diff --git a/docs/analysis/2026-09-21-integration-mutation-order.md b/docs/analysis/2026-09-21-integration-mutation-order.md new file mode 100644 index 000000000..645d837b5 --- /dev/null +++ b/docs/analysis/2026-09-21-integration-mutation-order.md @@ -0,0 +1,28 @@ +# Connector mutation ordering + +Status: stacked draft PR #3336, 2026-09-21. Parent: PR #3332, itself stacked on #3329. + +## Reproduced defects + +Update, delete, enable and disable started independently for one connector even though the API accepts no expected revision and the entity has no configured concurrency token. Same-connector commits and responses could therefore diverge from user submission order. A queued pre-logout intent also had no transport-time lifecycle check. + +Independent mutation failures exposed a second ownership gap during review. A queued same-connector mutation cleared the store's shared error when its transport started. If another connector failed while the queued intent was waiting, the queued start erased that unrelated receipt. + +## Contract + +- One queue exists per connector ID. Update, delete, enable and disable for that connector run in submission order; different connectors remain concurrent. +- The first intent starts transport synchronously. Later same-connector work waits for its predecessor, regardless of success or failure. +- Immediately before transport, queued work rechecks the lifecycle epoch from submission. Identity, token, authentication or demo replacement clears queue registration and prevents old intent from using later credentials. +- A predecessor failure does not cancel the next intent. The queued transport clears an error only when that receipt is owned by its own predecessor; an unrelated concurrent connector failure remains visible. +- Existing stale-session cache, detail, toast and error settlement rules from #3332 remain unchanged. +- Delete remains ordered rather than magical: later intent still reaches the server and may receive NotFound; no client resurrection is introduced. + +The client queue preserves one client's submission order only. It does not solve cross-device concurrency; the backend currently exposes no revision precondition. + +## Evidence and remaining gates + +The initial ordering negative control executed the actual production store with only framework/API/session boundaries stubbed: one independent-connector control passed and five ordering/session schedules failed on the parent, then all six passed after serialization. + +Codex review identified the independent-error case. A dedicated deferred Pinia regression was committed before the correction: connector 2 fails while connector 1's second update waits, then connector 1's queued transport starts without erasing connector 2's error. + +This child is rebuilt on the parent token-rotation correction rather than retaining its older copy of the integration store. Exact-head Pinia/Vitest, lint, project typecheck, build, the complete hosted matrix and repeat independent review remain required. Because this is a third-level stack, retarget only after #3329 and #3332 land, verify the child-only diff and requalify against current `main`. No merge, release or deployment qualification is claimed. diff --git a/frontend/taskdeck-web/src/store/integrationStore.ts b/frontend/taskdeck-web/src/store/integrationStore.ts index 80fea1be4..96eca688e 100644 --- a/frontend/taskdeck-web/src/store/integrationStore.ts +++ b/frontend/taskdeck-web/src/store/integrationStore.ts @@ -27,14 +27,31 @@ export const useIntegrationStore = defineStore('integration', () => { token: symbol } + interface MutationTail { + promise: Promise + ownerToken: symbol + } + let lifecycleEpoch = 0 + let errorOwner: symbol | null = null const readOwners = new Map() const activeReadTokens = new Set() + const mutationTails = new Map() function syncLoading() { loading.value = activeReadTokens.size > 0 } + function clearError() { + error.value = null + errorOwner = null + } + + function recordError(ownerToken: symbol, message: string) { + error.value = message + errorOwner = ownerToken + } + function beginRead(lane: ReadLane): ReadOwner { const previous = readOwners.get(lane) if (previous?.epoch === lifecycleEpoch) activeReadTokens.delete(previous.token) @@ -42,7 +59,7 @@ export const useIntegrationStore = defineStore('integration', () => { const owner = { epoch: lifecycleEpoch, token: Symbol(lane) } readOwners.set(lane, owner) activeReadTokens.add(owner.token) - error.value = null + clearError() syncLoading() return owner } @@ -63,6 +80,7 @@ export const useIntegrationStore = defineStore('integration', () => { lifecycleEpoch += 1 readOwners.clear() activeReadTokens.clear() + mutationTails.clear() loading.value = false } @@ -70,6 +88,35 @@ export const useIntegrationStore = defineStore('integration', () => { return epoch === lifecycleEpoch } + async function enqueueConnectorMutation( + connectorId: string, + task: (epoch: number, ownerToken: symbol) => Promise, + ): Promise { + const epoch = lifecycleEpoch + const ownerToken = Symbol(`mutation:${connectorId}`) + const predecessor = mutationTails.get(connectorId) + let release!: () => void + const tail = new Promise((resolve) => { release = resolve }) + mutationTails.set(connectorId, { promise: tail, ownerToken }) + + try { + if (predecessor) await predecessor.promise + if (!ownsLifetime(epoch)) return undefined + + // Retire only an error produced by this connector's predecessor. Another + // connector can fail while this intent waits and must keep its receipt. + if (predecessor) { + if (errorOwner === predecessor.ownerToken) clearError() + } else { + clearError() + } + return await task(epoch, ownerToken) + } finally { + release() + if (mutationTails.get(connectorId)?.promise === tail) mutationTails.delete(connectorId) + } + } + function guardDemoMutation(): never | void { if (isDemoMode) { toast.info('This action is view-only in demo mode.') @@ -80,6 +127,7 @@ export const useIntegrationStore = defineStore('integration', () => { async function fetchConnectors() { if (isDemoMode) { loading.value = false + clearError() error.value = 'Integrations are not available in demo mode.' return } @@ -93,7 +141,7 @@ export const useIntegrationStore = defineStore('integration', () => { if (!ownsRead('list', owner)) return connectors.value = [] const msg = getErrorDisplay(e, 'Failed to fetch integrations').message - error.value = msg + recordError(owner.token, msg) toast.error(msg) } finally { finishRead('list', owner) @@ -102,6 +150,7 @@ export const useIntegrationStore = defineStore('integration', () => { async function fetchConnectorDetail(id: string) { if (isDemoMode) { + clearError() error.value = 'Integrations are not available in demo mode.' return } @@ -114,7 +163,7 @@ export const useIntegrationStore = defineStore('integration', () => { } catch (e: unknown) { if (!ownsRead('detail', owner)) return const msg = getErrorDisplay(e, 'Failed to fetch connector details').message - error.value = msg + recordError(owner.token, msg) selectedConnector.value = null toast.error(msg) } finally { @@ -125,8 +174,9 @@ export const useIntegrationStore = defineStore('integration', () => { async function registerConnector(request: CreateIntegrationConnectorRequest) { guardDemoMutation() const epoch = lifecycleEpoch + const ownerToken = Symbol('register') try { - error.value = null + clearError() const connector = await integrationsApi.registerConnector(request) if (!ownsLifetime(epoch)) return connector @@ -136,7 +186,7 @@ export const useIntegrationStore = defineStore('integration', () => { } catch (e: unknown) { if (ownsLifetime(epoch)) { const msg = getErrorDisplay(e, 'Failed to register connector').message - error.value = msg + recordError(ownerToken, msg) toast.error(msg) } throw e @@ -145,102 +195,102 @@ export const useIntegrationStore = defineStore('integration', () => { async function updateConnector(id: string, request: UpdateIntegrationConnectorRequest) { guardDemoMutation() - const epoch = lifecycleEpoch - try { - error.value = null - const updated = await integrationsApi.updateConnector(id, request) - if (!ownsLifetime(epoch)) return updated + return await enqueueConnectorMutation(id, async (epoch, ownerToken) => { + try { + const updated = await integrationsApi.updateConnector(id, request) + if (!ownsLifetime(epoch)) return updated - connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) - if (selectedConnector.value?.id === id) { - selectedConnector.value = { ...selectedConnector.value, ...updated } + connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) + if (selectedConnector.value?.id === id) { + selectedConnector.value = { ...selectedConnector.value, ...updated } + } + toast.success('Connector updated.') + return updated + } catch (e: unknown) { + if (ownsLifetime(epoch)) { + const msg = getErrorDisplay(e, 'Failed to update connector').message + recordError(ownerToken, msg) + toast.error(msg) + } + throw e } - toast.success('Connector updated.') - return updated - } catch (e: unknown) { - if (ownsLifetime(epoch)) { - const msg = getErrorDisplay(e, 'Failed to update connector').message - error.value = msg - toast.error(msg) - } - throw e - } + }) } async function deleteConnector(id: string) { guardDemoMutation() - const epoch = lifecycleEpoch - try { - error.value = null - await integrationsApi.deleteConnector(id) - if (!ownsLifetime(epoch)) return + await enqueueConnectorMutation(id, async (epoch, ownerToken) => { + try { + await integrationsApi.deleteConnector(id) + if (!ownsLifetime(epoch)) return - connectors.value = connectors.value.filter((connector) => connector.id !== id) - if (selectedConnector.value?.id === id) { - selectedConnector.value = null + connectors.value = connectors.value.filter((connector) => connector.id !== id) + if (selectedConnector.value?.id === id) { + selectedConnector.value = null + } + toast.success('Connector removed.') + } catch (e: unknown) { + if (ownsLifetime(epoch)) { + const msg = getErrorDisplay(e, 'Failed to remove connector').message + recordError(ownerToken, msg) + toast.error(msg) + } + throw e } - toast.success('Connector removed.') - } catch (e: unknown) { - if (ownsLifetime(epoch)) { - const msg = getErrorDisplay(e, 'Failed to remove connector').message - error.value = msg - toast.error(msg) - } - throw e - } + }) } async function enableConnector(id: string) { guardDemoMutation() - const epoch = lifecycleEpoch - try { - error.value = null - const updated = await integrationsApi.enableConnector(id) - if (!ownsLifetime(epoch)) return + await enqueueConnectorMutation(id, async (epoch, ownerToken) => { + try { + const updated = await integrationsApi.enableConnector(id) + if (!ownsLifetime(epoch)) return - connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) - if (selectedConnector.value?.id === id) { - selectedConnector.value = { ...selectedConnector.value, ...updated } + connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) + if (selectedConnector.value?.id === id) { + selectedConnector.value = { ...selectedConnector.value, ...updated } + } + toast.success('Connector enabled.') + } catch (e: unknown) { + if (ownsLifetime(epoch)) { + const msg = getErrorDisplay(e, 'Failed to enable connector').message + recordError(ownerToken, msg) + toast.error(msg) + } + throw e } - toast.success('Connector enabled.') - } catch (e: unknown) { - if (ownsLifetime(epoch)) { - const msg = getErrorDisplay(e, 'Failed to enable connector').message - error.value = msg - toast.error(msg) - } - throw e - } + }) } async function disableConnector(id: string) { guardDemoMutation() - const epoch = lifecycleEpoch - try { - error.value = null - const updated = await integrationsApi.disableConnector(id) - if (!ownsLifetime(epoch)) return + await enqueueConnectorMutation(id, async (epoch, ownerToken) => { + try { + const updated = await integrationsApi.disableConnector(id) + if (!ownsLifetime(epoch)) return - connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) - if (selectedConnector.value?.id === id) { - selectedConnector.value = { ...selectedConnector.value, ...updated } - } - toast.success('Connector disabled.') - } catch (e: unknown) { - if (ownsLifetime(epoch)) { - const msg = getErrorDisplay(e, 'Failed to disable connector').message - error.value = msg - toast.error(msg) + connectors.value = connectors.value.map((connector) => connector.id === id ? updated : connector) + if (selectedConnector.value?.id === id) { + selectedConnector.value = { ...selectedConnector.value, ...updated } + } + toast.success('Connector disabled.') + } catch (e: unknown) { + if (ownsLifetime(epoch)) { + const msg = getErrorDisplay(e, 'Failed to disable connector').message + recordError(ownerToken, msg) + toast.error(msg) + } + throw e } - throw e - } + }) } function $reset() { invalidateReads() connectors.value = [] selectedConnector.value = null - error.value = null + clearError() } watch( diff --git a/frontend/taskdeck-web/src/tests/store/integrationStoreMutationErrorOwnership.spec.ts b/frontend/taskdeck-web/src/tests/store/integrationStoreMutationErrorOwnership.spec.ts new file mode 100644 index 000000000..1537fd4c0 --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/integrationStoreMutationErrorOwnership.spec.ts @@ -0,0 +1,115 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { integrationsApi } from '../../api/integrationsApi' +import { useIntegrationStore } from '../../store/integrationStore' +import { useSessionStore } from '../../store/sessionStore' +import type { IntegrationConnector } from '../../types/integration' + +vi.mock('../../utils/demoMode', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, isDemoMode: false } +}) + +vi.mock('../../api/integrationsApi', () => ({ + integrationsApi: { + listConnectors: vi.fn(), + getConnector: vi.fn(), + registerConnector: vi.fn(), + updateConnector: vi.fn(), + deleteConnector: vi.fn(), + enableConnector: vi.fn(), + disableConnector: vi.fn(), + }, +})) + +vi.mock('../../api/authApi', () => ({ + authApi: { + login: vi.fn(), + register: vi.fn(), + changePassword: vi.fn(), + }, +})) + +vi.mock('../../store/toastStore', () => ({ + useToastStore: () => ({ + error: vi.fn(), + success: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + }), +})) + +vi.mock('../../composables/useErrorMapper', () => ({ + getErrorDisplay: (error: unknown, fallback: string) => ({ + message: error instanceof Error ? error.message : fallback, + }), +})) + +function deferred() { + let resolve!: (value: T) => void + let reject!: (reason: unknown) => void + const promise = new Promise((yes, no) => { + resolve = yes + reject = no + }) + return { promise, resolve, reject } +} + +function connector(id: string, name: string): IntegrationConnector { + return { + id, + name, + connectorType: 'BrowserClipper', + direction: 'Inbound', + status: 'Active', + configuration: null, + createdAt: '2026-09-21T00:00:00Z', + updatedAt: '2026-09-21T00:00:00Z', + } +} + +async function flushQueue() { + await Promise.resolve() + await Promise.resolve() +} + +describe('integrationStore mutation error ownership', () => { + beforeEach(() => { + setActivePinia(createPinia()) + const session = useSessionStore() + session.userId = 'account-a' + vi.clearAllMocks() + }) + + it('does not erase an independent failure when queued same-connector work starts', async () => { + const store = useIntegrationStore() + store.connectors = [connector('connector-1', 'One'), connector('connector-2', 'Two')] + + const first = deferred() + const independent = deferred() + const queued = deferred() + vi.mocked(integrationsApi.updateConnector) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(independent.promise) + .mockReturnValueOnce(queued.promise) + + const firstRequest = store.updateConnector('connector-1', { name: 'First' }) + const queuedRequest = store.updateConnector('connector-1', { name: 'Queued' }) + const independentRequest = store.updateConnector('connector-2', { name: 'Independent' }) + + independent.reject(new Error('independent connector failed')) + await expect(independentRequest).rejects.toThrow('independent connector failed') + expect(store.error).toBe('independent connector failed') + + first.resolve(connector('connector-1', 'First')) + await firstRequest + await flushQueue() + + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(3) + expect(store.error).toBe('independent connector failed') + + queued.resolve(connector('connector-1', 'Queued')) + await queuedRequest + expect(store.error).toBe('independent connector failed') + }) +}) diff --git a/frontend/taskdeck-web/src/tests/store/integrationStoreMutationOrder.spec.ts b/frontend/taskdeck-web/src/tests/store/integrationStoreMutationOrder.spec.ts new file mode 100644 index 000000000..dffa2061a --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/integrationStoreMutationOrder.spec.ts @@ -0,0 +1,224 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { integrationsApi } from '../../api/integrationsApi' +import { useIntegrationStore } from '../../store/integrationStore' +import { useSessionStore } from '../../store/sessionStore' +import type { IntegrationConnector, IntegrationConnectorDetail } from '../../types/integration' + +const toastMocks = vi.hoisted(() => ({ + error: vi.fn(), + success: vi.fn(), + info: vi.fn(), + warning: vi.fn(), +})) + +vi.mock('../../utils/demoMode', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, isDemoMode: false } +}) + +vi.mock('../../api/integrationsApi', () => ({ + integrationsApi: { + listConnectors: vi.fn(), + getConnector: vi.fn(), + registerConnector: vi.fn(), + updateConnector: vi.fn(), + deleteConnector: vi.fn(), + enableConnector: vi.fn(), + disableConnector: vi.fn(), + }, +})) + +vi.mock('../../api/authApi', () => ({ + authApi: { + login: vi.fn(), + register: vi.fn(), + changePassword: vi.fn(), + }, +})) + +vi.mock('../../store/toastStore', () => ({ + useToastStore: () => toastMocks, +})) + +vi.mock('../../composables/useErrorMapper', () => ({ + getErrorDisplay: (_error: unknown, fallback: string) => ({ message: fallback }), +})) + +function deferred() { + let resolve!: (value: T) => void + let reject!: (reason: unknown) => void + const promise = new Promise((yes, no) => { + resolve = yes + reject = no + }) + return { promise, resolve, reject } +} + +function connector( + id: string, + name: string, + status: IntegrationConnector['status'] = 'Active', +): IntegrationConnector { + return { + id, + name, + connectorType: 'BrowserClipper', + direction: 'Inbound', + status, + configuration: null, + createdAt: '2026-09-21T00:00:00Z', + updatedAt: '2026-09-21T00:00:00Z', + } +} + +function detail( + id: string, + name: string, + status: IntegrationConnector['status'] = 'Active', +): IntegrationConnectorDetail { + return { ...connector(id, name, status), recentEvents: [] } +} + +async function flushQueue() { + await Promise.resolve() + await Promise.resolve() +} + +describe('integrationStore mutation ordering', () => { + let session: ReturnType + let store: ReturnType + + beforeEach(() => { + setActivePinia(createPinia()) + session = useSessionStore() + session.userId = 'account-a' + store = useIntegrationStore() + store.connectors = [connector('connector-1', 'Connector')] + store.selectedConnector = detail('connector-1', 'Connector') + vi.clearAllMocks() + }) + + it('serializes enable then disable for one connector', async () => { + const enable = deferred() + const disable = deferred() + vi.mocked(integrationsApi.enableConnector).mockReturnValue(enable.promise) + vi.mocked(integrationsApi.disableConnector).mockReturnValue(disable.promise) + + const enableRequest = store.enableConnector('connector-1') + const disableRequest = store.disableConnector('connector-1') + expect(integrationsApi.disableConnector).not.toHaveBeenCalled() + + enable.resolve(connector('connector-1', 'Connector', 'Active')) + await enableRequest + await flushQueue() + expect(integrationsApi.disableConnector).toHaveBeenCalledTimes(1) + + disable.resolve(connector('connector-1', 'Connector', 'Disabled')) + await disableRequest + expect(store.connectors[0]?.status).toBe('Disabled') + expect(store.selectedConnector?.status).toBe('Disabled') + }) + + it('serializes two updates and keeps the final intent', async () => { + const first = deferred() + const second = deferred() + vi.mocked(integrationsApi.updateConnector) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updateConnector('connector-1', { name: 'First' }) + const secondRequest = store.updateConnector('connector-1', { name: 'Second' }) + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(1) + + first.resolve(connector('connector-1', 'First')) + await firstRequest + await flushQueue() + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(2) + + second.resolve(connector('connector-1', 'Second')) + await secondRequest + expect(store.connectors[0]?.name).toBe('Second') + expect(store.selectedConnector?.name).toBe('Second') + }) + + it('continues with queued intent after a failed predecessor', async () => { + const first = deferred() + const second = deferred() + vi.mocked(integrationsApi.updateConnector) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updateConnector('connector-1', { name: 'First' }) + const secondRequest = store.updateConnector('connector-1', { name: 'Second' }) + first.reject(new Error('first failed')) + await expect(firstRequest).rejects.toThrow('first failed') + await flushQueue() + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(2) + + second.resolve(connector('connector-1', 'Second')) + await secondRequest + expect(store.connectors[0]?.name).toBe('Second') + expect(store.error).toBeNull() + }) + + it('orders update before delete and leaves the connector removed', async () => { + const update = deferred() + const remove = deferred() + vi.mocked(integrationsApi.updateConnector).mockReturnValue(update.promise) + vi.mocked(integrationsApi.deleteConnector).mockReturnValue(remove.promise) + + const updateRequest = store.updateConnector('connector-1', { name: 'Updated' }) + const deleteRequest = store.deleteConnector('connector-1') + expect(integrationsApi.deleteConnector).not.toHaveBeenCalled() + + update.resolve(connector('connector-1', 'Updated')) + await updateRequest + await flushQueue() + expect(integrationsApi.deleteConnector).toHaveBeenCalledTimes(1) + + remove.resolve() + await deleteRequest + expect(store.connectors).toEqual([]) + expect(store.selectedConnector).toBeNull() + }) + + it('does not start queued old-session transport after logout', async () => { + const first = deferred() + vi.mocked(integrationsApi.updateConnector).mockReturnValue(first.promise) + + const firstRequest = store.updateConnector('connector-1', { name: 'First' }) + const queuedRequest = store.updateConnector('connector-1', { name: 'Second' }) + session.userId = null + session.userId = 'account-a' + + first.resolve(connector('connector-1', 'First')) + await firstRequest + await queuedRequest + + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(1) + expect(store.connectors).toEqual([]) + expect(store.error).toBeNull() + }) + + it('keeps different connectors concurrent', async () => { + store.connectors = [ + connector('connector-1', 'One'), + connector('connector-2', 'Two'), + ] + const first = deferred() + const second = deferred() + vi.mocked(integrationsApi.updateConnector) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updateConnector('connector-1', { name: 'One updated' }) + const secondRequest = store.updateConnector('connector-2', { name: 'Two updated' }) + expect(integrationsApi.updateConnector).toHaveBeenCalledTimes(2) + + first.resolve(connector('connector-1', 'One updated')) + second.resolve(connector('connector-2', 'Two updated')) + await Promise.all([firstRequest, secondRequest]) + expect(store.connectors.map(item => item.name)).toEqual(['One updated', 'Two updated']) + }) +})