From e3581add2f927239f5482e5a4d7baac421c2ace0 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Wed, 15 Jul 2026 16:55:33 +0200 Subject: [PATCH] fix(sync): recover project notes/sections/repeat-cfgs on losing delete (#9048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sync): recover project notes/sections/repeat-cfgs on losing delete When a legacy (pre-schema-v4) deleteProject loses an LWW race to a concurrent project UPDATE, the recovery path previously recreated only the project entity and its active/backlog tasks. Notes, sections, and task-repeat-cfgs stayed deleted, so every client converged to a lossy shape on hydration replay of the durable loser row — data loss against the winning "keep the project" intent (#9037). Recreate those three adapter entities at resolution time on the winner (which still holds them) via the generic lwwUpdateMetaReducer recreate path. Sections/repeat-cfgs are enumerated from the store (not carried in the delete payload) through the non-throwing selectEntities selectors; notes come from the delete payload's noteIds. Guard against resurrecting an entity another device is concurrently deleting, and strip dead task refs from recreated sections. Deferred (own snapshot design): time-tracking and menu-tree (singletons, no safe single-project restore) and archived tasks (separate persistence). Documented, converging limitations: section/repeat-cfg have no modified field so a concurrent content edit can be clobbered; a note's todayOrder slot is not restored. Tests: producer recreation + concurrency guard (conflict-resolution spec) and a SECTION recreate round-trip (lww-update meta-reducer spec). * test(sync): converge losing deleteProject cascade recovery (#9037) End-to-end convergence test through the real cascade reducers, op-log persistence, and conflict-resolution service: a losing deleteProject's cascade wipes the project's note, section, and repeat-cfg, and replaying the full durable log (delete + compensations) restores all three on both the restarted local client and the originating delete client. Closes the round-trip coverage gap left by the unit-level producer and meta-reducer specs. * test(sync): cover cross-batch deleteProject cascade recovery (#9037) Extract the losing-deleteProject convergence fixture into a shared helper and add a cross-batch case: the note/section/cfg recreations are replayed in a separate page from the durable loser and the project/task compensations. Proves the recreation ops carry no cross-batch dependency — they stay wiped after batch 1 (delete + project/task comps) and converge once the later batch arrives. --- ...resolution-persistence.integration.spec.ts | 297 ++++++++++++++++++ .../sync/conflict-resolution.service.spec.ts | 265 ++++++++++++++++ .../sync/conflict-resolution.service.ts | 210 +++++++++++++ .../lww-update.meta-reducer.spec.ts | 34 ++ 4 files changed, 806 insertions(+) diff --git a/src/app/op-log/sync/conflict-resolution-persistence.integration.spec.ts b/src/app/op-log/sync/conflict-resolution-persistence.integration.spec.ts index d38e68da0d..8f0e5253f7 100644 --- a/src/app/op-log/sync/conflict-resolution-persistence.integration.spec.ts +++ b/src/app/op-log/sync/conflict-resolution-persistence.integration.spec.ts @@ -10,6 +10,22 @@ import { TIME_TRACKING_FEATURE_KEY } from '../../features/time-tracking/store/ti import { RootState } from '../../root-store/root-state'; import { createCombinedTaskSharedMetaReducer } from '../../root-store/meta/task-shared-meta-reducers/test-helpers'; import { createBaseState } from '../../root-store/meta/task-shared-meta-reducers/test-utils'; +import { sectionSharedMetaReducer } from '../../root-store/meta/task-shared-meta-reducers/section-shared.reducer'; +import { + NOTE_FEATURE_NAME, + initialNoteState, + noteReducer, +} from '../../features/note/store/note.reducer'; +import { + SECTION_FEATURE_NAME, + initialSectionState, +} from '../../features/section/store/section.reducer'; +import { + TASK_REPEAT_CFG_FEATURE_NAME, + initialTaskRepeatCfgState, + taskRepeatCfgReducer, +} from '../../features/task-repeat-cfg/store/task-repeat-cfg.reducer'; +import { WorkContextType } from '../../features/work-context/work-context.model'; import { lwwUpdateMetaReducer } from '../../root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer'; import { bulkApplyOperations } from '../apply/bulk-hydration.action'; import { bulkOperationsMetaReducer } from '../apply/bulk-hydration.meta-reducer'; @@ -1014,6 +1030,287 @@ describe('ConflictResolutionService persistence (integration, real store)', () = ]); }); + // Shared fixture: a legacy deleteProject that loses LWW to a concurrent project + // edit, with the project holding a task, note, section and repeat-cfg. Returns + // the seeded pre-delete state, a full-slice replay reducer (wiring the cascades + // this suite's default reducer omits — sections via the section meta-reducer, + // notes/repeat-cfgs via their feature reducers, recreations via + // lwwUpdateMetaReducer), and the durable ops the service produced. + const setupLosingProjectDeleteFixture = async (): Promise<{ + noteId: string; + sectionId: string; + cfgId: string; + seededState: RootState; + replay: (state: RootState, operations: Operation[], clientId: string) => RootState; + hasEntity: (state: RootState, feature: string, id: string) => boolean; + storedOps: Operation[]; + localProjectEdit: Operation; + remoteProjectDelete: Operation; + }> => { + const noteId = 'note1'; + const sectionId = 'section1'; + const cfgId = 'cfg1'; + const rootTaskId = 'root-task'; + + const rootTask: Task = { + ...DEFAULT_TASK, + id: rootTaskId, + title: 'Root task', + projectId: 'project1', + subTaskIds: [], + } as Task; + const baseTaskState = createTaskReplayState([rootTask], [rootTaskId], []); + const seededState: RootState = { + ...baseTaskState, + [NOTE_FEATURE_NAME]: { + ...initialNoteState, + ids: [noteId], + entities: { + [noteId]: { id: noteId, content: 'Kept note', modified: 5_000 }, + }, + }, + [SECTION_FEATURE_NAME]: { + ...initialSectionState, + ids: [sectionId], + entities: { + [sectionId]: { + id: sectionId, + title: 'Project section', + contextType: WorkContextType.PROJECT, + contextId: 'project1', + taskIds: [rootTaskId], + }, + }, + }, + [TASK_REPEAT_CFG_FEATURE_NAME]: { + ...initialTaskRepeatCfgState, + ids: [cfgId], + entities: { [cfgId]: { id: cfgId, projectId: 'project1' } }, + }, + } as unknown as RootState; + const project = seededState[PROJECT_FEATURE_NAME].entities.project1; + if (!project) { + throw new Error('Test fixture project1 is missing.'); + } + + const cascadeRootReducer: ActionReducer = ( + state = seededState, + action, + ) => ({ + ...state, + [TASK_FEATURE_NAME]: taskReducer(state[TASK_FEATURE_NAME], action), + [NOTE_FEATURE_NAME]: noteReducer(state[NOTE_FEATURE_NAME], action), + [TASK_REPEAT_CFG_FEATURE_NAME]: taskRepeatCfgReducer( + state[TASK_REPEAT_CFG_FEATURE_NAME], + action, + ), + }); + const cascadeReplayReducer = bulkOperationsMetaReducer( + sectionSharedMetaReducer( + createCombinedTaskSharedMetaReducer(lwwUpdateMetaReducer(cascadeRootReducer)), + ), + ) as ActionReducer; + const replay = ( + state: RootState, + operations: Operation[], + clientId: string, + ): RootState => + cascadeReplayReducer( + state, + bulkApplyOperations({ operations, localClientId: clientId }), + ); + const hasEntity = (state: RootState, feature: string, id: string): boolean => + !!(state[feature as keyof RootState] as { entities: Record }) + .entities[id]; + + const localProjectEdit: Operation = { + id: 'local-project-edit', + actionType: toLwwUpdateActionType('PROJECT'), + opType: OpType.Update, + entityType: 'PROJECT', + entityId: 'project1', + payload: { + actionPayload: project as unknown as Record, + entityChanges: [], + lwwUpdateMode: 'replace', + }, + clientId: LOCAL_CLIENT_ID, + vectorClock: { [LOCAL_CLIENT_ID]: 1 }, + timestamp: 2_000, + schemaVersion: CURRENT_SCHEMA_VERSION, + }; + const remoteProjectDelete: Operation = { + id: 'remote-project-delete', + actionType: ActionType.TASK_SHARED_DELETE_PROJECT, + opType: OpType.Delete, + entityType: 'PROJECT', + entityId: 'project1', + payload: { + actionPayload: { + projectId: 'project1', + allTaskIds: [rootTaskId], + noteIds: [noteId], + }, + entityChanges: [], + }, + clientId: REMOTE_CLIENT_ID, + vectorClock: { [REMOTE_CLIENT_ID]: 1 }, + timestamp: 1_000, + schemaVersion: CURRENT_SCHEMA_VERSION, + }; + + const registry = TestBed.inject(ENTITY_REGISTRY); + const noteSel = registry.NOTE!.selectEntities; + const sectionSel = registry.SECTION!.selectEntities; + const cfgSel = registry.TASK_REPEAT_CFG!.selectEntities; + // Resolution-time snapshot the recovery reads (the winner still holds all). + store.select.and.callFake((selector: unknown, props?: { id?: string }) => { + if (props?.id === 'project1') return of(project); + if (props?.id === rootTaskId) return of(rootTask); + if (selector === noteSel) { + return of(seededState[NOTE_FEATURE_NAME].entities); + } + if (selector === sectionSel) { + return of(seededState[SECTION_FEATURE_NAME].entities); + } + if (selector === cfgSel) { + return of(seededState[TASK_REPEAT_CFG_FEATURE_NAME].entities); + } + return of(undefined); + }); + await opLogStore.append(localProjectEdit, 'local'); + + await service.autoResolveConflictsLWW([ + { + entityType: 'PROJECT', + entityId: 'project1', + localOps: [localProjectEdit], + remoteOps: [remoteProjectDelete], + suggestedResolution: 'manual', + }, + ]); + + const storedOps = (await opLogStore.getOpsAfterSeq(0)).map(({ op }) => op); + return { + noteId, + sectionId, + cfgId, + seededState, + replay, + hasEntity, + storedOps, + localProjectEdit, + remoteProjectDelete, + }; + }; + + it('converges a losing deleteProject by recreating notes, sections and repeat-cfgs (#9037)', async () => { + const { + noteId, + sectionId, + cfgId, + seededState, + replay, + hasEntity, + storedOps, + localProjectEdit, + remoteProjectDelete, + } = await setupLosingProjectDeleteFixture(); + + // Sanity: the recovery emitted a recreation op for each victim type. + expect(storedOps.some((op) => op.entityType === 'NOTE')).toBeTrue(); + expect(storedOps.some((op) => op.entityType === 'SECTION')).toBeTrue(); + expect(storedOps.some((op) => op.entityType === 'TASK_REPEAT_CFG')).toBeTrue(); + + // The delete cascade alone wipes all three — proves the recreation does real work. + const afterDeleteOnly = replay(seededState, [remoteProjectDelete], REMOTE_CLIENT_ID); + expect(hasEntity(afterDeleteOnly, NOTE_FEATURE_NAME, noteId)).toBeFalse(); + expect(hasEntity(afterDeleteOnly, SECTION_FEATURE_NAME, sectionId)).toBeFalse(); + expect(hasEntity(afterDeleteOnly, TASK_REPEAT_CFG_FEATURE_NAME, cfgId)).toBeFalse(); + + // Replaying the full durable log (delete + compensations) restores all three + // on the restarted local client — the convergence the fix guarantees. + const restartedClientState = replay(seededState, storedOps, LOCAL_CLIENT_ID); + expect(hasEntity(restartedClientState, NOTE_FEATURE_NAME, noteId)) + .withContext('note recreated on restart') + .toBeTrue(); + expect(hasEntity(restartedClientState, SECTION_FEATURE_NAME, sectionId)) + .withContext('section recreated on restart') + .toBeTrue(); + expect(hasEntity(restartedClientState, TASK_REPEAT_CFG_FEATURE_NAME, cfgId)) + .withContext('repeat-cfg recreated on restart') + .toBeTrue(); + + // The originating (delete) client converges to the same shape: apply its own + // delete, then the winner's compensations. + const compensationOps = storedOps.filter( + (op) => op.id !== localProjectEdit.id && op.id !== remoteProjectDelete.id, + ); + let remoteClientState = replay(seededState, [remoteProjectDelete], REMOTE_CLIENT_ID); + remoteClientState = replay(remoteClientState, compensationOps, REMOTE_CLIENT_ID); + expect(hasEntity(remoteClientState, NOTE_FEATURE_NAME, noteId)) + .withContext('note recreated on originating client') + .toBeTrue(); + expect(hasEntity(remoteClientState, SECTION_FEATURE_NAME, sectionId)) + .withContext('section recreated on originating client') + .toBeTrue(); + expect(hasEntity(remoteClientState, TASK_REPEAT_CFG_FEATURE_NAME, cfgId)) + .withContext('repeat-cfg recreated on originating client') + .toBeTrue(); + }); + + it('converges when the note/section/cfg recreations arrive in a later sync batch (#9037)', async () => { + const { + noteId, + sectionId, + cfgId, + seededState, + replay, + hasEntity, + storedOps, + localProjectEdit, + remoteProjectDelete, + } = await setupLosingProjectDeleteFixture(); + + const cascadeTypes = new Set(['NOTE', 'SECTION', 'TASK_REPEAT_CFG']); + const compensationOps = storedOps.filter( + (op) => op.id !== localProjectEdit.id && op.id !== remoteProjectDelete.id, + ); + const cascadeRecreations = compensationOps.filter((op) => + cascadeTypes.has(op.entityType), + ); + const projectAndTaskComps = compensationOps.filter( + (op) => !cascadeTypes.has(op.entityType), + ); + expect(cascadeRecreations.length).toBeGreaterThan(0); + + // Batch 1: durable loser + project/task compensations only. Pagination puts + // the note/section/cfg recreations on a later page, so they stay wiped for now. + let state = replay( + seededState, + [remoteProjectDelete, ...projectAndTaskComps], + LOCAL_CLIENT_ID, + ); + expect(hasEntity(state, NOTE_FEATURE_NAME, noteId)) + .withContext('note still absent before its later batch') + .toBeFalse(); + expect(hasEntity(state, SECTION_FEATURE_NAME, sectionId)).toBeFalse(); + expect(hasEntity(state, TASK_REPEAT_CFG_FEATURE_NAME, cfgId)).toBeFalse(); + + // Batch 2: the cascade recreations arrive in a separate page and converge + // the state — they carry no cross-batch dependency on the delete/other comps. + state = replay(state, cascadeRecreations, LOCAL_CLIENT_ID); + expect(hasEntity(state, NOTE_FEATURE_NAME, noteId)) + .withContext('note recreated from a later batch') + .toBeTrue(); + expect(hasEntity(state, SECTION_FEATURE_NAME, sectionId)) + .withContext('section recreated from a later batch') + .toBeTrue(); + expect(hasEntity(state, TASK_REPEAT_CFG_FEATURE_NAME, cfgId)) + .withContext('repeat-cfg recreated from a later batch') + .toBeTrue(); + }); + it('keeps concurrently bulk-deleted tasks deleted on every client during project recovery (#8997)', async () => { // A wins a project rename vs B's deleteProject (B loses LWW). In the same // batch, C's bulk deleteTasks([bulk-1, bulk-2]) lands as a non-conflicting diff --git a/src/app/op-log/sync/conflict-resolution.service.spec.ts b/src/app/op-log/sync/conflict-resolution.service.spec.ts index 86e8c85404..154a83e1b9 100644 --- a/src/app/op-log/sync/conflict-resolution.service.spec.ts +++ b/src/app/op-log/sync/conflict-resolution.service.spec.ts @@ -27,6 +27,7 @@ import { CLIENT_ID_PROVIDER } from '../util/client-id.provider'; import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service'; import { MAX_VECTOR_CLOCK_SIZE } from '../core/operation-log.const'; import { buildEntityRegistry, ENTITY_REGISTRY } from '../core/entity-registry'; +import { WorkContextType } from '../../features/work-context/work-context.model'; import { OperationLogEffects } from '../capture/operation-log.effects'; import { IncompleteRemoteOperationsError } from '../core/errors/sync-errors'; import { ConflictJournalService } from './conflict-journal.service'; @@ -1879,6 +1880,270 @@ describe('ConflictResolutionService', () => { expect(mockOperationApplier.applyOperations).not.toHaveBeenCalled(); }); + it('recreates notes, sections and repeat-cfgs of a losing deleteProject (#9037)', async () => { + const remoteProjectDelete: Operation = { + ...createOpWithTimestamp( + 'remote-project-delete', + 'client-b', + 1_000, + OpType.Delete, + 'project-1', + ), + actionType: ActionType.TASK_SHARED_DELETE_PROJECT, + entityType: 'PROJECT', + payload: { + actionPayload: { + projectId: 'project-1', + allTaskIds: [], + noteIds: ['noteA'], + }, + entityChanges: [], + }, + }; + const localProjectEdit: Operation = { + ...createOpWithTimestamp( + 'local-project-edit', + 'client-a', + 2_000, + OpType.Update, + 'project-1', + ), + entityType: 'PROJECT', + }; + const noteSel = mockEntityRegistry.NOTE!.selectEntities; + const sectionSel = mockEntityRegistry.SECTION!.selectEntities; + const cfgSel = mockEntityRegistry.TASK_REPEAT_CFG!.selectEntities; + mockStore.select.and.callFake((selector: unknown, props?: { id: string }) => { + if (props?.id === 'project-1') { + return of({ + id: 'project-1', + title: 'Winning project', + taskIds: [], + backlogTaskIds: [], + }); + } + if (selector === noteSel) { + return of({ + noteA: { id: 'noteA', content: 'Kept note', modified: 5_000 }, + }); + } + if (selector === sectionSel) { + return of({ + sectionA: { + id: 'sectionA', + title: 'Project section', + contextType: WorkContextType.PROJECT, + contextId: 'project-1', + taskIds: [], + }, + // Belongs to a different project — must NOT be recreated. + sectionOther: { + id: 'sectionOther', + title: 'Other section', + contextType: WorkContextType.PROJECT, + contextId: 'project-2', + taskIds: [], + }, + }); + } + if (selector === cfgSel) { + return of({ + cfgA: { id: 'cfgA', title: 'Repeat', projectId: 'project-1' }, + // Different project — must NOT be recreated. + cfgOther: { id: 'cfgOther', title: 'Other', projectId: 'project-2' }, + }); + } + return of(undefined); + }); + + await service.autoResolveConflictsLWW([ + { + entityType: 'PROJECT', + entityId: 'project-1', + localOps: [localProjectEdit], + remoteOps: [remoteProjectDelete], + suggestedResolution: 'manual', + }, + ]); + + const localOps = getMixedLocalOps(); + const projectComps = localOps.filter((op) => op.entityType === 'PROJECT'); + const noteRecreations = localOps.filter((op) => op.entityType === 'NOTE'); + const sectionRecreations = localOps.filter((op) => op.entityType === 'SECTION'); + const cfgRecreations = localOps.filter( + (op) => op.entityType === 'TASK_REPEAT_CFG', + ); + + expect(noteRecreations.map(({ entityId }) => entityId)).toEqual(['noteA']); + expect(sectionRecreations.map(({ entityId }) => entityId)).toEqual(['sectionA']); + expect(cfgRecreations.map(({ entityId }) => entityId)).toEqual(['cfgA']); + + for (const op of [...noteRecreations, ...sectionRecreations, ...cfgRecreations]) { + expect( + (op.payload as { recreatesEntityAfterDelete?: boolean }) + .recreatesEntityAfterDelete, + ) + .withContext(`recreate flag on ${op.entityType}:${op.entityId}`) + .toBeTrue(); + expect((op.payload as { lwwUpdateMode?: string }).lwwUpdateMode) + .withContext(`replace mode on ${op.entityType}:${op.entityId}`) + .toBe('replace'); + expect(compareVectorClocks(op.vectorClock, remoteProjectDelete.vectorClock)) + .withContext(`clock domination for ${op.entityType}:${op.entityId}`) + .toBe(VectorClockComparison.GREATER_THAN); + } + // The note carries `modified`, so its own timestamp is preserved (protects a + // concurrent note edit). Sections/cfgs have no `modified`, so they fall back + // to the project compensation timestamp. + expect(noteRecreations[0].timestamp).toBe(5_000); + expect(sectionRecreations[0].timestamp).toBe(projectComps[0].timestamp); + expect(cfgRecreations[0].timestamp).toBe(projectComps[0].timestamp); + expect(extractActionPayload(noteRecreations[0].payload)['id']).toBe('noteA'); + }); + + it('skips notes/sections/cfgs concurrently deleted in-batch and strips dead section taskIds (#9037)', async () => { + const remoteProjectDelete: Operation = { + ...createOpWithTimestamp( + 'remote-project-delete', + 'client-b', + 1_000, + OpType.Delete, + 'project-1', + ), + actionType: ActionType.TASK_SHARED_DELETE_PROJECT, + entityType: 'PROJECT', + payload: { + actionPayload: { + projectId: 'project-1', + allTaskIds: ['task-live'], + noteIds: ['noteKeep', 'noteGone'], + }, + entityChanges: [], + }, + }; + const localProjectEdit: Operation = { + ...createOpWithTimestamp( + 'local-project-edit', + 'client-a', + 2_000, + OpType.Update, + 'project-1', + ), + entityType: 'PROJECT', + }; + // Concurrent non-conflicting deletes from a third device, not yet applied + // to the pre-batch store this recovery reads. + const concurrentNoteDelete: Operation = { + ...createOpWithTimestamp( + 'del-note', + 'client-c', + 1_500, + OpType.Delete, + 'noteGone', + ), + entityType: 'NOTE', + actionType: ActionType.TASK_SHARED_DELETE, + }; + const concurrentSectionDelete: Operation = { + ...createOpWithTimestamp( + 'del-sec', + 'client-c', + 1_500, + OpType.Delete, + 'sectionGone', + ), + entityType: 'SECTION', + actionType: ActionType.TASK_SHARED_DELETE, + }; + const concurrentTaskDelete: Operation = { + ...createOpWithTimestamp( + 'del-task', + 'client-c', + 1_500, + OpType.Delete, + 'task-gone', + ), + entityType: 'TASK', + actionType: ActionType.TASK_SHARED_DELETE, + }; + const noteSel = mockEntityRegistry.NOTE!.selectEntities; + const sectionSel = mockEntityRegistry.SECTION!.selectEntities; + const cfgSel = mockEntityRegistry.TASK_REPEAT_CFG!.selectEntities; + mockStore.select.and.callFake((selector: unknown, props?: { id: string }) => { + if (props?.id === 'project-1') { + return of({ + id: 'project-1', + title: 'Winning project', + taskIds: ['task-live'], + backlogTaskIds: [], + }); + } + if (props?.id === 'task-live') { + return of({ + id: 'task-live', + title: 'Live task', + projectId: 'project-1', + subTaskIds: [], + }); + } + if (selector === noteSel) { + return of({ + noteKeep: { id: 'noteKeep', content: 'Keep', modified: 5_000 }, + noteGone: { id: 'noteGone', content: 'Gone', modified: 5_000 }, + }); + } + if (selector === sectionSel) { + return of({ + sectionKeep: { + id: 'sectionKeep', + title: 'Keep', + contextType: WorkContextType.PROJECT, + contextId: 'project-1', + taskIds: ['task-live', 'task-gone'], + }, + sectionGone: { + id: 'sectionGone', + title: 'Gone', + contextType: WorkContextType.PROJECT, + contextId: 'project-1', + taskIds: [], + }, + }); + } + if (selector === cfgSel) { + return of({}); + } + return of(undefined); + }); + + await service.autoResolveConflictsLWW( + [ + { + entityType: 'PROJECT', + entityId: 'project-1', + localOps: [localProjectEdit], + remoteOps: [remoteProjectDelete], + suggestedResolution: 'manual', + }, + ], + [concurrentNoteDelete, concurrentSectionDelete, concurrentTaskDelete], + ); + + const localOps = getMixedLocalOps(); + const noteRecreations = localOps.filter((op) => op.entityType === 'NOTE'); + const sectionRecreations = localOps.filter((op) => op.entityType === 'SECTION'); + + // note-gone / section-gone were concurrently deleted → not resurrected. + expect(noteRecreations.map(({ entityId }) => entityId)).toEqual(['noteKeep']); + expect(sectionRecreations.map(({ entityId }) => entityId)).toEqual([ + 'sectionKeep', + ]); + // The surviving section drops its ref to the concurrently-deleted task. + expect(extractActionPayload(sectionRecreations[0].payload)['taskIds']).toEqual([ + 'task-live', + ]); + }); + it('does not resurrect a task deleted by a concurrent non-conflicting op (#8997 review)', async () => { // Device A wins a project rename vs Device B's deleteProject(P) (loses // LWW). In the SAME sync batch, Device C's independent deleteTask lands diff --git a/src/app/op-log/sync/conflict-resolution.service.ts b/src/app/op-log/sync/conflict-resolution.service.ts index 058d18d8e1..775231c906 100644 --- a/src/app/op-log/sync/conflict-resolution.service.ts +++ b/src/app/op-log/sync/conflict-resolution.service.ts @@ -41,6 +41,7 @@ import { } 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 { WorkContextType } from '../../features/work-context/work-context.model'; import { OperationApplierService } from '../apply/operation-applier.service'; import { HydrationStateService } from '../apply/hydration-state.service'; import { @@ -1193,6 +1194,16 @@ export class ConflictResolutionService { remoteOp, concurrentlyDeletedTaskIds, )), + // Emitted after the task recreations so sections referencing recreated + // tasks land in seq order after them (#9037). + ...(await this._createCascadeRecreationOpsForWinningProject( + parentCompensationOp, + remoteOp, + { + concurrentlyDeletedTaskIds, + batchOps: [...nonConflictingOps, ...remoteDeleteWinnerOps], + }, + )), ]; // Not queued for live apply: the pure loser is never applied live, so // this client's state already holds the cascade victims. The rows @@ -3038,6 +3049,205 @@ export class ConflictResolutionService { return deletedTaskIds; } + /** + * Collects the ids removed by single/bulk DELETE ops of one entity type in the + * same resolution batch. Unlike `_collectDeletedTaskIds` this does not scan + * multi-entity `entityChanges`: `deleteNote`/`deleteSection`/`deleteTaskRepeatCfg(s)` + * are all single- or bulk-entity deletes, so `getOpEntityIds` covers them. Used + * to keep the project cascade recovery from resurrecting a note/section/repeat-cfg + * another device is concurrently deleting (same divergence guard as tasks, #8997). + */ + private _collectDeletedEntityIds( + ops: readonly Operation[], + entityType: EntityType, + ): Set { + const deletedIds = new Set(); + for (const op of ops) { + if (op.entityType === entityType && op.opType === OpType.Delete) { + for (const id of getOpEntityIds(op)) deletedIds.add(id); + } + } + return deletedIds; + } + + /** + * Reads the full current entity dictionary for an adapter entity type from the + * store. Used to enumerate a deleted project's still-present sections and repeat + * configs at resolution time (they are not carried in the `deleteProject` + * payload). Returns `{}` for non-adapter types or when no selector is + * registered. Unlike `getCurrentEntityState`, this never routes through the + * per-id selectors, some of which THROW on a missing id (`selectNoteById`, + * `selectTaskRepeatCfgById`) — enumerating a stale id set would spam errors. + */ + private async _getCurrentEntitiesOfType( + entityType: EntityType, + ): Promise> { + const config = getEntityConfigFromRegistry(this.entityRegistry, entityType); + if (!config || !isAdapterEntity(config) || !config.selectEntities) { + return {}; + } + const dict = await firstValueFrom(this.store.select(config.selectEntities)); + return (dict as Record) ?? {}; + } + + /** + * Recreates the non-task main-state cascade victims of a losing remote + * `deleteProject` that the task-recovery path leaves deleted: notes, sections, + * and task-repeat-cfgs. Runs alongside `_createTaskRecreationOpsForWinningProject` + * on the winner, which still holds every victim at resolution time (the losing + * delete is never applied live). Without this, the durable loser row replays on + * every client's status-blind hydration and strips these entities, so the whole + * fleet converges to a lossy shape (#9037) even though the winning UPDATE meant + * "keep the project". + * + * All three are adapter entities recreated by `lwwUpdateMetaReducer`'s generic + * `addOne` path (TASK-only logic is gated there). One shared `recreationClock` + * dominates the delete; every op targets a distinct id, so they never conflict + * with each other. + * + * KNOWN LIMITATIONS (all converge — no split-brain — and are strictly better + * than losing the entity outright): + * - Sections and repeat-cfgs have no `modified` field, so their LWW timestamp + * falls back to the project timestamp; a CONCURRENT content edit on another + * device can be clobbered by the replace snapshot. Notes carry `modified`, + * which keeps a concurrent note edit winning. + * - A note's `NoteState.todayOrder` slot is not restored (the adapter recreate + * only touches `entities`/`ids`); a today-pinned note reappears but loses its + * today-list ordering. Project-level note membership IS restored via the + * project compensation snapshot's `noteIds`. + * - A section/cfg concurrently ADDED on a third device (not yet applied, so not + * enumerable here) is still removed by the loser's dynamic-filter replay. + * - Like the task path, the merged delete clock can over-resurrect an entity a + * third device had already durably deleted before the losing delete synced. + * + * Archived tasks, archive time-tracking, current time-tracking, and menu-tree + * stay outside this path (separate persistence / singleton state) — their own + * snapshot design is deferred (#9037). + */ + private async _createCascadeRecreationOpsForWinningProject( + projectCompensationOp: Operation, + remoteDeleteOp: Operation, + guard: { + concurrentlyDeletedTaskIds: ReadonlySet; + batchOps: readonly Operation[]; + }, + ): Promise { + if ( + projectCompensationOp.entityType !== 'PROJECT' || + remoteDeleteOp.entityType !== 'PROJECT' || + remoteDeleteOp.actionType !== ActionType.TASK_SHARED_DELETE_PROJECT || + !projectCompensationOp.entityId + ) { + return []; + } + const projectId = projectCompensationOp.entityId; + + const clientId = await this.clientIdProvider.loadClientId(); + if (!clientId) { + OpLog.err( + 'ConflictResolutionService: Cannot recreate winning project cascade - no client ID', + ); + return []; + } + const recreationClock = this.mergeAndIncrementClocks( + [remoteDeleteOp.vectorClock, projectCompensationOp.vectorClock], + clientId, + ); + + const buildRecreationOp = ( + entityType: EntityType, + entityId: string, + entityState: Record, + ): Operation => { + const modified = entityState['modified']; + return markLwwDeleteRecreation( + this.createLWWUpdateOp( + entityType, + entityId, + entityState, + clientId, + recreationClock, + typeof modified === 'number' ? modified : projectCompensationOp.timestamp, + ), + ); + }; + + const recreationOps: Operation[] = []; + + // Notes: enumerable from the delete payload's `noteIds`. Guard the array for + // legacy pre-`noteIds` deleteProject ops (the #9037 rollout window is exactly + // legacy deletes losing). + const deletePayload = extractActionPayload(remoteDeleteOp.payload); + const noteIds = deletePayload['noteIds']; + if (Array.isArray(noteIds) && noteIds.length > 0) { + const noteEntities = await this._getCurrentEntitiesOfType('NOTE' as EntityType); + const deletedNoteIds = this._collectDeletedEntityIds( + guard.batchOps, + 'NOTE' as EntityType, + ); + for (const noteId of noteIds) { + if (typeof noteId !== 'string' || deletedNoteIds.has(noteId)) continue; + const note = noteEntities[noteId] as Record | undefined; + if (!note || note['id'] !== noteId) continue; + recreationOps.push(buildRecreationOp('NOTE' as EntityType, noteId, note)); + } + } + + // Sections: not in the payload — scan the store for project-owned sections + // (same predicate as `removeProjectSections`). Strip taskIds pointing at a + // concurrently-deleted task so the recreated section carries no dangling ref. + const sectionEntities = await this._getCurrentEntitiesOfType('SECTION' as EntityType); + const deletedSectionIds = this._collectDeletedEntityIds( + guard.batchOps, + 'SECTION' as EntityType, + ); + for (const [sectionId, sectionRaw] of Object.entries(sectionEntities)) { + const section = sectionRaw as Record; + if ( + section['id'] !== sectionId || + section['contextType'] !== WorkContextType.PROJECT || + section['contextId'] !== projectId || + deletedSectionIds.has(sectionId) + ) { + continue; + } + const taskIds = section['taskIds']; + const cleanedSection = Array.isArray(taskIds) + ? { + ...section, + taskIds: taskIds.filter( + (id) => typeof id === 'string' && !guard.concurrentlyDeletedTaskIds.has(id), + ), + } + : section; + recreationOps.push( + buildRecreationOp('SECTION' as EntityType, sectionId, cleanedSection), + ); + } + + // Task-repeat-cfgs: not in the payload — scan the store by projectId. + const repeatCfgEntities = await this._getCurrentEntitiesOfType( + 'TASK_REPEAT_CFG' as EntityType, + ); + const deletedRepeatCfgIds = this._collectDeletedEntityIds( + guard.batchOps, + 'TASK_REPEAT_CFG' as EntityType, + ); + for (const [cfgId, cfgRaw] of Object.entries(repeatCfgEntities)) { + const cfg = cfgRaw as Record; + if ( + cfg['id'] !== cfgId || + cfg['projectId'] !== projectId || + deletedRepeatCfgIds.has(cfgId) + ) { + continue; + } + recreationOps.push(buildRecreationOp('TASK_REPEAT_CFG' as EntityType, cfgId, cfg)); + } + + return recreationOps; + } + /** * Recreates the active tasks removed by a losing remote `deleteProject`. * diff --git a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts index 334be15c16..a5b9b7d2c0 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/lww-update.meta-reducer.spec.ts @@ -333,6 +333,40 @@ describe('lwwUpdateMetaReducer', () => { expect(recreatedTask.doneOn).toBe(12345); }); + it('recreates a deleted SECTION from a [SECTION] LWW Update (project-delete recovery #9037)', () => { + const state = createMockState(); + // The losing deleteProject cascade removed the project's section locally. + state[SECTION_FEATURE_NAME] = { ids: [], entities: {} }; + const action = { + type: '[SECTION] LWW Update', + id: SECTION_ID, + contextId: PROJECT_ID, + contextType: WorkContextType.PROJECT, + title: 'Recovered Section', + taskIds: ['task-1'], + meta: { + isPersistent: true, + entityType: 'SECTION', + entityId: SECTION_ID, + recreatesEntityAfterDelete: true, + lwwUpdateMode: 'replace', + }, + }; + + reducer(state, action); + + const updatedState = mockReducer.calls.mostRecent().args[0] as Partial; + const section = updatedState[SECTION_FEATURE_NAME]?.entities[SECTION_ID] as Section; + expect(section).toBeDefined(); + expect(section.id).toBe(SECTION_ID); + expect(section.title).toBe('Recovered Section'); + expect(section.contextId).toBe(PROJECT_ID); + // Sections are not orphan-filtered by the meta-reducer; the snapshot's + // taskIds are preserved verbatim (producer strips dead refs upstream). + expect(section.taskIds).toEqual(['task-1']); + expect(updatedState[SECTION_FEATURE_NAME]?.ids).toContain(SECTION_ID); + }); + it('should add recreated entity to the ids array', () => { const state = createMockState(); const action = {