fix(ai): deterministic stream recovery for global assistant on mobile

[2witstudios]

Summary

On mobile, when an AI stream is in progress and the app backgrounds, the stream keeps generating server-side (disconnect-immune by design). But the client couldn't rejoin it on foreground — both GlobalAssistantView.tsx and SidebarChatTab.tsx had defects in their useAppStateRecovery wiring that orphaned the stream.

This is the bug behind the "invisible AI stream on mobile" issue. PR #2061 fixed the flashing/clobbering on send/reload/switch (client-side state machine). This PR fixes the recovery path that never fired.

Root cause

Defect 1: enabled was a render-time boolean

enabled: !isStreaming && currentConversationId !== null && !useEditingStore.getState().isAnyEditing(),

iOS freezes JS the moment the app backgrounds. The boolean is captured at render — so when the app went away mid-stream, !isStreaming was false. The recovery hook was gated off in exactly the case it was written for.

The useAppStateRecovery docstring documents this exact failure:

A boolean is captured at render, and iOS freezes JS the moment the app backgrounds — so the value that ends up gating the resume is whatever was true when the app went away.

That comment was written when AiChatView.tsx was fixed. GlobalAssistantView and SidebarChatTab were never updated.

Defect 2: onResume was a dumb DB fetch

onResume: handlePullUpRefresh, // or handleAppResume

Both handlers just fetch messages from the DB and setMessages. But the reply isn't persisted until the run completes — so mid-stream the fetch returns a snapshot with no assistant message, and writing it clobbers the in-progress bubble. No local stop, no server rejoin.

The fix

1. enabled → callback form — evaluated at fire time, gating on user-editing only, never on streaming.
2. onResume → stop, then tryRecover() — and critically, no blind DB read.

const action = resolveResumeAction({ native: isCapacitorApp(), isStreaming: effectiveIsStreaming });
if (action === 'noop') return;
if (action === 'refresh') { await handlePullUpRefresh(); return; }  // web, idle: safe

// A stream of OUR OWN, for the conversation on screen, was running when iOS froze us.
const hadTurnInFlight = selectedAgent
  ? isOwnAgentStreamForCurrentConversation
  : isOwnGlobalStreamForCurrentConversation;
const conversationAtResume = currentConversationId;

rawStop();                              // local-only; releases the channel's `consuming` mark
let attempt = await tryRecover();       // /active-streams first; no DB read while a run is live
if (attempt.recovered) return;

// Silence is not an answer. Re-ask until BOTH questions come back, bounded.
for (let i = 1; !(attempt.probeAnswered && attempt.dbAnswered) && i <= 2; i++) {
  await delay(i * 1000);
  attempt = await tryRecover();
  if (attempt.recovered) return;
}

if (hadTurnInFlight
    && currentConversationIdRef.current === conversationAtResume
    && canConcludeTurnIsLost(attempt)) {
  await handleRetry();
}

Why tryRecover, and not a refresh

Both components already have tryRecover — the rejoin-first probe useStreamRecovery runs on a network error. A background/foreground cycle is a network error on iOS; it's just one we get told about. It asks /active-streams first — the server's authoritative answer — and only touches the DB when nothing is live:

/active-streams says	action
stream still live	rejoin it — no DB read at all
nothing live, reply persisted	refetch the completed reply (it finished while backgrounded)
neither	fall through to the plain refresh

Earlier iterations of this PR tried to make a mid-stream DB refresh safe (stale-guards, mergeServerAndPending, careful ordering). That was the wrong tree to bark up: an own stream is deliberately never in the pending-streams store while its POST body is being consumed (markChannelConsuming → shouldAttachStream returns false → chat:stream_start is skipped). So on the resume path the store is empty for the very stream being recovered, the merge had nothing to merge, and the "guarded" refresh still wrote a pre-reply snapshot over the half-streamed bubble. The right answer is to not read the DB while a run is generating.

Why the rejoin must evict the local partial

The server mints one id and uses it for both the assistant UI message (generateId: () => serverAssistantMessageId) and the stream registry row. So the id useChat holds for the half-streamed bubble is the live stream's messageId. Chat.stop() is explicit that it "keeps the generated tokens", so that bubble survives the stop — and the rejoin then re-adds the same stream to the pending store under the same id. Both surfaces drop a pending stream whose messageId already appears in messages (dedupRemoteStreams, ChatMessagesArea.visibleRemoteStreams), so the rejoined stream would be filtered straight back out and not one token of it would render.

tryRecover's rejoin branch therefore reads the live messageId off /active-streams and evicts the matching local message before rejoining. (This also fixes the same latent bug on the network-error rejoin path.)

But only when the server has something to put in its place. /active-streams also returns parts — the registry's debounced checkpoint, persisted every 20 parts, so it is empty for a stream only a few parts old. (Counted with isValidPartFrame, the same predicate the bootstrap seeds with, since it is that post-filter count which becomes skipReplayCount.) Evicting against an empty checkpoint and then failing the SSE join (the documented multi-instance case — the multicast registry is per-process) makes the bootstrap remove the stream, leaving the user with nothing: strictly worse than the frozen partial. So eviction is gated on parts.length > 0; otherwise we keep the partial and still attempt the rejoin.

Why handlePullUpRefresh must NOT reconcile with the pending store

On these two surfaces the pending store is the bubble — an in-flight stream renders from remoteStreams, not from messages. mergeServerAndPending synthesizes a message under the pending stream's messageId, i.e. exactly the id the renderers dedup on. Merging here would freeze the live bubble at a static snapshot and stop it updating until completion. So this loader deliberately writes the DB snapshot as-is; the id stays absent from messages and the stream keeps rendering live.

Why silence is not an answer

"We recovered nothing" is not the same claim as "there was nothing to recover". tryRecover asks two questions, and either can come back unanswered — the first request after a foreground is the one most likely to fail, radio still coming up:

unanswered	what might still exist	what regenerating would do
/active-streams silent	a run may still be live	every generation start calls takeOverConversationStreams, so the regenerate does not race it — it aborts it: re-running write tools it already executed (a turn that created a page creates it twice), billing the discarded tokens, stranding its partial
messages GET silent	the reply may already be saved	handleRetry deletes the trailing assistant by id — the same id the server persisted the reply under — so it deletes the finished reply and pays for it again

So RecoveryAttempt carries probeAnswered and dbAnswered alongside recovered, each set only after a body we actually parsed, and canConcludeTurnIsLost requires both. The resume loop re-asks (bounded) until both come back. On persistent silence it does nothing — which is safe: the stop() released the consuming mark, so a live run is picked up by the socket-reconnect bootstrap once the network returns, a persisted reply is picked up by the next load, and a genuinely dead turn leaves the user their prompt and its retry action.

Was the user's turn persisted? Asked by identity, not by counting

The old guard compared user-message counts (dbUserCount >= localUserCount). That silently breaks past one page: this GET is unpaginated, so the route applies its default limit: 50 and returns only the newest 50 rows, while local messages is the initial 50 plus every turn since. Beyond that boundary the guard is permanently false — step 2 could never recover on a long conversation, and every interrupted turn there fell through to a regenerate that deleted the reply it should have refetched.

It now checks whether the DB contains the id of the last local user message. That message is by definition the newest, so it is always inside the returned window; and both routes persist the user turn under the client's id (userMessage.id || createId()), so the identity round-trips. Correct and pagination-proof.

Why the fallback is a regenerate, not a DB refresh

tryRecover returning false means one of exactly two things, and a DB write is unsafe in both:

• the probe failed — the first request after a foreground is the likeliest to, radio still coming up. A stream may well be live, and a DB snapshot cannot contain an unpersisted reply, so a refresh would erase the in-progress bubble.
• the DB is behind local state — step 2's dbUserCount >= localUserCount guard rejected it, e.g. a send whose POST never reached the server. A refresh would erase the user's own prompt.

But doing nothing is wrong too, and this is subtle: Chat.stop() aborts the fetch, so useChat settles at ready with no error — and useStreamRecovery only fires on status === 'error'. The stop we added therefore destroys the very signal that used to drive recovery. A turn whose POST died on the background transition would find no stream, no reply, and no error, and the user's prompt would sit unanswered forever. (Master recovered it: the dead fetch raised an error, useStreamRecovery probed, came up empty, regenerated.)

So we regenerate — the same fallback useStreamRecovery applies — but gated four ways:

1. a turn of our own, for the conversation on screen, really was in flight (the broad effectiveIsStreaming stays true for a stream still running against a conversation the user has since left);
2. both of tryRecover's questions came back (above);
3. we are still on that conversation — the recovery spans seconds of network, and handleRetry always acts on the live conversation, so a switch mid-recovery would otherwise fire a generation for the turn the user moved to;
4. nothing has already restarted the turn. useStreamRecovery watches the same failure from the other side and regenerates too — and it calls clearError() before its own probes, so throughout its decision window the status reads ready and looks idle. A single regenerationInFlightRef mutex wraps handleRetry (regenerateTurnOnce), and both paths go through it, so the same turn can never be regenerated twice; a !isStreamingRef.current belt then stops us regenerating on top of a turn that has already restarted and moved on (the mutex releases when handleRetry's DELETEs finish, but the generation it kicked off is still running).

Why the stop must come first

rawStop() / stop() is the local-only useChat stop. It does not signal the server (that's abortActiveStreamByMessageId), so the run keeps generating and stays rejoinable. But it also ends the dead response body, which releases the channel's consuming mark — and without that, the rejoin's bootstrap classifies the stream as one this tab is already reading off its POST body and skips attaching it. The rejoin would silently do nothing.

Ordering is safe because both tryRecover and the bootstrap await their /active-streams fetch before consulting isChannelConsuming, so the abort-triggered unmark (a microtask) always lands first.

Changes

File	Change
GlobalAssistantView.tsx	enabled → callback; onResume → stop + tryRecover (no DB fallback); tryRecover evicts the stale partial on rejoin (gated on the server checkpoint being non-empty, and written through the store-syncing setter); handlePullUpRefresh gains a live-conversation stale-guard
SidebarChatTab.tsx	Same resume wiring (hook relocated below tryRecover, which it now closes over); same eviction; handleAppResume funnels through loadGlobalMessages (the documented single writer) instead of its own raw fetch
lib/ai/streams/canResumeRecovery.ts	new — the resume gate, extracted. Takes no streaming argument by construction
lib/ai/streams/evictStalePartial.ts	new — the eviction rule, extracted (including the checkpoint guard, counted with the bootstrap's own isValidPartFrame)
lib/ai/streams/recoveryAttempt.ts	new — RecoveryAttempt + canConcludeTurnIsLost
all __tests__	+50 tests. The gate, the eviction and the regenerate predicate are asserted against the real implementations, not mirrors

Tests

The resume wiring previously had zero coverage on either surface — which is how the render-time enabled boolean survived. Following each file's established pattern (pure mirrors of the hook-heavy component's logic), the decision is not mirrored: planResume calls the real resolveResumeAction, so the tests pin the wiring against the real policy.

• the gate takes no streaming argument — the original regression is not expressible in it
• native + live stream → stop → tryRecover, and the DB is never read
• native → never falls back to a DB refresh (the two cases where tryRecover returns false are exactly the two where a DB write is unsafe)
• stop always precedes the probe (it's what releases the consuming mark)
• native + idle → still probes (deterministic, not flag-gated)
• web + live fetch → noop (a live fetch survives a tab switch)
• web + idle → refresh only
• the rejoin evicts the local partial carrying the live messageId — and only that one message, never the user's turn
• eviction is skipped entirely when the server's checkpoint is empty (keep what the user has rather than risk an empty screen)
• regenerate fires only when both questions were answered, for a turn of ours, on the conversation still on screen — never on silence, never on a foreign conversation's stream, never after a mid-recovery switch

Validation

• bun run typecheck — clean
• bun run build (apps/web) — exits 0
• bunx eslint on all touched files — clean
• vitest — 815 passing across components/layout + lib/ai/streams

Behaviour change worth calling out

Replacing the native DB refresh with a regenerate means two narrow cases no longer refresh on resume: a conversation whose last DB message is a user message where no turn was in flight, and another device having deleted the trailing assistant message. Both are healed by the socket-reconnect refreshSignal that fires on resume anyway.

On the tests

The resume wiring previously had zero coverage on either surface — which is how the render-time enabled boolean survived in the first place.

The three load-bearing rules are now shared pure modules, so the tests assert against the shipped implementation rather than a copy of it: canResumeRecovery, evictStalePartial, and canConcludeTurnIsLost. planResume remains a mirror of the sequencing only (the onResume body is genuinely imperative — stop → probe → bounded re-probe → regenerate), but its two decisions call the real resolveResumeAction and the real canConcludeTurnIsLost.

Known follow-ups (deliberately not in this PR)

• resolveResumeAction treats all Capacitor platforms as "the fetch is always dead" — isCapacitorApp() is true on Android too, so a healthy Android stream gets stopped and rejoined on every foreground ≥5s. It renders correctly now that the eviction is in place, but it is needless churn. Pre-existing shared policy (AiChatView already does this); worth gating on platform === 'ios' separately.
• AiChatView still does stop → rejoin → refresh, and carries the same dedup collision on its rejoin path. The same fix applies. Left alone rather than widening this PR's blast radius.

Refs: #2061, #2018

[coderabbitai]

📝 Walkthrough

Walkthrough

App resume recovery now dynamically gates execution, resolves a resume action, protects asynchronous message loading, conditionally evicts stale stream messages, and rejoins agent or global streams across both assistant views.

Changes

App resume recovery

Layer / File(s)	Summary
Global assistant resume and refresh flow apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx	Global recovery uses live conversation checks, deterministic native/web resume actions, guarded message refreshes, and conditional stale-message eviction before stream reattachment.
Sidebar resume and rejoin flow apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx	Sidebar recovery awaits the shared global message loader, dynamically gates resume handling, and rejoin-probes agent or global streams after conditional stale-message eviction.
Resume recovery regression coverage apps/web/src/components/layout/middle-content/page-views/dashboard/__tests__/GlobalAssistantView.test.tsx, apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarChatTab.test.tsx	Tests cover resume gating, action ordering, native/web behavior, and preservation or eviction of stale assistant partials based on server stream parts.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AppStateRecovery
  participant ResumeResolver
  participant TryRecover
  participant MessageStore
  AppStateRecovery->>ResumeResolver: Resolve native and streaming state
  ResumeResolver-->>AppStateRecovery: Return resume action
  AppStateRecovery->>TryRecover: Stop locally and recover
  TryRecover->>MessageStore: Probe live stream and conditionally evict stale message
  TryRecover-->>AppStateRecovery: Rejoin stream or load persisted messages

Loading

Possibly related PRs

• 2witstudios/PageSpace#1689: Both modify deterministic app-resume actions and stream rejoin behavior.
• 2witstudios/PageSpace#1693: Both use stale-response protection for assistant message loading and recovery.
• 2witstudios/PageSpace#1308: Both modify global assistant refresh and message-loading behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: deterministic mobile stream recovery for the global assistant.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/global-assistant-stream-recovery

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb8b15ac02

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Import the resume helpers in SidebarChatTab

This new resume handler calls resolveResumeAction and isCapacitorApp, but SidebarChatTab.tsx does not import either helper. Any build or render path that reaches this component now fails on undefined identifiers instead of mounting the sidebar/recovery hook; add the same imports used by the other chat views before using them here.

Useful? React with 👍 / 👎.

[2witstudios]

Confirmed and fixed in c2cd36d — both imports were genuinely missing. Added to SidebarChatTab.tsx:

import { isCapacitorApp } from '@/hooks/useCapacitor';
import { resolveResumeAction } from '@/lib/ai/streams/resolveResumeAction';

Same two imports AiChatView.tsx uses. This was the other half of the web#build failure.

While confirming the fix I also checked the stop this handler calls: it comes from usePageAgentSidebarChat, which resolves to the raw mode-selected useChat stop (selectedAgent ? agentStop : globalStop) — i.e. local-only, matching the chatStop semantics in AiChatView. It clears local useChat state so the rejoin attaches cleanly and does not signal the server, so the server-owned run keeps generating and is rejoined rather than killed. Documented that in a comment at the call site, since it is the non-obvious part.

Verified locally: tsc --noEmit clean, bun run build exits 0, lint clean.

[chatgpt-codex-connector]

Declare the rejoin callback before the resume hook

This dependency array is evaluated during render, but rejoinAgentStream is only initialized later by useAgentChannelMultiplayer in the same component. Rendering GlobalAssistantView therefore hits the temporal dead zone before the later hook runs; move the multiplayer hook above this useAppStateRecovery call or use the existing ref when wiring the resume callback.

Useful? React with 👍 / 👎.

[2witstudios]

Confirmed and fixed in c2cd36d — this was a real build break, not a nit: CI failed web#build with Type error: Block-scoped variable 'rejoinAgentStream' used before its declaration.

Fixed by routing the agent rejoin through the existing rejoinAgentStreamRef — declared near the top of the component and assigned immediately after useAgentChannelMultiplayer runs. That ref exists for exactly this hook-ordering problem and is already the in-file idiom: tryRecover calls rejoinAgentStreamRef.current() the same way. I preferred it over reordering the hooks, since hoisting useAgentChannelMultiplayer would drag its dependencies (setAgentMessages, loadConversation, …) up with it for no benefit.

Also collapsed the agent/global stop branch to the already-mode-selected rawStop.

Verified locally: tsc --noEmit clean and bun run build exits 0 (both previously failed).

[coderabbitai]

🧹 Nitpick comments (1)

apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx (1)

710-751: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Resume-recovery logic is duplicated with SidebarChatTab.tsx.

This entire block (comments, resumeEnabled, onResume action resolution and rejoin/refresh sequencing) is near-identical to SidebarChatTab.tsx Lines 622-662. The two already diverged once (this file needed a ref to fix a TDZ that the sidebar never had), which is exactly the kind of drift risk duplicated logic creates. Consider extracting a shared useResumeStreamRecovery hook (native check, action resolution, stop/rejoin/refresh sequencing) parameterized by stop, rejoinAgent, rejoinGlobal, refresh, isStreaming, selectedAgent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx`
around lines 710 - 751, The resume-recovery implementation around resumeEnabled
and the useAppStateRecovery onResume callback duplicates SidebarChatTab.tsx and
should be centralized. Extract a shared useResumeStreamRecovery hook that owns
native action resolution, enabled gating, stop, agent/global rejoin, and refresh
sequencing, parameterized by stop, rejoinAgent, rejoinGlobal, refresh,
isStreaming, and selectedAgent; then replace both component-local blocks with
the hook while preserving the ref-based agent rejoin behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx`:
- Around line 710-751: The resume-recovery implementation around resumeEnabled
and the useAppStateRecovery onResume callback duplicates SidebarChatTab.tsx and
should be centralized. Extract a shared useResumeStreamRecovery hook that owns
native action resolution, enabled gating, stop, agent/global rejoin, and refresh
sequencing, parameterized by stop, rejoinAgent, rejoinGlobal, refresh,
isStreaming, and selectedAgent; then replace both component-local blocks with
the hook while preserving the ref-based agent rejoin behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6926b1ce-08db-4c3f-b511-f486e6395731

📥 Commits

Reviewing files that changed from the base of the PR and between 2e48bc4 and c2cd36d.

📒 Files selected for processing (2)

• apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx
• apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx

[2witstudios]

Self-review found a blocker the PR itself introduced — fixed in 496e0a7

Running an adversarial pass over my own diff surfaced that this PR was fixing only half of its own stated "Defect 2". It added the stop+rejoin, but left the DB refresh as the same dumb, unguarded fetch — and then wired that fetch to fire on the rejoin path, where it is at its most dangerous.

The reply is not persisted until the run completes, so a refresh issued while a stream is live returns a snapshot with no assistant message. Two distinct problems fell out of that:

1. The write was unguarded.
GlobalAssistantView.handlePullUpRefresh was a raw fetchWithAuth(...) → setMessages(data.messages). SidebarChatTab.handleAppResume did its own raw fetch, bypassing loadGlobalMessages — the documented "single writer for the global-mode server→view path" sitting directly above it. Neither carried the two guards that make a mid-stream DB read safe, and which every other loader in this codebase has (AiChatView.loadMessagesForConversation is explicit that it exists so "there is never a competing write from a stale in-flight fetch"):

• shouldApplyLoadedMessages — drop a response that arrives after a conversation switch
• mergeServerAndPending — re-attach the in-flight assistant bubble, which is by definition absent from the DB snapshot mid-run

Both paths now funnel through guarded, reconciled loaders.

2. The order was racy.
The refresh was fired after the rejoin, so its request could still be in flight when the rejoined stream completed — landing last and overwriting the just-completed reply with the pre-reply snapshot.

Concretely (agent mode, the worst variant): resume at T issues the GET; the rejoined stream completes at T+150ms and renders the reply; the GET from T resolves at T+800ms (first request after a mobile resume is routinely slow) and wipes it. Global mode self-heals via the completion→refreshSignal refetch. Agent mode does not — there is no completion-triggered refetch there, so the reply stays invisible until the user reselects the conversation.

The order is now stop → await refresh → rejoin, so no DB request is ever outstanding when the stream completes: the refresh catches a run that finished while backgrounded, and the rejoin attaches to one that is still live.

Also

• The gate's regression test was vacuous (it asserted the same thing as the test above it, against the test's own copy of the logic). It now asserts on resumeEnabled.length, so an isStreaming parameter cannot be reintroduced silently.
• Added ordering tests pinning refresh strictly before rejoin.

Known follow-up (not this PR)

AiChatView has the same stop → rejoin → refresh ordering and therefore the same latent completion race, though it self-heals because its loader is the authoritative single writer. Worth aligning it to this order separately rather than expanding this PR's blast radius.

Validation

typecheck clean · bun run build exits 0 · eslint clean on all touched files · 291 tests green across the 34 affected suites · CI green.

[2witstudios]

Rewrote the fix: resume now goes through tryRecover, not a DB refresh (2fe46d1)

A second self-review round found my previous approach was still wrong — and in a way that made the primary symptom worse. Recording it here because the reasoning matters for review.

What was wrong

I had been trying to make a mid-stream DB refresh safe: stale-guards, mergeServerAndPending, careful stop→refresh→rejoin ordering. All of that was barking up the wrong tree.

An own stream is deliberately never in the pending-streams store while its POST body is being consumed — markChannelConsuming runs synchronously before the POST leaves, and shouldAttachStream({isOwn, isConsuming}) then returns false, so chat:stream_start is skipped and no store entry is ever created.

So on the resume path the store is empty for the very stream we are recovering. mergeServerAndPending had nothing to merge and was inert. The "guarded" refresh still wrote a pre-reply DB snapshot straight over the half-streamed assistant bubble — the partial reply would vanish for a full round-trip until the bootstrap re-seeded it. I had moved the bug, not fixed it.

What it does now

The mistake was reaching for a DB read at all. Both components already have tryRecover — the rejoin-first probe useStreamRecovery uses on a network error. A background/foreground cycle is a network error on iOS; it's just one we get told about.

if (action === 'rejoin-and-refresh') {
  rawStop();
  if (await tryRecover()) return;   // rejoined a live stream, or refetched a persisted reply
}
await handlePullUpRefresh();        // only when nothing was live and nothing persisted

tryRecover asks /active-streams first — the server's authoritative answer — and only touches the DB when nothing is live. The DB is never read while a run is still generating. No bespoke ordering, no inert merge, and it reuses logic that already exists and is already exercised on the network-error path.

The subtle part: why stop() must come first

stop() is the local-only useChat stop — it does not signal the server, so the run keeps generating and stays rejoinable. But it also ends the dead response body, which is what releases the channel's consuming mark. Without that release, the rejoin's bootstrap classifies the stream as one this tab is already reading off its POST body and skips attaching it — the rejoin would silently do nothing and the whole recovery would be a no-op.

That's safe here because both tryRecover and runBootstrap await their /active-streams fetch before consulting isChannelConsuming, so the abort-triggered unmark (a microtask) always lands first. Worth knowing if anyone reorders this later.

Also

• Dropped a vacuous test assertion I'd added (it asserted the arity of the test file's own helper — it could never fail).
• handlePullUpRefresh keeps its stale-guard + reconciliation: it's still reachable from the pull-up and refreshSignal paths, where a late-landing load can clobber a conversation the user has since switched away from. The stale-guard there now compares against the live conversation, because the "id the load was requested for" ref pattern I first copied from AiChatView is vacuous in this component — nothing else advances that ref here, since this surface doesn't load-on-select through handlePullUpRefresh.

Validation

typecheck clean · bun run build exits 0 · eslint clean · 289 tests green across the 34 affected suites.

[2witstudios]

Round 3 found the PR didn't actually work — fixed in 5fefc3c

Third adversarial pass. The rejoin was attaching correctly and then rendering nothing. Two findings, both real.

1. Dedup collision — the blocker

The server mints one id and uses it for both the assistant UI message and the stream registry row:

const serverAssistantMessageId = createId();      // app/api/ai/global/[id]/messages/route.ts:1027
…
messageId: serverAssistantMessageId,               // :1089  → aiStreamSessions row
generateId: () => serverAssistantMessageId,        // :1119  → the UI message

So the id useChat holds for the half-streamed bubble is the live stream's messageId. And Chat.stop() is explicit — "Abort the current request immediately, keep the generated tokens if any." The bubble survives the stop.

The rejoin then re-adds that same stream to the pending store under the same id — and both surfaces drop a pending stream whose messageId already appears in messages (dedupRemoteStreams, ChatMessagesArea.visibleRemoteStreams). The rejoined stream was filtered straight back out.

Net effect: we stopped the fetch, rejoined the server stream, and then displayed zero of its tokens. The user would sit in front of a frozen partial reply until completion. The PR was a no-op on its own headline path.

tryRecover's rejoin branch now reads the live stream's messageId off /active-streams and evicts the matching local message before rejoining. Nothing is lost — the bootstrap seeds the stream from the server's registry buffer, which holds every part pushed so far, i.e. strictly more than the partial we froze with. This fixes the same latent bug on the network-error rejoin path too.

2. The DB fallback fired in exactly the cases where a DB write is unsafe

onResume fell through to a blind refresh whenever tryRecover returned false. But false means one of:

• the /active-streams probe failed — and the first request after a foreground is the likeliest one to fail, radio still coming up. A stream may well be live, and a DB snapshot cannot contain an unpersisted reply, so the refresh erased the in-progress bubble.
• the DB is behind local state — step 2's dbUserCount >= localUserCount guard rejected it, e.g. a send whose POST never reached the server. The refresh erased the user's own prompt.

The native path now ends at tryRecover, which already refetches when the run finished while we were away. In the other two cases doing nothing is correct: local state is newer than anything we could fetch, and the socket reconnect / refreshSignal path heals it.

Tests

Added coverage pinning the eviction (the dedup collision) and asserting the native path never contains a refresh step. 116 tests green across the two component suites; 770 across all of components/layout + lib/ai/streams.

Validation

typecheck clean · bun run build exits 0 · eslint clean.

Known follow-ups (deliberately not in this PR)

• resolveResumeAction treats all Capacitor platforms as "fetch is always dead" — isCapacitorApp() is true on Android too, so a healthy Android stream gets stopped and rejoined on every foreground ≥5s. It still renders correctly now that the eviction is in place, but it's needless churn. This is pre-existing shared policy (AiChatView already does it); worth gating on platform === 'ios' separately.
• AiChatView still does stop → rejoin → refresh and carries the same dedup collision on its rejoin path. Same fix applies; left alone rather than widening this PR's blast radius.

[2witstudios]

Round 4: the eviction was being undone, and could leave the screen empty — fixed in 1339151

1. mergeServerAndPending re-inserted the very id the eviction removes

This one was a regression I introduced myself two commits ago, while trying to make a mid-stream DB read safe.

mergeServerAndPending synthesizes an assistant message under the pending stream's messageId when that id is absent from the DB snapshot — which is precisely the id both renderers dedup on. So:

1. Resume → stop → tryRecover evicts the partial → rejoin. The pending stream renders live. useChat is idle, so isOwnGlobalStreamForCurrentConversation is now false.
2. The socket reconnects (near-certain right after an iOS foreground) → refreshSignal bumps.
3. The refreshSignal effect's only guard is !isOwnGlobalStreamForCurrentConversation → passes → handlePullUpRefresh() runs.
4. My merge appends a frozen snapshot of ownStream.parts under the live id into messages.
5. visibleRemoteStreams drops the live stream. The bubble stops updating — the exact symptom the eviction exists to prevent.

The insight I'd missed: on these two surfaces the pending store is the bubble — an in-flight stream renders from remoteStreams, not from messages. Merging a static copy into messages isn't needed for visibility and actively kills the live render. The merge is removed, which also restores master's behaviour here (master never had it).

2. Evicting against an empty checkpoint could leave the user with nothing

/active-streams returns parts — but that's the registry's debounced checkpoint, persisted every PERSIST_EVERY_N_PARTS = 20 parts. It is empty for a stream only a few parts old.

If we evicted the local partial and the SSE join then failed — the documented multi-instance case, since the multicast registry is per-process and prod runs several web instances — the bootstrap calls removeStream(messageId) when skipReplayCount === 0. Partial evicted, stream removed: the assistant bubble disappears entirely. Strictly worse than the frozen partial we started with.

Eviction is now gated on the server actually having parts to put in its place. Otherwise we keep the partial and still attempt the rejoin — worst case the user keeps exactly what they had.

I'd also written a comment claiming the bootstrap "seeds the stream from the server's registry buffer, which holds every part pushed so far — strictly more than the partial we froze with." That conflated the in-memory multicast buffer with the DB parts column. It was wrong and is corrected.

3. Eviction now goes through the store-syncing setter (fixed in 4278114)

Agent mode must drop the message from the dashboard store too, not just useChat — the store is the handoff payload to the sidebar on navigation, so a stale partial there re-creates the dedup collision on the other surface. Now uses the existing agentSetMessages wrapper, matching how tryRecover's step 2 already writes both.

Behaviour change worth calling out

Removing the native DB fallback means two narrow cases no longer refresh on resume: a conversation whose last DB message is a user message (an aborted run, or another device's turn whose reply hasn't started), and another device having deleted the trailing assistant message. Both are healed by the socket-reconnect refreshSignal that fires on resume anyway. I've corrected the comment that overclaimed the only skipped cases were unsafe ones.

Validation

typecheck clean · bun run build exits 0 · eslint clean · 770 tests green across components/layout + lib/ai/streams.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx`:
- Around line 780-833: Extract shared resumeEnabled and stale-partial eviction
helpers, then replace all local mirrors with imports so production code and
tests exercise the same implementations. In
apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx:780-833
and
apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx:1009-1067,
import and use the shared resumeEnabled while preserving the callback’s
conversation/editing checks. In
apps/web/src/components/layout/middle-content/page-views/dashboard/__tests__/GlobalAssistantView.test.tsx:115-162
and 334-360, and
apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarChatTab.test.tsx:133-180
and 613-639, remove the resumeEnabled/ResumeEffect/planResume and
evictStalePartial mirrors and import the shared implementations used by the
views and tryRecover.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3756dac7-a648-4a24-b4c2-14990119204c

📥 Commits

Reviewing files that changed from the base of the PR and between c2cd36d and 1339151.

📒 Files selected for processing (4)

• apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx
• apps/web/src/components/layout/middle-content/page-views/dashboard/__tests__/GlobalAssistantView.test.tsx
• apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsx
• apps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarChatTab.test.tsx

[2witstudios]

Round 5 — happy path verified clean; two fixes in e6ac419

Fifth adversarial pass traced the full native path end-to-end (background mid-stream → foreground → stop → probe → evict → rejoin → bootstrap → SSE replay → render → completion) and verified it correct, including:

• the consuming mark is genuinely released before the rejoin (stop() → tracked body cancel → finishOnce() → unmarkChannelConsuming, and tryRecover awaits a real network hop before rejoining, so the ordering is load-bearing and holds);
• skipReplayCount / persistedParts reconcile exactly — seeded checkpoint + un-skipped remainder = the full buffer, no gaps, no dupes;
• the completed reply lands exactly once in both modes (agent: onStreamComplete replaces-by-id, and since we evicted, appends; global: refreshSignal → DB refetch, store entry already removed);
• the eviction is safe on useStreamRecovery's original network-error path too.

Two things still needed fixing.

1. The stop suppressed the fallback — a regression against master

Chat.stop() aborts the fetch, so useChat settles at ready with no error. But useStreamRecovery only fires on status === 'error':

if (!error || status !== 'error' || isRetryingRef.current) return;

So the stop() this PR added destroys the very signal that used to drive recovery — and tryRecover()'s false return was being discarded on top of that.

Failure: iOS backgrounds during the TTFB window and the POST never reaches the server (a radio drop right on the background transition is common). On resume: stop aborts the dead POST → /active-streams reports nothing → the DB has neither the reply nor the user's turn → tryRecover() returns false → nothing happens. The user's prompt sits unanswered forever, no error, no retry.

Master handled this: the dead fetch raised an error → useStreamRecovery → probe → empty → handleRetry() → recovered.

onResume now falls through to handleRetry() when tryRecover comes up empty — the same fallback the network-error path uses. Still not a DB refresh (that remains unsafe here, for the reasons documented at the call site). Gated on a turn actually having been in flight when we backgrounded, so an ordinary resume on an idle conversation can never fire a spurious generation — the frozen render-time streaming flag is a faithful record of exactly that, which is all it's used for.

I'd also written that the reconnect/refreshSignal path "heals" every skipped case. It doesn't heal a run that never started server-side. Comment corrected.

2. The eviction gate counted parts differently from the seeder

The gate used raw liveStream.parts.length, but the bootstrap seeds (stream.parts ?? []).filter(isValidPartFrame) — and it's that count which becomes skipReplayCount, and a skipReplayCount === 0 is what makes a failed join drop the stream. A checkpoint of malformed frames would read as "safe to evict" while seeding nothing, and a failed SSE join would then leave an empty screen: precisely what the gate exists to prevent. Both now use isValidPartFrame.

Validation

typecheck clean · bun run build exits 0 · eslint clean · 770 tests green (120 in the two component suites, +4 pinning the regenerate fallback and the no-spurious-generation guard).

[2witstudios]

Round 6 — the regenerate fallback could destroy a healthy run. Fixed in 25746fb (+1e13c8d15)

Two fixes to the fallback I added last round.

1. Silence is not an answer (ship-blocker)

tryRecover returns false for two semantically opposite outcomes:

• "the server says nothing is live" → the run died; regenerating is the recovery.
• "the probe never reached the server" → we know nothing; a run may well still be live.

I was treating the second as if it were the first. And on this path the second is common, not exotic — the first request after an iOS foreground is the one most likely to fail, with the radio still coming up.

It matters because every generation start calls takeOverConversationStreams. So a regenerate issued while the run is in fact still live does not race it — it aborts it. The fallback would have:

1. killed a healthy, possibly nearly-finished generation and re-run it from scratch;
2. re-run write tools that had already executed — a turn that created a page creates it twice; the side effects are not rolled back;
3. billed the discarded tokens;
4. stranded an orphan partial in the DB — handleRetry deletes the trailing assistant before POSTing, but the aborted run's onFinish persists its partial after that delete.

tryRecover now records reachability separately from its verdict (only a 2xx means the server actually told us what is live). onResume re-probes twice with backoff before concluding — the radio returns well inside that — and regenerates only on an answered probe.

On persistent silence it does nothing, which is safe: the stop() released this channel's consuming mark, so a live run is picked up by the socket-reconnect bootstrap once the network returns, and a dead one leaves the user their prompt and the retry action on it.

2. The regenerate gate was not conversation-scoped

effectiveIsStreaming / displayIsStreaming stay true for a stream still running against a conversation the user has since navigated away from (the useChat id is stable across a switch). Regenerating on that would have fired a generation for the turn the user is now looking at rather than the one that was interrupted — a spurious reply, and a spurious charge, on an untouched conversation.

Both files already carry the conversation-scoped flag for exactly this hazard (isOwnAgentStream/isOwnGlobalStreamForCurrentConversation, isOwnStreamForCurrentConversation — each latching the conversation the stream actually started in). The gate now uses it.

Verified sound (not defects)

• No concurrent double-generation. takeOverConversationStreams runs on every generation start, so a regenerate takes over rather than running beside the live stream.
• handleRetry is safe on a stale partial — it strips trailing assistant messages, and AI SDK's regenerate() trims one too. It does not duplicate the user turn.
• No retry loop. The stop() flips useChat to ready, so the conversation-scoped gate is false on the next resume — it self-clears, and handleRetry cannot fire twice for the same interrupted turn.

Tests

planResume now models ownTurnInFlight and probeAnswered as inputs distinct from the broad streaming flag. Both gates were previously unfalsifiable — reverting either would have kept the suite green. 124 tests in the two component suites, 772 across components/layout + lib/ai/streams.

Validation

typecheck clean · bun run build exits 0 · eslint clean on all touched files.

[2witstudios]

Round 7 — the probe signal was leaky three ways. Fixed in 9bb092f

All three landed on the same failure: regenerating over a healthy run, which takeOverConversationStreams turns into an abort of it (re-running its already-executed write tools, billing its discarded tokens, stranding its partial).

1. probeAnsweredRef.current = res.ok was set before the body was parsed. res.json() can still throw on a body that dies mid-read — exactly the cold-radio-after-foreground case this path exists for — and the catch swallowed it while we went on believing we had an answer. Now set only after a body we actually parsed.

2. One shared mutable slot, two callers. tryRecover is called by both the resume handler and useStreamRecovery's network-error retry, and they can be in flight together — so one caller's probe could answer the other caller's question.

3. The reset sat after tryRecover's early returns. A bail-out (!channelId — e.g. user transiently null while auth rehydrates on foreground, and the token very likely expired while backgrounded) left the ref holding a previous call's true, and the resume path would regenerate on the strength of a probe it never made.

All three are gone by construction: tryRecover now returns { recovered, probeAnswered } (RecoveryAttempt), so each caller reads its own probe and there is no shared state to race or go stale. useStreamRecovery gets a thin adapter reading only .recovered — the reachability half of the answer exists for the resume path, which is the one that would otherwise regenerate over a live run.

Also fixed: tryRecover step 2 read defensively (Array.isArray(data) ? data : data.messages) but wrote data.messages raw, which would blank the surface outright if the route ever answered with a bare array. It now writes the same normalized array its guards were computed from.

Verified sound this round

• The re-probe loop cannot spin, hang, or double-fire: bounded at 3 probes / ~3s, every success path returns, and useAppStateRecovery's pendingRefreshRef serializes resumes.
• Re-running step 2 per attempt is harmless — a pure GET plus one full-replace write that returns the instant it applies.
• No stale closures, no TDZ, deps complete.

Tests

planResume now takes a sequence of attempts, so the re-probe loop is actually modelled: a probe that only answers on a later attempt still regenerates (a cold radio must not strand the turn), one that never answers never does, and a re-probe that finds the live stream rejoins instead. 128 tests in the two component suites; 782 across components/layout + lib/ai/streams.

Validation

typecheck clean · bun run build exits 0 · eslint clean on all touched files.

[2witstudios]

Round 8 — the fallback could DELETE a completed reply. Fixed in 264055d

Three defects; two of them combine into the worst thing this PR could do.

1. Silence from the DB check was read as "nothing is persisted"

Round 7 taught the /active-streams probe to distinguish "the server says nothing is live" from "the probe got no answer." Step 2 had exactly the same hole — and it's the more destructive one.

If the messages GET throws or returns non-ok, we learned nothing about what is persisted. The code treated that as "no reply exists" and regenerated. But handleRetry deletes the trailing assistant by id — and that id is the one the server persisted the reply under (serverAssistantMessageId names the registry row, the UI message via generateId, and the DB row alike).

So a run that finished while backgrounded would have its reply deleted from the DB and regenerated from scratch: content loss, plus a second bill.

RecoveryAttempt now carries dbAnswered alongside probeAnswered, and canConcludeTurnIsLost requires both. The resume loop re-asks until both come back.

2. The "was the user's turn persisted?" guard was broken by pagination

It compared user-message counts (dbUserCount >= localUserCount). But tryRecover's GET is unpaginated, so the route applies its default limit: 50 and returns only the newest 50 rows — while local messages is the initial 50 plus every turn since.

Past one page that guard is permanently false. Step 2 could never recover on a long conversation, and every interrupted turn there fell through to the regenerate in (1) — deleting the very reply it should have refetched.

Now asked by identity: does the DB contain the id of our last local user message? That message is by definition the newest, so it is always inside the returned window. Correct and pagination-proof.

3. tryRecover's writes had no stale-conversation guard

It writes after two awaits into a useChat instance whose id is constant across conversation switches, so a recovery for the conversation the user just left could land in the one they moved to. Every sibling loader guards this (handlePullUpRefresh, loadGlobalMessages) — ironically including the one this PR migrated handleAppResume onto, for exactly this reason. tryRecover now guards too, against the live conversation.

Verified sound this round

• The tryRecoverForError adapter doesn't disturb useStreamRecovery (it only ever consumed the boolean), and its identity is stable.
• The normalized step-2 write is the same runtime array the guards were computed from.
• Hook order, deps, closures all clean.

Tests

The regenerate predicate is now the real canConcludeTurnIsLost, not a mirror of it. Added a case for DB-silence (probe answered, DB didn't → never regenerate). 778 passing.

Validation

typecheck clean · bun run build exits 0 · eslint clean on all touched files.

[2witstudios]

Rebased onto master, and the last of the duplication is gone

Rebase (master had landed in these same files)

Master picked up two commits touching GlobalAssistantView.tsx / SidebarChatTab.tsx while this was open — #2072 (deterministic chat rendering + synchronous send with contextRef) and #2071 (per-conversation send serialization). Rebased clean, and I checked the integration rather than trusting the auto-merge:

• The eviction is still load-bearing. feat(ai): deterministic chat rendering + synchronous send with contextRef #2072 changed when remote streams render (!isLoading → areaMode !== 'skeleton') but not the dedup rule — dedupRemoteStreams / visibleRemoteStreams still drop a pending stream whose messageId is already in messages. So without the eviction the rejoined stream would still render nothing. (feat(ai): deterministic chat rendering + synchronous send with contextRef #2072 actually helps: it stops hiding remote streams behind a skeleton during loading.)
• feat(ai): deterministic chat rendering + synchronous send with contextRef #2072's synchronous send makes the regenerate gate more accurate. status now flips to submitted in the same tick, so heldStreamConvIdRef latches immediately — a turn backgrounded inside the old 0.5–3s await window would previously have had hadTurnInFlight === false and never recovered.
• fix(ai): best-effort per-conversation send serialization (advisory lock) #2071's advisory lock cannot block or reject the regenerate. The critical section covers takeover + lifecycle-create only and releases before generation begins; on contention startGenerationExclusive retries 3×300ms then proceeds unlocked. takeOverConversationStreams still runs on every generation start — which is exactly why the regenerate stays gated behind canConcludeTurnIsLost.

Duplication (CodeRabbit's thread, and every self-review round before it)

The resume gate and the eviction rule existed in six copies — once per component, then hand-mirrored again in each test file — and the mirrors could not fail. Both are now single pure modules the components and the tests share: canResumeRecovery and evictStalePartial (plus recoveryAttempt's canConcludeTurnIsLost, already shared). The tests assert against the shipped implementation now, and I added the case only the real helper can have: a checkpoint of malformed frames counts as empty, because that is what the bootstrap would seed from — the raw-length version of that gate would have said "safe to evict" and, on a failed SSE join, left an empty screen.

One thing the extraction got wrong on the first pass, caught on review: it swapped the eviction from a setMessages updater to a value read from a render-time ref, so it could compare references and skip a no-op write. Equivalent in GlobalAssistantView (its setters resolve updaters against their own refs anyway) but not in SidebarChatTab, whose setters are the raw useChat ones — passing them an updater resolves against the SDK store's live array, which the transport and useAgentChannelMultiplayer write synchronously without waiting for a render. Reading a ref instead opened a lost-update window. Fixed by splitting the rule rather than weakening it: canEvictStalePartial is exported so callers can ask before writing (an unsafe checkpoint still costs no write), and the eviction goes back through the updater form. evictStalePartial stays self-guarding on the same predicate, and a test pins the two against each other so they cannot drift.

Validation

typecheck clean · bun run build exits 0 · eslint clean · 818 tests green.

[2witstudios]

Final review pass — one real gap closed (7a6c2c8)

A fresh end-to-end read of the final state. The core design verified sound — the eviction has real server data behind it (/active-streams genuinely returns messageId and parts), both stops are the local-only useChat stop rather than the server-aborting wrapper (the one thing that would have silently defeated the whole PR), silence is never treated as an answer, and every post-await write is conversation-guarded. Three findings.

1. onResume and useStreamRecovery could both regenerate the same turn

They watch the same failure from opposite sides — useStreamRecovery fires on status === 'error', onResume on the app-resume event — and share no lock.

If iOS delivers the dead fetch's rejection as an error before the resume listener runs, useStreamRecovery may already have retried. And rawStop() cannot have cancelled it: Chat.stop() returns early unless the status is streaming/submitted, so on an already-errored chat it is a no-op. onResume would then regenerate on top — exactly the double destruction its own comments warn about, since takeOverConversationStreams aborts the run that just started and handleRetry deletes its assistant message on the way in.

The gate now also requires that nothing has restarted the turn, read through a ref because the closure's copy of isStreaming is the value captured before we were frozen.

2. A comment that lied — and contradicted a test in this same PR

It still described neither → falls through to handlePullUpRefresh, which is the DB-refresh fallback deliberately removed two commits earlier, and which the tests explicitly assert never happens ("never a DB refresh"). Fixed in both files.

3. Dead expression

Step 1's guard called decideRecovery with hasLiveStream hardcoded true, so it could only ever answer 'rejoin' — decoration dressed as a decision. The rejoin-first priority is expressed by the order of the two steps; step 2 is where decideRecovery genuinely decides.

Validation

typecheck clean · bun run build exits 0 · eslint clean · 814 tests green.

[2witstudios]

The double-regenerate fix was incomplete — real mutex now (e2268d6)

Verifying the previous commit surfaced that the nothingHasRestarted status check I'd added doesn't actually close the race it was written for.

useStreamRecovery calls clearError() before running its own probes. So for the entire window in which it is deciding whether to regenerate (two fetches, ~1s+), the chat status reads ready and looks idle to us — my status guard passes, and both paths can still reach handleRetry for the same turn. The status check only caught the narrow sub-case where the other path had already reached a running generation and a render had committed.

Closed with an actual lock: one regenerationInFlightRef wraps handleRetry as regenerateTurnOnce, and both callers go through it — useStreamRecovery (via its handleRetry prop) and the resume handler. Held across the whole of handleRetry, whose message DELETEs are a network round-trip.

The status guard stays as the belt to this mutex's braces: the lock releases when handleRetry returns, but the generation it kicked off is still running, and that generation's own submitted status is what keeps the other path out afterwards.

It's a mutex, not a one-shot latch — released in a finally, so a later genuine failure can still recover and a throwing handleRetry can't wedge it shut. Tested with a real concurrent-invocation harness (both paths firing while the first is still awaiting): exactly one regenerate, plus a later-regenerate-allowed case and a throws-still-releases case.

Also fixed a stale comment referencing rawStop() in SidebarChatTab — that file calls the (equally local-only) stop().

Validation

typecheck clean · bun run build exits 0 · eslint clean · 820 tests green (the mutex now has a genuine concurrency test, not just a policy mirror).