From 1fab6da05e6dfe1f9678548bdd1195c804c8d9d0 Mon Sep 17 00:00:00 2001 From: Cristian Tcaci <59696583+Chris0Jeky@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:20:00 +0100 Subject: [PATCH 1/3] test(board): pin per-card move/delete settlement ordering --- .../cardStoreMoveMutationOrdering.spec.ts | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) create mode 100644 frontend/taskdeck-web/src/tests/store/board/cardStoreMoveMutationOrdering.spec.ts diff --git a/frontend/taskdeck-web/src/tests/store/board/cardStoreMoveMutationOrdering.spec.ts b/frontend/taskdeck-web/src/tests/store/board/cardStoreMoveMutationOrdering.spec.ts new file mode 100644 index 000000000..4a479cb28 --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/board/cardStoreMoveMutationOrdering.spec.ts @@ -0,0 +1,231 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ref } from 'vue' + +const { mockCardsApi } = vi.hoisted(() => ({ + mockCardsApi: { + setArchived: vi.fn(), + getCards: vi.fn(), + createCard: vi.fn(), + updateCard: vi.fn(), + deleteCard: vi.fn(), + moveCard: vi.fn(), + getCardProvenance: vi.fn(), + }, +})) + +vi.mock('../../../api/cardsApi', () => ({ cardsApi: mockCardsApi })) +vi.mock('../../../utils/errorMessage', () => ({ + getErrorMessage: vi.fn((_error: unknown, fallback: string) => fallback), +})) + +import { createCardActions } from '../../../store/board/cardStore' + +interface TestCard { + id: string + boardId: string + columnId: string + title: string + description: string + dueDate: null + isBlocked: boolean + blockReason: null + position: number + labels: never[] + createdAt: string + updatedAt: string +} + +const originalCard: TestCard = { + id: 'card-1', + boardId: 'board-1', + columnId: 'col-a', + title: 'Ship release', + description: '', + dueDate: null, + isBlocked: false, + blockReason: null, + position: 0, + labels: [], + createdAt: '2026-09-20T10:00:00Z', + updatedAt: '2026-09-20T10:00:00Z', +} + +function createState() { + return { + currentBoard: ref<{ + id: string + columns: Array<{ id: string; name: string; cardCount: number }> + } | null>({ + id: 'board-1', + columns: [ + { id: 'col-a', name: 'Todo', cardCount: 1 }, + { id: 'col-b', name: 'Doing', cardCount: 0 }, + { id: 'col-c', name: 'Done', cardCount: 0 }, + ], + }), + currentBoardCards: ref([{ ...originalCard }]), + cardCommentsByCardId: ref>({ + 'card-1': [{ id: 'comment-1' }], + }), + loading: ref(false), + error: ref(null), + } +} + +function createHelpers(state: ReturnType) { + const updateColumnCardCount = vi.fn((columnId: string, delta: number) => { + const column = state.currentBoard.value?.columns.find(candidate => candidate.id === columnId) + if (column) column.cardCount += delta + }) + + return { + guardDemoMutation: vi.fn(), + handleApiError: vi.fn(), + isHttpConflict: vi.fn().mockReturnValue(false), + isDemoMode: false, + toast: { success: vi.fn(), error: vi.fn() }, + updateColumnCardCount, + markBoardDetailMutation: vi.fn(), + } +} + +function deferred() { + let resolve!: (value: T) => void + let reject!: (reason?: unknown) => void + const promise = new Promise((settle, fail) => { + resolve = settle + reject = fail + }) + return { promise, resolve, reject } +} + +async function flushPromises() { + await Promise.resolve() + await Promise.resolve() +} + +function moved(columnId: string, updatedAt: string): TestCard { + return { ...originalCard, columnId, updatedAt } +} + +function counts(state: ReturnType) { + return state.currentBoard.value!.columns.map(column => column.cardCount) +} + +describe('cardStore move/delete mutation ordering', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCardsApi.moveCard.mockReset() + mockCardsApi.deleteCard.mockReset() + }) + + it('ignores an older move response after authoritative state advanced to another column', async () => { + const state = createState() + const helpers = createHelpers(state) + const firstMove = deferred() + mockCardsApi.moveCard.mockReturnValueOnce(firstMove.promise) + const actions = createCardActions(state as never, helpers as never, vi.fn().mockResolvedValue(true)) + + const pendingMove = actions.moveCard('board-1', 'card-1', 'col-b', 0) + + state.currentBoardCards.value[0] = moved('col-c', '2026-09-20T10:02:00Z') + state.currentBoard.value!.columns[0].cardCount = 0 + state.currentBoard.value!.columns[2].cardCount = 1 + + firstMove.resolve(moved('col-b', '2026-09-20T10:01:00Z')) + await pendingMove + + expect(state.currentBoardCards.value).toEqual([ + moved('col-c', '2026-09-20T10:02:00Z'), + ]) + expect(counts(state)).toEqual([0, 0, 1]) + expect(helpers.updateColumnCardCount).not.toHaveBeenCalled() + }) + + it('serializes two accepted moves so the later intent settles last', async () => { + const state = createState() + const helpers = createHelpers(state) + const firstMove = deferred() + const secondMove = deferred() + mockCardsApi.moveCard + .mockReturnValueOnce(firstMove.promise) + .mockReturnValueOnce(secondMove.promise) + const actions = createCardActions(state as never, helpers as never, vi.fn().mockResolvedValue(true)) + + const pendingFirst = actions.moveCard('board-1', 'card-1', 'col-b', 0) + const pendingSecond = actions.moveCard('board-1', 'card-1', 'col-c', 0) + + await flushPromises() + expect(mockCardsApi.moveCard).toHaveBeenCalledTimes(1) + + firstMove.resolve(moved('col-b', '2026-09-20T10:01:00Z')) + await pendingFirst + await flushPromises() + expect(mockCardsApi.moveCard).toHaveBeenCalledTimes(2) + expect(state.currentBoardCards.value).toEqual([ + moved('col-b', '2026-09-20T10:01:00Z'), + ]) + expect(counts(state)).toEqual([0, 1, 0]) + + secondMove.resolve(moved('col-c', '2026-09-20T10:02:00Z')) + await pendingSecond + + expect(state.currentBoardCards.value).toEqual([ + moved('col-c', '2026-09-20T10:02:00Z'), + ]) + expect(counts(state)).toEqual([0, 0, 1]) + }) + + it('does not let a move response reinsert a card deleted by the later intent', async () => { + const state = createState() + const helpers = createHelpers(state) + const firstMove = deferred() + mockCardsApi.moveCard.mockReturnValueOnce(firstMove.promise) + mockCardsApi.deleteCard.mockResolvedValueOnce(undefined) + const actions = createCardActions(state as never, helpers as never, vi.fn().mockResolvedValue(true)) + + const pendingMove = actions.moveCard('board-1', 'card-1', 'col-b', 0) + const pendingDelete = actions.deleteCard('board-1', 'card-1') + + await flushPromises() + expect(mockCardsApi.moveCard).toHaveBeenCalledTimes(1) + expect(mockCardsApi.deleteCard).not.toHaveBeenCalled() + + firstMove.resolve(moved('col-b', '2026-09-20T10:01:00Z')) + await pendingMove + await pendingDelete + + expect(mockCardsApi.deleteCard).toHaveBeenCalledTimes(1) + expect(state.currentBoardCards.value).toEqual([]) + expect(counts(state)).toEqual([0, 0, 0]) + expect(state.cardCommentsByCardId.value).not.toHaveProperty('card-1') + }) + + it('keeps the earlier confirmed move when the later serialized move fails', async () => { + const state = createState() + const helpers = createHelpers(state) + const firstMove = deferred() + const secondMove = deferred() + mockCardsApi.moveCard + .mockReturnValueOnce(firstMove.promise) + .mockReturnValueOnce(secondMove.promise) + const actions = createCardActions(state as never, helpers as never, vi.fn().mockResolvedValue(true)) + + const pendingFirst = actions.moveCard('board-1', 'card-1', 'col-b', 0) + const pendingSecond = actions.moveCard('board-1', 'card-1', 'col-c', 0) + + firstMove.resolve(moved('col-b', '2026-09-20T10:01:00Z')) + await pendingFirst + await flushPromises() + + const failure = new Error('newer move failed') + secondMove.reject(failure) + await expect(pendingSecond).rejects.toBe(failure) + + expect(state.currentBoardCards.value).toEqual([ + moved('col-b', '2026-09-20T10:01:00Z'), + ]) + expect(counts(state)).toEqual([0, 1, 0]) + expect(helpers.handleApiError).toHaveBeenCalledWith(failure, 'Failed to move card') + }) +}) From 435a3258a0d8dcabc7013821e3913be9a57c228b Mon Sep 17 00:00:00 2001 From: Cristian Tcaci <59696583+Chris0Jeky@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:20:49 +0100 Subject: [PATCH 2/3] fix(board): serialize per-card move and delete settlements --- .../taskdeck-web/src/store/board/cardStore.ts | 237 ++++++++++++------ 1 file changed, 162 insertions(+), 75 deletions(-) diff --git a/frontend/taskdeck-web/src/store/board/cardStore.ts b/frontend/taskdeck-web/src/store/board/cardStore.ts index 4e43f6fb1..98667d949 100644 --- a/frontend/taskdeck-web/src/store/board/cardStore.ts +++ b/frontend/taskdeck-web/src/store/board/cardStore.ts @@ -1,6 +1,7 @@ /** * Card operations: fetch, create, update, delete, move cards, and provenance. */ +import { watch } from 'vue' import { cardsApi } from '../../api/cardsApi' import { getErrorMessage } from '../../utils/errorMessage' import type { CardDetachPreview, CreateCardDto, UpdateCardDto, CardCaptureProvenance } from '../../types/board' @@ -8,11 +9,94 @@ import type { BoardState } from './boardState' import type { BoardHelpers } from './boardStoreHelpers' import type { BoardFetchOptions } from './boardCrudStore' +interface CardMutationVisit { + boardId: string + generation: number +} + +class StaleBoardVisitError extends Error { + constructor() { + super('The board visit that queued this card change has ended.') + this.name = 'StaleBoardVisitError' + } +} + export function createCardActions( state: BoardState, helpers: BoardHelpers, refreshBoard: (boardId: string, options?: BoardFetchOptions) => Promise, ) { + // Move and delete target the same durable card and neither API exposes a + // shared client mutation token. Serialize only that per-card lane so server + // commit order follows user intent, while unrelated cards remain concurrent. + // The visit generation prevents a queued pre-logout intent from starting + // under a later session's credentials. + const mutationTailByCardId = new Map>() + let boardVisitGeneration = 0 + + watch( + () => state.currentBoard.value?.id ?? null, + (nextBoardId, previousBoardId) => { + if (nextBoardId !== previousBoardId) boardVisitGeneration++ + }, + { flush: 'sync' }, + ) + + function captureCardMutationVisit(boardId: string): CardMutationVisit { + return { boardId, generation: boardVisitGeneration } + } + + function isCurrentCardMutationVisit(visit: CardMutationVisit) { + const currentBoard = state.currentBoard.value + return ( + (currentBoard === null || currentBoard.id === visit.boardId) && + boardVisitGeneration === visit.generation + ) + } + + async function runCardMutation( + cardId: string, + visit: CardMutationVisit, + mutation: () => Promise, + ): Promise { + const previous = mutationTailByCardId.get(cardId) + let operation: Promise + + if (previous) { + operation = previous.catch(() => undefined).then(() => { + if (!isCurrentCardMutationVisit(visit)) throw new StaleBoardVisitError() + return mutation() + }) + } else { + // The first intent is already submitted by the caller; do not defer its + // transport to a microtask where immediate navigation could cancel it. + if (!isCurrentCardMutationVisit(visit)) throw new StaleBoardVisitError() + operation = mutation() + } + + const tail = operation.then( + () => undefined, + () => undefined, + ) + mutationTailByCardId.set(cardId, tail) + + try { + return await operation + } finally { + if (mutationTailByCardId.get(cardId) === tail) { + mutationTailByCardId.delete(cardId) + } + } + } + + function isOlderCardSnapshot(candidateUpdatedAt: string, currentUpdatedAt: string) { + const candidateTime = Date.parse(candidateUpdatedAt) + const currentTime = Date.parse(currentUpdatedAt) + return Number.isFinite(candidateTime) && + Number.isFinite(currentTime) && + candidateTime < currentTime + } + async function refreshDetachedChildren(boardId: string) { // The mutation already committed. It only changes hierarchy ownership, not // surviving comment threads, so keep the open editor's same-board cache @@ -130,44 +214,44 @@ export function createCardActions( async function deleteCard(boardId: string, cardId: string, confirmation?: CardDetachPreview) { helpers.guardDemoMutation() - let refreshChildren = false - try { - state.loading.value = true - state.error.value = null - await cardsApi.deleteCard(boardId, cardId, confirmation) - helpers.markBoardDetailMutation(boardId) + const visit = captureCardMutationVisit(boardId) + return runCardMutation(cardId, visit, async () => { + let refreshChildren = false + try { + state.loading.value = true + state.error.value = null + await cardsApi.deleteCard(boardId, cardId, confirmation) + helpers.markBoardDetailMutation(boardId) - // A move, realtime refresh, or navigation can replace this state while the - // DELETE is in flight. Commit only into the initiating board's current - // collection, and derive the count delta from the card that exists NOW. - // If an authoritative refresh already removed it, its count is already - // settled and must not be decremented again. - const ownsCurrentCards = - state.currentBoard.value === null || state.currentBoard.value.id === boardId - if (ownsCurrentCards) { - const committedCard = state.currentBoardCards.value.find((card) => card.id === cardId) - state.currentBoardCards.value = state.currentBoardCards.value.filter((card) => card.id !== cardId) - if (state.cardCommentsByCardId.value[cardId]) { - const { [cardId]: _, ...remainingComments } = state.cardCommentsByCardId.value - state.cardCommentsByCardId.value = remainingComments - } + // A move, realtime refresh, or navigation can replace this state while + // the DELETE is in flight. Commit only into the exact initiating board + // visit and derive the count delta from the card that exists NOW. If an + // authoritative refresh already removed it, its count is already settled. + if (isCurrentCardMutationVisit(visit)) { + const committedCard = state.currentBoardCards.value.find((card) => card.id === cardId) + state.currentBoardCards.value = state.currentBoardCards.value.filter((card) => card.id !== cardId) + if (state.cardCommentsByCardId.value[cardId]) { + const { [cardId]: _, ...remainingComments } = state.cardCommentsByCardId.value + state.cardCommentsByCardId.value = remainingComments + } - if (committedCard) { - helpers.updateColumnCardCount(committedCard.columnId, -1) - } + if (committedCard) { + helpers.updateColumnCardCount(committedCard.columnId, -1) + } - refreshChildren = state.currentBoard.value?.id === boardId && - state.currentBoardCards.value.some(card => card.parentCardId === cardId) + refreshChildren = state.currentBoard.value?.id === boardId && + state.currentBoardCards.value.some(card => card.parentCardId === cardId) + helpers.toast.success('Card deleted successfully') + } + } catch (e: unknown) { + helpers.handleApiError(e, 'Failed to delete card') + throw e + } finally { + state.loading.value = false } - helpers.toast.success('Card deleted successfully') - } catch (e: unknown) { - helpers.handleApiError(e, 'Failed to delete card') - throw e - } finally { - state.loading.value = false - } - // Finish mutation-owned loading/error writes before a refresh can outlive navigation. - if (refreshChildren) await refreshDetachedChildren(boardId) + // Finish mutation-owned loading/error writes before a refresh can outlive navigation. + if (refreshChildren) await refreshDetachedChildren(boardId) + }) } async function moveCard( @@ -177,53 +261,56 @@ export function createCardActions( targetPosition: number, ) { helpers.guardDemoMutation() - try { - state.loading.value = true - state.error.value = null + const visit = captureCardMutationVisit(boardId) + return runCardMutation(cardId, visit, async () => { + try { + state.loading.value = true + state.error.value = null - const existingCard = - state.currentBoardCards.value.find((c) => c.id === cardId) ?? null - const previousColumnId = existingCard?.columnId ?? null - const updatedCard = await cardsApi.moveCard(boardId, cardId, { - targetColumnId, - targetPosition, - }) - helpers.markBoardDetailMutation(boardId) + const updatedCard = await cardsApi.moveCard(boardId, cardId, { + targetColumnId, + targetPosition, + }) + helpers.markBoardDetailMutation(boardId) - // The board can change while the move is in flight. Committing to another - // board's array would splice an unrelated card out and push this one in. - // Skip only when a board IS selected and it is a different one; a null - // currentBoard still owns currentBoardCards (integration tests and the - // pre-load window). - if (state.currentBoard.value && state.currentBoard.value.id !== boardId) { - return updatedCard - } + if (!isCurrentCardMutationVisit(visit)) { + return updatedCard + } - // Re-resolve by id AFTER the await, exactly as updateCard does. An index - // captured before the await goes stale whenever anything else mutates the - // array first -- a second concurrent move, a realtime-triggered refetch, a - // teammate's delete -- and splicing it removes the WRONG card: the moved - // card survives as a duplicate while an innocent one disappears. - const commitIndex = state.currentBoardCards.value.findIndex((c) => c.id === cardId) - if (commitIndex !== -1) { - state.currentBoardCards.value.splice(commitIndex, 1) - } + // Resolve the committed card after the await. Its current column owns + // any count delta; a pre-request snapshot has no settlement authority. + const commitIndex = state.currentBoardCards.value.findIndex((card) => card.id === cardId) + if (commitIndex === -1) { + // A later delete or authoritative refresh removed the card. Never + // resurrect it from an older move response. Preserve the historical + // null-board preload behavior only when no board session exists yet. + if (state.currentBoard.value === null && state.currentBoardCards.value.length === 0) { + state.currentBoardCards.value.push(updatedCard) + helpers.toast.success('Card moved successfully') + } + return updatedCard + } - state.currentBoardCards.value.push(updatedCard) + const committedCard = state.currentBoardCards.value[commitIndex] + if (isOlderCardSnapshot(updatedCard.updatedAt, committedCard.updatedAt)) { + return updatedCard + } - if (previousColumnId && previousColumnId !== updatedCard.columnId) { - helpers.updateColumnCardCount(previousColumnId, -1) - helpers.updateColumnCardCount(updatedCard.columnId, 1) - } + state.currentBoardCards.value[commitIndex] = updatedCard + if (committedCard.columnId !== updatedCard.columnId) { + helpers.updateColumnCardCount(committedCard.columnId, -1) + helpers.updateColumnCardCount(updatedCard.columnId, 1) + } - helpers.toast.success('Card moved successfully') - return updatedCard - } catch (e: unknown) { - helpers.handleApiError(e, 'Failed to move card') - throw e - } finally { - state.loading.value = false - } + helpers.toast.success('Card moved successfully') + return updatedCard + } catch (e: unknown) { + helpers.handleApiError(e, 'Failed to move card') + throw e + } finally { + state.loading.value = false + } + }) } async function fetchCardProvenance( From 8f89bfcefce03576c48cf6dbb040983854eac47b Mon Sep 17 00:00:00 2001 From: Cristian Tcaci <59696583+Chris0Jeky@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:50:45 +0100 Subject: [PATCH 3/3] test(board): separate shifted-index and no-resurrection move contracts --- .../src/tests/store/board/cardStore.spec.ts | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/frontend/taskdeck-web/src/tests/store/board/cardStore.spec.ts b/frontend/taskdeck-web/src/tests/store/board/cardStore.spec.ts index 42816f20f..0d54a26f2 100644 --- a/frontend/taskdeck-web/src/tests/store/board/cardStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/board/cardStore.spec.ts @@ -445,6 +445,7 @@ describe('cardStore', () => { */ it('re-resolves the card by id after the await, so a shifted array cannot splice the wrong card', async () => { state.currentBoard.value!.columns.push({ id: 'col-2', name: 'Done', cardCount: 0 }) + const unrelatedCard = { ...state.currentBoardCards.value[1] } // Baseline: card-1 at index 0, card-2 at index 1. expect(state.currentBoardCards.value.map((c: { id: string }) => c.id)).toEqual(['card-1', 'card-2']) @@ -452,20 +453,37 @@ describe('cardStore', () => { ...state.currentBoardCards.value[0], id: 'card-1', columnId: 'col-2', updatedAt: '2024-01-06T00:00:00Z', } mockCardsApi.moveCard.mockImplementationOnce(async () => { - // While the move is in flight, card-1 is removed by something else (a - // realtime refetch, a teammate's delete). The pre-await index 0 now - // points at card-2 -- an innocent bystander. - state.currentBoardCards.value.shift() + // A refresh reorders the array while retaining both cards. The saved + // pre-await index now points at card-2, not the requested card. + state.currentBoardCards.value.reverse() return movedCard }) const { moveCard } = createCardActions(state as any, helpers as any, vi.fn().mockResolvedValue(true)) await moveCard('board-1', 'card-1', 'col-2', 0) - // card-2 must survive. Against the stale-index commit it was spliced out - // and the array came back as ['card-1'] alone. + // Exact IDs catch both loss of the unrelated card and duplication of the target. const ids = state.currentBoardCards.value.map((c: { id: string }) => c.id).sort() expect(ids).toEqual(['card-1', 'card-2']) + expect(state.currentBoardCards.value.find((card) => card.id === 'card-1')).toEqual(movedCard) + expect(state.currentBoardCards.value.find((card) => card.id === 'card-2')).toEqual(unrelatedCard) + }) + + it('does not resurrect a card removed by an authoritative refresh while its move is pending', async () => { + const unrelatedCard = { ...state.currentBoardCards.value[1] } + const movedCard = { ...state.currentBoardCards.value[0], columnId: 'col-2' } + mockCardsApi.moveCard.mockImplementationOnce(async () => { + state.currentBoardCards.value.shift() + return movedCard + }) + const { moveCard } = createCardActions(state as any, helpers as any, vi.fn().mockResolvedValue(true)) + + const result = await moveCard('board-1', 'card-1', 'col-2', 0) + + expect(result).toEqual(movedCard) + expect(state.currentBoardCards.value).toEqual([unrelatedCard]) + expect(helpers.updateColumnCardCount).not.toHaveBeenCalled() + expect(helpers.toast.success).not.toHaveBeenCalled() }) it('still commits into currentBoardCards when currentBoard is unset', async () => { @@ -495,7 +513,7 @@ describe('cardStore', () => { expect(state.currentBoardCards.value.map((c: { id: string }) => c.id)).toEqual(['other-board-card']) }) - it('removes from old position and pushes updated card', async () => { + it('replaces the moved card by stable id without changing unrelated cards', async () => { const movedCard = { id: 'card-1', boardId: 'board-1', @@ -522,9 +540,11 @@ describe('cardStore', () => { }) expect(result).toEqual(movedCard) expect(helpers.markBoardDetailMutation).toHaveBeenCalledWith('board-1') - expect(state.currentBoardCards.value[state.currentBoardCards.value.length - 1]).toEqual( - movedCard, - ) + expect(state.currentBoardCards.value.filter((card) => card.id === 'card-1')).toEqual([movedCard]) + expect(state.currentBoardCards.value.find((card) => card.id === 'card-2')).toMatchObject({ + title: 'Second', columnId: 'col-1', position: 1, + }) + expect(state.currentBoardCards.value).toHaveLength(2) expect(helpers.toast.success).toHaveBeenCalledWith('Card moved successfully') expect(state.loading.value).toBe(false) })