Fix websocket closing and reopening connections too eagerly

[juliusmarminge]

Summary

• Keeps terminal drawers mounted for recently viewed threads so terminal sessions survive thread switches instead of resetting on navigation.
• Reconciles mounted terminal thread IDs with an LRU-style cap so hidden drawers stay bounded while the active drawer remains available.
• Replays buffered terminal events after hydration and only applies unread events, preventing lost output across remounts and recovery paths.
• Refactors server state bootstrap into the root route and tightens orchestration recovery replay bookkeeping.
• Adds coverage for mounted-thread reconciliation, terminal replay selection, and recovery state transitions.

Testing

• Not run (PR description only).
• Added unit test coverage in apps/web/src/components/ChatView.logic.test.ts, apps/web/src/components/ThreadTerminalDrawer.test.ts, and apps/web/src/orchestrationRecovery.test.ts.

---

Note

Medium Risk
Changes terminal lifecycle and websocket event handling by keeping multiple drawers mounted and replaying buffered events, which could introduce memory/perf regressions or missed/duplicated output if the reconciliation/replay logic is wrong.

Overview
Keeps thread terminals alive across navigation by mounting a bounded set of background ThreadTerminalDrawers (MRU-capped via MAX_HIDDEN_MOUNTED_TERMINAL_THREADS) and only toggling visibility for the active thread.

Adds a transient terminal event buffer in terminalStateStore (non-persisted, capped) and updates ThreadTerminalDrawer to hydrate from a snapshot then replay/apply only unseen buffered events, while __root.tsx now records terminal websocket events into the shared buffer.

Refactors server state bootstrap into __root.tsx, tightens orchestration recovery replay bookkeeping/logging, and adds unit tests for mounted-terminal reconciliation, terminal event replay selection, and recovery state transitions.

^{Written by Cursor Bugbot for commit 28bae95. This will update automatically on new commits. Configure here.}

Note

Fix terminal drawers closing and reopening by keeping them persistently mounted across thread switches

• Introduces PersistentThreadTerminalDrawer in ChatView.tsx to keep terminal drawers mounted even when hidden, preserving session state across thread switches.
• Adds reconcileMountedTerminalThreadIds in ChatView.logic.ts to maintain a bounded MRU list (capped at MAX_HIDDEN_MOUNTED_TERMINAL_THREADS = 10) of mounted terminal drawers.
• Terminal events from the server are now buffered in terminalStateStore (up to 200 per terminal) and replayed on mount via snapshot + event log, preventing output loss during remounts.
• TerminalViewport in ThreadTerminalDrawer.tsx now hydrates from snapshot history and replays buffered events after mount; resize/refit work is suppressed while the drawer is hidden.
• Behavioral Change: terminal drawers are no longer unmounted when switching threads; hidden drawers are pruned to the configured cap rather than destroyed immediately.

^{Macroscope summarized 28bae95.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8a55bc55-8e4c-4941-9c70-d269387bd826

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/persist-terminals

---

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

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Stale pathname after await causes unwanted redirect
  • Reintroduced a pathnameRef that is updated on every render, so the post-await check reads the live pathname instead of the stale closure-captured value.
• ✅ Fixed: Identity check never short-circuits due to eager spread
  • Deferred the spread of terminalEventEntriesByKey until after checking whether any matching keys exist, using a boolean flag (like removeTerminalState) so the identity check can correctly short-circuit when nothing changed.

Or push these changes by commenting:

@cursor push 2dab2f10a0

Preview (2dab2f10a0)

diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx
--- a/apps/web/src/routes/__root.tsx
+++ b/apps/web/src/routes/__root.tsx
@@ -213,6 +213,8 @@
   const seenServerConfigUpdateIdRef = useRef(getServerConfigUpdatedNotification()?.id ?? 0);
   const disposedRef = useRef(false);
   const bootstrapFromSnapshotRef = useRef<() => Promise<void>>(async () => undefined);
+  const pathnameRef = useRef(pathname);
+  pathnameRef.current = pathname;
   const serverConfig = useServerConfig();
 
   const handleWelcome = useEffectEvent((payload: ServerLifecycleWelcomePayload | null) => {
@@ -230,7 +232,7 @@
       }
       setProjectExpanded(payload.bootstrapProjectId, true);
 
-      if (pathname !== "/") {
+      if (pathnameRef.current !== "/") {
         return;
       }
       if (handledBootstrapThreadIdRef.current === payload.bootstrapThreadId) {

diff --git a/apps/web/src/terminalStateStore.ts b/apps/web/src/terminalStateStore.ts
--- a/apps/web/src/terminalStateStore.ts
+++ b/apps/web/src/terminalStateStore.ts
@@ -587,18 +587,29 @@
               threadId,
               () => createDefaultThreadTerminalState(),
             );
-            const nextTerminalEventEntriesByKey = { ...state.terminalEventEntriesByKey };
-            for (const key of Object.keys(nextTerminalEventEntriesByKey)) {
+            let removedEventEntries = false;
+            for (const key of Object.keys(state.terminalEventEntriesByKey)) {
               if (key.startsWith(`${threadId}\u0000`)) {
-                delete nextTerminalEventEntriesByKey[key];
+                removedEventEntries = true;
+                break;
               }
             }
             if (
               nextTerminalStateByThreadId === state.terminalStateByThreadId &&
-              nextTerminalEventEntriesByKey === state.terminalEventEntriesByKey
+              !removedEventEntries
             ) {
               return state;
             }
+            const nextTerminalEventEntriesByKey = removedEventEntries
+              ? { ...state.terminalEventEntriesByKey }
+              : state.terminalEventEntriesByKey;
+            if (removedEventEntries) {
+              for (const key of Object.keys(nextTerminalEventEntriesByKey)) {
+                if (key.startsWith(`${threadId}\u0000`)) {
+                  delete nextTerminalEventEntriesByKey[key];
+                }
+              }
+            }
             return {
               terminalStateByThreadId: nextTerminalStateByThreadId,
               terminalEventEntriesByKey: nextTerminalEventEntriesByKey,

_{You can send follow-ups to this agent here.}

[macroscopeapp]

Approvability

Verdict: Needs human review

This PR makes substantial changes to terminal/websocket connection lifecycle management, introducing persistent terminal mounting and event buffering to prevent eager connection cycling. The behavioral changes to how terminals are mounted/unmounted and how events are replayed warrant human review despite good test coverage.

^{You can customize Macroscope's approvability policy. Learn more.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.}

[cursor]

Spurious state reference change in removeTerminalState

Low Severity

When hasThreadState is false but removedEventEntries is true, removeTerminalState unconditionally copies terminalStateByThreadId and deletes a non-existent key, producing a new object reference identical to the original. Because ChatView now subscribes to state.terminalStateByThreadId directly (rather than a per-thread selector), this spurious new reference triggers an unnecessary re-render of the entire ChatView component tree. The fix is to conditionally copy terminalStateByThreadId only when hasThreadState is true, or reuse the existing reference otherwise.