From 2261160436de202fc7e4641d1b823beea37bd620 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 2 Jul 2026 16:37:39 +0200 Subject: [PATCH] fix(sync): scan all tags when archiving to avoid dangling tag refs (#8710) * fix(sync): scan all tags when archiving to avoid dangling tag refs The archive path only cleaned tags named in each task's own tagIds, so a one-sided tag->task reference (tag.taskIds holds an id the task omits) left behind by a sync replay was never removed. That dangling reference later tripped cross-model validation and forced a reconciliation/REPAIR. Lift removeTasksFromAllTags into the shared helpers and reuse it in the archive path so cleanup is symmetric with the delete path (scans every tag). Add regression coverage. * refactor(store): route project-delete tag cleanup through shared helper Replace the hand-rolled all-tags scan in handleDeleteProject with the shared removeTasksFromAllTags helper, matching the delete and archive paths (and gaining its no-op skip for tags that don't reference the deleted tasks). Move the helper into the STATE UPDATE HELPERS section (it is a state->state transform, not a list helper) and let its JSDoc own the one-sided-tag-ref rationale, trimming the duplicated archive call-site comment to a pointer. No behavior change: final tag taskIds are identical; only unaffected tags now keep their reference identity instead of being rewritten to equal arrays. --- .../project-shared.reducer.ts | 19 ++------ .../task-shared-crud.reducer.ts | 23 +--------- .../task-shared-helpers.ts | 28 ++++++++++++ .../task-shared-lifecycle.reducer.spec.ts | 43 +++++++++++++++++++ .../task-shared-lifecycle.reducer.ts | 26 ++--------- 5 files changed, 80 insertions(+), 59 deletions(-) diff --git a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts index cb98515dbe..a784b5a2b7 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts @@ -3,23 +3,20 @@ import { Update } from '@ngrx/entity'; import { RootState } from '../../root-state'; import { TaskSharedActions } from '../task-shared.actions'; import { PROJECT_FEATURE_NAME } from '../../../features/project/store/project.reducer'; -import { TAG_FEATURE_NAME } from '../../../features/tag/store/tag.reducer'; import { TASK_FEATURE_NAME, taskAdapter, } from '../../../features/tasks/store/task.reducer'; import { TIME_TRACKING_FEATURE_KEY } from '../../../features/time-tracking/store/time-tracking.reducer'; import { TimeTrackingState } from '../../../features/time-tracking/time-tracking.model'; -import { Tag } from '../../../features/tag/tag.model'; import { Task, TaskWithSubTasks } from '../../../features/tasks/task.model'; import { unique } from '../../../util/unique'; import { ActionHandlerMap, getProject, - getTag, + removeTasksFromAllTags, removeTasksFromList, updateProject, - updateTags, } from './task-shared-helpers'; import { TASK_REPEAT_CFG_FEATURE_NAME } from '../../../features/task-repeat-cfg/store/task-repeat-cfg.selectors'; import { TaskRepeatCfgState } from '../../../features/task-repeat-cfg/task-repeat-cfg.model'; @@ -140,17 +137,9 @@ const handleDeleteProject = ( projectId: string, allTaskIds: string[], ): ExtendedState => { - const tagUpdates = (state[TAG_FEATURE_NAME].ids as string[]).map( - (tagId): Update => ({ - id: tagId, - changes: { - taskIds: removeTasksFromList(getTag(state, tagId).taskIds, allTaskIds), - }, - }), - ); - - // First update tags - const stateWithUpdatedTags = updateTags(state, tagUpdates) as ExtendedState; + // First strip the deleted project's tasks from every tag (same all-tags + // scan the delete/archive paths use — handles one-sided refs after sync). + const stateWithUpdatedTags = removeTasksFromAllTags(state, allTaskIds) as ExtendedState; // Cleanup TIME_TRACKING for deleted project const updatedTimeTracking = cleanupTimeTrackingForProject( diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts index 8ea4d1e425..7ab1eb23ca 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts @@ -39,6 +39,7 @@ import { ProjectTaskList, filterOutTodayTag, removeTaskFromPlannerDays, + removeTasksFromAllTags, removeTasksFromList, TaskWithTags, updateProject, @@ -140,28 +141,6 @@ const handleAddTask = ( return updatedState; }; -/** - * Removes the given task IDs from every tag's `taskIds`. Scans all tags from - * the CURRENT state (not a payload-provided list) so sync replays with - * divergent tag associations are fully cleaned up. Uses a Set for O(1) lookup. - * Returns state unchanged when no tag references the tasks. - */ -const removeTasksFromAllTags = (state: RootState, taskIds: string[]): RootState => { - const taskIdSet = new Set(taskIds); - const tagUpdates = (state[TAG_FEATURE_NAME].ids as string[]) - .map((tagId) => state[TAG_FEATURE_NAME].entities[tagId]) - .filter((tag): tag is Tag => !!tag && tag.taskIds.some((id) => taskIdSet.has(id))) - .map( - (tag): Update => ({ - id: tag.id, - changes: { - taskIds: removeTasksFromList(tag.taskIds, taskIds), - }, - }), - ); - return updateTags(state, tagUpdates); -}; - const handleConvertToMainTask = ( state: RootState, task: Task, diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts index d158ab1172..f101d98a36 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts @@ -46,6 +46,34 @@ export const updateTags = (state: RootState, updates: Update[]): RootState [TAG_FEATURE_NAME]: tagAdapter.updateMany(updates, state[TAG_FEATURE_NAME]), }); +/** + * Removes the given task IDs from every tag's `taskIds`. Scans ALL tags from + * the CURRENT state (not a payload-provided list or the task's own `tagIds`) + * so sync replays with divergent tag associations are fully cleaned up: a + * receiving client can hold a one-sided `tag.taskIds` → task reference even + * when the task's own `tagIds` omits that tag, and only scanning `tagIds` + * would leave it dangling. Uses a Set for O(1) lookup; tags that reference + * none of the ids are left untouched. + */ +export const removeTasksFromAllTags = ( + state: RootState, + taskIds: string[], +): RootState => { + const taskIdSet = new Set(taskIds); + const tagUpdates = (state[TAG_FEATURE_NAME].ids as string[]) + .map((tagId) => state[TAG_FEATURE_NAME].entities[tagId]) + .filter((tag): tag is Tag => !!tag && tag.taskIds.some((id) => taskIdSet.has(id))) + .map( + (tag): Update => ({ + id: tag.id, + changes: { + taskIds: removeTasksFromList(tag.taskIds, taskIds), + }, + }), + ); + return updateTags(state, tagUpdates); +}; + // ============================================================================= // ENTITY GETTERS // ============================================================================= diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.spec.ts index eec36eeed7..b0e6e2e88a 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.spec.ts @@ -296,6 +296,49 @@ describe('taskSharedLifecycleMetaReducer', () => { ); }); + // Regression: divergent/one-sided tag→task reference from a sync replay. + // A receiving client can end up with `tag.taskIds` containing a task id that + // is NOT listed in the task's own `tagIds`. The archive path used to only + // visit tags named in `task.tagIds`, so it left this reference dangling — + // which later tripped cross-model validation and forced a reconciliation + // (observed in SP-logs: an archived repeating-task instance still held by a + // regular tag). Cleanup must scan ALL tags, like the delete path does. + it('should remove archived task from a tag that references it even when the task.tagIds omits that tag', () => { + const testState = createStateWithExistingTasks( + ['rpt-task', 'keep-task'], + [], + ['rpt-task', 'keep-task'], // regular tag1 references rpt-task + [], // TODAY empty + ); + + // One-sided reference: the task itself does NOT list tag1. + testState[TASK_FEATURE_NAME].entities['rpt-task'] = createMockTask({ + id: 'rpt-task', + projectId: 'project1', + tagIds: [], + }); + + const tasksToArchive: TaskWithSubTasks[] = [ + createTaskWithSubTasks({ + id: 'rpt-task', + projectId: 'project1', + tagIds: [], // payload also omits the tag + }), + ]; + + const action = createArchiveAction(tasksToArchive); + + metaReducer(testState, action); + expectStateUpdate( + { + ...expectTagUpdate('tag1', { taskIds: ['keep-task'] }), + }, + action, + mockReducer, + testState, + ); + }); + it('should handle archiving tasks from multiple projects', () => { const testState = createStateWithExistingTasks( ['task1'], diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.ts index b3f7e1b3b3..10b04fb683 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-lifecycle.reducer.ts @@ -22,6 +22,7 @@ import { ActionHandlerMap, getProject, getTag, + removeTasksFromAllTags, removeTasksFromList, TaskEntity, updateTags, @@ -71,28 +72,9 @@ const handleMoveToArchive = (state: RootState, tasks: TaskWithSubTasks[]): RootS } } - // Get tag associations from CURRENT STATE for the same reason as above. - // Always include TODAY_TAG to ensure cleanup even if tasks aren't in it. - const affectedTagIds = unique([ - TODAY_TAG.id, - ...taskIdsToArchive.flatMap((taskId) => { - const task = state[TASK_FEATURE_NAME].entities[taskId]; - return task?.tagIds ?? []; - }), - ]); - - const tagUpdates = affectedTagIds - .filter((tagId) => !!state[TAG_FEATURE_NAME].entities[tagId]) - .map( - (tagId): Update => ({ - id: tagId, - changes: { - taskIds: removeTasksFromList(getTag(state, tagId).taskIds, taskIdsToArchive), - }, - }), - ); - - return updateTags(updatedState, tagUpdates); + // Scan every tag, not just each task's own `tagIds` — see + // removeTasksFromAllTags for why (one-sided tag refs after sync). + return removeTasksFromAllTags(updatedState, taskIdsToArchive); }; /**