mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): make marked project deletions win LWW conflicts (#9009)
deleteProject cascade-deletes a project's tasks, notes, sections, repeat config, and archive data in one reducer pass. When that op lost an LWW conflict to a concurrent project edit, only the PROJECT entity was reversed: every client resurrected an empty project and the winning client's status-blind hydration replay cascaded its tasks away after a restart (live state != post-restart replay). Rather than recreate every cascaded entity (payload scales with project size and cannot restore every side effect safely), give schema-v4 deleteProject operations explicit delete-wins precedence: - new deleteProject actions carry a shared PROJECT_DELETE_WINS_MARKER; the shared LWW planner accepts a host-supplied delete-wins classifier. A marked remote delete is applied regardless of timestamps; a marked local delete is replaced with one op whose vector clock dominates both sides. - historical unmarked (schema-v3) deletions keep timestamp-based LWW; the absence of the marker (never added by the no-op v3->v4 migration) is the real discriminator, and a schema v3->v4 barrier (mirroring v2->v3) makes older clients block on the newer-schema gate instead of mis-resolving. Delete-wins plans reuse the archive-win resolution pipeline, so they inherit its atomic persistence and losing-op rejection, and disjoint merge leaves them untouched (the delete must win the whole entity). Hardening from multi-agent review: - union allTaskIds/noteIds across multiple concurrent marked deletes for the same project, so a single replacement cannot leave orphan tasks on clients that only receive it (the task reducer removes by allTaskIds). - gate the classifier on the AUTHENTICATED payload projectId matching the plaintext entityId, so a tampered/replayed delete retargeted onto a live entity cannot silently drop a concurrent edit. - guard a null/undefined delete payload in the classifier instead of throwing and wedging the conflict pass. - pin the server's legacy-misc conflict alias to the fixed v1->v2 split boundary, not CURRENT_SCHEMA_VERSION, so this bump does not fabricate false GLOBAL_CONFIG:misc/tasks conflicts during rollout. - bind the marker with a shared const (compiler-checked on producer and consumer) and rename _isArchivePlan -> _isWholeEntityWinPlan. Documents the policy as ARCHITECTURE-DECISIONS.md #7. Addresses #8997.
This commit is contained in:
Johannes Millan
GitHub
parent
2864a39c85
commit
8e810edbe7
17 changed files with 679 additions and 17 deletions
|
|
@ -259,6 +259,70 @@ promoted to the user's active `Passkey` set only when that token is consumed.
|
|||
|
||||
---
|
||||
|
||||
### 7. Versioned Delete-Wins Semantics for Project Deletion
|
||||
|
||||
**Status**: ✅ Active (since July 2026)
|
||||
|
||||
**Decision**: Project deletions created with schema v4 or newer carry an explicit
|
||||
`projectDeleteWins` marker and beat concurrent project updates. Historical,
|
||||
unmarked deletions keep timestamp-based LWW semantics.
|
||||
|
||||
This is a deliberate semantic trade-off: a concurrent project rename or field
|
||||
edit that is vector-clock CONCURRENT with a marked delete **loses**, regardless
|
||||
of which has the newer wall-clock timestamp. Deleting an entity another device is
|
||||
editing wins over the edit — the alternative (timestamp LWW) resurrects an empty
|
||||
project shell and silently loses its task subtree. The lost edit is only
|
||||
recoverable via local undo, not via sync.
|
||||
|
||||
**Rationale**:
|
||||
|
||||
- `deleteProject` is one user intent whose reducer cascade removes the project,
|
||||
active tasks, notes, sections, repeat configuration, and related archive data.
|
||||
Reversing only the project entity after that operation loses data and violates
|
||||
replay determinism.
|
||||
- Capturing every cascaded entity in the delete payload or emitting restoration
|
||||
sidecars makes payload size scale with project size and still cannot restore
|
||||
every side effect safely.
|
||||
- Deletion is the only complete, deterministic result already represented by the
|
||||
operation. A concurrent rename or project-field edit must not partially undo it.
|
||||
- The schema-v4 barrier makes clients that do not understand this conflict policy
|
||||
stop before applying the operation (they block on the newer-schema gate rather
|
||||
than mis-resolving). The **absence** of the payload marker on historical
|
||||
deletions — never added by the no-op v3→v4 migration — is what preserves their
|
||||
timestamp-LWW semantics; the marker, not the version number, is the real
|
||||
discriminator. The classifier additionally requires the marked delete's
|
||||
plaintext `entityId` to match its authenticated payload `projectId`, so a
|
||||
tampered/replayed delete retargeted onto a live entity cannot win.
|
||||
|
||||
**Implementation**:
|
||||
|
||||
- New `deleteProject` actions include `projectDeleteWins: true`; replacement
|
||||
delete operations preserve that payload.
|
||||
- The shared LWW planner accepts a host-supplied delete-wins classifier. A remote
|
||||
marked delete is applied regardless of timestamps. A local marked delete is
|
||||
replaced with one operation whose vector clock dominates both conflict sides.
|
||||
- SuperSync keeps its generic conflict protocol: if the first delete upload is
|
||||
rejected, the existing retry path uploads the causally dominant replacement.
|
||||
File-based providers use the same client planner and marker.
|
||||
- Do not add per-task/note restoration operations or project-sized snapshots to
|
||||
compensate a losing marked project delete.
|
||||
|
||||
**Key Files**:
|
||||
|
||||
- [`task-shared.actions.ts`](src/app/root-store/meta/task-shared.actions.ts) — the `PROJECT_DELETE_WINS_MARKER` producer
|
||||
- [`conflict-resolution.ts`](packages/sync-core/src/conflict-resolution.ts)
|
||||
- [`conflict-resolution.service.ts`](src/app/op-log/sync/conflict-resolution.service.ts) — the delete-wins classifier
|
||||
- [`schema-version.ts`](packages/shared-schema/src/schema-version.ts)
|
||||
- [`project-delete-wins-barrier-v3-to-v4.ts`](packages/shared-schema/src/migrations/project-delete-wins-barrier-v3-to-v4.ts) (registered in [`migrations/index.ts`](packages/shared-schema/src/migrations/index.ts))
|
||||
|
||||
**When to Update This Pattern**:
|
||||
|
||||
- Changing the cascade performed by `deleteProject`
|
||||
- Adding another operation with delete-wins conflict semantics
|
||||
- Changing schema compatibility or LWW replacement behavior
|
||||
|
||||
---
|
||||
|
||||
## How to Use This Document
|
||||
|
||||
### When Making Architectural Changes
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
// Schema version constants
|
||||
export { CURRENT_SCHEMA_VERSION, MIN_SUPPORTED_SCHEMA_VERSION } from './schema-version';
|
||||
export {
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
MIN_SUPPORTED_SCHEMA_VERSION,
|
||||
PROJECT_DELETE_WINS_SCHEMA_VERSION,
|
||||
} from './schema-version';
|
||||
|
||||
// Types
|
||||
export type {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import type { SchemaMigration } from '../migration.types';
|
||||
import { MiscToTasksSettingsMigration_v1v2 } from './misc-to-tasks-settings-migration-v1-to-v2';
|
||||
import { LwwReplacementBarrierMigration_v2v3 } from './lww-replacement-barrier-v2-to-v3';
|
||||
import { ProjectDeleteWinsBarrierMigration_v3v4 } from './project-delete-wins-barrier-v3-to-v4';
|
||||
|
||||
/**
|
||||
* Registry of all schema migrations.
|
||||
|
|
@ -30,4 +31,5 @@ import { LwwReplacementBarrierMigration_v2v3 } from './lww-replacement-barrier-v
|
|||
export const MIGRATIONS: SchemaMigration[] = [
|
||||
MiscToTasksSettingsMigration_v1v2,
|
||||
LwwReplacementBarrierMigration_v2v3,
|
||||
ProjectDeleteWinsBarrierMigration_v3v4,
|
||||
];
|
||||
|
|
|
|||
|
|
@ -0,0 +1,17 @@
|
|||
import type { SchemaMigration } from '../migration.types';
|
||||
|
||||
/**
|
||||
* Compatibility barrier for delete-wins project deletions.
|
||||
*
|
||||
* The state shape and historical operations are unchanged. New schema-v4
|
||||
* deleteProject operations carry an explicit payload marker; leaving older
|
||||
* operations untouched ensures they retain their original timestamp-based LWW
|
||||
* semantics after receiver-side migration.
|
||||
*/
|
||||
export const ProjectDeleteWinsBarrierMigration_v3v4: SchemaMigration = {
|
||||
fromVersion: 3,
|
||||
toVersion: 4,
|
||||
description: 'Gate marked project delete-wins conflict semantics',
|
||||
requiresOperationMigration: false,
|
||||
migrateState: (state: unknown): unknown => state,
|
||||
};
|
||||
|
|
@ -2,7 +2,8 @@
|
|||
* Current schema version for all operations and state snapshots.
|
||||
* Increment this BEFORE adding a new migration.
|
||||
*/
|
||||
export const CURRENT_SCHEMA_VERSION = 3;
|
||||
export const PROJECT_DELETE_WINS_SCHEMA_VERSION = 4;
|
||||
export const CURRENT_SCHEMA_VERSION = PROJECT_DELETE_WINS_SCHEMA_VERSION;
|
||||
|
||||
/**
|
||||
* Minimum schema version that this codebase can still handle.
|
||||
|
|
|
|||
|
|
@ -11,11 +11,22 @@ import {
|
|||
import {
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
MIN_SUPPORTED_SCHEMA_VERSION,
|
||||
PROJECT_DELETE_WINS_SCHEMA_VERSION,
|
||||
} from '../src/schema-version';
|
||||
import type { OperationLike, SchemaMigration } from '../src/migration.types';
|
||||
import { MIGRATIONS } from '../src/migrations';
|
||||
|
||||
describe('shared-schema migration functions', () => {
|
||||
it('includes the schema-v4 project-delete conflict-policy barrier', () => {
|
||||
expect(CURRENT_SCHEMA_VERSION).toBe(4);
|
||||
expect(PROJECT_DELETE_WINS_SCHEMA_VERSION).toBe(CURRENT_SCHEMA_VERSION);
|
||||
expect(MIGRATIONS.at(-1)).toMatchObject({
|
||||
fromVersion: 3,
|
||||
toVersion: 4,
|
||||
requiresOperationMigration: false,
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCurrentSchemaVersion', () => {
|
||||
it('returns the current schema version', () => {
|
||||
expect(getCurrentSchemaVersion()).toBe(CURRENT_SCHEMA_VERSION);
|
||||
|
|
@ -117,6 +128,36 @@ describe('shared-schema migration functions', () => {
|
|||
});
|
||||
|
||||
describe('migrateOperation', () => {
|
||||
it('keeps a historical project delete unmarked while crossing the v4 barrier', () => {
|
||||
const op: OperationLike = {
|
||||
id: 'legacy-project-delete',
|
||||
opType: 'DEL',
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
payload: {
|
||||
actionPayload: {
|
||||
projectId: 'project-1',
|
||||
noteIds: [],
|
||||
allTaskIds: [],
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
schemaVersion: 3,
|
||||
};
|
||||
|
||||
const result = migrateOperation(op);
|
||||
|
||||
expect(result).toMatchObject({ success: true, migratedToVersion: 4 });
|
||||
expect(result.data).toEqual({ ...op, schemaVersion: 4 });
|
||||
expect(
|
||||
(
|
||||
(result.data as OperationLike).payload as {
|
||||
actionPayload: { projectDeleteWins?: boolean };
|
||||
}
|
||||
).actionPayload.projectDeleteWins,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns unchanged operation when already at target version', () => {
|
||||
const op: OperationLike = {
|
||||
id: 'op1',
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import type { OperationLike } from '../../src/migration.types';
|
|||
|
||||
describe('LWW replacement compatibility barrier v2 -> v3', () => {
|
||||
it('makes replacement-mode operations visible as a new schema generation', () => {
|
||||
expect(CURRENT_SCHEMA_VERSION).toBe(3);
|
||||
expect(CURRENT_SCHEMA_VERSION).toBeGreaterThanOrEqual(3);
|
||||
expect(LwwReplacementBarrierMigration_v2v3.fromVersion).toBe(2);
|
||||
expect(LwwReplacementBarrierMigration_v2v3.toVersion).toBe(3);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,49 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
PROJECT_DELETE_WINS_SCHEMA_VERSION,
|
||||
} from '../../src/schema-version';
|
||||
import { migrateOperation, migrateState } from '../../src/migrate';
|
||||
import { ProjectDeleteWinsBarrierMigration_v3v4 } from '../../src/migrations/project-delete-wins-barrier-v3-to-v4';
|
||||
import type { OperationLike } from '../../src/migration.types';
|
||||
|
||||
describe('project delete-wins compatibility barrier v3 -> v4', () => {
|
||||
it('makes marked project deletions visible as a new schema generation', () => {
|
||||
expect(CURRENT_SCHEMA_VERSION).toBe(4);
|
||||
expect(PROJECT_DELETE_WINS_SCHEMA_VERSION).toBe(4);
|
||||
expect(ProjectDeleteWinsBarrierMigration_v3v4.fromVersion).toBe(3);
|
||||
expect(ProjectDeleteWinsBarrierMigration_v3v4.toVersion).toBe(4);
|
||||
});
|
||||
|
||||
it('leaves v3 state data unchanged', () => {
|
||||
const state = { project: { ids: ['project-1'] } };
|
||||
|
||||
const result = migrateState(state, 3, 4);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toBe(state);
|
||||
});
|
||||
|
||||
it('preserves historical project-delete semantics while stamping schema v4', () => {
|
||||
const operation: OperationLike = {
|
||||
id: 'legacy-project-delete',
|
||||
opType: 'DEL',
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
payload: {
|
||||
actionPayload: {
|
||||
projectId: 'project-1',
|
||||
noteIds: [],
|
||||
allTaskIds: [],
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
schemaVersion: 3,
|
||||
};
|
||||
|
||||
const result = migrateOperation(operation, 4);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toEqual({ ...operation, schemaVersion: 4 });
|
||||
});
|
||||
});
|
||||
|
|
@ -416,8 +416,18 @@ export const getConflictEntityIds = (op: Operation): string[] => {
|
|||
return Array.from(new Set(rawEntityIds));
|
||||
};
|
||||
|
||||
/**
|
||||
* The misc→tasks settings split happened in the v1→v2 migration
|
||||
* (`MiscToTasksSettingsMigration_v1v2`), so ONLY pre-v2 `GLOBAL_CONFIG:misc`
|
||||
* writes also touched what is now `GLOBAL_CONFIG:tasks`. Gate on that fixed
|
||||
* boundary, not the moving `CURRENT_SCHEMA_VERSION`: otherwise every schema bump
|
||||
* (e.g. v3→v4) newly aliases already-split misc writes to tasks and fabricates
|
||||
* conflicts between disjoint settings during rollout.
|
||||
*/
|
||||
const MISC_TASKS_SPLIT_SCHEMA_VERSION = 2;
|
||||
|
||||
const isLegacyMiscConfigOperation = (op: Operation): boolean =>
|
||||
op.schemaVersion < CURRENT_SCHEMA_VERSION &&
|
||||
op.schemaVersion < MISC_TASKS_SPLIT_SCHEMA_VERSION &&
|
||||
op.entityType === 'GLOBAL_CONFIG' &&
|
||||
op.entityId === 'misc';
|
||||
|
||||
|
|
|
|||
|
|
@ -91,6 +91,20 @@ describe('conflict helpers', () => {
|
|||
},
|
||||
);
|
||||
|
||||
it.each([2, 3, 4])(
|
||||
'does NOT alias a post-split (v%i) misc write to tasks',
|
||||
(schemaVersion) => {
|
||||
// The misc→tasks split was the v1→v2 migration; v2+ misc writes touch only
|
||||
// misc. The legacy boundary must stay fixed at v2 so schema bumps do not
|
||||
// fabricate conflicts between disjoint settings (regression: v3→v4 bump).
|
||||
expect(
|
||||
getConflictEntityIds(
|
||||
op({ entityType: 'GLOBAL_CONFIG', entityId: 'misc', schemaVersion }),
|
||||
),
|
||||
).toEqual(['misc']);
|
||||
},
|
||||
);
|
||||
|
||||
it('accepts matching duplicate operations regardless of JSON key order', () => {
|
||||
const incoming = op({
|
||||
payload: { title: 'A', nested: { b: 2, a: 1 } },
|
||||
|
|
|
|||
|
|
@ -11,11 +11,13 @@ import type { SyncLogger } from './sync-logger';
|
|||
|
||||
export type ConflictResolutionSuggestion = 'local' | 'remote' | 'manual';
|
||||
export type LwwConflictResolutionWinner = 'local' | 'remote';
|
||||
export type LwwLocalWinOperationKind = 'archive-win' | 'update';
|
||||
export type LwwLocalWinOperationKind = 'archive-win' | 'delete-win' | 'update';
|
||||
export type LwwConflictResolutionReason =
|
||||
| 'remote-archive'
|
||||
| 'local-archive'
|
||||
| 'local-archive-sibling'
|
||||
| 'remote-delete-wins'
|
||||
| 'local-delete-wins'
|
||||
| 'local-timestamp'
|
||||
| 'remote-timestamp-or-tie';
|
||||
|
||||
|
|
@ -42,6 +44,7 @@ export interface LwwConflictResolutionPlanningOptions<
|
|||
TOperation extends Operation<string> = Operation,
|
||||
> {
|
||||
isArchiveAction: (op: TOperation) => boolean;
|
||||
isDeleteWinsAction?: (op: TOperation) => boolean;
|
||||
toEntityKey?: (entityType: string, entityId: string) => string;
|
||||
}
|
||||
|
||||
|
|
@ -334,9 +337,10 @@ export const suggestConflictResolution = <TOperation extends Operation<string>>(
|
|||
* Plans last-write-wins conflict resolution without looking up host state or
|
||||
* creating operations.
|
||||
*
|
||||
* The host supplies archive-action detection because archive semantics are
|
||||
* domain-specific. The returned plan tells the host whether a local-win op must
|
||||
* be created and which app-side factory should create it.
|
||||
* The host supplies archive-action detection and may opt specific operations
|
||||
* into delete-wins precedence because both policies are domain-specific. The
|
||||
* returned plan tells the host whether a local-win op must be created and which
|
||||
* app-side factory should create it.
|
||||
*/
|
||||
export const planLwwConflictResolutions = <
|
||||
TOperation extends Operation<string> = Operation,
|
||||
|
|
@ -386,6 +390,31 @@ export const planLwwConflictResolutions = <
|
|||
};
|
||||
}
|
||||
|
||||
const isDeleteWinsAction = options.isDeleteWinsAction;
|
||||
const remoteHasDeleteWins = isDeleteWinsAction
|
||||
? conflict.remoteOps.some(isDeleteWinsAction)
|
||||
: false;
|
||||
const localHasDeleteWins = isDeleteWinsAction
|
||||
? conflict.localOps.some(isDeleteWinsAction)
|
||||
: false;
|
||||
|
||||
if (remoteHasDeleteWins) {
|
||||
return {
|
||||
conflict,
|
||||
winner: 'remote',
|
||||
reason: 'remote-delete-wins',
|
||||
};
|
||||
}
|
||||
|
||||
if (localHasDeleteWins) {
|
||||
return {
|
||||
conflict,
|
||||
winner: 'local',
|
||||
reason: 'local-delete-wins',
|
||||
localWinOperationKind: 'delete-win',
|
||||
};
|
||||
}
|
||||
|
||||
const localMaxTimestamp = Math.max(...conflict.localOps.map((op) => op.timestamp));
|
||||
const remoteMaxTimestamp = Math.max(...conflict.remoteOps.map((op) => op.timestamp));
|
||||
|
||||
|
|
|
|||
|
|
@ -192,6 +192,92 @@ describe('suggestConflictResolution', () => {
|
|||
});
|
||||
|
||||
describe('planLwwConflictResolutions', () => {
|
||||
const isDeleteWinsAction = (op: Operation): boolean =>
|
||||
op.actionType === '[Test] Delete Wins';
|
||||
|
||||
it('lets a remote delete-wins action beat a newer local update', () => {
|
||||
const conflict = createConflict(
|
||||
[createOp({ id: 'local-update', timestamp: 2_000 })],
|
||||
[
|
||||
createOp({
|
||||
id: 'remote-delete',
|
||||
actionType: '[Test] Delete Wins',
|
||||
opType: OpType.Delete,
|
||||
timestamp: 1_000,
|
||||
}),
|
||||
],
|
||||
);
|
||||
|
||||
expect(
|
||||
planLwwConflictResolutions([conflict], {
|
||||
isArchiveAction,
|
||||
isDeleteWinsAction,
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
conflict,
|
||||
winner: 'remote',
|
||||
reason: 'remote-delete-wins',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('plans a replacement delete when a local delete-wins action is older', () => {
|
||||
const conflict = createConflict(
|
||||
[
|
||||
createOp({
|
||||
id: 'local-delete',
|
||||
actionType: '[Test] Delete Wins',
|
||||
opType: OpType.Delete,
|
||||
timestamp: 1_000,
|
||||
}),
|
||||
],
|
||||
[createOp({ id: 'remote-update', timestamp: 2_000 })],
|
||||
);
|
||||
|
||||
expect(
|
||||
planLwwConflictResolutions([conflict], {
|
||||
isArchiveAction,
|
||||
isDeleteWinsAction,
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
conflict,
|
||||
winner: 'local',
|
||||
reason: 'local-delete-wins',
|
||||
localWinOperationKind: 'delete-win',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps timestamp semantics for an unmarked delete', () => {
|
||||
const conflict = createConflict(
|
||||
[
|
||||
createOp({
|
||||
id: 'local-delete',
|
||||
opType: OpType.Delete,
|
||||
timestamp: 1_000,
|
||||
}),
|
||||
],
|
||||
[createOp({ id: 'remote-update', timestamp: 2_000 })],
|
||||
);
|
||||
|
||||
expect(
|
||||
planLwwConflictResolutions([conflict], {
|
||||
isArchiveAction,
|
||||
isDeleteWinsAction,
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
conflict,
|
||||
winner: 'remote',
|
||||
reason: 'remote-timestamp-or-tie',
|
||||
localMaxTimestamp: 1_000,
|
||||
remoteMaxTimestamp: 2_000,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('lets a remote archive win over local non-archive operations', () => {
|
||||
const conflict = createConflict(
|
||||
[createOp({ id: 'local', timestamp: 2_000 })],
|
||||
|
|
|
|||
|
|
@ -47,10 +47,6 @@ describe('SchemaMigrationService', () => {
|
|||
it('should return the current schema version', () => {
|
||||
expect(service.getCurrentVersion()).toBe(CURRENT_SCHEMA_VERSION);
|
||||
});
|
||||
|
||||
it('should return 3 for the LWW replacement compatibility barrier', () => {
|
||||
expect(service.getCurrentVersion()).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getMigrations', () => {
|
||||
|
|
|
|||
|
|
@ -449,6 +449,27 @@ describe('ConflictResolutionService', () => {
|
|||
schemaVersion: 1,
|
||||
});
|
||||
|
||||
const createProjectDelete = (
|
||||
id: string,
|
||||
clientId: string,
|
||||
timestamp: number,
|
||||
marked: boolean = true,
|
||||
): Operation => ({
|
||||
...createOpWithTimestamp(id, clientId, timestamp, OpType.Delete, 'project-1'),
|
||||
actionType: ActionType.TASK_SHARED_DELETE_PROJECT,
|
||||
entityType: 'PROJECT',
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
payload: {
|
||||
actionPayload: {
|
||||
projectId: 'project-1',
|
||||
noteIds: ['note-1'],
|
||||
allTaskIds: ['task-1'],
|
||||
...(marked ? { projectDeleteWins: true } : {}),
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
});
|
||||
|
||||
// Helper to create conflict with suggestedResolution
|
||||
const createConflict = (
|
||||
entityId: string,
|
||||
|
|
@ -1093,6 +1114,226 @@ describe('ConflictResolutionService', () => {
|
|||
expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['local-del']);
|
||||
});
|
||||
|
||||
it('keeps a marked local project deletion when a concurrent update is newer', async () => {
|
||||
const localDelete = createProjectDelete(
|
||||
'local-project-delete',
|
||||
'client-a',
|
||||
1_000,
|
||||
);
|
||||
const remoteUpdate: Operation = {
|
||||
...createOpWithTimestamp(
|
||||
'remote-project-update',
|
||||
'client-b',
|
||||
2_000,
|
||||
OpType.Update,
|
||||
'project-1',
|
||||
),
|
||||
entityType: 'PROJECT',
|
||||
};
|
||||
const conflict: EntityConflict = {
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
localOps: [localDelete],
|
||||
remoteOps: [remoteUpdate],
|
||||
suggestedResolution: 'manual',
|
||||
};
|
||||
|
||||
const result = await service.autoResolveConflictsLWW([conflict]);
|
||||
|
||||
const replacementDelete = getFirstMixedLocalOp();
|
||||
expect(replacementDelete.opType).toBe(OpType.Delete);
|
||||
expect(replacementDelete.actionType).toBe(ActionType.TASK_SHARED_DELETE_PROJECT);
|
||||
expect(replacementDelete.payload).toEqual(localDelete.payload);
|
||||
expect(replacementDelete.timestamp).toBe(localDelete.timestamp);
|
||||
expect(replacementDelete.vectorClock['client-a']).toBeGreaterThanOrEqual(1);
|
||||
expect(replacementDelete.vectorClock['client-b']).toBeGreaterThanOrEqual(1);
|
||||
expect(replacementDelete.vectorClock[TEST_CLIENT_ID]).toBeGreaterThanOrEqual(1);
|
||||
expect(result.localWinOpsCreated).toBe(1);
|
||||
});
|
||||
|
||||
it('applies a marked remote project deletion even when the local update is newer', async () => {
|
||||
const localUpdate: Operation = {
|
||||
...createOpWithTimestamp(
|
||||
'local-project-update',
|
||||
'client-a',
|
||||
2_000,
|
||||
OpType.Update,
|
||||
'project-1',
|
||||
),
|
||||
entityType: 'PROJECT',
|
||||
};
|
||||
const remoteDelete = createProjectDelete(
|
||||
'remote-project-delete',
|
||||
'client-b',
|
||||
1_000,
|
||||
);
|
||||
const conflict: EntityConflict = {
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
localOps: [localUpdate],
|
||||
remoteOps: [remoteDelete],
|
||||
suggestedResolution: 'manual',
|
||||
};
|
||||
mockOperationApplier.applyOperations.and.resolveTo({
|
||||
appliedOps: [remoteDelete],
|
||||
});
|
||||
|
||||
const result = await service.autoResolveConflictsLWW([conflict]);
|
||||
|
||||
expect(mockOpLogStore.appendBatchSkipDuplicates).toHaveBeenCalledWith(
|
||||
[remoteDelete],
|
||||
'remote',
|
||||
{ pendingApply: true },
|
||||
);
|
||||
expect(getMixedLocalOps()).toEqual([]);
|
||||
const appliedOps = mockOperationApplier.applyOperations.calls.mostRecent()
|
||||
.args[0] as Operation[];
|
||||
expect(appliedOps).toContain(remoteDelete);
|
||||
expect(result.localWinOpsCreated).toBe(0);
|
||||
});
|
||||
|
||||
it('keeps timestamp LWW for a migrated project deletion without the marker', async () => {
|
||||
const localDelete = createProjectDelete(
|
||||
'legacy-project-delete',
|
||||
'client-a',
|
||||
1_000,
|
||||
false,
|
||||
);
|
||||
const remoteUpdate: Operation = {
|
||||
...createOpWithTimestamp(
|
||||
'remote-project-update',
|
||||
'client-b',
|
||||
2_000,
|
||||
OpType.Update,
|
||||
'project-1',
|
||||
),
|
||||
entityType: 'PROJECT',
|
||||
};
|
||||
const conflict: EntityConflict = {
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
localOps: [localDelete],
|
||||
remoteOps: [remoteUpdate],
|
||||
suggestedResolution: 'manual',
|
||||
};
|
||||
mockOperationApplier.applyOperations.and.resolveTo({
|
||||
appliedOps: [remoteUpdate],
|
||||
});
|
||||
|
||||
await service.autoResolveConflictsLWW([conflict]);
|
||||
|
||||
expect(mockOpLogStore.appendBatchSkipDuplicates).toHaveBeenCalledWith(
|
||||
[remoteUpdate],
|
||||
'remote',
|
||||
{ pendingApply: true },
|
||||
);
|
||||
});
|
||||
|
||||
it('unions allTaskIds/noteIds across multiple concurrent marked deletes (#8997)', async () => {
|
||||
// Concurrent tabs each captured a deleteProject with a different cascade
|
||||
// set; the local store applied BOTH. The single replacement must carry
|
||||
// the union or another client keeps entities only the other delete removed.
|
||||
const makeDelete = (
|
||||
id: string,
|
||||
taskIds: string[],
|
||||
noteIds: string[],
|
||||
): Operation => ({
|
||||
...createOpWithTimestamp(id, 'client-a', 1_000, OpType.Delete, 'project-1'),
|
||||
actionType: ActionType.TASK_SHARED_DELETE_PROJECT,
|
||||
entityType: 'PROJECT',
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
payload: {
|
||||
actionPayload: {
|
||||
projectId: 'project-1',
|
||||
noteIds,
|
||||
allTaskIds: taskIds,
|
||||
projectDeleteWins: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
});
|
||||
const deleteA = makeDelete('local-delete-a', ['task-1'], ['note-1']);
|
||||
const deleteB = makeDelete('local-delete-b', ['task-2'], ['note-2']);
|
||||
const remoteUpdate: Operation = {
|
||||
...createOpWithTimestamp(
|
||||
'remote-project-update',
|
||||
'client-b',
|
||||
2_000,
|
||||
OpType.Update,
|
||||
'project-1',
|
||||
),
|
||||
entityType: 'PROJECT',
|
||||
};
|
||||
const conflict: EntityConflict = {
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
localOps: [deleteA, deleteB],
|
||||
remoteOps: [remoteUpdate],
|
||||
suggestedResolution: 'manual',
|
||||
};
|
||||
|
||||
await service.autoResolveConflictsLWW([conflict]);
|
||||
|
||||
const replacementPayload = getFirstMixedLocalOp().payload as {
|
||||
actionPayload: { allTaskIds: string[]; noteIds: string[] };
|
||||
};
|
||||
expect(new Set(replacementPayload.actionPayload.allTaskIds)).toEqual(
|
||||
new Set(['task-1', 'task-2']),
|
||||
);
|
||||
expect(new Set(replacementPayload.actionPayload.noteIds)).toEqual(
|
||||
new Set(['note-1', 'note-2']),
|
||||
);
|
||||
});
|
||||
|
||||
it('ignores the marker when entityId does not match the authenticated projectId', async () => {
|
||||
// Tampered/replayed delete retargeted onto a live entity: marker present
|
||||
// but op.entityId ('project-1') != payload.projectId ('project-original').
|
||||
// Must NOT win delete-wins; falls back to timestamp LWW, so the newer
|
||||
// local update wins and the retargeted delete is not applied.
|
||||
const retargetedDelete: Operation = {
|
||||
...createProjectDelete('remote-retargeted-delete', 'client-b', 1_000),
|
||||
payload: {
|
||||
actionPayload: {
|
||||
projectId: 'project-original',
|
||||
noteIds: [],
|
||||
allTaskIds: [],
|
||||
projectDeleteWins: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
};
|
||||
const localUpdate: Operation = {
|
||||
...createOpWithTimestamp(
|
||||
'local-project-update',
|
||||
'client-a',
|
||||
2_000,
|
||||
OpType.Update,
|
||||
'project-1',
|
||||
),
|
||||
entityType: 'PROJECT',
|
||||
};
|
||||
const conflict: EntityConflict = {
|
||||
entityType: 'PROJECT',
|
||||
entityId: 'project-1',
|
||||
localOps: [localUpdate],
|
||||
remoteOps: [retargetedDelete],
|
||||
suggestedResolution: 'manual',
|
||||
};
|
||||
mockOperationApplier.applyOperations.and.resolveTo({ appliedOps: [] });
|
||||
|
||||
await service.autoResolveConflictsLWW([conflict]);
|
||||
|
||||
// Delete-wins did NOT fire (would apply the delete as a remote winner).
|
||||
expect(mockOpLogStore.appendBatchSkipDuplicates).not.toHaveBeenCalledWith(
|
||||
[retargetedDelete],
|
||||
'remote',
|
||||
{ pendingApply: true },
|
||||
);
|
||||
expect(mockOpLogStore.markRejected).toHaveBeenCalledWith([
|
||||
'remote-retargeted-delete',
|
||||
]);
|
||||
});
|
||||
|
||||
it('restamps a converted remote update to the current schema version (#8990)', async () => {
|
||||
// Conversion wraps an older-schema remote update in the v3-only
|
||||
// replacement envelope; the stored row's version must match its
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import {
|
|||
type LwwConflictResolutionPlan,
|
||||
type LwwResolvedConflict,
|
||||
} from '@sp/sync-core';
|
||||
import { PROJECT_DELETE_WINS_SCHEMA_VERSION } from '@sp/shared-schema';
|
||||
import {
|
||||
findLwwContentConflicts,
|
||||
type LwwContentConflict,
|
||||
|
|
@ -39,6 +40,7 @@ import {
|
|||
VectorClock,
|
||||
} from '../core/operation.types';
|
||||
import { toLwwUpdateActionType } from '../core/lww-update-action-types';
|
||||
import { PROJECT_DELETE_WINS_MARKER } from '../../root-store/meta/task-shared.actions';
|
||||
import { OperationApplierService } from '../apply/operation-applier.service';
|
||||
import { HydrationStateService } from '../apply/hydration-state.service';
|
||||
import {
|
||||
|
|
@ -114,6 +116,68 @@ interface AutoResolveConflictsLwwOptions {
|
|||
remoteApplyLifecycleOwnedByCaller?: boolean;
|
||||
}
|
||||
|
||||
const isProjectDeleteWinsOperation = (operation: Operation): boolean => {
|
||||
// `!(x >= n)` (not `x < n`) so a malformed op with an undefined schemaVersion
|
||||
// is treated as pre-v4 rather than slipping through. The `!operation.payload`
|
||||
// guard prevents a null/undefined-payload DEL op (the server permits one) from
|
||||
// throwing inside `extractActionPayload` and wedging the whole conflict pass.
|
||||
if (
|
||||
!(operation.schemaVersion >= PROJECT_DELETE_WINS_SCHEMA_VERSION) ||
|
||||
operation.actionType !== ActionType.TASK_SHARED_DELETE_PROJECT ||
|
||||
operation.opType !== OpType.Delete ||
|
||||
!operation.payload
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
const actionPayload = extractActionPayload(operation.payload);
|
||||
// Gate on the AUTHENTICATED `projectId` (inside the E2EE GCM auth tag), and
|
||||
// require it to match the plaintext `entityId` used to group the conflict.
|
||||
// A tampered/replayed marked delete retargeted onto a live entity therefore
|
||||
// fails to win delete-wins, so it cannot silently drop the victim's concurrent
|
||||
// edit — it falls back to timestamp LWW. (GHSA-8pxh metadata-tampering class.)
|
||||
return (
|
||||
actionPayload[PROJECT_DELETE_WINS_MARKER] === true &&
|
||||
operation.entityId === actionPayload['projectId']
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Concurrent tabs can capture more than one marked `deleteProject` for the same
|
||||
* project before syncing, and the local store has applied EVERY one's cascade
|
||||
* (the task reducer removes entities by explicit `allTaskIds`, not by
|
||||
* `projectId`). The single winning replacement must therefore carry the UNION of
|
||||
* all their cascaded `allTaskIds`/`noteIds`, or a client that only receives that
|
||||
* replacement keeps entities a later local delete already removed. Only the id
|
||||
* arrays are widened — `projectId` and every other field are identical across
|
||||
* same-project deletes, so the first op is a safe base.
|
||||
*/
|
||||
const mergeMarkedProjectDeleteOps = (localOps: Operation[]): Operation | undefined => {
|
||||
const deletes = localOps.filter(isProjectDeleteWinsOperation);
|
||||
if (deletes.length <= 1) {
|
||||
return deletes[0];
|
||||
}
|
||||
const unionIds = (key: string): string[] => {
|
||||
const merged = new Set<string>();
|
||||
for (const op of deletes) {
|
||||
const value = extractActionPayload(op.payload)[key];
|
||||
if (Array.isArray(value)) {
|
||||
value.forEach((id) => merged.add(id as string));
|
||||
}
|
||||
}
|
||||
return [...merged];
|
||||
};
|
||||
const base = deletes[0];
|
||||
const mergedActionPayload: Record<string, unknown> = {
|
||||
...extractActionPayload(base.payload),
|
||||
allTaskIds: unionIds('allTaskIds'),
|
||||
noteIds: unionIds('noteIds'),
|
||||
};
|
||||
const mergedPayload = isMultiEntityPayload(base.payload)
|
||||
? { ...base.payload, actionPayload: mergedActionPayload }
|
||||
: mergedActionPayload;
|
||||
return { ...base, payload: mergedPayload };
|
||||
};
|
||||
|
||||
const getTaskProjectMoveEntityIds = (operation: Operation): string[] | undefined => {
|
||||
if (
|
||||
operation.actionType === toLwwUpdateActionType('TASK') &&
|
||||
|
|
@ -1444,6 +1508,7 @@ export class ConflictResolutionService {
|
|||
|
||||
const plans = planLwwConflictResolutions(conflicts, {
|
||||
isArchiveAction: (op) => op.actionType === ActionType.TASK_SHARED_MOVE_TO_ARCHIVE,
|
||||
isDeleteWinsAction: isProjectDeleteWinsOperation,
|
||||
toEntityKey: (entityType, entityId) =>
|
||||
toEntityKey(entityType as EntityType, entityId),
|
||||
});
|
||||
|
|
@ -1506,6 +1571,15 @@ export class ConflictResolutionService {
|
|||
|
||||
if (plan.localWinOperationKind === 'archive-win') {
|
||||
localWinOp = await this._createArchiveWinOp(plan.conflict);
|
||||
} else if (plan.localWinOperationKind === 'delete-win') {
|
||||
const deleteOp = mergeMarkedProjectDeleteOps(plan.conflict.localOps);
|
||||
if (!deleteOp) {
|
||||
throw new Error(
|
||||
`ConflictResolutionService: Missing delete-wins operation for ` +
|
||||
`${plan.conflict.entityType}:${plan.conflict.entityId}`,
|
||||
);
|
||||
}
|
||||
localWinOp = await this._createReplacementDeleteOp(plan.conflict, deleteOp);
|
||||
} else if (plan.localWinOperationKind === 'update') {
|
||||
localWinOp = await this._createLocalWinUpdateOp(plan.conflict);
|
||||
}
|
||||
|
|
@ -1527,6 +1601,15 @@ export class ConflictResolutionService {
|
|||
`(${plan.reason === 'remote-archive' ? 'remote' : 'local'} archive) for ` +
|
||||
`${plan.conflict.entityType}:${plan.conflict.entityId}`,
|
||||
);
|
||||
} else if (
|
||||
plan.reason === 'remote-delete-wins' ||
|
||||
plan.reason === 'local-delete-wins'
|
||||
) {
|
||||
OpLog.normal(
|
||||
`ConflictResolutionService: Project deletion wins over concurrent update ` +
|
||||
`(${plan.winner} delete) for ` +
|
||||
`${plan.conflict.entityType}:${plan.conflict.entityId}`,
|
||||
);
|
||||
} else if (plan.winner === 'local') {
|
||||
OpLog.normal(
|
||||
`ConflictResolutionService: LWW resolved ${plan.conflict.entityType}:${plan.conflict.entityId} as LOCAL ` +
|
||||
|
|
@ -1845,15 +1928,20 @@ export class ConflictResolutionService {
|
|||
}
|
||||
|
||||
/**
|
||||
* SPAP-14: whether this plan is an archive plan. Archive/delete-wins semantics
|
||||
* are left 100% untouched by disjoint-merge — the archive must win the whole
|
||||
* entity, never be partially merged with a concurrent edit.
|
||||
* SPAP-14: whether this plan must win the WHOLE entity and so is excluded from
|
||||
* disjoint-field merge. Both archive and project-delete-wins have this
|
||||
* property — the winner replaces the entity outright, never partially merged
|
||||
* with a concurrent edit.
|
||||
*/
|
||||
private _isArchivePlan(plan: LwwConflictResolutionPlan<EntityConflict>): boolean {
|
||||
private _isWholeEntityWinPlan(
|
||||
plan: LwwConflictResolutionPlan<EntityConflict>,
|
||||
): boolean {
|
||||
return (
|
||||
plan.reason === 'remote-archive' ||
|
||||
plan.reason === 'local-archive' ||
|
||||
plan.reason === 'local-archive-sibling' ||
|
||||
plan.reason === 'remote-delete-wins' ||
|
||||
plan.reason === 'local-delete-wins' ||
|
||||
plan.localWinOperationKind === 'archive-win'
|
||||
);
|
||||
}
|
||||
|
|
@ -1881,7 +1969,7 @@ export class ConflictResolutionService {
|
|||
private async _tryCreateDisjointMergeOp(
|
||||
plan: LwwConflictResolutionPlan<EntityConflict>,
|
||||
): Promise<Operation | undefined> {
|
||||
if (this._isArchivePlan(plan)) {
|
||||
if (this._isWholeEntityWinPlan(plan)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,15 @@ import { BatchOperation } from '@super-productivity/plugin-api';
|
|||
import { PersistentActionMeta } from '../../op-log/core/persistent-action.interface';
|
||||
import { OpType } from '../../op-log/core/operation.types';
|
||||
|
||||
/**
|
||||
* Payload marker stamped on every new `deleteProject` operation so the LWW
|
||||
* conflict planner can give it delete-wins precedence. Shared with the sync
|
||||
* classifier (`conflict-resolution.service.ts`) so a rename is compiler-checked
|
||||
* on both ends — dropping/renaming it silently would regress to resurrecting an
|
||||
* empty project (see ARCHITECTURE-DECISIONS.md #7).
|
||||
*/
|
||||
export const PROJECT_DELETE_WINS_MARKER = 'projectDeleteWins';
|
||||
|
||||
/**
|
||||
* Shared actions that affect multiple reducers (tasks, projects, tags)
|
||||
* These actions are handled by the task-shared meta-reducer
|
||||
|
|
@ -310,6 +319,7 @@ export const TaskSharedActions = createActionGroup({
|
|||
allTaskIds: string[];
|
||||
}) => ({
|
||||
...taskProps,
|
||||
[PROJECT_DELETE_WINS_MARKER]: true as const,
|
||||
meta: {
|
||||
isPersistent: true,
|
||||
entityType: 'PROJECT',
|
||||
|
|
|
|||
|
|
@ -1859,6 +1859,16 @@ describe('taskSharedMetaReducer', () => {
|
|||
});
|
||||
|
||||
describe('deleteProject action', () => {
|
||||
it('marks new project deletions with delete-wins conflict semantics', () => {
|
||||
const action = TaskSharedActions.deleteProject({
|
||||
projectId: 'project-1',
|
||||
noteIds: [],
|
||||
allTaskIds: [],
|
||||
});
|
||||
|
||||
expect(action.projectDeleteWins).toBeTrue();
|
||||
});
|
||||
|
||||
it('should remove all project tasks from all tags', () => {
|
||||
const testState = createStateWithExistingTasks(
|
||||
[],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue