feat: persist and reattach terminal sessions - #10
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements durable terminal session persistence for the Electron runtime by moving PTY ownership into a detached local “session host” process, persisting workspace/layout + terminal session IDs to an app-owned userData JSON store (mirrored to renderer localStorage), and reattaching saved sessions on restore.
Changes:
- Add a local Session Host (socket/pipe protocol + client + detached host process) to keep PTYs alive across Baton UI restarts and enable attach/reattach.
- Introduce an Electron app-state bridge (
window.baton.appState) backed by an atomic JSON store, plus renderer hydration/serialization changes to retain terminal session IDs. - Add automated verification (unit tests + built-runtime reattach validation) and CI steps to run tests/typecheck/verification.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/terminal-types.ts | Adds attach request/response DTOs (incl. buffer + exit status). |
| src/shared/session-host-protocol.ts | Defines client/server message protocol for the detached session host. |
| src/renderer/src/services/terminalClient.ts | Adds attachTerminal to clients; buffered client seeds replay buffer from attach response. |
| src/renderer/src/services/terminalClient.test.ts | Tests that attachTerminal seeds the buffered replay state. |
| src/renderer/src/persistence.ts | Adds sanitization/serialization split, Electron hydration via appState bridge, and persists terminalId for reattach. |
| src/renderer/src/persistence.test.ts | Adds tests for Electron app-state hydration/mirroring and terminalId persistence/restore behavior. |
| src/renderer/src/env.d.ts | Extends Baton bridge types with terminal.attach and appState API. |
| src/renderer/src/components/TerminalWindow.tsx | Implements attach-first logic when a saved terminalId exists; updates UI for unavailable sessions. |
| src/renderer/src/App.tsx | Hydrates state from Electron app-state store on boot, then debounced saves to both stores. |
| src/preload/index.ts | Exposes terminal.attach and appState.get/set via contextBridge. |
| src/main/terminal-runtime.ts | Extracts shared runtime helpers (cwd resolution + integer clamping). |
| src/main/session-host-process.ts | Implements the detached PTY owner, ring buffer, attach/reattach, and idle shutdown. |
| src/main/session-host-path.ts | Computes stable per-userData endpoint (UDS/named pipe). |
| src/main/session-host-client.ts | Electron-side client that spawns/attaches to the session host and multiplexes events. |
| src/main/index.ts | Bootstraps either session-host mode or normal Electron main entry. |
| src/main/electron-main.ts | Refactors Electron main logic; wires IPC to session host and app-state store. |
| src/main/app-state-store.ts | Adds atomic JSON persistence with in-memory cache and serialized writes. |
| src/main/app-state-store.test.ts | Tests app-state store behavior (round-trip, invalid JSON recovery, concurrent saves). |
| scripts/verify-session-persistence.mjs | Built-runtime verification proving PID-stable detach/reattach + buffer replay. |
| package.json | Adds verification scripts. |
| docs/verification/session-persistence.md | Documents verification plan and gates. |
| docs/adrs/0002-terminal-session-persistence.md | Adds ADR capturing architecture + scope. |
| README.md | Updates persistence model documentation and CI description. |
| .github/workflows/build.yml | Runs tests/typecheck + session persistence verification in CI (non-Windows). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (terminal.terminalId) { | ||
| client.attachTerminal(terminal.terminalId) | ||
| .then((response) => { | ||
| if (cancelled) return; | ||
|
|
||
| onPatchRef.current({ | ||
| title: `${response.shell} · ${response.cwd ?? "~"}`, | ||
| status: response.status, | ||
| exitCode: response.exitCode, | ||
| }); | ||
| }) | ||
| .catch((error: unknown) => { | ||
| hasStartedRef.current = false; | ||
| onPatchRef.current({ | ||
| status: "error", | ||
| title: error instanceof Error | ||
| ? error.message | ||
| : "Terminal session is unavailable", | ||
| }); | ||
| }); |
There was a problem hiding this comment.
In the attach error path, the effect may run its .catch(...) after the component unmounts (or after the cleanup sets cancelled = true). Unlike the success path, the catch block doesn't check cancelled before calling onPatchRef.current(...), so it can patch state for a terminal window that is no longer mounted / relevant. Add a if (cancelled) return; guard in the catch block (and consider doing the same for the create catch below for symmetry).
There was a problem hiding this comment.
there's no reason to do that because there's no action being taken other than reporting.
| async function save(next: unknown): Promise<void> { | ||
| cache = next | ||
| cacheLoaded = true | ||
|
|
||
| const currentWrite = ++writeCount | ||
| writeSequence = writeSequence | ||
| .catch(() => undefined) | ||
| .then(async () => { | ||
| await fs.mkdir(path.dirname(filePath), { recursive: true }) | ||
| const tmp = `${filePath}.${process.pid}.${currentWrite}.tmp` | ||
| await fs.writeFile(tmp, JSON.stringify(next, null, 2), 'utf8') | ||
| await fs.rename(tmp, filePath) | ||
| }) | ||
|
|
||
| await writeSequence | ||
| } |
There was a problem hiding this comment.
save() awaits the mutable writeSequence variable after reassigning it. With concurrent save() calls, an earlier call can end up awaiting a later call’s appended promise chain (if writeSequence is reassigned before the first call reaches await writeSequence), which can unnecessarily delay resolution and make timing harder to reason about. Capture the newly created promise in a local variable (e.g., const nextWrite = writeSequence = ...; await nextWrite;) so each save() awaits only the write it scheduled (while still serializing on the shared chain).
| export function serializeAppState(state: PersistedAppState): PersistedAppState { | ||
| return { | ||
| ...state, | ||
| workspaces: state.workspaces.map((workspace) => ({ | ||
| ...workspace, | ||
| terminals: workspace.terminals.map((terminal) => ({ | ||
| ...terminal, | ||
| terminalId: undefined, | ||
| status: terminal.status === 'exited' ? 'exited' : 'starting' | ||
| })) | ||
| })) | ||
| terminalId: sanitizeTerminalId(terminal.terminalId), | ||
| status: terminal.terminalId ? 'starting' : terminal.status === 'exited' ? 'exited' : 'starting', | ||
| })), | ||
| })), | ||
| } |
There was a problem hiding this comment.
serializeAppState() now persists terminalId for all runtimes. On non-Electron backends this appears to regress restore behavior: WebSocketTerminalClient.attachTerminal() always throws, and DemoTerminalClient can only attach if the in-memory buffer map still exists (so a reload makes saved terminalIds unattached). Because TerminalWindow prioritizes attachTerminal when terminalId is present and does not fall back to createTerminal on attach failure, reloading in web/demo mode will leave terminals stuck in an error/unavailable state rather than spawning a fresh session. Consider stripping terminalId unless the current backend supports attach/reattach (or add fallback logic on attach failure to create a new terminal and clear the stored id).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…llback fix: avoid stale terminal reattach on web runtimes
Summary
userDataJSON store, mirror it to renderer cache, and reattach saved terminal ids on restoreTesting