[alfredolopez80]

Summary

Fixes four Desktop chat-loading problems introduced or made visible in v0.11 (#849).

What this PR fixes

1. The sidebar and the first chat open waited for deleted-chat cleanup. chat:getIndex awaited the cleanup of every pending deleted chat before returning the local index, so with pending cleanups the sidebar stayed empty for seconds after launch (v0.11: p95 1.3–29.6 s in the benchmark below). The index is now returned at once and the cleanup runs in the background (426eb72da).
2. The stateful Host's sessions waited for the stateless Host to wake. The latest-sessions list was published only after every Host's catalog arrived (Promise.all), so while chatllm-stateless was suspended and waking, the chatllm rows did not appear either. Each Host's sessions are now published as its own catalog arrives (b319b516e); the stateless Host keeps its lazy load.
3. Returning to a chat on the same Host blanked the transcript. Every switch cleared the view, showed "Loading conversation" and re-read the transcript from disk, then reconciled with the Host. The renderer now keeps the last 8 opened transcripts in memory and shows the remembered one on the click (A → B → A shows A at once), then checks it against disk and keeps the background Host reconcile (afterTurn = latest-1) unchanged (432a458c4, 48fcd0b30).
4. Cached chats painted as "New Chat" until the Host answered. A chat created on the server was stored in the local index as "New Chat", and its server title was shown only after the catalog arrived. Before this PR the list waited for every catalog, so the placeholder was never visible; with the cache painted first (fixes 1 and 2) it was, for the whole wake of a suspended stateless Host. The local index now keeps the last title the server reported: each catalog page writes it for chats already in the index (c010b9002), and a chat first opened from the catalog is stored with that title instead of "New Chat" (e4166fcd5). Opening a chat whose history starts at turn 1 no longer replaces a known server title with the first user message; that message names the chat only while it is still "New Chat" (b91528a57). A later server title still replaces it, and a pending local rename is never overwritten. The title written is the server's own; client-derived titles are still not persisted (spec 15 Phase A).

Adjustments that keep existing behaviour

• fix(desktop-ui): route immediate New chat sends to a fresh chat #976 compatibility. History is still read only after the last-active write, as fix(desktop-ui): route immediate New chat sends to a fresh chat #976 requires. A remembered transcript shown before that write settles is tracked as an unconfirmed selection and withdrawn (blank view, request retired) if the write fails or a new selection intent starts: Host hold, Host deselect, logout, notification route change, clearActiveChat, New chat (15e009561, f1f8f720e). The mark lives only at the revision that set it: a newer intent retires it, and a click on the same row takes it over as a new mark, so a superseded click's failed write cannot blank a message sent after it (bdccf042f). While unconfirmed, send and "Load older" return without effect and keep the draft. New chat restores the chat through a fresh switch when its create fails.
• Memory invalidation. The remembered transcripts are cleared on logout, on auth-scope change (team, user, environment) and when a Host loses access. A held Host's chat is not recorded, a deleted chat is never served from memory, and an empty disk page (a tombstoned chat) replaces the remembered transcript (941ee014d, 16d3c570c).
• Cleanup idempotence. A cleanup pass skips a tombstone that an overlapping pass already finished, so overlapping chat:getIndex calls do not repeat the removal and the index save (5ab4cffff).
• setLastActiveChatId skips the index write when the chat is already the last-active one (63e3a5f92).
• Sidebar order. Rows with the same time are ordered by Host, then chat id, so the order no longer depends on which Host's catalog arrived first (9f4e378e8). The same order applies to the selected Host's own list: tied rows follow chat id instead of index order, so after the active chat is deleted, the chat opened next is the lowest id among the remaining chats with the newest time.
• Offline renames. A pending offline rename is retried on its own Host's poll and on reconnect, so another Host's polls do not spend its retry budget (46c8dbea1). A local rename is registered as pending before its local write, and a catalog page requested before the rename settled (in flight, cached, or processed during the rename IPC) neither shows nor stores the old server title over it (d62c072bf). A superseded rename whose local write fails is settled, so it cannot block later server titles (1c24949dc).

Not in this PR

• Opening a chat on another Host from the sidebar (cross-Host 'specific' selection) still clears the view while the target Host's catalog loads; that path, including loadChatList returning null, belongs to spec 06 (d94532904 removed the memo seeds there).
• The sidebar 'Latest sessions' flicker (held-Host auto-select loop, repeated same-kind holds, local-only republication on reload) is handled in a follow-up PR stacked on this one.

Refs #1091
Refs #827 (step 1)

Recorded Desktop runs

Runs 1 and 2 use the PR's own Electron build on e58825bf4, run 3 on e4166fcd5, all against the branch profile with Playwright video and trace and one password login per run.

Run 1: same-Host A → B → A and relaunch with pending cleanups

• On chatllm and chatllm-stateless, open chats A, B, C and A again. A MutationObserver on the return to A fails the test on any empty message list or "Loading conversation" spinner; none appeared (fix 3).
• Send one message in A on each Host: the reply appears and the chat grows by two messages.
• Relaunch with 250 pending deleted-chat cleanups (chatllm): the first sidebar row appeared 249 ms after window load, before any click; reopening A shows the message sent in the first session (fix 1).

Run 2: stateful and stateless Hosts while the stateless Host wakes

Launch 1 opened two chats per Host to fill the local cache. The HCC stateless idle window was lowered to 5 minutes for the run (restored to 30 afterwards, checked with kubectl), and the run waited until chatllm-stateless was suspended (replicas=0, no pods; 5.9 min). Launch 2 then started with the same cache (PLAYWRIGHT_EXIT=0). Times are from window launch; the pod's Ready time is the Kubernetes lastTransitionTime, truncated to the second.

Event	Time
First chatllm sidebar row, before any click (fix 1, fix 2)	1171 ms
Both chatllm-stateless rows listed from the local cache	2314 ms
Stateless A opened with its full transcript (from disk)	2498 ms
Stateless B opened	2903 ms
Back to stateless A: no empty list and no spinner (fix 3)	3253 ms
chatllm-stateless pod Ready	7366 ms
First stateless Host response with data (/models, 200)	7723 ms

Every paint above happened before the stateless pod was Ready; the requests to that Host were still waiting for the wake. After the wake, stateless A was still shown in full.

Run 3: titles from the cache, then a server-only chat after the wake

Launch 1 opened two titled server chats A and B on chatllm-stateless and checked that the local index on disk holds their server titles (fix 4). A second launch, on a copy of that data directory with the session restored (no second password login), created one new chat on the server; the original directory was then put back, so its cache does not know the new chat. With the idle window lowered as in run 2, the run waited until chatllm-stateless was suspended (6.2 min), and launch 2 started from the first cache (PLAYWRIGHT_EXIT=0). Times are from window launch.

Event	Time
Rows A and B painted from the local cache, each with its server title from the first frame; no "New Chat" frame	1851 ms
Wake request sent (POST …/wake, 202)	1853 ms
chatllm-stateless pod Ready	7386 ms
Session list returned (GET …/sessions, 200)	9217 ms
New server-only chat row appeared, without any click	9221 ms

No cached row disappeared before the server list arrived.

A/B results (head 62f62def4)

BEFORE = 62de22cf3 (C1: test ids only, v0.11 behavior), AFTER = 62f62def4. Both builds run against the same branch profile and the same server chats.

Two limits on what the A/B attributes to this PR:

• BEFORE was cut from an older dev than the PR's merge base (ebbd31544). AFTER also contains 211 dev commits that BEFORE does not, 108 of them touching desktop-app/, including fix(desktop-ui): route immediate New chat sends to a fresh chat #976. Each gain is attributed to this PR by its vacuity run below: the memo disabled for O, the inline cleanup await restored for L.
• The PR head is now 1c24949dc. Since 62f62def4 it changes the provisional-selection guard used by send, "Load older" and New chat (f1f8f720e), adds one test (e58825bf4), stores server titles in the local index (c010b9002, e4166fcd5) and keeps a known server title when a chat is opened from turn 1 (b91528a57) and fixes two review findings in the provisional-selection mark and in rename/catalog ordering (bdccf042f, d62c072bf, 1c24949dc), none of which the A/B measures. Values are p50 per Host. Phase O rounds are ordered BEFORE, AFTER, AFTER, BEFORE (ABBA). Phases L and W run every BEFORE round, then every AFTER round.

Phase OL (HCC cadence as recorded, 22.7 min)

Metric	Host	Before	After	Gate
open_cached_ms (first open after launch, disk)	chatllm	19.7 ms	18.3 ms	pass (budget: before + max(A/A spread 1.0 ms, 10%))
open_cached_ms	chatllm-stateless	19.7 ms	17.1 ms	pass (budget 1.97 ms)
open_memo_ms (reopen in the same session)	chatllm	21.1 ms	7.4 ms	pass (after < before)
open_memo_ms	chatllm-stateless	16.9 ms	6.8 ms	pass
reopen_blank_witness (reopens showing the "Loading conversation" skeleton)	chatllm	16	0	pass (AFTER first opens: 20)
reopen_blank_witness	chatllm-stateless	16	0	pass (AFTER first opens: 19)
relaunch_rows_ms (window load → first stateful sidebar row, before any click; 10 pending cleanups)		8366 ms	438 ms	pass (ratio 0.052 ≤ 0.5)
relaunch_ms (window load → row → explicit open → full transcript)		9255 ms	882 ms	pass (ratio 0.095 ≤ 0.5)
Rounds invalidated by a pod or readiness change (an HTTP 429 aborts the run instead)			0	pass

A first open does not gain: it waits for the last-active write as #976 requires, so open_cached_ms stays within the budget of BEFORE rather than below it. The gain is on reopen and on relaunch.

Each seeded chat has 4 messages (2 turns); the clock runs in the renderer from the click to the first frame that shows every message, checked once per animation frame (~16 ms resolution). The values compare the two builds under the same conditions; they are not an estimate of production latency. On reopen, the user-visible change is the blank skeleton going from 16 of 16 reopens to 0.

Phase W (stateless Host suspended, 13.9 min)

Metric	Before	After	Gate
first_stateful_row_ms (max over AFTER rounds)		1215 ms	pass (< 2000 ms)
first_stateful_row_ms (p50)	8938 ms	1164 ms	pass (after < before)
Invalidated rounds		0	pass

Machine load (loadavg_spread, reported, not gated)

Phase	Spread of 1-minute loadavg across rounds	Reference budget
O	3.51	1
L	2.69	1
W	2.54	1

The machine (Apple M4 Pro, 14 cores) ran other branch profiles during the run. In phase O, ABBA ordering puts each build in the same load positions. Phases L and W are not interleaved; their ratios (L 0.052 and 0.095, W 0.13) are far larger than a load effect, and round_validity invalidated 0 rounds.

Witness vacuity

• O. With the memo disabled (maxChats: 0, local-only commit on ba75871a8) the spec failed with 6 blank reopens on chatllm and 4 on chatllm-stateless; open_memo_ms alone still passed, so the blank witness is what proves the memo.
• L. With the deleted-chat cleanup awaited inline in chat:getIndex (local-only 37c6619aa on 62f62def4): relaunch_rows_ms 8427 → 5881 ms (ratio 0.698) and relaunch_ms 9204 → 6196 ms (ratio 0.673), so both gates fail, where the PR head gives 0.052 and 0.095.

T2

make minikube-t2 on the PR head 1c24949dc, on a freshly created branch profile (full bootstrap, images built from HEAD): MINIKUBE_T2_PASS, T2_EXIT=0. T0, T1 and T2 pass; T1 626 tests in 82 files, Vitest exit 0.

Benchmark (chat:getIndex after bind, idle machine)

node scripts/bench-chat-open.mjs on 62f62def4, Node 24.18.0, Apple M4 Pro, 30 repetitions per row. v0.10.0 (d84385a62) reads the index directly, v0.11.0 (cfa72530f) awaits the deleted-chat cleanup and then reads, HEAD reads and starts the cleanup in the background. No Minikube cluster was running. Condition on every healthy row: HEAD firstGetIndexAfterBind p95 ≤ max(2 × v0.10.0 p95, 2 ms), evaluated on the JSON output. Result: 18 of 18 healthy rows pass. EACCES rows (a cleanup that fails on a read-only directory) are reported, not gated.

Values in ms, p50 / p95. "Pending" is the number of queued deleted-chat cleanups per agent.

Agents	Chats/agent	Pending	Filesystem	v0.10.0 p50 / p95	v0.11.0 p50 / p95	HEAD p50 / p95	Budget	Result
5	20	0	healthy	0.11 / 0.24	0.60 / 1.02	0.12 / 0.24	2.00	pass
5	20	10	EACCES	0.09 / 0.13	1.06 / 2.06	0.11 / 0.18	2.00	reported
5	20	10	healthy	0.08 / 0.16	1026.95 / 1268.55	0.14 / 0.29	2.00	pass
5	200	0	healthy	0.14 / 0.22	1.03 / 8.08	0.16 / 0.28	2.00	pass
5	200	10	EACCES	0.15 / 0.32	1.36 / 2.01	0.18 / 0.30	2.00	reported
5	200	10	healthy	0.14 / 0.19	1076.66 / 1512.66	0.16 / 0.25	2.00	pass
5	1000	0	healthy	0.44 / 0.72	2.81 / 3.64	0.45 / 0.93	2.00	pass
5	1000	10	EACCES	0.49 / 0.66	3.09 / 6.30	0.60 / 0.97	2.00	reported
5	1000	10	healthy	0.49 / 0.63	1091.59 / 1664.58	0.50 / 1.11	2.00	pass
25	20	0	healthy	0.10 / 0.17	2.20 / 2.79	0.12 / 0.27	2.00	pass
25	20	10	EACCES	0.09 / 0.11	3.33 / 4.03	0.11 / 0.18	2.00	reported
25	20	10	healthy	0.08 / 0.21	6016.11 / 8393.96	0.13 / 0.37	2.00	pass
25	200	0	healthy	0.17 / 0.25	3.46 / 8.61	0.15 / 0.24	2.00	pass
25	200	10	EACCES	0.14 / 0.17	5.61 / 9.52	0.19 / 0.23	2.00	reported
25	200	10	healthy	0.16 / 0.26	6758.81 / 12820.80	0.21 / 0.34	2.00	pass
25	1000	0	healthy	0.49 / 0.83	10.72 / 14.50	0.57 / 0.63	2.00	pass
25	1000	10	EACCES	0.46 / 0.57	13.43 / 19.28	0.56 / 0.88	2.00	reported
25	1000	10	healthy	0.46 / 0.57	5947.95 / 8802.51	0.59 / 0.95	2.00	pass
100	20	0	healthy	0.08 / 0.17	6.84 / 9.25	0.09 / 0.22	2.00	pass
100	20	10	EACCES	0.09 / 0.11	13.42 / 26.08	0.11 / 0.22	2.00	reported
100	20	10	healthy	0.08 / 0.16	22925.38 / 29539.55	0.13 / 0.44	2.00	pass
100	200	0	healthy	0.15 / 0.18	15.23 / 21.99	0.22 / 0.42	2.00	pass
100	200	10	EACCES	0.15 / 0.23	21.27 / 30.73	0.16 / 0.23	2.00	reported
100	200	10	healthy	0.15 / 0.18	24228.90 / 29429.05	0.18 / 0.35	2.00	pass
100	1000	0	healthy	0.45 / 0.66	40.79 / 48.98	0.44 / 0.50	2.00	pass
100	1000	10	EACCES	0.50 / 0.70	49.38 / 52.48	0.58 / 2.04	2.00	reported
100	1000	10	healthy	0.48 / 0.59	23785.68 / 29585.14	0.49 / 0.67	2.00	pass

Tests

• Desktop Vitest on 1c24949dc: 373 files, 4037 tests, exit 0. tsc (main and ui) exit 0, verify:electron exit 0, Node 24. fix(desktop-ui): route immediate New chat sends to a fresh chat #976's test files are unchanged, except for the shared __fixtures__/controllerHarness.tsx, which gains an additive onRender hook (3 lines).
• Memo (transcriptMemo.test.ts, switchToChat.transcriptMemo.test.tsx) and provisional selection (switchToChat.provisionalSelection.test.tsx, 24 tests), every case for chatllm and chatllm-stateless: A → B → A never renders an empty list, disk check and keep path, invalidations, write failures and double clicks, holds, deletes, New chat failure, and send / "Load older" guards.
• Per-Host publication (chatListPerHostPublication.test.tsx): Host authority epoch, tie order, offline rename budget per Host.
• Server titles (chatStore.test.ts, chatListTitleMerge.test.tsx, renameSessionQueue.test.tsx): a cached "New Chat" takes the server title and the index on disk holds it; a chat first opened from the catalog is stored with the title that page reported, kept per agent and used once; a session sent without a title (pending rename) forgets the remembered one and never overwrites the local rename; a deleted chat stays deleted; a relaunch paints the stored title before any catalog resolves. switchToChat.hydrationTitle.test.tsx (both Hosts): opening a chat from turn 1 keeps its server title in the sidebar and the Latest list; an untitled chat is still named after its first message. chatListRenameCatalogOrder.test.tsx: a local rename survives a catalog page in flight, a cached page and a page processed during the rename IPC, on disk and in the sidebar, Latest list and selected list; a superseded rename whose local write fails is settled.
• chat:getIndex: returns while the cleanup is still pending; overlapping calls remove a tombstone once; a rejecting cleanup still returns the index (cleanupErrors === 1).
• Each fix has a hand-applied mutation that turns its tests red (memo disabled, disk check skipped, each invalidation removed, cleanup awaited inline, ownership transfer, request retirement and revision check removed, guards removed, tiebreak removed, server title ignored by the store or not sent by the renderer, pending-rename filter removed, remembered title not stored, not forgotten or not consumed, first-message title applied over a server title); results are recorded with the run evidence.
• Benchmark harness (node:test, wired into CI): 11/11. tools/e2e_static_audit.py on the changed specs: exit 0.

Known, not fixed here

• useActivityController.ts maps any failure of the initial activity import to null (catch { return [agentName, null] }), so a read error looks like "no activity". Pre-existing.
• The two reconcileServerSessions calls in useChatListController.ts ignore a failed write (.catch(() => undefined) and an empty catch, from f6abe0c8b and 879b862ba), so a failed title write is silent and the next catalog load retries it. Pre-existing.
• Desktop: expanded agent chat list collapses on a transient short list during refresh #1109 — expanded chat list collapses on a transient short list (ChatThread.tsx:409-416), pre-existing.
• Desktop: hasLocalMessagesBeyond hides chat read failures as 'no older messages' #1110 — hasLocalMessagesBeyond hides read failures as "no older messages", pre-existing.
• Desktop chat-open A/B and memo tests: follow-ups from the PR #1112 review #1123 — follow-ups from the zach88 review (A/B witness sensitivity, per-run Keychain item, memo test seeding, tombstone filter test).

🤖 Generated with Claude Code

[alfredolopez80]

Evenfire Reviewer — Round 1

Reviewed head: 8348a10fae8ab804a7d41b762acf38e344c2ff59

Decision: Approve with follow-ups

The three latency changes hold: chat:getIndex returns before deleted-chat cleanup, opening a chat no longer waits for the last-active write, and each Host publishes its latest sessions as its own catalog arrives. The transcript memo is the open item. It paints a remembered transcript before this session has loaded that Host's durable tombstones, and a tombstoned chat's disk read returns an empty page, which the new keep rule treats as "disk has nothing, keep memory."

This comment reviews 8348a10fa only. Before publication the PR head moved to f1bede1ccf02b770c4846e4966a2273b8544c61a (test(desktop): detect the loading skeleton in the chat-open reopen witness, chat-open-ab.spec.ts +48/−21). That commit was not reviewed. Another review is required for it.

Must fix

No must-fix items found.

Should fix

• R1-M1 — Medium — desktop-app/ui/src/hooks/domain/useAgentChatController.ts:1074 — Do not keep a remembered transcript when the chat is durably deleted. presentSelectedTranscript runs before deletedChatIdsByAgentRef is populated, ChatStore.loadMessages returns [] for a tombstoned chat (chatStore.ts:2037), and the keep rule at lines 1117-1130 treats that empty page as "keep memory." Nothing afterwards re-checks the tombstone.

Verified

• Background cleanup is detached with its rejection handler attached synchronously, and the index still carries tombstones.
• setLastActive is started before the cache read and its rejection is rethrown from finally. The unchanged-selection skip runs inside the per-agent index chain; A→B→A still commits A.
• Per-Host publication merges only the arriving Host's rows, re-checks epoch, scope, cancellation and tombstones at arrival, and flushes renames for that Host only. Reconnect still retries every Host.
• The memo is keyed by auth scope, Host and chat, capped, and cleared on resetChat, scope change, and Host revocation or hold.

Validation gaps

• Previous-head regression proof: not verified in this environment. Desktop dependencies are not installed here; Vitest was not run.
• CI/CD was excluded at the caller's request.
• The A/B reopen_blank_witness failure is already disclosed in the PR body. ChatThread only shows the skeleton when the message list is empty, so the witness is likely counting the pre-commit frame. Not raised as a product defect. The new commit on f1bede1cc touches this witness and is outside this review.

PR description

Accurate on the latency changes and honest about the open A/B gates. Add R1-M1, or describe the fix once it lands. 30 files, one service.

Pre-existing

Pre-existing: none identified during the scoped review.

Next action

Fix R1-M1 with a regression that reopens a remembered chat whose index carries a tombstone before loadChatList has filled deletedChatIdsByAgentRef, then request a review of f1bede1cc.

Full review archive: pr-reviews/pr-1112/r1-26-10-14.md (reviewed head only). Head-move note: pr-reviews/pr-1112/r1-26-10-14-head-moved.md.

— Review performed by Evenfire Reviewer

[alfredolopez80]

Thanks for the review. Response to round 1:

R1-M1 — fixed in 16d3c570c. The finding holds: ChatStore.loadMessages returns [] for a tombstoned chat (chatStore.ts:2037), and the keep rule treated that empty page as "keep memory". A failed disk read leaves the same empty page and was kept the same way.

• The remembered transcript is now kept only when the disk page is non-empty and equals its newest part. An empty page replaces it, which matches the path without a memo. Recording [] removes the memo entry, because TranscriptMemo.set drops empty lists.
• New case b2 in switchToChat.transcriptMemo.test.tsx, run for chatllm and chatllm-stateless:
  • The first reopen is a memo hit while the disk read is pending.
  • The disk then returns [] while the renderer's deleted-chat set is still unaware of the tombstone.
  • The list ends empty, and the next open is blank behind the spinner instead of remembered.
• Vacuity: restoring the old rule turns b2 red (2 failures).
• The memo and switch suites pass: 7 files, 162 tests, exit 0. tsc and prettier are clean.

PR description — updated with R1-M1 and the current A/B status.

Please review f1bede1cc (chat-open A/B reopen witness) and 16d3c570c (this fix).

🤖 Generated with Claude Code

[jozer-rami]

Automated review by Evenfire — comments only, not an approval.

This pull request is too large for automated review: it changes more than 3000 lines or more than 100 files.

[zach88]

Took a third pass over this one at 62f62def4, focused on the four commits since round 2 (15e009561, c19dbd069, 9f4e378e8, 62f62def4). Writing the decision table first paid off. The unconfirmed selection holds up. I tried to find a sequence of events and promise orderings that shows another chat's or a deleted chat's transcript, leaves the mark dangling, double-reports a failed write, or loses a draft or sent message, and I couldn't find one. I re-ran all the mutations you listed and every one turned its rows red. Nothing since round 2 regressed. What's left is mostly small test and body accuracy stuff. The one real gate is the evidence you already called a merge condition.

Verdict: 🔴 Not ready to approve

• Must resolve before approval: 1
• Optional / non-blocking: 6

This is my read from the review, not a formal GH approval. The single blocker is the A/B re-run you already agreed to. Once that lands with the numbers, this flips to 🟡.

Must resolve before approval

1. [R1-M1] A/B, live L vacuity and T2 on the final head (still open since round 1)

The body still reports the A/B from 48fcd0b30. The path open_memo_ms, reopen_blank_witness and the relaunch gates measure has changed since then. You already listed the A/B re-run, the live L vacuity run and T2 on the final head as merge conditions, so I'm only tracking it here. Replace the "A/B results" and "T2" sections with the final-head numbers and it's closed.

Heads up before you launch it: see optional item 4 (R3-L3). As written, the wake in 62f62def4 can miss a draining Host and kill the L run again.

Optional (non-blocking)

You can merge without these. They're follow-ups or your call.

2. [R3-L1] The post-write withdrawal is only pinned by case l, which uses a synthetic hold

desktop-app/ui/src/hooks/domain/useAgentChatController.ts:1181-1186: if you remove that block, only the two l cases go red (switchToChat.transcriptMemo.test.tsx:709). l holds the Host through controller.hostAuthority.hold(...) (:729). In production every hold goes through hideHostAccess, which already withdraws in beginSelectionIntent, so for holds this branch never fires. The producer that actually needs the branch has no test: a tombstone discovered while the write is in flight (useChatListController.ts:541, :1645, which mark the chat deleted without clearActiveChat). Without the branch you'd see a deleted chat's remembered transcript and a dangling mark. Case 9 deletes through clearActiveChat, so it doesn't cover this, as the body already says.

Repro: dropping the block makes only l red on both Hosts.

Fix idea: rewrite l so getIndex returns a deletedChatTombstones entry for chat-a during the write. Or drop l and call the hold half hardening.

3. [R3-L2] Double click on a remembered chat with a rejected disk read blanks the view

useAgentChatController.ts:1157 together with :1241-1269. The second click reuses the request with remembered = undefined, so keepRemembered is false and cached = []. The view goes empty and gets confirmed, and the memo effect then records [] and drops the entry. With a single click the transcript is kept, which is what the body and the comment at :1238 promise ("A failed disk read keeps the remembered transcript").

Repro: throwaway test, double click, W1 ok, W2 ok, newest-page read rejected. View is [] on both Hosts, even after the reconcile. The same test with a single click keeps the memo. Merge-base also blanks here, so this isn't a regression, just an inconsistency with the new contract.

My suggestion: don't patch it in this PR. It would be the 4th fix commit on the controller. Narrow the body sentence to the single-click case and track the double-click case in #1123. The likely fix later is to treat chatMessagesRef.current as remembered on the reuse path while the mark still belongs to this request.

4. [R3-L3] wakeStateless doesn't cover a draining Host

desktop-app/test/e2e-playwright/chat-open-ab.spec.ts:584-591 only wakes when readyReplicas === 0. The Host CRD has active | draining | suspended (charts/clerum-crds/crds/host.yaml:561-563), and the drain grace defaults to 60 s (host-context-controller/src/config.ts:865). While draining, readyReplicas is still 1, so there's no wake. The round's 65 s awaitQuietWindow then outlasts the grace, the Host suspends right before the measured launch, the pod changes, and round_validity fails the run at the end. The most likely spot is the first L round, after O has only touched the stateful Host for a couple of minutes. It fails loudly and never passes falsely, but it can burn another 60 to 90 minute run on the merge condition above.

Fix idea: also wake when lifecycleState !== 'active', or always wake when the remaining idle time is shorter than the quiet window plus the round. Worth doing before the final A/B.

5. [R3-L4] starting.close() has no bound, so "always closed" isn't guaranteed

chat-open-ab.spec.ts:512-520. Playwright's Electron close() runs app.quit() through the inspector (playwright-core/lib/server/electron/electron.js:81) with no timeout. That is exactly the case you're diagnosing: main blocked in safeStorage (KEYCHAIN_PROMPT_PENDING). The finally never returns, and the test hits its 90 minute timeout with no diagnosis. The unbounded close was already there before 9f4e378e8. What changed is the body claim ("the starting process is always closed").

Fix idea: Promise.race([starting.close(), delay(10_000)]), then SIGKILL starting.process(). Or soften the sentence.

6. [R3-L5] The total tie order has effects the body doesn't mention, and no hook test asserts the visible order

desktop-app/ui/src/lib/catalogMerge.ts:67 also orders the selected Host's own chatList (useChatListController.ts:538, :543). Those rows have no agentRef, so ties now fall back to chat id where they used to keep index order. One visible effect: after you delete the active chat and the remaining chats share an updatedAt, sorted[0] (:1529) opens the lowest id. It's harmless and deterministic, but the body only describes the cross-Host sidebar.

On the test side, chatListPerHostPublication.test.tsx:31-88 already builds the cross-Host tie in both arrival orders, but only checks toHaveLength(2) and uses find().

Repro: with the old comparator mocked into that suite it stays 12/12 green. Adding expect(latestChatSessions.map(c => c.agentRef)).toEqual([HOST_A, HOST_B]) gives 1 red with the old comparator and 12/12 with the real one.

Fix: add that assertion to both it.each cases, plus one sentence in the body about the per-Host list.

7. [R3-L6] The A/B spec runs only half of the global setup guard

chat-open-ab.spec.ts:377-381 together with playwright.chat-open-ab.config.ts (globalSetup: undefined). The spec re-runs assertAllowedKubeContext but skips three checks:

• validateBaseUrls, which ties REST/RPC to the context and requires localhost for minikube or branch profiles;
• the kubectl config current-context match;
• the pre-gate marker check.

With CHAT_OPEN_AB_KUBE_CONTEXT=clerum-test and QA_RECORDER_ALLOW_REMOTE=1, the URLs can point at a different environment than the one kubectl reads. The operator controls those env vars, and kubectl goes through execFile with a validated --context, so this is hardening only. The comment saying it "runs the same context guard" overstates it a bit.

Fix idea: call validateBaseUrls({ expectedContext: context, ... }) in the precondition step.

Nits

• chat-open-ab.spec.ts:581 and the 62f62def4 message say "Between L rounds no Desktop is open". That's not true for iteration >= 1, because the previous round's app stays open until the next launch closes it. The conclusion still holds; just fix the premise.
• report.statelessWakes records {build, iteration} but not the round.id that follows, which would make it easier to audit which sample came after a wake.
• The spec never checks app.getName() against /Evenfire Issue1091Ab/. Today only the later userData check would catch a non-isolated BEFORE build.
• The CodeQL "useless conditional" at chat-open-ab.spec.ts:639 is a false positive: stopped is flipped by the finally while the loop awaits.
• 9f4e378e8 bundles four review items (tiebreak, deleteChat comment, A/B diagnostics, updater comment) plus the Route Profile desktop handoffs to matching environments #978 memo rows. Splitting it one fix per commit would make bisecting easier next time.

What checked out clean

• Round 2 items closed for real. R2-M1 through case 7 on the real 403 path through hideHostAccess. R2-M2 through ownership transfer (cases 3, 5, 6). R2-M3 through getChatSwitchInFlight (case 10; fix(desktop-ui): route immediate New chat sends to a fresh chat #976's createChatFailure* suites still green). R2-L1 through the send and load-older guards (12, 13). R2-L5 through the rejecting bench case. R2-L6 through the narrowed comment, reproduced with a real ChatStore: 2 removals and 3 saveIndex, idempotent as stated. R2-L2 as an invariant comment: your layout probe makes the case, and I'm fine closing it that way. R2-L3 and R2-L4 are fixed in the body. R1-L4 and R1-L8 are fixed. R1-M4 is closed in writing, and the body now matches the code (per-Host filter at :1327-1331, unfiltered online retry).
• The new suite discriminates. switchToChat.provisionalSelection.test.tsx fails 14/20 against 45f1584c1, all on assertions. The 6 passes are the controls (8, 11) and row 9, as the body says.
• Unconfirmed selection edge cases.
  • A switch superseded by another chat can't confirm or withdraw.
  • Confirmation happens on both the rejected-read keep path and the empty tombstone page.
  • No guard can stay stuck, because provisionalSelectionFor checks request, agentRef and chatId.
  • getChatSwitchInFlight is correct across Hosts.
  • Every caller of beginSelectionIntent(false) was checked; withdrawing is right or a no-op in each.
• Main process.
  • The renderer can't receive or revive a tombstoned chat from the non-blocking chat:getIndex.
  • All 12 saveIndex call sites run inside serializeIndex and re-read disk. A concurrent cleanup, create and last-active race lost no update.
  • The setLastActiveChatId skip can't desync anything.
• Contract. chat:getIndex shape, nullability and order are unchanged. Memo page comparison lets the disk win on optimistic or local-error tails. Every cross-Host row carries agentRef and id.
• Security.
  • The memo is keyed by auth, env, user and team, and cleared at every identity boundary, including hold and revoke.
  • Notification and deeplink opens go through the same gate.
  • Scripts use execFile with no shell.
  • The real Evenfire Safe Storage item is never touched.
  • Output files are 0600 under the gitignored .local-notes/.
  • The CI step is hermetic.
• Harness. The per-round wake can't move an L gate: ratios are 0.054 and 0.100 against a 0.5 gate. global-setup.ts, launchDesktopApp (30 callers) and human-stateless-continuity show no collateral.
• Suites on 62f62def4.
  • UI hooks/domain, components/agents and SidebarNav: 978/978.
  • Main (5 files): 137/137.
  • Bench node:test: 11/11.
  • globalSetupGuard + qaRecorderHelpers: 22/22.
  • tsc main and ui: 0.

Nice work on the redesign. Once the final-head A/B and T2 are in the body, I'd be happy to see this go in.

[alfredolopez80]

Thanks @zach88 for the third pass. #1112 (comment)

R1-M1. The body has carried the final-head evidence since the edit at 2026-10-15 23:53 UTC, about two hours before this review was posted, so I think the review read an earlier copy of the body. On 62f62def4, the current body has:

• "A/B results (head 62f62def4)": BEFORE 62de22cf3, AFTER 62f62def4, phases OL and W. All gates pass. relaunch_rows_ms has a ratio of 0.052 and relaunch_ms 0.095, against the 0.5 gate. reopen_blank_witness goes from 16 to 0 on both Hosts.
• "Witness vacuity → L, live run on the final head": the inline cleanup await was cherry-picked on 62f62def4 as a local-only commit, 37c6619aa, which changes only desktop-app/src/chatGetIndexHandler.ts. With it, the ratios become 0.698 and 0.673, and both L gates fail.
• "T2": make minikube-t2 on 62f62def4 gives MINIKUBE_T2_PASS and T2_EXIT=0, with T1 at 626 tests in 82 files.

R3-L3. The draining case did not occur in that run. The stateless Host stayed Ready through phase L (statelessWakes: []), so the measured L rounds include no wake.

Could you re-check against the current body?

[alfredolopez80]

Evenfire Reviewer — Round 3

Reviewed head: 62f62def4907a9bf4cfb0f048d37e5ff320ceee6

Decision: Approve with follow-ups

The unconfirmed-selection redesign, the total catalog tie order, the overlapping-cleanup skip and the relaunch-ratio gate hold on this head, and none of them regresses a closed finding. R1-M1 stays closed. What remains is accuracy in the PR body, not another code change.

Must fix

No must-fix items found.

Should fix

• R3-L1 — Low — desktop-app/ui/src/lib/catalogMerge.ts:67 — byUpdatedDesc now breaks a time tie by chat id for one Host's own list too, so after deleting the active chat sorted[0] opens the lowest id instead of the index order. The body only describes the cross-Host sidebar. Add one sentence; no code change.
• R3-L2 — Low — desktop-app/test/e2e-playwright/chat-open-ab.spec.ts:512 — The body says the starting process is always closed, but close() waits on app.quit() with no timeout, which never returns while main is blocked in safeStorage. Drop the sentence, or bound the close and then kill the process.

Verified

• The provisional selection is one owned mark. Only its owner confirms it, and only after the local page settles; a failed write, a hold or a delete withdraws it and retires the switch request so an earlier page cannot publish.
• finishPendingDeleteCleanup re-reads the index under the per-chat serialization and skips a tombstone another pass already finished.
• The relaunch gates require after_p50 <= 0.5 * before_p50.
• catalogMerge.prop.test.ts 10/10 and chatGetIndexHandler.test.ts 6/6, run against this head. CI: 43 checks succeeded, claude skipped. mergeable_state is clean.

Validation gaps

• switchToChat.provisionalSelection.test.tsx was not executed: Electron is not installed in this environment, so the suite fails at import before any test.
• The A/B numbers and the T2 run are the author's report, now pinned to this head, and were not reproduced here.

Collateral audit

No regression from the ten commits since ba75871a8. useAgentChatController.ts is past the corrective-fix budget (8 fix: commits); the last two commits correctly stop patching it. R3-L1 and R3-L2 are body edits.

Follow-up

• R3-L3 — Low — chat-open-ab.spec.ts:584 — wakeStateless wakes only when readyReplicas === 0, so a draining Host (60 s grace) is not woken and the run fails loud. Not a false pass.

PR description

Update required. The A/B tables, the live L vacuity run and the T2 result are now reported at this head. Two sentences do not match the code: the tie-order sentence (R3-L1) and "the starting process is always closed" (R3-L2).

Pre-existing

Pre-existing: none identified during the scoped review.

Next action

Correct the two body sentences. No further patch to useAgentChatController.ts.

Full review archive: pr-reviews/pr-1112/r3-26-10-16.md

— Review performed by Evenfire Reviewer

[jozer-rami]

Deep review at 62f62def4, adversarial, four lenses: renderer correctness, main process + chat list, security/tenancy, test integrity. Every candidate was checked against the code, and the one below was reproduced with a test. This comment carries only the blocking/major tier. There are no blockers and one major. It isn't in the three earlier rounds or in #1123.

Major: a superseded switch leaves its unconfirmed selection behind, and send then fails silently

Impact is high and likelihood is low. Dev accepts the same send.

Where: desktop-app/ui/src/hooks/domain/useAgentChatController.ts:1187, with :4196.

Cause. setPendingChatSelection calls beginSelectionIntent(true) (:4196). That is the only way a new selection intent starts without withdrawing the unconfirmed selection or taking it over. It covers cross-Host sidebar opens, deferSwitch, and notification opens from another route. It also leaves activeChatSwitchRequestRef and the switch key untouched. ownsProvisionalSelection() compares the mark with the switch's own revision, not with selectionIntentRevisionRef.current. So when the write resolves, the superseded switch still owns the mark and returns at :1187 without clearing it.

Trigger:

1. On Host H1, open A, then B, then reopen A. A is remembered, so it is painted provisionally while setLastActive is pending.
2. Open H2:C from the sidebar's latest sessions while H2's getIndex is still pending.
3. A's write resolves. The mark {request: RA, revision: r1, H1, A} is left dangling.
4. Before H2's list lands, open H1:A from the sidebar again. H1's getIndex fails twice, so loadChatList returns null and the effect settles at :1711-1713 without starting a switch.

Result. Chat A is blank, with loading:false and the composer enabled. provisionalSelectionFor(H1, A) still matches the mark, so handleSendAgentMessage (:3170) and handleLoadOlderMessages (:1477) return with no effect, and getChatSwitchInFlight() stays true. Clicking A's row again recovers. This is the same failure family as d94532904: loadChatList returning null after a seed. Neither new suite drives setPendingChatSelection across Hosts while a mark exists.

Fix, verified locally. Clear an owned mark before the superseded return:

if (selectionIntentRevisionRef.current !== selectionIntentRevision) {
  if (ownsProvisionalSelection()) provisionalSelectionRef.current = null
  return
}

This is safe because every other owner has already moved on by then:

• a same-row takeover has already made owns false;
• a new switch has overwritten the mark;
• beginSelectionIntent(false) has withdrawn it.

So the only owned mark that can reach this line comes from a (true) navigation, and that navigation has already blanked the view. With the change, the repro below passes and switchToChat.provisionalSelection + switchToChat.transcriptMemo stay at 64/64.

Repro test: remembered=true fails on 62f62def4 (0 sends); remembered=false is the control (1 send)

// @vitest-environment jsdom
// Repro: remembered=true fails on 62f62def4 (send dropped); remembered=false is the control.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { act, cleanup, waitFor } from '@testing-library/react'
import { resetComposerDraftStore, setComposerDraft } from '@lib/composerDraftStore'
import type { AgentChatMessage } from '../../../uiTypes'
import { deferred } from './__fixtures__/catalogFixtures'
import { renderController } from './__fixtures__/controllerHarness'
import { type MockClerum, installMockClerum, uninstallMockClerum } from './__fixtures__/mockClerum'

;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true

const H1 = 'chatllm'
const H2 = 'chatllm-stateless'
let clerum: MockClerum

beforeEach(() => {
  clerum = installMockClerum()
})
afterEach(() => {
  cleanup()
  vi.restoreAllMocks()
  resetComposerDraftStore()
  uninstallMockClerum()
})

function transcript(chatId: string): AgentChatMessage[] {
  return [1, 2].map(n => ({
    id: `${chatId}-${n}`,
    role: n === 1 ? ('user' as const) : ('assistant' as const),
    content: `${chatId} message ${n}`,
    timestamp: n,
  }))
}

describe('F1 probe', () => {
  it.each([true, false])('remembered=%s: send after cross-Host round trip with failed list', async remembered => {
    const disk: Record<string, AgentChatMessage[]> = {
      'chat-a': transcript('chat-a'),
      'chat-b': transcript('chat-b'),
    }
    clerum.chat.loadMessages.mockImplementation(
      async (_agentRef: string, chatId: string, limit = 80, offset = 0) => {
        const messages = disk[chatId] ?? []
        const end = Math.max(0, messages.length - offset)
        return messages.slice(Math.max(0, end - limit), end)
      }
    )
    const defaultIndex = clerum.chat.getIndex.getMockImplementation()!
    const h2Index = deferred<never>()
    let h1Fails = false
    clerum.chat.getIndex.mockImplementation(async (agentRef: string) => {
      if (agentRef === H2) return h2Index.promise
      if (h1Fails) throw new Error('Not authenticated')
      return defaultIndex(agentRef)
    })
    const controller = renderController({ selectedAgent: H1, agentNames: [H1, H2] })
    await waitFor(() => expect(clerum.chat.getIndex).toHaveBeenCalled())
    if (remembered) {
      await act(async () => {
        await controller.result.current.switchToChat(H1, 'chat-a')
      })
    }
    await act(async () => {
      await controller.result.current.switchToChat(H1, 'chat-b')
    })

    // 1. Click A (remembered → provisional mark) with its write held.
    const write = deferred<void>()
    clerum.chat.setLastActive.mockImplementationOnce(() => write.promise)
    let click!: Promise<void>
    await act(async () => {
      click = controller.result.current.switchToChat(H1, 'chat-a').catch(() => undefined)
    })

    // 2. Cross-Host open of H2:C (setPendingChatSelection → beginSelectionIntent(true)).
    await act(async () => {
      controller.result.current.setPendingChatSelection(H2, 'chat-c')
      controller.rerender({ selectedAgent: H2 })
    })

    // 3. A's write resolves while H2's list read is pending.
    await act(async () => {
      write.resolve()
      await click
    })

    // 4. Back to H1:A from the sidebar; H1's list read fails twice.
    h1Fails = true
    await act(async () => {
      controller.result.current.setPendingChatSelection(H1, 'chat-a')
      controller.rerender({ selectedAgent: H1 })
    })
    await waitFor(
      () => {
        expect(controller.result.current.chatMessagesLoading).toBe(false)
        expect(controller.result.current.chatListLoading).toBe(false)
      },
      { timeout: 3000 }
    )
    expect(controller.result.current.activeChatId).toBe('chat-a')

    act(() => setComposerDraft('chat-a', 'hello', H1))
    await act(async () => {
      await controller.result.current.handleSendAgentMessage('hello')
    })
    expect(clerum.rpc.invokeHostMessage).toHaveBeenCalledTimes(1)
  })
})

Local evidence on 62f62def4, Node 24:

• verify:electron ok.
• PR-touched Vitest suites: 147 files / 1576 tests, exit 0.
• Bench harness 11/11.
• tsc main and ui exit 0.

I did not re-run the A/B, T2 or the benchmark. Lower-severity notes (a memo entry evicted by New chat during the write window, two mutation survivors in tests, a stale memo count after a server backfill) are available if you want them; they don't gate this.

[alfredolopez80]

Thanks @jozer-rami. Your scenario is real on 62f62def4 and is fixed on the current head.

Reproduction. I ran your repro through the real controller hook on both Hosts:

Head	Result
62f62def4	fails: after the return whose catalog load fails, send is refused (invokeHostMessage called 0 times)
f1f8f720e	passes 2/2 (chatllm, chatllm-stateless)

Why it is fixed. f1f8f720e makes provisionalSelectionFor also require selection.revision === selectionIntentRevisionRef.current. setPendingChatSelection advances the selection-intent revision, so the superseded click's mark no longer matches anything: send, load-older and getChatSwitchInFlight all ignore it.

Why I did not add the clear on the superseded return. With the revision check the leftover mark is inert. The next beginSelectionIntent(false) still withdraws it, and that withdrawal ends in the base state for that boundary. Clearing it at the superseded return as well would be hygiene, not a behaviour change, so I kept the diff to the one check.

Regression test. e58825bf4 adds your scenario as case 16 in switchToChat.provisionalSelection.test.tsx. It runs for both Hosts. It opens a chat on the other Host from the sidebar while the click's write is pending, then returns to A while A's catalog load fails, and asserts that send dispatches and the draft clears. A witness asserts that the return really issued A's catalog load.

• Vacuity: with the revision check removed, cases 15 and 16 turn red on both Hosts (4 failed, EXIT=1).
• Full desktop suite on e58825bf4: 371 files, 4003 tests, EXIT=0; tsc main and ui 0.
• T2 on f1f8f720e: MINIKUBE_T2_PASS, T2_EXIT=0. e58825bf4 changes only that test file.

[alfredolopez80]

@claude Please do a closing review of this PR at head b91528a57. Two questions:

1. Is every reviewer finding fixed, or tracked in a follow-up issue? Check each one against the code at b91528a57 and the linked reply or issue. Report anything that is neither fixed nor tracked.

Review	Findings	Resolution
Evenfire Reviewer R1 (comment)	R1-M1	Fixed in 16d3c570c (reply); confirmed closed in R2 and R3
Evenfire Reviewer R2 (comment)	none	—
Evenfire Reviewer R3 (comment)	R3-L1 tie-order sentence, R3-L2 "always closed" sentence, R3-L3 draining Host	R3-L1 and R3-L2: PR body corrected. R3-L3: #1123 item 12
zach88 round 1 (comment)	H1, M1–M7, L1–L9	Per-item verdicts in the reply; deferred items in #1123
zach88 round 2 (comment)	R2-M1–M3, L1, L2, L5 and the optional items	Fixed in 15e009561, c19dbd069, 9f4e378e8, 62f62def4 (reply); the rest in #1123
zach88 round 3 (comment)	R1-M1 evidence on the final head, R3-L1–L6, nits	Evidence in the body (reply); R3-L1–L6 and nits in #1123 items 10–16. Approved on 62f62def4
jozer-rami (comment)	Major: a superseded switch leaves its unconfirmed selection and send fails silently	Fixed in f1f8f720e, regression test in e58825bf4 (reply)
@claude closing reviews (1, 2, 3)	F1	Closed in review 3

jozer-rami also mentioned lower-severity notes that were not posted: a memo entry evicted by New chat during the write window, two mutation survivors, and a stale memo count after a server backfill. The stale count is #1123 item 25. The other two are not tracked yet.

Other follow-ups: #1109 (expanded list collapses on a transient short list), #1110 (hasLocalMessagesBeyond hides read failures), and #1136 for renaming a never-opened chat (renameChat is a no-op for a chat not in the index; present on the merge base).

2. Do the commits after zach88's approval on 62f62def4 introduce a bug or regression?

Commit	Change
f1f8f720e	provisionalSelectionFor also requires the current selection-intent revision, so a superseded click's mark no longer blocks send, "Load older" or New chat
e58825bf4	Test only: jozer-rami's scenario as case 16, both Hosts
c010b9002	ChatStore.upsertServerSessions stores the server title for chats already in the index; the renderer omits the title while a rename is pending
e4166fcd5	ChatStore remembers the server title of a server-only chat and uses it once in createChat
b91528a57	Opening a chat whose history starts at turn 1 keeps a known server title; the first user message names the chat only while it is "New Chat"

Points to check:

• A pending local rename is never overwritten by a server title (c010b9002, e4166fcd5).
• A deleted chat is not recreated, and a remembered title cannot leak to another agent or user.
• b91528a57 does not change the case where the server has no title.
• f1f8f720e keeps the withdrawal on logout, Host hold and delete (beginSelectionIntent(false)).

Evidence on b91528a57: Desktop Vitest 372 files / 4021 tests, exit 0; tsc main and ui 0; verify:electron 0; Node 24. T2 (make minikube-t2): MINIKUBE_T2_PASS, T2_EXIT=0, T1 626 tests in 82 files. Each fix has a vacuity mutation that turns its tests red; #976's test files are unchanged.

cc @zach88 @jozer-rami: these are the commits made after your reviews. Please take a look if you want to re-check them.

[claude]

Claude finished @alfredolopez80's task in 3m 53s —— View job

---

Closing review @ b91528a57

I checked each reviewer finding against the code at this head, and read every source change in the five commits made after zach88's 62f62def4 approval. I reviewed source and diffs only — Electron isn't installed in this environment, so I did not run Vitest, the benchmark, or T2; the test/T2 numbers are the author's report.

• Q1: every reviewer finding fixed or tracked
• Q2: commits after 62f62def4 — bugs / regressions

---

Q1 — findings ledger

The table is accurate. Spot-checking the non-trivial closures against the code at b91528a57:

• R1-M1 / F1 (empty disk page kept) — keepRemembered (useAgentChatController.ts:1244) requires !loadedLocalPage or a non-empty disk page equal to the memo tail; an empty page (tombstone) falls through and the record effect drops the entry. Closed.
• zach R2-M1/M2/M3, L1 (write-window family) — resolved by the unconfirmed-selection design, and the one superseded-mark gap jozer-rami found is closed by f1f8f720e (see Q2). Closed.
• zach R1-M2 (cleanup single-flight) — finishPendingDeleteCleanup re-reads the index under serializeChat. Closed.
• R3-L1 / R3-L2 (body wording) — body corrected. R3-L3 (draining-Host wake) → Desktop chat-open A/B and memo tests: follow-ups from the PR #1112 review #1123.

Two items are genuinely untracked, and the author says so openly: jozer-rami's unposted low notes — "a memo entry evicted by New chat during the write window" and "two mutation survivors". The stale-memo-count note is #1123 item 25; these two are not in any issue yet. Both are low severity and neither blocks, but if you want a fully-closed ledger they should get a one-line #1123 entry (or a sentence in "Known, not fixed here"). Nothing else is left unresolved: every other finding is fixed in-tree or tracked in #1123 / #1109 / #1110 / #1136.

Q2 — the five commits after approval

I read every non-test change. No new bug or regression. Each of the four checkpoints holds:

f1f8f720e — revision check on the provisional mark. The only change is one added clause in provisionalSelectionFor (useAgentChatController.ts:566-573): a mark now also requires selection.revision === selectionIntentRevisionRef.current. That function has exactly three callers, all read-side guards — getChatSwitchInFlight (:1443), "Load older" (:1480), send (:3180) — so a superseded (stale-revision) mark no longer blocks them. The confirm/withdraw path uses a separate local closure ownsProvisionalSelection (:1169-1171) that compares the captured revision, so it is untouched — a superseded switch still can't confirm or withdraw, which is why the author leaves the stale mark in place (it's now inert, and revisions are monotonic so it can never re-match). Withdrawal on logout / Host-hold / delete is unchanged: those go through beginSelectionIntent(false) → withdrawProvisionalSelection() (:596), which f1f8f720e does not modify. ✅

e58825b — test-only (provisional case 16). ✅

c010b9002 + e4166fcd5 — server title in the index. Verified against the four points:

• Pending local rename never overwritten. persistServerSessions (useChatListController.ts:391-397) omits the title unless session.title is truthy and pendingRenameStateFor(agentRef, chatId) === 'none'. A pending rename sends no title, and upsertServerSessions only writes chat.title when a title is present and differs (chatStore.ts:2302). ✅
• Deleted chat not recreated / no leak. A tombstoned chat is absent from index.chats, so upsertServerSessions treats it as server-only and never creates it (:2286-2292); createChat independently throws on isDeletedInScope before using any remembered title (:1783). serverOnlyTitles is keyed by agentRef, and the whole store is per-(envKey, userId) dir — unbindChatStore/a different bind drops the instance and builds a fresh new ChatStore(userDir) (chatStoreBinding.ts:104, :352-358), so a remembered title can't cross an agent, user, or environment. ✅
• createChat consumes the remembered title once — it reads then rememberedTitles?.delete(chatId) before use (chatStore.ts:1789-1791). ✅

b91528a57 — keep the server title on a turn-1 open. The auto-title block now guards on title === 'New Chat' (useAgentChatController.ts:2127), so a server title in meta.title is not replaced by the first user message. The ephemeral applyLatestTitle is unchanged, nothing new is persisted via renameChat (spec 15 Phase A holds). No-title case unchanged: when the server reported no title, createChat falls back to 'New Chat' (:1795), the guard passes, and the first message still names the chat — identical to before. The send-path auto-title (:3418) was already 'New Chat'-gated, so it's consistent. ✅

Verdict

Nothing in the post-approval commits introduces a bug or regression; the title plumbing is correctly gated against pending renames, deletes, and cross-scope leaks, and the revision fix narrows the guard without touching the confirm/withdraw invariants. The only open item for a clean ledger is tracking jozer-rami's two unposted low notes — non-blocking. From a code standpoint this is ready to merge; the test/T2/A-B evidence is the author's report and was not re-run here.
· branch fix/issue-1091-chat-open-latency-v2

[alfredolopez80]

Round 7: R5-M1 and R6-M1 fixed, plus one defect found during the simplification pass

Pushed 7b43ddaa1..1c24949dc (3 commits). The T2 lane on 1c24949dc is running; its result will be posted when it finishes.

Problems detected and fixed

1. R5-M1 (Medium; regression of f1f8f720e) — fixed in bdccf042f

• Problem: a same-row click that reused the switch request revived a provisional selection mark whose selection intent had already been superseded. If that click's last-active write then failed, the withdrawal blanked the view, including a message the user had sent in the meantime.
• Fix: the mark exists only at the current selection revision.
  • beginSelectionIntent retires it on every advance.
  • switchToChat passes a live mark to the click that supersedes it, as a new object.
  • Effect-driven switches inherit nothing.
  • The revision clause in provisionalSelectionFor is removed, because the invariant now holds at the only place that advances the revision.
• Tests: rows 17 (late same-row click) and 18 (late effect-driven switch), each for chatllm and chatllm-stateless, in switchToChat.provisionalSelection.test.tsx. Removing the retirement turns rows 15–18 red.

2. R6-M1 (Medium; regression of c010b9002) — fixed in d62c072bf

• Problem: a catalog page requested before a rename settled wrote the old server title over the local rename on disk. There were three windows:

  • (a) the page was in flight when the rename settled;
  • (b) the page was served from the 5 s catalog cache;
  • (c) the page was processed during the rename's local IPC write, because the pending entry was registered after that await.

  In (c), with a failed PATCH, the rename stayed pending while the sidebar showed the old title, even after a restart.

• Fix:

  • listSessions stamps each catalog page with a monotonic sequence number taken when the request is sent; a cache hit keeps that number.
  • A rename registers its pending entry (with sending: true) before the local write.
  • Settling a rename records a per-chat settle stamp (current entry only).
  • One predicate, catalogTitleRenameState, gates persistServerSessions and the three in-memory merges: a pending rename, or a page issued at or before the settle stamp, keeps the local title.
  • If a re-rename's local write fails, the key goes back to the rename it replaced, but only while that rename is unsettled.

• Tests: new chatListRenameCatalogOrder.test.tsx. It uses a real ChatStore on a temp directory and asserts the index on disk after a restart, not the IPC payload.

  • T1/T7: window (a).
  • T2: window (b).
  • T3/T4: window (c), with the PATCH succeeding and failing (offline restart).
  • T5: a page requested before and after the 200.
  • T6a–c: re-renames.
  • Every test ends with a positive control: another device's title, requested after the settle, is accepted.

• Vacuity mutations: N1 (pending-only predicate), N2 (register after the write), N3 (no issue stamp), N5, N6 and N7 each turn their tests red. N4 (opening the stillOwned fence in applyLocalTitleOnly) is not caught by any test. It is reported here and not fixed in this PR.

• useChatStore.test.tsx: the late-reject witness now asserts that the cached promise is the fresh one (toBe(fresh)). A new test checks that a request is stamped when it is sent and that a cache hit keeps the stamp. Mutation: a constant stamp turns it red.

3. F9 (found by a YAGNI/KISS review of the two fixes above) — fixed in 1c24949dc

• Problem: three renames of one chat in a row, where the second and third local writes both fail. The second write fails while the third rename is current, so the second entry was never marked settled. The third's failure then put the second back as the pending rename with sending: true. It never sent its PATCH, and the chat refused every server title until the next rename.
• Fix: a failed local write always settles its own entry. The one exception is when it is the current entry and the rename it replaced is unsettled; then the key goes back to that rename, as before.
• Test T6d is red on d62c072bf and green on 1c24949dc. Mutations:
  • the old catch form → T6d red;
  • no check that the entry is current → T6d red;
  • settle instead of restore → T6b red;
  • restore while ignoring settled → T6c and T6d red.
• Known residual, with no reported case: in that triple, the first rename's protection is gone once the third write fails. A page requested before the first PATCH's 200 can show the old server title until the next page arrives.

Simplification review (YAGNI/KISS) of the two fixes

Three read-only reviewers looked at reuse/efficiency, simplification/YAGNI and altitude/KISS. None found speculative machinery: every new piece of state traces to R5-M1, one of the three R6-M1 windows, or a named mutation. They also checked simpler alternatives for R6:

• Invalidating the catalog cache on rename leaves windows (a) and (c) open.
• A single global settle counter would refuse titles for chats nobody renamed.

Only F9 was applied. The optional cleanups (test-helper reuse, a duplicated type, a no-op settle) were left out to keep the diff to the defects.

Validation on 1c24949dc

• Node 24.18.0: npm run verify:electron 0, tsc main 0, tsc ui 0, prettier 0.
• Desktop Vitest: 373 files / 4037 tests passed, exit 0.
• None of the 40 fix(desktop-ui): route immediate New chat sends to a fresh chat #976 test files changed.
• T2 (make minikube-t2) is running on this HEAD. Result to follow.

@claude please re-validate this round:

1. Do bdccf042f, d62c072bf and 1c24949dc fix R5-M1 and R6-M1 correctly, including all three R6 windows and the F9 case, without introducing a new regression?
2. Do the tests assert the right observable (the index on disk after a restart, the visible view) and fail when each fix is removed?
3. Is anything still open (including the N4 note and the F9 residual) that should block the merge?
4. In your view, is PR perf(desktop): reduce chat-open latency and keep reopened transcripts in memory #1112 ready to merge? Answer yes or no, with the reasons.

[claude]

Claude finished @alfredolopez80's task in 5m 59s —— View job

---

Round 7 re-validation @ 1c24949dc

• Read bdccf042f (R5-M1 provisional-mark fix)
• Read d62c072bf (R6-M1 rename-vs-catalog fix, windows a/b/c)
• Read 1c24949dc (F9 triple-rename fix)
• Check the tests assert on-disk index / visible view and fail on mutation
• Assess N4 note and F9 residual
• Answer Q1–Q4

I read all three commits against origin/dev. The round-7 diff touches only renderer code and tests (useAgentChatController.ts, useChatListController.ts, useChatStore.ts + 3 test files) — no main-process, IPC, or on-disk-format change — so the blast radius is bounded to the two hooks. Source review only: vitest is not installed and this box is Node 22 (repo wants 24), so I did not run the suite or T2; those numbers are the author's report.

---

Q1 — do the three commits fix R5-M1, R6-M1 (all three windows) and F9 without a new regression? Yes.

bdccf042f (R5-M1) — correct, and the invariant is airtight.
The fix replaces the old "revive the mark's revision on reuse" with a strict invariant: the provisional mark only exists at the current selection revision.

• beginSelectionIntent now retires the mark on every advance — null on startsHistoryLoad, withdrawProvisionalSelection() otherwise (useAgentChatController.ts:593-595).
• switchToChat captures the still-live mark before advancing (inherited = provisionalSelectionFor(...), :1389) and threads it to switchToChatForIntent, which restores it as a new object only for the reuse click that supersedes it (else if (inherited?.request === switchRequest), :1173-1174).
• Effect-driven switches pass no inherited, so they inherit nothing (matches row 18).
• Because the revision is advanced and the mark is set in the same synchronous tick — runChatSwitchRequest calls execute() synchronously (chatSwitchRequest.ts:7) and switchToChatForIntent sets the mark before its first await (the setLastActive at :1186) — no other beginSelectionIntent can interleave. So a mark can never outlive its revision, which is exactly what lets the revision clause be dropped from provisionalSelectionFor (:1152-1159). The confirm/withdraw gate ownsProvisionalSelection still checks the captured revision (:1176-1178), so a superseded switch still cannot confirm or withdraw. No regression to jozer-rami's case 16: the dangling mark it relied on is now retired at the supersede, so the guards never see it.

d62c072bf (R6-M1) — all three windows closed.
A monotonic nextCatalogSequence() (useChatStore.ts) stamps each catalog page with the seq taken when the request is sent, and settlePendingRename stamps renameSettledSeqRef when a rename settles. One predicate, catalogTitleRenameState(agentRef, chatId, issueSeq), gates the disk write and all three in-memory merges: a pending rename, or issueSeq <= settledSeq (or undefined), keeps the local title.

• (a) in flight: page issued before settle → issueSeq <= settledSeq → kept.
• (b) 5 s cache: a cache hit returns the original promise, so the page keeps the original (earlier) issueSeq (useChatStore.ts, cached entry resolves to {...result, issueSeq}); verified by the new useChatStore.test.tsx case (cached.issueSeq === first.issueSeq, forced/other strictly greater).
• (c) during the local write: the pending entry is now registered before applyLocalTitleOnly, with sending: true so a flush poll can't fire the PATCH early; while it is pending catalogTitleRenameState returns the pending state → kept.
  Since issueSeq is monotonic, a fresh poll always eventually exceeds settledSeq, so a legitimate newer server title is never refused forever — the per-test positive controls prove this.

1c24949dc (F9) — correct.
The d62c072bf catch only acted when the failing entry was still current, so a superseded second entry whose write failed was never settled; the third's failure then re-inserted it with sending: true → a stuck entry that never PATCHes and refuses every server title. The fix makes a failed local write always settle its own entry, except the one case where it is current and hands the key back to a still-unsettled predecessor (:1423-1429). I traced the triple (writes 2 and 3 fail): entry2 is now marked settled when its write fails out-of-current, so when entry3 fails existing.settled is true → entry3 settles and the key is left empty. No stuck sending:true entry survives.

No new regression surfaced in review: the mark invariant holds synchronously, and the rename state threading is closure-local per page.

Q2 — do the tests assert the right observable and fail when each fix is removed? Yes.

• Provisional (rows 17/18): assert the visible view — the message 'hello' the user sent during the window must still render after a later same-row click (17) or effect switch (18) whose write fails, with liveness witnesses that the reclick issued its own setLastActive and the failure reached the caller once. Author: removing the retirement turns rows 15–18 red.
• Rename (chatListRenameCatalogOrder.test.tsx): every case drives a real ChatStore on a temp dir and asserts diskTitleAfterRestart() — a fresh new ChatStore(dir) reading the index, i.e. the on-disk state after a restart — not the IPC payload. T1/T7 (window a), T2 (window b, with a sourceTitles === ['old'] witness that the page was a cache hit), T3/T4 (window c, PATCH ok/fail + an actual remount for T4), T5 (page before/after the 200), T6a–d (re-renames). T6d is the F9 witness: it asserts patchTitles === ['first'], disk 'first', and then expectFreshPageAccepted — a later server title must be accepted. On d62c072bf the stuck entry would keep catalogTitleRenameState === 'in-flight', so that final control fails; on 1c24949dc it passes. Each test ends with that positive control, so a fix that over-refuses titles can't pass vacuously.

Q3 — anything still open that should block? No.

• N4 — the surviving mutation is opening the stillOwned() fence in applyLocalTitleOnly (:1180). That is a test-coverage gap, not a live defect: the fence is defense-in-depth against a mid-write auth-scope change, and renameChat is also guarded by bindingGeneration at the store. Correctly disclosed; non-blocking.
• F9 residual — in the triple-double-failure case, a catalog page landing in the sub-second window between the third rename settling and the first PATCH's 200 could briefly paint the stale server title, self-corrected by the next page. Transient, no data corruption (disk ends at 'first'), no reported case, disclosed. Non-blocking.
• Housekeeping (carried over): jozer-rami's two unposted low notes ("memo entry evicted by New chat during the write window", "two mutation survivors") are still untracked; a one-line Desktop chat-open A/B and memo tests: follow-ups from the PR #1112 review #1123 entry would close the ledger. Non-blocking.

Q4 — is PR #1112 ready to merge? Yes, from a code standpoint, with one evidence gate I can't close here.

The three commits correctly fix R5-M1, R6-M1 (all three windows) and F9; the invariant that makes R5-M1 sound is airtight; the rename changes are scoped and self-correcting; and the tests assert the right durable observable (on-disk index after restart, visible view) with discriminating positive controls. Nothing I found regresses a closed finding or introduces a new bug or silent fallback.

The remaining condition is non-code and not reproducible in this environment: the T2 run on 1c24949dc is still in progress per your message — that final-HEAD green (MINIKUBE_T2_PASS, T2_EXIT=0) plus the Vitest exit-0 you reported are the author's evidence and must land. Once T2 posts green on this exact head, I see no blocker to merging. (N4, the F9 residual, and the two low notes are follow-ups, not gates.)

---

Validation gaps: vitest/Node 24 not available here, so the test, benchmark, A/B and T2 results are the author's report and were not re-run; I reviewed source and diffs only.
· branch fix/issue-1091-chat-open-latency-v2

[alfredolopez80]

T2 on the PR head 1c24949dc: MINIKUBE_T2_PASS, T2_EXIT=0. T0, T1 and T2 pass; T1 626 tests in 82 files, Vitest exit 0.

The branch profile was recreated from scratch for this run, so the orchestrator ran a full bootstrap with every image built from HEAD. origin/dev 567cac208 is an ancestor of HEAD.

Earlier attempts on this head did not reach a verdict for reasons outside the PR: one died in NP-08 with exit 137 (#757), and three were refused at the preflight because my local launcher restarted the node runtime on a running profile just before it. The launcher now starts the profile only when its apiserver is down and waits for every deployment to be Ready before the preflight.

The PR body now lists the R5/R6/F9 fixes (bdccf042f, d62c072bf, 1c24949dc) and the updated test counts.

🤖 Generated with Claude Code