diff --git a/src/client/TerminalView.tsx b/src/client/TerminalView.tsx index 2fcbdfa..a226fc9 100644 --- a/src/client/TerminalView.tsx +++ b/src/client/TerminalView.tsx @@ -8,16 +8,28 @@ * retry, and repeated unreasoned failures surface the close code after three * attempts, so the banner never spins forever. * + * Three control frames shape the pty lifecycle on unmount: + * - `{type:'close'}` — the user closed the tab. The host kills the pty + * immediately (quota released). + * - `{type:'park'}` — the user switched to another conversation. The tab is + * still open in its session's persisted state but its view unmounted; the + * host keeps the pty alive indefinitely (no grace countdown), so switching + * back reattaches the same shell instead of respawning one. + * - bare socket drop (no frame) — page refresh, crash, plugin teardown, or a + * same-session re-render. The host's reconnect grace keeps the shell alive + * for a quick reconnect. + * * Two attach modes share one upgrade endpoint: * - `tabId` starting with `agent:` is an agent-owned terminal (created by * the `terminal_create` tool). The uuid is the suffix after `agent:`; the * view connects with `?uuid=...`. A close frame kills the pty (the agent's * terminal closes when the user closes the tab); a bare socket drop - * leaves the pty alive (the agent owns the lifetime). + * leaves the pty alive (the agent owns the lifetime) — agent terminals + * never send park (their lifetime is already indefinite on bare drop). * - Any other `tabId` is a UI-tab terminal (the user created it from the + * menu). The view connects with `?tab=...&sessionId=...&cwd=...`. A close - * frame schedules a 0-ms close; a bare socket drop gets the host's - * reconnect grace. + * frame schedules a 0-ms close; a park frame marks the pty as parked; a + * bare socket drop gets the host's reconnect grace. */ import { useEffect, useRef, useState } from 'react' import { Terminal, type ITheme } from '@xterm/xterm' @@ -274,18 +286,31 @@ export function TerminalView(props: { scope: SessionScope; tabId: string; store: fontSub() schemeSub() inputSub.dispose() - // The close frame tells the host the owning tab is GONE (immediate - // pty release). A bare unmount — conversation switch, re-render, - // page unload — leaves the tab open, so the socket drop alone hands - // the process to the host's reconnect grace: switching back or - // refreshing reattaches the SAME shell instead of respawning one. - // (The host respawns on its own when the authoritative cwd changed.) - // Agent terminals follow the same rule: a close frame kills the pty - // (the user closed the sidebar tab); a bare socket drop leaves it - // alive (the agent owns the lifetime). - if (!store.tabOpen(scope.sessionId, tabId) + // Three unmount cases, distinguished by the store's tab/open state and + // the active session id: + // 1. The tab was closed by the user (NOT in its session's state): send + // `{type:'close'}` — the host releases the pty immediately. + // 2. The user switched to another conversation (the tab IS still open + // in scope.sessionId's state, but the active session is now a + // different one): send `{type:'park'}` — the host keeps the pty + // alive indefinitely (no grace countdown), so switching back + // reattaches the SAME shell. Without this, the bare socket drop + // would start the 30s reconnect-grace countdown and kill the shell + // while the user is still actively working in the other session. + // 3. A same-session unmount (page refresh, crash, plugin teardown, a + // re-render that re-mounts the view): bare socket drop — the host's + // reconnect grace keeps the shell alive for a quick reconnect. + // Agent terminals follow the close-frame rule; their lifetime is owned + // by the agent, so a bare drop (case 3) already leaves them alive + // indefinitely — no park frame needed. + const tabStillOpen = store.tabOpen(scope.sessionId, tabId) + const sessionSwitched = store.getSnapshot().sessionId !== scope.sessionId + if (!tabStillOpen && socket !== null && socket.readyState === WebSocket.OPEN) { socket.send(JSON.stringify({ type: 'close' })) + } else if (tabStillOpen && sessionSwitched && !isAgentTabId(tabId) + && socket !== null && socket.readyState === WebSocket.OPEN) { + socket.send(JSON.stringify({ type: 'park' })) } socket?.close() term.dispose() diff --git a/src/index.ts b/src/index.ts index 2deb39f..c5c98b9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -895,6 +895,10 @@ async function attachAgentList( * - `?tab=...&sessionId=...` attaches to a UI-tab terminal (the user * created it from the + menu). The close frame schedules a 0-ms close * (the host's reconnect grace keeps the shell alive across a refresh). + * The park frame (sent when the user switches to another conversation) + * marks the pty as parked so the upcoming bare socket drop does NOT start + * the grace countdown — the tab is still open in its session's state, so + * the shell must survive until the user switches back or closes the tab. */ async function attachTerminal( ctx: Context, @@ -971,6 +975,16 @@ async function attachTerminal( ptyManager.scheduleClose(handle.key, 0) return } + if (control !== null && control.type === 'park') { + // The user switched to another conversation: the tab is still open in + // its session's persisted state, but its view unmounted. Park the pty + // so the upcoming bare socket drop does NOT start the reconnect-grace + // countdown — the pty stays alive until the user switches back (a + // reconnecting view clears the parked state) or explicitly closes the + // tab (a close frame's scheduleClose clears it). + ptyManager.park(handle.key) + return + } if (handle.exited) return if ( control !== null @@ -986,10 +1000,14 @@ async function attachTerminal( ws.on('close', () => { dataSub.dispose() exitSub.dispose() - // A bare socket drop (refresh, tab switch) leaves the process alive - // for a grace period so a quick reconnect keeps it; the reconnect's - // open() cancels the pending close. - ptyManager.scheduleClose(handle.key, resolved.reconnectGraceMs) + // A parked pty (the user switched conversations and sent `{type:'park'}`) + // stays alive indefinitely — do NOT start the grace countdown. A bare + // socket drop without a prior park (refresh, crash) starts the grace + // period so a quick reconnect keeps the process; the reconnect's open() + // cancels the pending close. + if (!ptyManager.isParked(handle.key)) { + ptyManager.scheduleClose(handle.key, resolved.reconnectGraceMs) + } }) } catch (error) { ws.close(1011, error instanceof Error ? error.message : String(error)) diff --git a/src/pty-manager.ts b/src/pty-manager.ts index 67e553a..0b0a068 100644 --- a/src/pty-manager.ts +++ b/src/pty-manager.ts @@ -64,10 +64,32 @@ export interface SidebarPty { /** * The terminal registry. `maxPerSession` bounds concurrent processes per * conversation (the client caps tabs at the same number). + * + * Lifecycle of a UI-tab pty when its WebSocket drops: + * - **Close frame** (`{type:'close'}`): the user closed the tab → schedule a + * 0-ms close (quota released immediately). + * - **Park frame** (`{type:'park'}`): the user switched to another + * conversation; the tab is still open in its session's persisted state but + * its view unmounted → mark the pty as parked (no auto-close countdown). + * The pty stays alive until the user switches back (a reconnecting view + * calls `open()` which clears the parked state) or the tab is later closed + * (a `{type:'close'}` frame from a fresh connection). Without `park`, a + * bare socket drop would start the reconnect-grace countdown and kill the + * shell after `reconnectGraceMs` — wrong for a session switch, where the + * user is still actively using the app, just in another conversation. + * - **Bare socket drop** (no frame): page refresh, crash, plugin teardown → + * schedule a close after `reconnectGraceMs` so a quick reconnect reattaches + * the same shell. */ export class PtyManager { private readonly sessions = new Map() private readonly pendingCloses = new Map>() + /** Tabs whose view unmounted because the user switched conversations — the + * tab is still open in its session's state, so the pty must NOT enter the + * reconnect-grace countdown. Cleared by `cancelClose` (a reconnecting + * view's `open()` cancels it) or by `scheduleClose` (an explicit close + * frame still kills a parked pty). */ + private readonly parked = new Set() constructor( private readonly shell: string, @@ -159,7 +181,9 @@ export class PtyManager { * Schedule the terminal's destruction after `delayMs`. A tab close sends * delay 0 (release the quota immediately); a bare socket drop (refresh, * crash) uses the grace period so a quick reconnect keeps the process. - * `open()` cancels any pending close. + * `open()` cancels any pending close. Clears the parked state — an explicit + * close frame on a parked pty (the user switched back and closed the tab) + * still kills it. */ scheduleClose(key: string, delayMs: number): void { const handle = this.sessions.get(key) @@ -169,13 +193,36 @@ export class PtyManager { this.pendingCloses.set(key, timer) } - /** Cancel a pending scheduled close (the terminal is being reopened). */ + /** + * Park a terminal: the owning tab's view unmounted because the user + * switched to another conversation, but the tab is still open in its + * session's persisted state. Cancels any pending grace close and marks + * the pty so the host's `ws.on('close')` handler does NOT start the + * reconnect-grace countdown — the pty stays alive until the user switches + * back (a reconnecting view's `open()` clears this) or explicitly closes + * the tab (a `{type:'close'}` frame's `scheduleClose` clears this). + */ + park(key: string): void { + if (this.sessions.get(key) === undefined) return + this.cancelClose(key) + this.parked.add(key) + } + + /** Whether this pty was parked (its view unmounted for a session switch). */ + isParked(key: string): boolean { + return this.parked.has(key) + } + + /** Cancel a pending scheduled close (the terminal is being reopened). + * Also clears the parked state — a reconnecting view reattaches a parked + * pty and resumes normal lifecycle. */ cancelClose(key: string): void { const timer = this.pendingCloses.get(key) if (timer !== undefined) { clearTimeout(timer) this.pendingCloses.delete(key) } + this.parked.delete(key) } /** Resolve a live handle by key, or undefined. */ diff --git a/tests/smoke.spec.ts b/tests/smoke.spec.ts index 17ec67e..ba6ffc7 100644 --- a/tests/smoke.spec.ts +++ b/tests/smoke.spec.ts @@ -198,6 +198,60 @@ describe('host plugin smoke', () => { } }) + it('pty manager: a parked pty survives past the reconnect grace (session switch)', async () => { + const manager = new PtyManager(defaultShell(), 3) + try { + const handle = manager.open('s2', 't1', process.cwd(), 80, 24) + manager.park(handle.key) + expect(manager.isParked(handle.key)).toBe(true) + // A parked pty does NOT enter the grace countdown — it stays alive + // well past any realistic reconnectGraceMs. + await new Promise(resolve => setTimeout(resolve, 300)) + expect(manager.get(handle.key)).toBeDefined() + expect(manager.isParked(handle.key)).toBe(true) + } finally { + manager.disposeAll() + } + }) + + it('pty manager: a reconnecting view clears the parked state (switch back)', () => { + const manager = new PtyManager(defaultShell(), 3) + try { + const handle = manager.open('s2', 't1', process.cwd(), 80, 24) + manager.park(handle.key) + expect(manager.isParked(handle.key)).toBe(true) + // open() calls cancelClose(), which clears the parked state — the + // user switched back to the session and the view reattached. + manager.open('s2', 't1', process.cwd(), 80, 24) + expect(manager.isParked(handle.key)).toBe(false) + expect(manager.get(handle.key)).toBeDefined() + } finally { + manager.disposeAll() + } + }) + + it('pty manager: an explicit close frame on a parked pty still kills it', async () => { + const manager = new PtyManager(defaultShell(), 3) + try { + const handle = manager.open('s2', 't1', process.cwd(), 80, 24) + manager.park(handle.key) + // The user switched back and closed the tab — scheduleClose (the + // close-frame handler) clears the parked state and kills the pty. + manager.scheduleClose(handle.key, 0) + expect(manager.isParked(handle.key)).toBe(false) + await new Promise(resolve => setTimeout(resolve, 50)) + expect(manager.get(handle.key)).toBeUndefined() + } finally { + manager.disposeAll() + } + }) + + it('pty manager: park on an unknown key is a no-op', () => { + const manager = new PtyManager(defaultShell(), 3) + expect(() => manager.park('s2:nonexistent')).not.toThrow() + expect(manager.isParked('s2:nonexistent')).toBe(false) + }) + it('pty manager: reopening with a different cwd respawns in the new directory', async () => { const manager = new PtyManager(defaultShell(), 3) // A real second directory: os.tmpdir() exists on every platform ('/tmp'