diff --git a/src/app/op-log/sync/operation-log-sync.service.spec.ts b/src/app/op-log/sync/operation-log-sync.service.spec.ts index ae06333496..e7e6538e3e 100644 --- a/src/app/op-log/sync/operation-log-sync.service.spec.ts +++ b/src/app/op-log/sync/operation-log-sync.service.spec.ts @@ -260,6 +260,202 @@ describe('OperationLogSyncService', () => { .and.returnValue(Promise.resolve(1)); // Not fresh (has seq) }); + describe('example-task cleanup on first adoption of a populated remote (#7985)', () => { + const exampleCreate = (taskId: string): OperationLogEntry => ({ + seq: 1, + op: { + id: `ex-op-${taskId}`, + clientId: 'client-A', + actionType: ActionType.TASK_SHARED_ADD, + opType: OpType.Create, + entityType: 'TASK', + entityId: taskId, + payload: { + actionPayload: { task: { id: taskId }, isExampleTask: true }, + entityChanges: [], + }, + vectorClock: { clientA: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + appliedAt: Date.now(), + source: 'local', + }); + + const realTaskCreate = (taskId: string): OperationLogEntry => ({ + seq: 2, + op: { + id: `real-op-${taskId}`, + clientId: 'client-A', + actionType: '[Task] Add Task' as ActionType, + opType: OpType.Create, + entityType: 'TASK', + entityId: taskId, + payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] }, + vectorClock: { clientA: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + appliedAt: Date.now(), + source: 'local', + }); + + const configOp = (): OperationLogEntry => ({ + seq: 3, + op: { + id: 'config-op-1', + clientId: 'client-A', + actionType: '[Global Config] Update Global Config Section' as ActionType, + opType: OpType.Update, + entityType: 'GLOBAL_CONFIG', + entityId: 'sync', + payload: { sectionKey: 'sync' }, + vectorClock: { clientA: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + appliedAt: Date.now(), + source: 'local', + }); + + // A reorder of an example task: a Move op (entityType TASK, entityId = the example id) + // that hasMeaningfulPendingOps() does NOT flag — would strand a dangling reference if + // the cleanup rejected the create but let this Move upload (cross-validated by review). + const exampleMove = (taskId: string): OperationLogEntry => ({ + seq: 4, + op: { + id: `mov-op-${taskId}`, + clientId: 'client-A', + actionType: '[Project] Move Task In Backlog/Today' as ActionType, + opType: OpType.Move, + entityType: 'TASK', + entityId: taskId, + payload: {}, + vectorClock: { clientA: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + appliedAt: Date.now(), + source: 'local', + }); + + const exampleUpdate = (taskId: string): OperationLogEntry => ({ + seq: 5, + op: { + id: `upd-op-${taskId}`, + clientId: 'client-A', + actionType: '[Task] Update Task' as ActionType, + opType: OpType.Update, + entityType: 'TASK', + entityId: taskId, + payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] }, + vectorClock: { clientA: 1 }, + timestamp: Date.now(), + schemaVersion: 1, + }, + appliedAt: Date.now(), + source: 'local', + }); + + const mockProvider = { isReady: () => Promise.resolve(true) } as any; + + beforeEach(() => { + uploadServiceSpy.uploadPendingOps.and.resolveTo({ + uploadedCount: 0, + piggybackedOps: [], + rejectedCount: 0, + rejectedOps: [], + }); + opLogStoreSpy.markRejected.and.resolveTo(); + }); + + it('discards untouched example-task ops when a never-synced client adopted a populated remote', async () => { + // isNeverSynced captured pre-download = true; hasSyncedOps now true = adopted this cycle. + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([ + exampleCreate('ex-1'), + exampleCreate('ex-2'), + ]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([ + 'ex-op-ex-1', + 'ex-op-ex-2', + ]); + }); + + it('does NOT discard when the client had already synced before this cycle (isNeverSynced false)', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: false }); + + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); + }); + + it('does NOT discard when nothing was adopted this cycle (empty-server seeding, hasSyncedOps false)', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(false); + opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); + }); + + it('does NOT discard when meaningful user work is also pending (a real task) — keeps everything', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([ + exampleCreate('ex-1'), + realTaskCreate('real-1'), + ]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); + }); + + it('still discards when only default config ops coexist with the example tasks', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([ + exampleCreate('ex-1'), + configOp(), + exampleCreate('ex-2'), + ]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([ + 'ex-op-ex-1', + 'ex-op-ex-2', + ]); + }); + + it('does NOT discard when a reorder Move of an example task is pending (would otherwise strand a dangling ref)', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([ + exampleCreate('ex-1'), + exampleMove('ex-1'), + ]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); + }); + + it('does NOT discard when an example task was edited (its Update keeps the whole batch)', async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + opLogStoreSpy.getUnsynced.and.resolveTo([ + exampleCreate('ex-1'), + exampleUpdate('ex-1'), + ]); + + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); + + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); + }); + }); + describe('uploadPendingOps', () => { it('should return localWinOpsCreated: 0 when no piggybacked ops', async () => { opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve([])); diff --git a/src/app/op-log/sync/operation-log-sync.service.ts b/src/app/op-log/sync/operation-log-sync.service.ts index fd9018fb43..227094dd14 100644 --- a/src/app/op-log/sync/operation-log-sync.service.ts +++ b/src/app/op-log/sync/operation-log-sync.service.ts @@ -198,6 +198,47 @@ export class OperationLogSyncService { return { kind: 'blocked_fresh_client' }; } + // #7985: identity-based example-task cleanup (the deferred "Fix C" of #7980). + // `isNeverSyncedAtSyncStart` is captured PRE-download; if hasSyncedOps() is now true, + // this cycle's download just ADOPTED a populated remote. A genuinely-fresh client's + // onboarding example tasks are throwaway scaffolding every install regenerates; + // uploading them onto the adopted remote pollutes it and propagates to every device + // (the residual #7980 documented for non-encrypted accounts seeded by normal upload, + // where no SYNC_IMPORT fires the incoming-import gate's discard). Reject the example + // create ops so the upload never sends them. + // + // A status read alone is unsafe (download flips hasSyncedOps mid-cycle — #7980 §10); + // the PRE-download `isNeverSynced` is what makes this reliable. + // + // Stranding safety: we discard only from a PRISTINE post-boot batch — example creates + // plus the default GLOBAL_CONFIG writes, nothing else. hasMeaningfulPendingOps() is NOT + // enough here: it ignores Move/Batch (and planner/board/section) ops, which can reference + // an example task id without being "meaningful". Unlike the incoming-SYNC_IMPORT discard + // — where SyncImportFilterService drops any concurrent dependent op against the import — + // this adoption path has no import to filter them, so a surviving Move would strand a + // dangling reference once its create is rejected. Requiring an all-examples+config batch + // also preserves the "edited example syncs as real data" property: any user action (edit, + // reorder, real task) adds a non-config op and skips the cleanup, so everything uploads. + if (isNeverSyncedAtSyncStart && (await this.opLogStore.hasSyncedOps())) { + const pendingOps = await this.opLogStore.getUnsynced(); + const isPristinePostBootBatch = pendingOps.every( + (entry) => + isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG', + ); + if (isPristinePostBootBatch) { + const exampleOpIds = pendingOps + .filter(isExampleTaskCreateOp) + .map((entry) => entry.op.id); + if (exampleOpIds.length > 0) { + await this._discardExampleTaskOps(exampleOpIds); + OpLog.normal( + `OperationLogSyncService: Discarded ${exampleOpIds.length} untouched example-task ` + + 'op(s) — never-synced client adopted a populated remote this sync (#7985).', + ); + } + } + } + // SERVER MIGRATION CHECK: Passed as callback to execute INSIDE the upload lock. // This prevents race conditions where multiple tabs could both detect migration // and create duplicate SYNC_IMPORT operations.