Tasks: Fix UI State Bugs on Browser Refresh
Input: Design documents from docs/docs/specs/2026-05-15-fix-refresh-state-bugs/
Prerequisites: plan.md (required), spec.md (required), research.md, data-model.md, quickstart.md
Tests: Included. The plan explicitly maps each invariant (Inv-A, Inv-B, Inv-C, Inv-C2, Inv-D, Inv-E, Inv-F, Inv-G) to specific test files, and CAIPE Constitution VI ("CI Gates Are Non-Negotiable") requires lint + test gates.
Organization: Tasks are grouped by user story so each story can be implemented, tested, and shipped independently.
Format: [ID] [P?] [Story] Description
- [P]: Can run in parallel (different files, no dependencies on incomplete tasks)
- [Story]: User story label (US1, US2, US3). No story label on Setup / Foundational / Polish tasks.
- Include exact file paths.
Path Conventions
UI-only feature. All paths are under ui/ in the repository root. No backend (Python) files are touched. No db-migration.md (storage unchanged).
Phase 1: Setup (Shared Infrastructure)
Purpose: Verify the UI workspace is ready for editing and the existing test suites pass before any code change.
- T001 Verify UI workspace is installed and tests pass on
main. Runcd ui && npm ci && npm run lint && npm testand confirm green baseline. No file changes. - T002 Confirm the four target source files compile and the three target test files exist in the workspace:
ui/src/lib/api-middleware.ts,ui/src/store/chat-store.ts,ui/src/components/chat/ChatContainer.tsx,ui/src/app/api/__tests__/admin-audit-access.test.ts,ui/src/store/__tests__/chat-store.test.ts,ui/src/components/chat/__tests__/ChatPanel.test.tsx. No file changes.
Phase 2: Foundational (Blocking Prerequisites)
Purpose: (a) Document the persistence-scope invariant that all subsequent tasks must respect, and (b) verify that the planned server-side authorization change is safe with respect to write-path enforcement and audit logging (analyze findings S1 and S2 — must be confirmed before T025 lands).
⚠️ BLOCKING: T028 must complete before T025. T003 should complete before T013.
-
T003 Add a code comment directly above the
partializeblock inui/src/store/chat-store.ts(around L1870) documenting Inv-E fromdata-model.md. The comment must list the two scopes (kept in sync with theTOP_LEVEL_DENYLIST/RECURSIVE_DENYLISTconstants T013a introduces):- Top-level keys of the persisted output and of each
ConversationMUST NOT include (11 entries):access_level,accessLevel,readOnlyReason,readOnly,adminOrigin,isAdmin,canViewAdmin,sessionRole,authRole,role,userRole. - Any nesting depth MUST NOT include (10 entries —
TOP_LEVEL_DENYLISTminus the singleroleexception):access_level,accessLevel,readOnlyReason,readOnly,adminOrigin,isAdmin,canViewAdmin,sessionRole,authRole,userRole. The barerolekey is the ONLY recursive-denylist omission and is exempted becauseChatMessage.role: 'user' \| 'assistant' \| 'system'is the canonical message-sender field on every persisted message; stripping it would break every persisted conversation. Any future authorization-role field MUST use one of the disambiguated names (sessionRole,authRole,userRole) — never barerole.
Broader principle (finding N9 in
/speckit.analyzere-run): the comment must ALSO state the general guideline beyond the denylist —partializepersists conversation data only. Transient/UI/loading/timing flags (e.g.,isLoadingConversations,isLoadingAutonomous,streamingConversations,pendingMessage,inputDraft) belong in module-level variables (see the existingisLoadingConversationsat L883) or component-localuseState(seeChatContainer'saccessLevel/agentInfo), NEVER in the persisted store state. The denylist is keyed signals; the principle is the broader safeguard against an inadvertent persistence of any session-scoped flag the future may add.Reference
docs/docs/specs/2026-05-15-fix-refresh-state-bugs/data-model.md(Inv-E) and state that the enforcing test (T013) walks the object tree, not the serialized string. - Top-level keys of the persisted output and of each
-
T028 Security audit (defense in depth — findings S1, S2 in
/speckit.analyze). Conduct a read-only codebase audit before T025 reordersrequireConversationAccess. Produce a findings note appended toresearch.md(under a new section "Phase 0 addendum: write-path and audit-log audit") that answers:- Write-path safety (S1): Run
rg "admin_audit|shared_readonly|access_level" ui/src/app/api -n -t tsand inspect every match in a write-side route handler (POST, PUT, PATCH, DELETE). For each match, confirm the handler treatsadmin_auditandshared_readonlyidentically as "no write" (i.e., no handler grants write capability based onaccess_level === 'admin_audit'). If any handler does grant additional power onadmin_audit, document the specific file:line and STOP — escalate to a spec clarification before continuing. - Audit-log behaviour (S2): Inspect
ui/src/app/api/admin/audit-logs/and any code path that writes audit-log entries (rg "audit_log|auditLog|access_level.*admin_audit" ui/src/app/api -n -t ts). Determine whether admin views of autonomous-source conversations are currently logged via theadmin_auditsignal. Document the finding. If admin views of autonomous conversations are currently logged and the spec requires that to continue, add a follow-up task to T025 that preserves the logging on an orthogonal signal (e.g.,is_admin && !is_ownerregardless ofsource); if not required, document that the logging change is intentional with a one-line rationale. - Output: A short "Findings" paragraph in
research.mdPhase 0 addendum, plus a green/red flag for each of (1) and (2). Green on both → T025 can proceed unchanged. Red on either → block T025 and update plan.
(FR-008 / Inv-F persistence-side coverage is enforced by the T013 + T013a pair: T013a installs
stripDenylistedKeysinsidepartializeso every denylisted key is removed at write-time regardless of how it got onto store state, and T013 is a real Inv-E gate that explicitly INJECTS those keys onto the seed state and asserts the output is clean — together they close the seed-shape blind spot that an earlier draft of T013 had. A separatergaudit for store-setter writes was considered and dropped as redundant: a regression in T013a's strip would surface as a T013 failure on the next test run, and any future code that writes a denylisted key to store state still produces clean persisted output because the strip is unconditional.) - Write-path safety (S1): Run
Checkpoint: Persistence-scope invariant is documented (T003), and the write-path / audit-log audit (T028) is green. US1 and US2 work can now begin in parallel.
Phase 3: User Story 1 — No duplicate conversation tab after refresh (Priority: P1) 🎯 MVP
Goal: After a browser refresh, the chat sidebar contains exactly one entry per unique conversation id, regardless of source. The previously-active conversation is highlighted exactly once. (Spec FR-001, FR-002, FR-003, FR-007, FR-010; Inv-A, Inv-B.)
Independent Test: Open an autonomous conversation as any user, send a typed reply, hard-refresh the browser; verify exactly one sidebar entry remains (DevTools: const ids = useChatStore.getState().conversations.map(c => c.id); new Set(ids).size === ids.length). Repeat with a non-autonomous conversation as the active chat.
Tests for User Story 1 (write FIRST, ensure they FAIL before implementation)
-
T010 [P] [US1] Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedloadAutonomousConversationsFromService dedupes by idthat seedsstate.conversationswith one entry{ id: 'X', source: undefined }(mimicking a persisted/API-fetched entry that lost itssourcetag) AND seedsstate.activeConversationId = 'X', mocksautonomousApi.listTasks/listRunsto produce a task whose canonical id is'X', calls the action, and asserts: (a) the resultingconversationsarray has exactly one entry withid: 'X'andsource === 'autonomous'(FR-001, FR-003); AND (b)state.activeConversationId === 'X'still maps to exactly one entry instate.conversations, satisfying FR-002. Must fail against currentmain. -
T011 [P] [US1] Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedloadAutonomousConversationsFromService preserves user-typed messages on dedupethat seeds an existing entry{ id: 'X', source: undefined, messages: [{ id: 'user-msg-1', role: 'user', content: 'hello', ... }] }and a synthesized autonomous task with canonical id'X'and its own synth messages, then asserts the final entry forid: 'X'contains both the synth messages anduser-msg-1, sorted bytimestamp. Must fail against currentmain. -
T012 [P] [US1] Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedloadConversationsFromServer produces unique ids after local-only preservationthat seeds an existing local-only active conversation{ id: 'Y', source: undefined }, mocks the API to return the same id in itsitemsarray, calls the loader, and asserts the resulting list has exactly one entry withid: 'Y'. Must fail against currentmain. -
T012a [P] [US1] Back-to-back-refresh resilience test (finding C2 in
/speckit.analyze). Add a Jest test inui/src/store/__tests__/chat-store.test.tsnamedonRehydrateStorage dedupes duplicate ids in persisted localStorage. Pre-populatelocalStorage['caipe-chat-history']with a Zustand-persist payload whosestate.conversationsarray contains two entries that share the sameid(one withsource: undefined, one withsource: 'autonomous', each with a distinctmessageslist andupdatedAt). Re-import / re-construct the store soonRehydrateStorageruns. Assert: (a)useChatStore.getState().conversationscontains exactly one entry for that id; (b) the surviving entry hassource === 'autonomous'; (c) the surviving entry'smessagesis the union by message id of both seed entries' messages, sorted by timestamp; (d) no network loader was invoked between rehydrate and the assertion — to avoid brittle action-spying, assert this by spying on the underlying network calls (apiClient.getConversationsandautonomousApi.listTasks) and asserting both were called zero times. Must fail against currentmain(todayonRehydrateStoragedoes no dedupe).(Cross-cutting consistency between
selectedTurnIdsand the merged-messages output is covered by T011, which already asserts user-typed messages survive the dedupe bymessage.id— no additional T012a sub-assertion needed.) -
T013 [P] [US1] Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedpartialize never persists authorization or session fields. The test MUST be a real Inv-E gate, not a passive spread-shape regression: it has to inject every denylisted key onto the seed state at root, conversation, and message level (defeating the future-proofing failure mode wherepartializedoes{ ...conv, a2aEvents: [], ... }and silently carries through any new field added toConversationorChatMessage), then assert those keys are stripped from thepartializeoutput.Arrange (the seed MUST exercise both the substring path AND the key path):
- Build a base state with one conversation. The conversation's
titleand onemessage.contentMUST deliberately include the literal substringsaccess_level,accessLevel,isAdmin, andadminOrigin— this defeats any naive substring matcher a future author might write against the serialized JSON. - Include at least one message with
role: 'user'and one withrole: 'assistant'(the barerolefield on messages is the legitimate non-authorization use that motivates the split denylist below). - Inject denylisted KEYS onto the state via
as anycasts so the test compiles even if theChatState/Conversation/ChatMessagetypes don't declare them:This step is what makes T013 a real defense for FR-008 / Inv-F. Without it the test would pass trivially against the current shape and silently regress the moment any future setter wrote one of these keys onto store state.(state as any).accessLevel = 'admin_audit';
(state as any).readOnlyReason = 'admin_audit';
(state as any).adminOrigin = 'audit-logs';
(state as any).isAdmin = true;
(state.conversations[0] as any).access_level = 'admin_audit';
(state.conversations[0] as any).accessLevel = 'admin_audit';
(state.conversations[0] as any).readOnlyReason = 'admin_audit';
(state.conversations[0] as any).adminOrigin = 'audit-logs';
(state.conversations[0].messages[0] as any).accessLevel = 'admin_audit';
(state.conversations[0].messages[0] as any).adminOrigin = 'audit-logs';
(state.conversations[0].messages[0] as any).isAdmin = true;
Act: invoke the
partializefunction on that state and parse the result as JSON / treat it as a plain object.Assert — define two
constarrays at the top of the test:// Canonical denylists. RECURSIVE is TOP_LEVEL minus the single 'role'
// exception (ChatMessage.role is the legitimate message-sender field).
// Keep these constants in sync with the stripDenylistedKeys helper in
// chat-store.ts (T013a) and the comment above partialize (T003).
const TOP_LEVEL_DENYLIST = ['access_level', 'accessLevel', 'readOnlyReason', 'readOnly',
'adminOrigin', 'isAdmin', 'canViewAdmin', 'sessionRole', 'authRole', 'role', 'userRole'] as const;
const RECURSIVE_DENYLIST = ['access_level', 'accessLevel', 'readOnlyReason', 'readOnly',
'adminOrigin', 'isAdmin', 'canViewAdmin', 'sessionRole', 'authRole', 'userRole'] as const;Walk every nested object recursively and assert: (a) no own enumerable key of the root persisted object matches
TOP_LEVEL_DENYLIST; (b) no own enumerable key of any element ofconversations[]matchesTOP_LEVEL_DENYLIST; (c) at every nesting depth (root + conversations + messages + arbitrary nested objects), no own enumerable key matchesRECURSIVE_DENYLIST. The barerolefield on messages is the only key allowed to appear (it is the message-sender role, not an authorization role) —RECURSIVE_DENYLISTis preciselyTOP_LEVEL_DENYLISTminus'role'.Substring search on the serialized string is forbidden by the test author note (false positives on titles / message bodies; false negatives if a future field encodes its value differently).
Must fail against current
main(because today'spartializedoes{ ...conv, ... }which copies the injectedconv.access_level/conv.accessLevelthrough to the output). Must pass after the implementation explicitly strips the denylisted keys from each spread (e.g., by replacing{ ...conv, a2aEvents: [], ... }with an explicit pick of the persisted fields, or by reduce-then-delete). The implementation change required to make T013 green is part of the regression guard scope — ifpartializecurrently lets these keys through, that is itself the bug Inv-E exists to prevent.(Regression guard for Inv-E; closes finding S4 in
/speckit.analyze, finding C1 in/speckit.analyzere-run, and finding A1 in the latest re-analysis — the prior T013 wording exercised only the substring path and never the key path.) - Build a base state with one conversation. The conversation's
Implementation for User Story 1
-
T013a [US1] Defensive
partialize(Inv-E hardening; required to make T013 green per finding A1 in the latest/speckit.analyze). Inui/src/store/chat-store.ts, update thepartializeblock (around L1870) so that each persistedConversationand each persistedChatMessageis constructed by explicitly picking the persisted fields, OR by spreading and thendelete-ing every key inTOP_LEVEL_DENYLIST(forConversation) /RECURSIVE_DENYLIST(forChatMessage). The intent is: even if a future setter writes one of those keys ontostate.conversations[i]orstate.conversations[i].messages[j],partializeMUST strip it before the persist middleware writes tolocalStorage.Recommended approach (minimal, matches existing style):
const stripDenylistedKeys = <T extends Record<string, unknown>>(
obj: T,
keys: readonly string[],
): Partial<T> => {
const out: Record<string, unknown> = { ...obj };
for (const k of keys) delete out[k];
return out as Partial<T>;
};
// Inside partialize:
conversations: state.conversations.map((conv) => stripDenylistedKeys({
...conv,
a2aEvents: [],
streamEvents: [],
messages: conv.messages.map((msg) => stripDenylistedKeys({
...msg,
events: [],
}, RECURSIVE_DENYLIST)),
}, TOP_LEVEL_DENYLIST)),The
TOP_LEVEL_DENYLIST/RECURSIVE_DENYLISTarrays are declared once at module scope (or imported from a tiny helper module) so the test in T013 and the implementation in T013a reference the same lists by name — preventing them from drifting out of sync. Note that'role'is intentionally inTOP_LEVEL_DENYLIST(Conversation has no legitimaterolefield) but NOT inRECURSIVE_DENYLIST(ChatMessage.roleis legitimate); applyingRECURSIVE_DENYLISTto messages preserves the sender field while still stripping every authorization key.Add a short inline
// Inv-E:comment referencingdata-model.mdand noting this strip is the runtime enforcement of the persistence-scope invariant — the T013 test is the corresponding gate. -
T014 [US1] In
ui/src/store/chat-store.ts, modifyloadAutonomousConversationsFromService(theset((state) => { ... })block around L1108–L1177) so the final conversation list is deduplicated byid. Replace the currentconst final = [...others, ...merged].sort(...)line with aMap<string, Conversation>keyed byid, populated by first inserting every entry fromothersand then overwriting with every entry frommerged(autonomous-synth wins on collision). Convert the Map's values back to an array and sort withcompareConversationsForSidebar. Preserves the existing per-message merge insidemerged.map(...)for the autonomous-tagged case (L1157–L1161); the new dedupe handles the non-autonomous-tagged collision case. -
T015 [US1] In
ui/src/store/chat-store.ts, modifyloadConversationsFromServer(around L992–L1011 + L1011) so theallConversations = [...serverConversations, ...localOnlyPreserved]concatenation also goes through aMap<string, Conversation>dedupe keyed byid. Insertion semantics (server-wins-on-collision): insert every entry fromserverConversationsfirst; then iteratelocalOnlyPreservedand insert each entry only if itsidis not already present in the Map (i.e. do NOT overwrite an existing server entry — server is authoritative forsourceand metadata). Concretely the second pass isif (!map.has(conv.id)) map.set(conv.id, conv), NOTmap.set(conv.id, conv). In practice no collision occurs at this site because L1000–L1001 already filters local-only-preserved by!serverIds.has(conv.id); this dedupe is defense in depth so no future code path that drops that filter can introduce duplicates at this merge site. -
T015a [US1] Rehydrate-time dedupe (finding C2 in
/speckit.analyze— closes back-to-back-refresh andautonomousAgentsEnabled = falseedge cases; findings N2, N5, N7, N8 in/speckit.analyzere-run). Inui/src/store/chat-store.ts, modifyonRehydrateStorage(around L1883–L1943) to add a Map-based dedupe-by-id pass overstate.conversationsafter the existing legacy-id filter (L1892–L1894) and before the messages re-mapping (L1905–L1938).Defensive guards (run BEFORE the dedupe pass):
- Null-state guard (N7): if
!state || !Array.isArray(state.conversations),returnearly — rehydration failed or persisted shape is unexpected; let the store start empty. - Invalid-id guard (N8): before inserting any entry into the dedupe Map, skip entries where
typeof conv.id !== 'string' || conv.id.length === 0. Defense against corrupted/tamperedlocalStorage.
Insertion strategy when two persisted entries share an id (deterministic, NaN-safe per finding N5 in
/speckit.analyzere-run):- Iterate
state.conversationsonce, buildingMap<id, Conversation>. - Winner selection (apply in order; first rule that produces a winner wins):
a. If exactly one of the two has
source === 'autonomous', that one wins. b. Otherwise prefer the entry with moremessages.length. c. Otherwise prefer the entry whoseupdatedAtparses to a finite, valid number — i.e.,Number.isFinite(new Date(updatedAt).getTime())is true. If exactly one is valid, it wins. (Without this rule, missing/null/malformedupdatedAtproducesNaNand the Map insertion-order winner becomes non-deterministic.) d. Otherwise (both have validupdatedAt) prefer the most-recentnew Date(updatedAt).getTime(). e. Otherwise (both invalidupdatedAt, last-resort) prefer the entry whoseidsorts first lexicographically. - Message-level merge (N2 in original
/speckit.analyze): after selecting the winning entry, MERGE messages from BOTH collision entries bymessage.id. Concretely: buildMap<message.id, ChatMessage>populated first from the winner's messages, then from the loser's messages (winner wins per-message-id on collision). Convert back to an array and sort bytimestampascending with NaN-safe comparator (treat non-finite timestamps as+Infinityso malformed messages sort to the end without disrupting well-formed ordering). Assign this merged list to the surviving entry'smessagesfield. This guarantees no user-typed message is lost just because the entry that carried it lost the source-preference contest. - Replace
state.conversationswithArray.from(map.values()).
Document inline that this pass is the ONLY thing protecting users in
localStoragemode +autonomousAgentsEnabled = false(where neither network loader runs) from duplicate persistence carried over from before this fix shipped, and is also what makes back-to-back F5 refreshes self-healing without any network call. - Null-state guard (N7): if
-
T015c [US1] Cross-loader clobber fix (finding N1 in
/speckit.analyzere-run; Inv-G indata-model.md). Inui/src/store/chat-store.ts, convert the imperativeset({ conversations: sortedConversations, ... })at the end ofloadConversationsFromServer(around L1018) to the callback form so that any autonomous-source entries written byloadAutonomousConversationsFromServicebetween this loader's snapshot read (L954) and its write are NOT discarded. Concretely:set((state) => {
// Keep entries the other loader wrote between L954 and now (Inv-G).
// Autonomous-synth entries are the canonical case (cross-loader race);
// also keep any locally-streaming entries that appeared mid-load.
const serverIds = new Set(sortedConversations.map(c => c.id));
const crossLoaderAdditions = state.conversations.filter(c =>
!serverIds.has(c.id) && (c.source === 'autonomous' || state.streamingConversations.has(c.id))
);
const merged = [...sortedConversations, ...crossLoaderAdditions];
// Map-based dedupe-by-id (defense in depth; should be a no-op given the filter above)
const deduped = Array.from(new Map(merged.map(c => [c.id, c])).values())
.sort(compareConversationsForSidebar);
return {
conversations: deduped,
...(activeId && !deduped.some(c => c.id === activeId) ? {
activeConversationId: deduped.length > 0 ? deduped[0].id : null,
a2aEvents: [],
} : {}),
};
});Add an inline comment referencing Inv-G in
data-model.mdexplaining that the callback-formset()protects cross-loader interleaving (the same-loader case is already covered byisLoadingConversationsat L883 for the server loader; the autonomous loader's Map-based dedupe is idempotent on a single snapshot, so no equivalent guard is required there).Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedloadConversationsFromServer + loadAutonomousConversationsFromService interleave preserves both loaders' contributions. Setup:- Mock
apiClient.getConversationsto return a controllable promise (resolve manually) yielding one server item with id'S', nosource. - Mock
autonomousApi.listTasks/listRunsto return one autonomous task with canonical id'A'. - Invoke
loadConversationsFromServer()(do NOT await) and let it reach the snapshot read. - Invoke
loadAutonomousConversationsFromService()and await its completion (writes{id: 'A', source: 'autonomous'}to the store). - Resolve the server loader's promise so it proceeds to
set(...). - Await the server loader.
Assert the final
state.conversationscontains BOTH'S'and'A'with'A'retainingsource === 'autonomous'. Pre-fix, this test fails because the server loader's imperativesetclobbers the autonomous entry. Post-fix, both survive. - Mock
-
T016 [US1] Re-run the four new dedupe tests (T010, T011, T012, T012a), the partialize regression test (T013, which exercises T013a's defensive strip), and the cross-loader interleave test from T015c; confirm all seven are green. Run
npm run lintfor the UI workspace and fix any new lint warnings introduced by T013a/T014/T015/T015a/T015c.
Checkpoint: User Story 1 is fully functional and independently testable. The sidebar shows exactly one entry per id after any sequence of loadConversationsFromServer + loadAutonomousConversationsFromService calls.
Phase 4: User Story 2 — Refresh does not unexpectedly put the user in Audit Mode (Priority: P1)
Goal: After a browser refresh, the Read-Only Audit Mode banner appears only when (a) the user is an admin and (b) they actually navigated from the admin audit-logs or feedback views in the current session (?from=audit-logs|feedback is present). Non-admin users never see the banner. Autonomous conversations never resolve to admin_audit server-side. When the server returns admin_audit but the gate evaluates false (no ?from=, or unrecognized value), the UI routes the case through the existing shared_readonly treatment. (Spec FR-004, FR-005, FR-006, FR-008, FR-009; Inv-C, Inv-C2, Inv-D.)
Independent Test: Sign in as an admin. Open an autonomous conversation directly (no ?from=...), hard-refresh; verify no audit banner. Open another user's non-autonomous conversation via a deep link (no ?from=...), hard-refresh; verify no audit banner. Open a conversation via /admin?tab=audit-logs (URL becomes ?from=audit-logs), hard-refresh; verify audit banner still shows with "Back to Audit Logs" link.
Tests for User Story 2 (write FIRST, ensure they FAIL before implementation)
- T020 [P] [US2] Add a Jest test in
ui/src/app/api/__tests__/admin-audit-access.test.tsnamedadmin viewing autonomous conversation returns shared_readonlythat mocks an admin session (session.role === 'admin') and a non-owned conversation withsource: 'autonomous', callsrequireConversationAccess, and asserts the returnedaccess_level === 'shared_readonly'. Must fail against currentmain. - T021 [P] [US2] Add a Jest test in the same file named
admin viewing non-autonomous non-owned conversation still returns admin_auditthat mocks an admin session and a non-owned conversation withoutsource(orsource: 'web'), and assertsaccess_level === 'admin_audit'. This regression guard must pass against currentmain. - T022 [P] [US2] Add a Jest test in
ui/src/components/chat/__tests__/ChatContainer.test.tsx(create the file if it does not exist —ChatContaineris the unit that contains the new Inv-C derivation; this test must target that unit, notChatPanel) nameddoes not render audit banner when access_level is admin_audit but adminOrigin is null. MockuseSession(return an admin session),useParams(return auuid),useSearchParams(returnnullfor'from'), and the underlying conversation API call to return{ access_level: 'admin_audit', ... }. Render<ChatContainer />and assert (a) the string "Read-Only Audit Mode" is not in the rendered output, AND (b) the "Back to Audit Logs" / "Back to Feedback" link is not rendered. Must fail against currentmain(because today the banner renders wheneveraccessLevel === 'admin_audit'). - T023 [P] [US2]
ChatPanelrendering regression guard (NOT a gate test — finding C9 in/speckit.analyze). Add a Jest test inui/src/components/chat/__tests__/ChatPanel.test.tsxnamedrenders audit banner when readOnlyReason='admin_audit' is passed in. This test exercises ONLY<ChatPanel>in isolation — render with the propsreadOnlyReason='admin_audit'andadminOrigin='audit-logs'and assert the banner + "Back to Audit Logs" link are visible. The actual gate (accessLevel === 'admin_audit' && adminOrigin ∈ {...}) lives inChatContainerand is tested in T022 (negative) / T029 / T029a — this test is purely a guard against a future refactor that breaks the rendering side of the contract while the gate computation is correct. Must pass against currentmain(regression guard for legitimate audit-flow rendering). - T024 [P] [US2] Add a Jest test in
ui/src/components/chat/__tests__/ChatPanel.test.tsxnamednon-admin user with shared_readonly access never sees audit bannerrendering withreadOnlyReason={'shared_readonly'}and asserting the "Read-Only Audit Mode" string is absent. Must pass against currentmain(regression guard for FR-005). - T029 [P] [US2] Audit-context leak test (finding S5; spec edge case "audit context must not leak onto next own-conversation view"). Add a Jest test in
ui/src/components/chat/__tests__/ChatContainer.test.tsxnameddoes not render audit banner when user owns the conversation even if adminOrigin is set. MockuseSession(admin session),useParams(return owned conversation'suuid),useSearchParams(return'audit-logs'for'from'— simulating a stale URL param carried across navigation), and the conversation API to return{ access_level: 'owner', ... }. Render<ChatContainer />and assert the "Read-Only Audit Mode" banner is not rendered (becauseaccessLevel === 'owner', theadminAuditActivegate evaluates false regardless ofadminOrigin). Must pass against currentmain(already correct due toaccessLevel === 'admin_audit'clause); kept as a regression guard so a future refactor cannot accidentally introduce an OR withadminOriginalone. - T029a [P] [US2] Cross-conversation
?from=carry-over (finding C5 in/speckit.analyze). Add a Jest test inui/src/components/chat/__tests__/ChatContainer.test.tsxnamedrenders audit banner for a different non-owned conversation when ?from=audit-logs is carried over in the URL. MockuseSession(admin),useParams(return UUID of conversation B — admin does not own B),useSearchParams(return'audit-logs'for'from'— simulating the URL carry-over from a prior audit of conversation A in the same session), and the conversation API to return{ access_level: 'admin_audit', ... }for B. Assert the banner IS rendered (the gate is satisfied: server-side admin_audit AND URL says?from=audit-logs). This documents the intentional cross-conversation behaviour (data-model.md Inv-C, "Cross-conversation?from=carry-over", item 2) so a future change cannot tighten the gate to a per-conversation pin without an explicit spec update. - T029c [P] [US2]
shared_readonlyfallback tests for the admin-without-recognized-origin case (finding N3 in/speckit.analyzere-run; spec FR-004 clarification 2; Inv-C2 option A). Add three Jest tests inui/src/components/chat/__tests__/ChatContainer.test.tsx:routes admin + admin_audit + no ?from= through the shared_readonly UI branch (no silent write-fail)— MockuseSession(admin),useParams(non-owned UUID),useSearchParams(returnnullfor'from'), API returns{ access_level: 'admin_audit', ... }. Assert: (a) the rendered output does NOT contain "Read-Only Audit Mode"; (b) the rendered output does NOT contain "Back to Audit Logs" or "Back to Feedback"; (c) the composer is NOT rendered (read-only); (d) theChatView/SupervisorChatViewprop isreadOnly={true}ANDreadOnlyReason === 'shared_readonly'(NOT'admin_audit', NOT a new value). Must fail against currentmain(today the audit banner renders, so post-T026 this test pins the fallback choice).routes admin + admin_audit + unrecognized ?from= through the shared_readonly UI branch— Same setup exceptuseSearchParamsreturns'shared-link'(or'whatever','', or any string outside the closed set). Same assertions as test 1. Documents that the gate honours ONLY'audit-logs'and'feedback'.non-admin user with non-owned shared_readonly conversation receives shared_readonly directly— MockuseSession(non-admin), API returns{ access_level: 'shared_readonly', ... },useSearchParamsreturnsnull. AssertreadOnlyReason === 'shared_readonly'. Regression guard for FR-005: this branch is reachable from BOTHaccess_level === 'shared_readonly'(any user) ANDaccess_level === 'admin_audit' + no recognized ?from=(admin only, via the Inv-C2 fallback); the test confirms non-admins reach it through path A (which is unchanged by this fix), not path B (which non-admins cannot reach because the server cannot returnadmin_auditfor them).
Implementation for User Story 2
-
T025 [P] [US2] Depends on T028 (security audit) being green. In
ui/src/lib/api-middleware.ts, modifyrequireConversationAccess(around L443–L529). Move the autonomous-source branch (currently at L525–L527) to run before the admin fallback (currently at L514–L517). Add a comment referencing Inv-D fromdata-model.mdand citing T028's findings as the justification (no write-path or audit-log regression). The new order is: owner → public-share → user-share → team-share → email-grant → autonomous → shared_readonly → admin → 403. -
T026 [US2] In
ui/src/components/chat/ChatContainer.tsx(around L404–L405), replace the currentreadOnlyReasonderivation with the gated form perdata-model.md"New derivation (post-fix)" — implementing Inv-C (audit-banner gate) and Inv-C2 (option A:shared_readonlyfallback for the admin-without-recognized-origin case; finding N3). Concretely:// Inv-C: audit banner requires BOTH server admin_audit AND a recognized
// in-session ?from= value (closed set: 'audit-logs' or 'feedback').
const adminAuditActive = accessLevel === 'admin_audit'
&& (adminOrigin === 'audit-logs' || adminOrigin === 'feedback');
// Inv-C2 (option A): server says admin_audit but the gate failed → route
// through the EXISTING shared_readonly UI branch. No new readOnlyReason
// value is introduced. ChatPanel rendering and prop-type union are
// unchanged. FR-005 still holds because non-admins cannot reach
// access_level === 'admin_audit' server-side.
const isReadOnly =
adminAuditActive
|| accessLevel === 'admin_audit'
|| accessLevel === 'shared_readonly';
const readOnlyReason =
adminAuditActive ? 'admin_audit'
: (accessLevel === 'admin_audit' || accessLevel === 'shared_readonly') ? 'shared_readonly'
: undefined;No changes to
ChatPanel.tsx,ChatView, orSupervisorChatVieware required — the existingshared_readonlyrendering branch handles the new fallback case. ThereadOnlyReasonprop-type union does NOT change.Add a comment referencing Inv-C and Inv-C2 from
data-model.mdnoting that this gating is presentation-only andadminOrigin(from URL search params) is never trusted for authorization — the server-sideaccess_levelis authoritative.Scope note (finding N2 in
/speckit.analyzere-run; data-model.md "Where the gate runs"): This gate runs on the API-roundtrip path (ChatContainer.tsxL196–L218), where the server can returnaccess_level === 'admin_audit'. It is intentionally a no-op on the local-store-hit path (L130–L143), which never assigns'admin_audit'because Inv-E forbids persisting that signal. Add an inline comment documenting this so a future reviewer does not "fix" the local-store-hit path to ALSO produce'admin_audit'(that would be a feature expansion, out of scope for this spec). -
T027 [US2] Re-run the new tests T020, T021, T022, T023, T024, T029, T029a, T029c and confirm all eight are green. Run
npm run lintfor the UI workspace.
Checkpoint: User Story 2 is fully functional. Admins refreshing on autonomous chats or other-user chats without ?from=... see no audit banner. Legitimate admin audit flow (via ?from=audit-logs|feedback) still works.
Phase 5: User Story 3 — Active conversation state survives refresh cleanly (Priority: P2)
Goal: For every conversation source (web, autonomous, slack, shared), after refresh the selected sidebar entry and the conversation shown in the main panel reference the same id; if the persisted active id no longer matches a real server-known conversation, the app falls back cleanly without fabricating a placeholder or applying any read-only treatment. (Spec User Story 3 acceptance scenarios; FR-006, FR-009.)
Independent Test: For each source type, open a conversation, refresh, verify exactly one highlighted sidebar entry and the URL/active id matches. Then delete the active conversation in another tab and refresh this tab; verify graceful fallback (no duplicate stub, no audit banner).
Tests for User Story 3 (write FIRST, ensure they FAIL or pass-as-regression as noted)
-
T030 [P] [US3] Add a Jest test in
ui/src/store/__tests__/chat-store.test.tsnamedstale activeConversationId falls back without creating a stubcovering the full US3 Scenario 2 contract (finding M4 in/speckit.analyzePass 3 — was previously only asserting the negative half). Two sub-tests:Sub-test 1 — empty server response: seed
activeConversationId: 'STALE'withconversations: [], mockapiClient.getConversations(theloadConversationsFromServerdependency) to return{ items: [] }, callloadConversationsFromServer(), then assert:- (a)
state.conversationscontains no entry withid: 'STALE'(negative half — was the only prior assertion); - (b)
state.activeConversationId === null— no stub is fabricated to satisfy the stale pointer; the selection clears cleanly (FR-006, FR-009); - (c)
state.a2aEventsis[]— no event state carried over from the stale conversation.
Sub-test 2 — server returns a different conversation: same seed but the API returns
{ items: [{ id: 'REAL', ... }] }. Assertstate.conversationshas exactly one entry withid: 'REAL', then assert the active id against the spec contract (not the current implementation):expect(state.activeConversationId).not.toBe('STALE')— the stale pointer MUST be cleared (FR-006 + FR-009);expect([null, 'REAL']).toContain(state.activeConversationId)— the spec deliberately leaves the fallback as "no selection OR most-recent valid conversation"; either value is acceptable.
This wording (disjunctive, contract-shaped) replaces the earlier instruction that told the test author to read
chat-store.tsL1014–L1024 and pin the assertion to whichever value the implementation chose. Pinning to the implementation hides the spec's intentional ambiguity from the test surface and makes the test brittle to a legitimate future change of fallback policy (e.g., from "first conversation" to "null"). The contract-shaped form will continue to pass whichever branch lands as long as the spec contract holds.Both sub-tests should pass against current
main— theactiveStillExistsbranch already implements one valid fallback (most-recent valid conversation). Test acts as a regression guard for FR-006 + FR-009 + US3 Scenario 2's clean-fallback contract, and as documentation that EITHER fallback choice is acceptable. (Closes finding A4 in the latest/speckit.analyze.) - (a)
-
T031 [P] [US3] Add a single source-agnostic Jest test in
ui/src/store/__tests__/chat-store.test.tsnameddedupe is source-agnosticthat seeds two entries with the sameidbut differentsourcevalues (the canonical bug case:source: undefinedvssource: 'autonomous') and asserts the dedupe path leaves exactly one entry. The Map-based dedupe in T014/T015/T015a keys onidalone —sourcenever appears in the key — so a single test pins the source-agnostic property; parameterizing over everysourcevalue would only re-test the same Map-key behaviour. Must fail against currentmain(this is the same bug T010 already exercises; T031 is the US3 cross-source regression guard that documents the source-agnostic contract for future maintainers).
Implementation for User Story 3
- T032 [US3] No new code beyond what US1 and US2 already deliver — T014 and T015 (dedupe-by-id) together satisfy FR-006 and the cross-source coverage. T031 acts as the cross-source regression guard. If T031 fails for a source not covered by the Map-based dedupe (because that source label is dropped/changed elsewhere), extend the dedupe in T014/T015 to handle that source; otherwise mark T032 complete with a one-line note in the spec PR description that says "implementation satisfied by T014, T015; verified by T031."
Checkpoint: User Story 3 verified. Refresh restoration is clean for every source; stale active-id falls back without side effects.
Phase 6: Polish & Cross-Cutting Concerns
Purpose: Final verification, documentation, and quality gates.
- T040 [P] Run the full UI workspace test suite:
cd ui && npm run lint && npm test. Fix any unrelated test failures only if they appear flaky; otherwise report them and leave for a separate PR. - T041 [P] Walk through the three manual repro scenarios in
docs/docs/specs/2026-05-15-fix-refresh-state-bugs/quickstart.md(Case A: admin on autonomous; Case B: admin on other-user non-autonomous; Case C: legitimate audit flow; plus the non-admin Case D safety check). Record results in the PR description. - T042 [P] Update the spec's Status field at the top of
docs/docs/specs/2026-05-15-fix-refresh-state-bugs/spec.mdfromDrafttoImplementedand add a**Closed**:date line below**Created**:. - T043 Run
make caipe-ui-testsfrom the repo root (Constitution VI quality gate) and confirm green.
Dependencies & Execution Order
Phase Dependencies
- Setup (Phase 1): No dependencies — can start immediately.
- Foundational (Phase 2): Depends on Setup completion.
- T028 (security audit) BLOCKS T025 (server-side authorization reorder). T028 must produce green flags for both write-path safety and audit-log behaviour, or T025 is rebased to a different approach (see T028 for the escalation path).
- T003 (partialize comment) is a soft prerequisite for T013 (the test it grounds) and should land before US1 tests merge.
- User Story 1 (Phase 3): Can start as soon as Phase 1 is green. T013 should land after T003. Otherwise independent of US2.
- User Story 2 (Phase 4): Test-authoring (T020–T024, T029, T029a, T029c) can start as soon as Phase 1 is green. T025 must wait for T028. T026 is independent of T028 (client-side, presentation-only).
- User Story 3 (Phase 5): Verification phase that depends on US1's implementation tasks (T014, T015). Tests T030, T031 can be authored in parallel with US1 implementation but will only go green once T014 and T015 land.
- Polish (Phase 6): Depends on all desired user stories being complete.
User Story Dependencies
- US1 ↔ US2: Independent. Different files (
chat-store.tsvsapi-middleware.ts+ChatContainer.tsx). Can be implemented in parallel by different developers. - US3: Depends on US1 implementation; depends on nothing in US2.
Within Each User Story
- Tests authored and failing before implementation lands (TDD per Constitution VI).
- US1: tests T010–T013 + T012a → implementation T013a → T014 → T015 → T015a → T015c → verification T016.
- US2: T028 (security audit, BLOCKING for T025) → tests T020–T024, T029, T029a, T029c → implementation T025 (server, after T028 green) ∥ T026 (client) → verification T027.
- US3: tests T030, T031 → no implementation beyond US1; verification T032.
Parallel Opportunities
- All test-authoring tasks marked [P] across phases can be written in parallel (T010–T013, T012a, T020–T024, T029, T029a, T029c, T030, T031 — each lives in a distinct test file or a distinct
describeblock inchat-store.test.ts/ChatContainer.test.tsx/ChatPanel.test.tsx). - T013a, T014, T015, T015a, T015c (all in
chat-store.ts) must be sequential to avoid merge conflicts in the same file. Recommended order: T013a → T014 → T015 → T015a → T015c. - T025 (
api-middleware.ts) and T026 (ChatContainer.tsx) are different files → parallel after T028 is green. - T028 is read-only investigation and can run in parallel with all US1 work and all US2 test authoring; it only gates T025.
- All Polish tasks marked [P] can run in parallel.
Parallel Example: User Story 1
# Author all five US1 tests in parallel (different describe blocks in the same file):
Task: "T010 — Add 'loadAutonomousConversationsFromService dedupes by id' test in ui/src/store/__tests__/chat-store.test.ts"
Task: "T011 — Add 'preserves user-typed messages on dedupe' test in ui/src/store/__tests__/chat-store.test.ts"
Task: "T012 — Add 'loadConversationsFromServer produces unique ids' test in ui/src/store/__tests__/chat-store.test.ts"
Task: "T012a — Add 'onRehydrateStorage dedupes duplicate ids in persisted localStorage' test in ui/src/store/__tests__/chat-store.test.ts"
Task: "T013 — Add 'partialize never persists authorization fields' test in ui/src/store/__tests__/chat-store.test.ts"
# Then implementation (sequential — all five edits in chat-store.ts):
Task: "T013a — Defensive partialize: explicit denylist-strip per Conversation and ChatMessage (Inv-E runtime enforcement)"
Task: "T014 — Map-based dedupe in loadAutonomousConversationsFromService"
Task: "T015 — Map-based dedupe in loadConversationsFromServer"
Task: "T015a — Rehydrate-time dedupe in onRehydrateStorage (defensive guards + winner selection + message merge)"
Task: "T015c — Callback-form set() in loadConversationsFromServer to prevent cross-loader clobber (Inv-G)"
Parallel Example: User Story 2
# 1. Run the security audit FIRST (blocks T025):
Task: "T028 — Read-only audit of API write handlers and audit-log writers; append findings to research.md"
# 2. Author US2 tests in parallel (three files; can run alongside T028):
Task: "T020 — admin + autonomous → shared_readonly test in admin-audit-access.test.ts"
Task: "T021 — admin + non-autonomous non-owned → admin_audit regression test (same file)"
Task: "T022 — audit banner hidden without adminOrigin test in ChatContainer.test.tsx"
Task: "T023 — audit banner shown with adminOrigin=audit-logs (regression) test in ChatPanel.test.tsx"
Task: "T024 — non-admin shared_readonly never sees audit banner (regression) test in ChatPanel.test.tsx"
Task: "T029 — audit context does not leak onto own conversation (regression) test in ChatContainer.test.tsx"
Task: "T029a — cross-conversation ?from= carry-over renders banner on conversation B in ChatContainer.test.tsx"
Task: "T029c — shared_readonly fallback (Inv-C2) for admin without recognized ?from= (3 sub-tests) in ChatContainer.test.tsx"
# 3. Then implementation (T025 blocked on T028 green; T026 unblocked):
Task: "T025 — Reorder requireConversationAccess in api-middleware.ts (after T028 green)"
Task: "T026 — Gate admin_audit on adminOrigin in ChatContainer.tsx"
Implementation Strategy
MVP First (User Story 1 Only)
- Complete Phase 1: Setup (T001, T002).
- Complete Phase 2 foundational items needed for US1 only (T003). T028 is only required for US2's T025 and can be deferred until US2 work begins.
- Complete Phase 3: User Story 1 (tests T010–T013 + T012a → implementation T013a → T014 → T015 → T015a → T015c → verification T016).
- STOP and VALIDATE: Run quickstart Repro 1 (duplicate tab). Deploy if green.
This MVP alone closes the more visible of the two reported bugs (duplicate autonomous tab) and is independently shippable.
Incremental Delivery
- Setup + Foundational → baseline.
- US1 → Test (Repro 1) → Demo / merge → MVP shipped.
- US2 → Test (Repros 2A/B/C) → Demo / merge.
- US3 → Verify (parametric regression test) → Demo / merge.
- Polish → ship.
Parallel Team Strategy
With two developers:
- Developer A: US1 (chat-store dedupe) — owns
chat-store.tsedits. - Developer B: US2 (audit-mode gating) — owns
api-middleware.ts+ChatContainer.tsxedits. - Both run their respective test suites independently; integrate at Phase 6.
- US3 verification can be picked up by either developer once US1 lands.
Notes
- [P] markers indicate tasks in different files with no incomplete-task dependencies.
- All tests are co-located with the code under test (
ui/src/.../__tests__/) per existing repo convention. - No backend (Python) changes, no MongoDB schema changes, no migration file.
- Persistence key (
caipe-chat-history) andpartializeoutput shape are unchanged; existing user state remains valid across the upgrade. - Constitution VII (Security by Default): the server-side change in T025 is more restrictive (admin + autonomous →
shared_readonlyinstead ofadmin_audit), never less. Non-admin users remain locked out of audit mode at every layer. T028 explicitly verifies, before T025 lands, that no write-side handler or audit-log writer is silently weakened by the reclassification (defense in depth; findings S1 and S2 from/speckit.analyze). - The client-side gating in T026 is presentation-only.
adminOriginis sourced from a user-controllable URL query parameter and is NEVER trusted for authorization. The server-sideaccess_levelinrequireConversationAccessremains the sole authorization source of truth (finding S3 from/speckit.analyze). - Verify tests fail before implementing (TDD per Constitution VI).
- Commit after each task or logical group; use conventional commits with DCO sign-off (
-s).