fix(desktop): preserve live turns across session refreshes - #3189
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWhat this PR solvesThis PR preserves the active Desktop Turn when a session refresh follows interruption, replacement, or conversation remount. It prevents an older session snapshot from clearing a newer confirmed Turn. It also exposes observed nonterminal root Turns in Desktop session-list summaries so the Stop control remains available. Source of truthThe PR extends the existing sources of truth. The runtime observer supplies live Turn identity, and the session catalog remains the session-list authority. The PR does not create a separate persisted state path. Scope and complexityThe solution is coherent with the race condition:
No further deletion is evident without weakening the new regression coverage. The removed effect and cleanup path are replaced by refresh-time reconciliation, which is required to avoid stale cleanup. Complexity deltaAdded:
Removed:
The PR adds a small amount of explicit state and branching, but removes a competing cleanup path. Total maintenance complexity decreases or remains justified by the race-specific behavior and focused tests. ValidationThe reported validation includes Desktop tests, focused and full E2E runs, lint, formatting, build, typecheck, and Knip checks. Direct evidence for the final status of these checks is not available in the supplied diff context. Review-relevant risksThe diff affects user-visible Desktop live Turn behavior and the Stop-control flow. Material changes in this area require independent human review under repository policy. The diff changes internal TypeScript interfaces and hook contracts, including No security, licensing, release, or governance effect was identified in the current diff. The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughChangesThe Runtime Host now projects active root turn IDs into session listings. Renderer session refreshes capture live-turn context before authority reads and conditionally clear settled transient state. The previous shell-level reconciliation effect was removed. Live turn projection
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change preserves live turns across session refreshes and is supported by passing tests and validation checks; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant useAppShellSessionList
participant createSessionListRefresher
participant SessionCatalog
participant reconcileSettledSessionTransients
participant clearTurnTransientStateIfCurrent
useAppShellSessionList->>createSessionListRefresher: capture live-turn request context
createSessionListRefresher->>SessionCatalog: request session list
SessionCatalog-->>createSessionListRefresher: return session summaries
createSessionListRefresher->>reconcileSettledSessionTransients: reconcile captured projections
reconcileSettledSessionTransients->>clearTurnTransientStateIfCurrent: clear matching settled projection
createSessionListRefresher->>useAppShellSessionList: commit sessions with captured context
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoPreserve live turns across Desktop session refreshes
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more' |
A session-list result could be committed against a newer live Turn projection and clear the Stop witness. Runtime Host-backed Desktop lists also omitted observer-owned running Turn identities. Capture renderer live state at each authoritative list read, reconcile only accepted results, and clear transient state with a projection-identity compare-and-swap. Project observed nonterminal root Turn identities only into renderer list summaries. Generated-by: Codex
ce6a90d to
b88b41b
Compare
|
fast path: The current changes are minor and reversible |
1 participant
Summary
Fixes #3177
Root cause
Settlement previously combined two independently versioned inputs: a React
sessionsstate snapshot and whatever live-Turn projection happened to be current when an effect ran. Changing the effect phase could narrow one scheduling window, but it could not establish causality. A list result could already be queued, asessions:changednotification could then confirm or replace the Turn, and React could commit the older list afterward; reconciliation would treat that older catalog state as evidence about the newer projection and clear the Stop witness.Runtime Host-backed Desktop lists also omitted the observer's current running Turn identity, so later catalog refreshes could not preserve a live Turn when the durable header lagged or was stale.
Design
createSessionListRefreshernow captures renderer context immediately before each authority read and passes it only with the accepted generation. Settlement runs at that acceptance boundary, before the list enters React state. The state controller conditionally clears a projection only when it is still the same object captured for that read. Superseded responses discard both their catalog result and their context, while a trailing read captures a fresh context.This removes the React effect and adds no persisted state, protocol field, or renderer epoch.
SessionSummary.runningTurnIdsremains an ephemeral list projection sourced from the existing Runtime Host observer.Verification
npm run lintnpm run format:checknpm run typechecknpm --workspace @maka/desktop run buildnpx knip --workspace apps/desktopnpx knip --workspace packages/uiAI use
Select exactly one:
Tool(s) and scope: OpenAI Codex investigated the race, implemented the fix, and added regression coverage under M4n5ter's direction and review. M4n5ter accepts responsibility for the result.
Checklist
Does this PR entail a change in behavior?