diff --git a/docs/analysis/2026-09-21-agent-store-ownership.md b/docs/analysis/2026-09-21-agent-store-ownership.md new file mode 100644 index 0000000000..69dd7be1f6 --- /dev/null +++ b/docs/analysis/2026-09-21-agent-store-ownership.md @@ -0,0 +1,53 @@ +# Agent store request and session ownership + +Status: draft PR #3338, 2026-09-21. Base: `307c3b8b50bec1cb0bfaea3e570a942bcb1d4451`. + +## Reproduced defects + +`agentStore` previously let every profile, run-list and run-detail response write its shared surface. Route-clear helpers changed visible values but did not invalidate requests. The store also had no session lifetime, so old-account work could settle after logout/login, including login as the same user ID. + +This admitted reverse-settling reads, A-old → B → A-new route reuse, late settlement after route clear, stale error/toast state, false loading clears, and old-session repopulation. + +Independent review then exposed two token-refresh boundaries: + +1. treating a successful same-user refresh as full identity replacement cleared already loaded agent data; +2. preserving settled data alone still stranded an empty first-load route because the old request was invalidated and the unchanged route did not remount or refetch. + +## Contract + +Profiles, run lists and run details have independent owner lanes. Each owner carries the current session epoch and a unique request token. A newer request retires only the previous owner in its lane. Route-clear helpers invalidate their own lane before clearing visible state. + +User identity, authenticated state and demo-session replacement synchronously advance the epoch, retire all owners, and clear every agent surface, error and loading indicator. + +A token-only rotation: + +- preserves already loaded profiles, runs and detail; +- retires old-token owners and stale success/failure/toast/loading settlement; +- restarts only active lanes whose visible surface is still empty; +- preserves the exact agent/run parameters captured by the active read; +- never replays a mutation. + +Independent lanes remain concurrent. Demo mode retains its no-network behavior. No API, DTO, route, schema, dependency or backend behavior changes. + +## Test-first evidence + +Initial test-only head `ae52930c6eb5d5b80a2f6a7347242f4ad60a0036` ran the full frontend suite on Ubuntu and Windows. Both platform jobs passed lint, typecheck, production build and PWA validation, then failed in the new real Pinia suite. Ubuntu JUnit recorded **7,159 tests, 8 failures, 0 errors**, all in the intended ownership schedules. + +Review-regression head `7b6e137d9625ee6b2ba6ad747a4fdecc5fabe172` added the loaded-data refresh schedule. Ubuntu again passed lint, typecheck, build and PWA validation; all ten ownership cases ran and only the new preservation case failed. + +The first corrected head `c48346128982366c0abdf4f1f766246f5cc351dc` then passed Smart CI, Extended and the complete Required CI matrix. Repeat Codex review found the empty-first-load residual described above. + +Test-only head `11ab87461ed4773b99af8e35a6625873a52e2425` adds one deferred real-Pinia schedule spanning profiles, runs and detail. A dependency-free runner transpiled and executed the actual production module: + +- before the retry correction: each API was called once and all three loading flags became false after rotation; +- after the correction: each API was called twice, old-token settlement was suppressed, and fresh-token results populated all three lanes. + +Hosted qualification for the final correction remains authoritative; the supplemental runner does not replace it. + +## Remaining gates + +Current production correction: `c6a88e78cc9a306992dd95ae28569fb575c4e286` before this documentation commit. + +Exact final-head lint, typecheck, production build, complete Vitest on Ubuntu and Windows, Required CI, Extended, Self-Test, and repeat independent review are required. Review should focus on retry capture, route-clear cancellation, no mutation replay, and avoiding refresh loops. + +This is client-state integrity, not a claim of server-side authorization bypass or transport cancellation. No merge, release or deployment qualification is claimed. diff --git a/frontend/taskdeck-web/src/store/agentStore.ts b/frontend/taskdeck-web/src/store/agentStore.ts index 1a599de2d4..bbd89eeb4a 100644 --- a/frontend/taskdeck-web/src/store/agentStore.ts +++ b/frontend/taskdeck-web/src/store/agentStore.ts @@ -1,13 +1,15 @@ import { defineStore } from 'pinia' -import { ref } from 'vue' +import { ref, watch } from 'vue' import { agentApi } from '../api/agentApi' import { useToastStore } from './toastStore' +import { useSessionStore } from './sessionStore' import { isDemoMode } from '../utils/demoMode' import { getErrorDisplay } from '../composables/useErrorMapper' import type { AgentProfile, AgentRun, AgentRunDetail } from '../types/agent' export const useAgentStore = defineStore('agent', () => { const toast = useToastStore() + const session = useSessionStore() const profiles = ref([]) const profilesLoading = ref(false) @@ -21,80 +23,193 @@ export const useAgentStore = defineStore('agent', () => { const runDetailLoading = ref(false) const runDetailError = ref(null) + type ReadLane = 'profiles' | 'runs' | 'detail' + type ReadRetry = () => Promise + + interface ReadOwner { + epoch: number + token: symbol + } + + let sessionEpoch = 0 + const readOwners = new Map() + const readRetries = new Map() + + function setLaneLoading(lane: ReadLane, value: boolean): void { + if (lane === 'profiles') profilesLoading.value = value + else if (lane === 'runs') runsLoading.value = value + else runDetailLoading.value = value + } + + function clearLaneError(lane: ReadLane): void { + if (lane === 'profiles') profilesError.value = null + else if (lane === 'runs') runsError.value = null + else runDetailError.value = null + } + + function beginRead(lane: ReadLane, retry: ReadRetry): ReadOwner { + const owner = { epoch: sessionEpoch, token: Symbol(lane) } + readOwners.set(lane, owner) + readRetries.set(lane, retry) + clearLaneError(lane) + setLaneLoading(lane, true) + return owner + } + + function ownsRead(lane: ReadLane, owner: ReadOwner): boolean { + const current = readOwners.get(lane) + return owner.epoch === sessionEpoch && current?.token === owner.token + } + + function finishRead(lane: ReadLane, owner: ReadOwner): void { + if (!ownsRead(lane, owner)) return + readOwners.delete(lane) + readRetries.delete(lane) + setLaneLoading(lane, false) + } + + function invalidateLane(lane: ReadLane): void { + readOwners.delete(lane) + readRetries.delete(lane) + clearLaneError(lane) + setLaneLoading(lane, false) + } + + function invalidateReads(): void { + sessionEpoch += 1 + readOwners.clear() + readRetries.clear() + profilesLoading.value = false + runsLoading.value = false + runDetailLoading.value = false + profilesError.value = null + runsError.value = null + runDetailError.value = null + } + + function retryEmptyActiveReads(): void { + const retries: ReadRetry[] = [] + const profilesRetry = readOwners.has('profiles') && profiles.value.length === 0 + ? readRetries.get('profiles') + : undefined + const runsRetry = readOwners.has('runs') && runs.value.length === 0 + ? readRetries.get('runs') + : undefined + const detailRetry = readOwners.has('detail') && runDetail.value === null + ? readRetries.get('detail') + : undefined + + if (profilesRetry) retries.push(profilesRetry) + if (runsRetry) retries.push(runsRetry) + if (detailRetry) retries.push(detailRetry) + + invalidateReads() + for (const retry of retries) { + void retry().catch(() => { + // The retried store action owns current error/toast state. + }) + } + } + + function resetForSession(): void { + invalidateReads() + profiles.value = [] + runs.value = [] + runDetail.value = null + } + + watch( + () => [session.userId, session.isAuthenticated, session.isDemo], + resetForSession, + { flush: 'sync' }, + ) + + watch( + () => session.token, + retryEmptyActiveReads, + { flush: 'sync' }, + ) + async function fetchProfiles(): Promise { if (isDemoMode) { - profilesLoading.value = true - profilesError.value = null + invalidateLane('profiles') profiles.value = [] - profilesLoading.value = false return } + + const owner = beginRead('profiles', fetchProfiles) try { - profilesLoading.value = true - profilesError.value = null - profiles.value = await agentApi.listProfiles() + const result = await agentApi.listProfiles() + if (!ownsRead('profiles', owner)) return + profiles.value = result } catch (e: unknown) { - const msg = getErrorDisplay(e, 'Failed to load agent profiles').message - profilesError.value = msg - toast.error(msg) + if (ownsRead('profiles', owner)) { + const msg = getErrorDisplay(e, 'Failed to load agent profiles').message + profilesError.value = msg + toast.error(msg) + } throw e } finally { - profilesLoading.value = false + finishRead('profiles', owner) } } async function fetchRuns(agentId: string, limit = 100): Promise { if (isDemoMode) { - runsLoading.value = true - runsError.value = null + invalidateLane('runs') runs.value = [] - runsLoading.value = false return } + + const owner = beginRead('runs', () => fetchRuns(agentId, limit)) try { - runsLoading.value = true - runsError.value = null - runs.value = await agentApi.listRuns(agentId, limit) + const result = await agentApi.listRuns(agentId, limit) + if (!ownsRead('runs', owner)) return + runs.value = result } catch (e: unknown) { - const msg = getErrorDisplay(e, 'Failed to load agent runs').message - runsError.value = msg - toast.error(msg) + if (ownsRead('runs', owner)) { + const msg = getErrorDisplay(e, 'Failed to load agent runs').message + runsError.value = msg + toast.error(msg) + } throw e } finally { - runsLoading.value = false + finishRead('runs', owner) } } async function fetchRunDetail(agentId: string, runId: string): Promise { if (isDemoMode) { - runDetailLoading.value = true - runDetailError.value = null + invalidateLane('detail') runDetail.value = null - runDetailLoading.value = false return } + + const owner = beginRead('detail', () => fetchRunDetail(agentId, runId)) try { - runDetailLoading.value = true - runDetailError.value = null - runDetail.value = await agentApi.getRunDetail(agentId, runId) + const result = await agentApi.getRunDetail(agentId, runId) + if (!ownsRead('detail', owner)) return + runDetail.value = result } catch (e: unknown) { - const msg = getErrorDisplay(e, 'Failed to load run details').message - runDetailError.value = msg - toast.error(msg) + if (ownsRead('detail', owner)) { + const msg = getErrorDisplay(e, 'Failed to load run details').message + runDetailError.value = msg + toast.error(msg) + } throw e } finally { - runDetailLoading.value = false + finishRead('detail', owner) } } function clearRuns(): void { + invalidateLane('runs') runs.value = [] - runsError.value = null } function clearRunDetail(): void { + invalidateLane('detail') runDetail.value = null - runDetailError.value = null } return { diff --git a/frontend/taskdeck-web/src/tests/store/agentStoreOwnership.spec.ts b/frontend/taskdeck-web/src/tests/store/agentStoreOwnership.spec.ts new file mode 100644 index 0000000000..ef17332a3e --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/agentStoreOwnership.spec.ts @@ -0,0 +1,420 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { agentApi } from '../../api/agentApi' +import { useAgentStore } from '../../store/agentStore' +import { useSessionStore } from '../../store/sessionStore' +import type { AgentProfile, AgentRun, AgentRunDetail } from '../../types/agent' + +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/agentApi', () => ({ + agentApi: { + listProfiles: vi.fn(), + getProfile: vi.fn(), + listRuns: vi.fn(), + getRunDetail: vi.fn(), + }, +})) + +vi.mock('../../api/authApi', () => ({ + authApi: { + login: vi.fn(), + register: vi.fn(), + changePassword: vi.fn(), + refreshToken: vi.fn(), + exchangeOAuthCode: vi.fn(), + exchangeOidcCode: vi.fn(), + }, +})) + +vi.mock('../../store/toastStore', () => ({ + useToastStore: () => toastMocks, +})) + +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 sessionToken(id: string): string { + const payload = btoa(JSON.stringify({ exp: 4_102_444_800, jti: id })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/g, '') + return `header.${payload}.sig` +} + +function profile(id: string): AgentProfile { + return { + id, + userId: 'user-a', + name: `Agent ${id}`, + description: '', + templateKey: 'triage-assistant', + scopeType: 'Workspace', + scopeBoardId: null, + policyJson: '{}', + isEnabled: true, + createdAt: '2026-09-21T00:00:00Z', + updatedAt: '2026-09-21T00:00:00Z', + } +} + +function run(agentProfileId: string, id: string): AgentRun { + return { + id, + agentProfileId, + userId: 'user-a', + boardId: null, + triggerType: 'manual', + objective: id, + status: 'Completed', + summary: null, + failureReason: null, + proposalId: null, + stepsExecuted: 1, + tokensUsed: 1, + approxCostUsd: null, + startedAt: '2026-09-21T00:00:00Z', + completedAt: '2026-09-21T00:00:01Z', + createdAt: '2026-09-21T00:00:00Z', + updatedAt: '2026-09-21T00:00:01Z', + } +} + +function detail(agentProfileId: string, id: string): AgentRunDetail { + return { ...run(agentProfileId, id), events: [] } +} + +describe('agentStore async ownership', () => { + let session: ReturnType + let store: ReturnType + + beforeEach(() => { + setActivePinia(createPinia()) + session = useSessionStore() + session.userId = 'user-a' + store = useAgentStore() + vi.clearAllMocks() + }) + + it('keeps the newest profile read when responses settle in reverse order', async () => { + const older = deferred() + const newer = deferred() + vi.mocked(agentApi.listProfiles) + .mockReturnValueOnce(older.promise) + .mockReturnValueOnce(newer.promise) + + const oldRequest = store.fetchProfiles() + const newRequest = store.fetchProfiles() + newer.resolve([profile('new')]) + await newRequest + older.resolve([profile('old')]) + await oldRequest + + expect(store.profiles.map(item => item.id)).toEqual(['new']) + }) + + it('keeps the newest A run list across A-old to B to A-new navigation', async () => { + const oldA = deferred() + const boardB = deferred() + const newA = deferred() + vi.mocked(agentApi.listRuns) + .mockReturnValueOnce(oldA.promise) + .mockReturnValueOnce(boardB.promise) + .mockReturnValueOnce(newA.promise) + + const oldRequest = store.fetchRuns('agent-a') + store.clearRuns() + const bRequest = store.fetchRuns('agent-b') + store.clearRuns() + const newRequest = store.fetchRuns('agent-a') + + newA.resolve([run('agent-a', 'new-a')]) + await newRequest + boardB.resolve([run('agent-b', 'b')]) + await bRequest + oldA.resolve([run('agent-a', 'old-a')]) + await oldRequest + + expect(store.runs.map(item => item.id)).toEqual(['new-a']) + }) + + it('keeps the newest run detail across repeated route identities', async () => { + const oldA = deferred() + const other = deferred() + const newA = deferred() + vi.mocked(agentApi.getRunDetail) + .mockReturnValueOnce(oldA.promise) + .mockReturnValueOnce(other.promise) + .mockReturnValueOnce(newA.promise) + + const oldRequest = store.fetchRunDetail('agent-a', 'run-a') + store.clearRunDetail() + const otherRequest = store.fetchRunDetail('agent-b', 'run-b') + store.clearRunDetail() + const newRequest = store.fetchRunDetail('agent-a', 'run-a') + + newA.resolve(detail('agent-a', 'new-a')) + await newRequest + other.resolve(detail('agent-b', 'other')) + await otherRequest + oldA.resolve(detail('agent-a', 'old-a')) + await oldRequest + + expect(store.runDetail?.id).toBe('new-a') + }) + + it('clearRuns invalidates a pending success', async () => { + const pending = deferred() + vi.mocked(agentApi.listRuns).mockReturnValue(pending.promise) + + const request = store.fetchRuns('agent-a') + expect(store.runsLoading).toBe(true) + store.clearRuns() + expect(store.runsLoading).toBe(false) + + pending.resolve([run('agent-a', 'late')]) + await request + + expect(store.runs).toEqual([]) + expect(store.runsError).toBeNull() + }) + + it('clearRunDetail invalidates a pending failure without stale UI', async () => { + const pending = deferred() + vi.mocked(agentApi.getRunDetail).mockReturnValue(pending.promise) + + const request = store.fetchRunDetail('agent-a', 'run-a') + store.clearRunDetail() + pending.reject(new Error('stale detail failure')) + await expect(request).rejects.toThrow('stale detail failure') + + expect(store.runDetail).toBeNull() + expect(store.runDetailError).toBeNull() + expect(store.runDetailLoading).toBe(false) + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('does not let an older finally clear a newer same-lane loading owner', async () => { + const older = deferred() + const newer = deferred() + vi.mocked(agentApi.listRuns) + .mockReturnValueOnce(older.promise) + .mockReturnValueOnce(newer.promise) + + const oldRequest = store.fetchRuns('agent-a') + const newRequest = store.fetchRuns('agent-a') + older.resolve([run('agent-a', 'old')]) + await oldRequest + + expect(store.runsLoading).toBe(true) + + newer.resolve([run('agent-a', 'new')]) + await newRequest + expect(store.runsLoading).toBe(false) + }) + + it('preserves loaded route data while invalidating old-token reads on refresh', async () => { + setActivePinia(createPinia()) + session = useSessionStore() + session.userId = 'user-a' + session.token = sessionToken('old') + store = useAgentStore() + + store.profiles = [profile('existing')] + store.runs = [run('agent-a', 'existing')] + store.runDetail = detail('agent-a', 'existing') + + const profiles = deferred() + const runs = deferred() + const runDetail = deferred() + vi.mocked(agentApi.listProfiles).mockReturnValue(profiles.promise) + vi.mocked(agentApi.listRuns).mockReturnValue(runs.promise) + vi.mocked(agentApi.getRunDetail).mockReturnValue(runDetail.promise) + + const profileRequest = store.fetchProfiles() + const runsRequest = store.fetchRuns('agent-a') + const detailRequest = store.fetchRunDetail('agent-a', 'run-a') + + session.token = sessionToken('new') + + expect(store.profiles.map(item => item.id)).toEqual(['existing']) + expect(store.runs.map(item => item.id)).toEqual(['existing']) + expect(store.runDetail?.id).toBe('existing') + expect(store.profilesLoading).toBe(false) + expect(store.runsLoading).toBe(false) + expect(store.runDetailLoading).toBe(false) + + profiles.resolve([profile('old-token')]) + runs.reject(new Error('old-token failure')) + runDetail.resolve(detail('agent-a', 'old-token')) + + await profileRequest + await expect(runsRequest).rejects.toThrow('old-token failure') + await detailRequest + + expect(store.profiles.map(item => item.id)).toEqual(['existing']) + expect(store.runs.map(item => item.id)).toEqual(['existing']) + expect(store.runDetail?.id).toBe('existing') + expect(store.runsError).toBeNull() + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('retries empty initial route reads after same-user token rotation', async () => { + setActivePinia(createPinia()) + session = useSessionStore() + session.userId = 'user-a' + session.token = sessionToken('old') + store = useAgentStore() + + const oldProfiles = deferred() + const freshProfiles = deferred() + const oldRuns = deferred() + const freshRuns = deferred() + const oldDetail = deferred() + const freshDetail = deferred() + vi.mocked(agentApi.listProfiles) + .mockReturnValueOnce(oldProfiles.promise) + .mockReturnValueOnce(freshProfiles.promise) + vi.mocked(agentApi.listRuns) + .mockReturnValueOnce(oldRuns.promise) + .mockReturnValueOnce(freshRuns.promise) + vi.mocked(agentApi.getRunDetail) + .mockReturnValueOnce(oldDetail.promise) + .mockReturnValueOnce(freshDetail.promise) + + const profileRequest = store.fetchProfiles() + const runsRequest = store.fetchRuns('agent-a') + const detailRequest = store.fetchRunDetail('agent-a', 'run-a') + + session.token = sessionToken('new') + + expect(agentApi.listProfiles).toHaveBeenCalledTimes(2) + expect(agentApi.listRuns).toHaveBeenCalledTimes(2) + expect(agentApi.getRunDetail).toHaveBeenCalledTimes(2) + expect(store.profilesLoading).toBe(true) + expect(store.runsLoading).toBe(true) + expect(store.runDetailLoading).toBe(true) + + oldProfiles.resolve([profile('old-token')]) + oldRuns.reject(new Error('old-token failure')) + oldDetail.resolve(detail('agent-a', 'old-token')) + await profileRequest + await expect(runsRequest).rejects.toThrow('old-token failure') + await detailRequest + + expect(store.profiles).toEqual([]) + expect(store.runs).toEqual([]) + expect(store.runDetail).toBeNull() + expect(store.runsError).toBeNull() + expect(toastMocks.error).not.toHaveBeenCalled() + + freshProfiles.resolve([profile('fresh-token')]) + freshRuns.resolve([run('agent-a', 'fresh-token')]) + freshDetail.resolve(detail('agent-a', 'fresh-token')) + + await vi.waitFor(() => { + expect(store.profiles.map(item => item.id)).toEqual(['fresh-token']) + expect(store.runs.map(item => item.id)).toEqual(['fresh-token']) + expect(store.runDetail?.id).toBe('fresh-token') + expect(store.profilesLoading).toBe(false) + expect(store.runsLoading).toBe(false) + expect(store.runDetailLoading).toBe(false) + }) + }) + + it('clears every surface and invalidates pending reads on session replacement', async () => { + store.profiles = [profile('existing')] + store.runs = [run('agent-a', 'existing')] + store.runDetail = detail('agent-a', 'existing') + + const profiles = deferred() + const runs = deferred() + const runDetail = deferred() + vi.mocked(agentApi.listProfiles).mockReturnValue(profiles.promise) + vi.mocked(agentApi.listRuns).mockReturnValue(runs.promise) + vi.mocked(agentApi.getRunDetail).mockReturnValue(runDetail.promise) + + const profileRequest = store.fetchProfiles() + const runsRequest = store.fetchRuns('agent-a') + const detailRequest = store.fetchRunDetail('agent-a', 'run-a') + + session.userId = null + session.userId = 'user-a' + + expect(store.profiles).toEqual([]) + expect(store.runs).toEqual([]) + expect(store.runDetail).toBeNull() + expect(store.profilesLoading).toBe(false) + expect(store.runsLoading).toBe(false) + expect(store.runDetailLoading).toBe(false) + + profiles.resolve([profile('old-session')]) + runs.resolve([run('agent-a', 'old-session')]) + runDetail.resolve(detail('agent-a', 'old-session')) + await Promise.all([profileRequest, runsRequest, detailRequest]) + + expect(store.profiles).toEqual([]) + expect(store.runs).toEqual([]) + expect(store.runDetail).toBeNull() + }) + + it('suppresses a stale failure after logout and same-user login', async () => { + const pending = deferred() + vi.mocked(agentApi.listRuns).mockReturnValue(pending.promise) + const request = store.fetchRuns('agent-a') + + session.userId = null + session.userId = 'user-a' + pending.reject(new Error('old-session failure')) + await expect(request).rejects.toThrow('old-session failure') + + expect(store.runsError).toBeNull() + expect(store.runsLoading).toBe(false) + expect(toastMocks.error).not.toHaveBeenCalled() + }) + + it('keeps independent lanes concurrent with truthful loading', async () => { + const profiles = deferred() + const runs = deferred() + const runDetail = deferred() + vi.mocked(agentApi.listProfiles).mockReturnValue(profiles.promise) + vi.mocked(agentApi.listRuns).mockReturnValue(runs.promise) + vi.mocked(agentApi.getRunDetail).mockReturnValue(runDetail.promise) + + const profileRequest = store.fetchProfiles() + const runsRequest = store.fetchRuns('agent-a') + const detailRequest = store.fetchRunDetail('agent-a', 'run-a') + + profiles.resolve([profile('profile')]) + await profileRequest + expect(store.profilesLoading).toBe(false) + expect(store.runsLoading).toBe(true) + expect(store.runDetailLoading).toBe(true) + + runs.resolve([run('agent-a', 'run')]) + await runsRequest + expect(store.runsLoading).toBe(false) + expect(store.runDetailLoading).toBe(true) + + runDetail.resolve(detail('agent-a', 'run')) + await detailRequest + expect(store.runDetailLoading).toBe(false) + }) +})