diff --git a/browse/test/sidepanel-restart-dispose.test.ts b/browse/test/sidepanel-restart-dispose.test.ts new file mode 100644 index 000000000..8ec44690b --- /dev/null +++ b/browse/test/sidepanel-restart-dispose.test.ts @@ -0,0 +1,106 @@ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +// v1.44 Commit 2C — client-side restart + dispose wiring. +// +// Pre-v1.44 forceRestart only closed the client WS and disposed xterm; +// the old PTY died asynchronously via the agent's WS close handler. +// Race window between kill and mint, two claude instances briefly, +// no prompt visible until the user typed. +// +// Now forceRestart POSTs /pty-restart (one transaction: dispose + mint), +// opens the new WS with the fresh attachToken from the response, and +// sends {type:"start"} for the eager spawn. pagehide handler in +// sidepanel.js sendBeacon /pty-dispose so browser quit / panel close +// doesn't leak a 60s-zombie claude. + +const TERMINAL_JS = path.resolve( + new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel-terminal.js', +); +const SIDEPANEL_JS = path.resolve( + new URL(import.meta.url).pathname, '..', '..', '..', 'extension', 'sidepanel.js', +); + +describe('sidepanel-terminal: forceRestart via /pty-restart (v1.44+)', () => { + test('1. mintSession callers read the 4-tuple (sessionId + attachToken)', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + // The new shape lands in `minted.sessionId` and `minted.attachToken`. + expect(src).toContain('const { terminalPort, sessionId } = minted'); + expect(src).toContain('minted.attachToken || minted.ptySessionToken'); + // Backward-compat fallback to ptySessionToken kept so a partially- + // updated extension still works against a fresh server. + }); + + test('2. eager spawn via {type:"start"} on ws.open', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + // Replaces the legacy `ws.send(TextEncoder().encode("\\n"))` newline + // hack that nudged the lazy-binary-spawn. + expect(src).toMatch(/ws\.send\(JSON\.stringify\(\{\s*type:\s*'start'\s*\}\)\)/); + expect(src).not.toContain("TextEncoder().encode('\\n')"); + }); + + test('3. forceRestart sends 4001 close code (intentional restart)', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + expect(src).toMatch(/ws\.close\(4001/); + }); + + test('4. forceRestart POSTs /pty-restart with current sessionId', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + expect(src).toContain('/pty-restart'); + expect(src).toContain('priorSessionId ? { sessionId: priorSessionId } : {}'); + }); + + test('5. forceRestart 401 triggers sticky abort (no spam loop)', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + // Same defense pattern as connect() — 401 must flip the sticky flag + // or every 2s the user sees a fresh "Auth invalid" message. + const block = sliceBetween(src, 'async function forceRestart', 'function repaintIfLive'); + expect(block).toContain('resp.status === 401'); + expect(block).toContain('autoConnectAborted = true'); + }); + + test('6. currentSessionId is exposed on window for sidepanel.js pagehide', () => { + const src = fs.readFileSync(TERMINAL_JS, 'utf-8'); + expect(src).toContain('window.gstackPtySession = currentSessionId'); + }); +}); + +describe('sidepanel: pagehide → sendBeacon /pty-dispose (v1.44+)', () => { + test('7. pagehide handler fires sendBeacon to /pty-dispose', () => { + const src = fs.readFileSync(SIDEPANEL_JS, 'utf-8'); + expect(src).toMatch(/window\.addEventListener\('pagehide'/); + expect(src).toContain('navigator.sendBeacon'); + expect(src).toContain('/pty-dispose'); + }); + + test('8. pagehide payload carries sessionId + authToken in body (sendBeacon-compat)', () => { + const src = fs.readFileSync(SIDEPANEL_JS, 'utf-8'); + // sendBeacon can't set custom headers — server route accepts body-auth. + // Both fields must be in the payload or the server rejects. + expect(src).toMatch(/JSON\.stringify\(\{\s*sessionId,\s*authToken\s*\}\)/); + expect(src).toContain('window.gstackPtySession'); + expect(src).toContain('window.gstackAuthToken'); + }); + + test('9. pagehide handler is best-effort (try/catch swallows failures)', () => { + const src = fs.readFileSync(SIDEPANEL_JS, 'utf-8'); + // The 60s detach window catches any sendBeacon that fails, so the + // handler MUST not throw — uncaught throws can interfere with the + // browser's unload sequence. Slice between pagehide and end-of-file + // (it's the last addEventListener in sidepanel.js by design). + const i = src.indexOf("addEventListener('pagehide'"); + expect(i).toBeGreaterThan(-1); + const block = src.slice(i); + expect(block).toMatch(/try \{/); + expect(block).toMatch(/} catch /); + }); +}); + +function sliceBetween(source: string, start: string, end: string): string { + const i = source.indexOf(start); + if (i === -1) throw new Error(`marker not found: ${start}`); + const j = source.indexOf(end, i + start.length); + if (j === -1) throw new Error(`end marker not found: ${end}`); + return source.slice(i, j); +} diff --git a/extension/sidepanel-terminal.js b/extension/sidepanel-terminal.js index 88c3dbb10..4cc42810d 100644 --- a/extension/sidepanel-terminal.js +++ b/extension/sidepanel-terminal.js @@ -57,6 +57,14 @@ * fixing whatever was wrong. */ let autoConnectAborted = false; + /** + * v1.44 session identity. The stable, non-secret sessionId minted by + * /pty-session and surfaced back via window.gstackPtySession so the + * sidepanel.js pagehide handler can sendBeacon /pty-dispose for THIS + * specific session. forceRestart sends this to /pty-restart so the + * server can scope the disposal to one terminal rather than all. + */ + let currentSessionId = null; /** * 25s client-side WS keepalive interval (v1.44+). Belt-and-suspenders with * the server-side ping in terminal-agent.ts: server pings cover most @@ -347,11 +355,19 @@ setState(STATE.IDLE, { message: `Cannot start: ${minted.error}` }); return; } - const { terminalPort, ptySessionToken } = minted; - if (!ptySessionToken) { - setState(STATE.IDLE, { message: 'Cannot start: no session token returned' }); + // v1.44 4-tuple: { terminalPort, sessionId, attachToken, leaseExpiresAt } + // Falls back to the legacy `ptySessionToken` field for one minor release + // (server keeps the alias) so a partially-updated extension still works + // against a fresh server. + const { terminalPort, sessionId } = minted; + const attachToken = minted.attachToken || minted.ptySessionToken; + if (!attachToken) { + setState(STATE.IDLE, { message: 'Cannot start: no attach token returned' }); return; } + currentSessionId = sessionId || null; + // Expose for sidepanel.js pagehide handler — see Commit 2C wiring. + try { window.gstackPtySession = currentSessionId; } catch {} // Pre-flight: does claude even exist on PATH? const claudeStatus = await checkClaudeAvailable(terminalPort); @@ -373,7 +389,7 @@ // SameSite=Strict don't survive the jump from server.ts:34567 to the // agent's random port from a chrome-extension origin, so cookies // alone weren't reliable. - ws = new WebSocket(`ws://127.0.0.1:${terminalPort}/ws`, [`gstack-pty.${ptySessionToken}`]); + ws = new WebSocket(`ws://127.0.0.1:${terminalPort}/ws`, [`gstack-pty.${attachToken}`]); ws.binaryType = 'arraybuffer'; ws.addEventListener('open', () => { @@ -398,8 +414,11 @@ } }); } catch {} - // Send a single byte to nudge the agent to spawn claude (lazy-spawn trigger). - try { ws.send(new TextEncoder().encode('\n')); } catch {} + // v1.44 eager spawn: send {type:"start"} so the agent boots claude + // without requiring the user to type a keystroke. Pre-v1.44 the + // lazy-binary-spawn pattern made forceRestart look stuck for ~2-3s + // until the user pressed any key. + try { ws.send(JSON.stringify({ type: 'start' })); } catch {} // v1.44 client-side keepalive. Server pings every 25s; we ALSO send // keepalive frames at the same cadence so a paused timer on either // side still has the other to lean on. Both are silently dropped @@ -471,23 +490,175 @@ * Force a fresh session: close any open WS, dispose xterm, return to * IDLE, kick off auto-connect. Safe to call from any state. */ - function forceRestart() { + /** + * v1.44 forceRestart: hits the server's /pty-restart one-transaction + * endpoint with the current sessionId. The server kills the old PtySession + * scope-to-our-id, mints a fresh sessionId + lease + attachToken, and + * returns the new 4-tuple in one round trip. Zero race window between + * kill and mint (codex D8). + * + * If we don't have a sessionId (sidebar is in IDLE / ENDED state because + * the prior session ended cleanly), the route accepts that gracefully — + * skips the dispose step and just mints fresh. Either way the user sees + * the same "Restarting..." → fresh prompt UX. + */ + async function forceRestart() { if (keepaliveInterval) { clearInterval(keepaliveInterval); keepaliveInterval = null; } - try { ws && ws.close(); } catch {} + // Re-arm the auto-connect loop in case a prior auth failure stuck the + // sticky flag — explicit user action is the cleared-flag signal. + autoConnectAborted = false; + setState(STATE.IDLE, { message: 'Restarting Claude Code...' }); + + const serverPort = getServerPort(); + const authToken = getAuthToken(); + const priorSessionId = currentSessionId; + + // Close the local WS BEFORE the server-side kill so the agent's + // close handler doesn't race with the dispose call. + try { ws && ws.close(4001, 'intentional-restart'); } catch {} ws = null; if (term) { try { term.dispose(); } catch {} term = null; fitAddon = null; } - // User explicitly asked for a fresh start; re-arm the auto-connect - // polling loop in case a prior auth failure stuck the abort flag. - autoConnectAborted = false; - setState(STATE.IDLE, { message: 'Starting Claude Code...' }); - tryAutoConnect(); + + if (!serverPort || !authToken) { + // Server hasn't been discovered yet — fall back to the patient + // polling loop. forceRestart's promise of "fresh prompt now" can't + // be met without a live server; user sees the patient status path. + tryAutoConnect(); + return; + } + + let nextTuple = null; + try { + const resp = await fetch(`http://127.0.0.1:${serverPort}/pty-restart`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${authToken}`, + }, + body: JSON.stringify(priorSessionId ? { sessionId: priorSessionId } : {}), + credentials: 'include', + }); + if (resp.ok) { + nextTuple = await resp.json(); + } else if (resp.status === 401) { + autoConnectAborted = true; + setState(STATE.IDLE, { + message: 'Auth invalid — reload the sidebar or restart your gstack session.', + }); + return; + } else if (resp.status === 503) { + // Agent down — fall through to patient autoconnect which will + // surface the appropriate "waiting for server" status. + setState(STATE.IDLE, { message: 'Restart failed: terminal agent not ready. Retrying...' }); + } else { + const body = await resp.text().catch(() => ''); + setState(STATE.IDLE, { message: `Restart failed: ${resp.status} ${body || resp.statusText}` }); + } + } catch (err) { + setState(STATE.IDLE, { + message: `Restart failed: ${err && err.message ? err.message : String(err)}`, + }); + } + + if (!nextTuple) { + // Restart didn't yield a fresh tuple. Fall back to the regular + // connect path; tryAutoConnect will retry as the server recovers. + currentSessionId = null; + try { window.gstackPtySession = null; } catch {} + tryAutoConnect(); + return; + } + + // We have a fresh 4-tuple — open the new WS directly without going + // through mintSession again. This is the explicit "no race window" + // path the codex D8 redesign was after. + const { terminalPort, sessionId, attachToken, expiresAt: _expiresAt } = nextTuple; + const token = attachToken || nextTuple.ptySessionToken; + if (!terminalPort || !token) { + currentSessionId = null; + tryAutoConnect(); + return; + } + currentSessionId = sessionId || null; + try { window.gstackPtySession = currentSessionId; } catch {} + + // Pre-flight: claude still on PATH? + const claudeStatus = await checkClaudeAvailable(terminalPort); + if (!claudeStatus.available) { + setState(STATE.NO_CLAUDE); + return; + } + + setState(STATE.LIVE); + ensureXterm(); + ws = new WebSocket(`ws://127.0.0.1:${terminalPort}/ws`, [`gstack-pty.${token}`]); + ws.binaryType = 'arraybuffer'; + + ws.addEventListener('open', () => { + try { + ws.send(JSON.stringify({ type: 'resize', cols: term.cols, rows: term.rows })); + } catch {} + try { + chrome.runtime.sendMessage({ type: 'getTabState' }, (resp) => { + if (resp && ws && ws.readyState === WebSocket.OPEN) { + try { + ws.send(JSON.stringify({ + type: 'tabState', + active: resp.active, + tabs: resp.tabs, + reason: 'restart', + })); + } catch {} + } + }); + } catch {} + // Eager spawn — fresh claude prompt visible without user keystroke. + try { ws.send(JSON.stringify({ type: 'start' })); } catch {} + if (keepaliveInterval) clearInterval(keepaliveInterval); + keepaliveInterval = setInterval(() => { + if (!ws || ws.readyState !== WebSocket.OPEN) return; + try { ws.send(JSON.stringify({ type: 'keepalive' })); } catch {} + }, KEEPALIVE_INTERVAL_MS); + }); + + ws.addEventListener('message', (ev) => { + if (typeof ev.data === 'string') { + try { + const msg = JSON.parse(ev.data); + if (msg.type === 'error' && msg.code === 'CLAUDE_NOT_FOUND') { + setState(STATE.NO_CLAUDE); + try { ws.close(); } catch {} + return; + } + if (msg.type === 'ping') { + try { ws.send(JSON.stringify({ type: 'pong', ts: msg.ts })); } catch {} + return; + } + } catch {} + return; + } + const buf = ev.data instanceof ArrayBuffer ? new Uint8Array(ev.data) : ev.data; + term.write(buf); + }); + + ws.addEventListener('close', () => { + ws = null; + if (keepaliveInterval) { + clearInterval(keepaliveInterval); + keepaliveInterval = null; + } + if (state !== STATE.NO_CLAUDE) setState(STATE.ENDED); + }); + ws.addEventListener('error', (err) => { + console.error('[gstack terminal] ws error', err); + }); } /** diff --git a/extension/sidepanel.js b/extension/sidepanel.js index 6328d7c51..14834519b 100644 --- a/extension/sidepanel.js +++ b/extension/sidepanel.js @@ -1083,3 +1083,38 @@ chrome.runtime.onMessage.addListener((msg) => { })); } }); + +// ─── v1.44 pagehide: explicit PTY dispose on sidebar close ────────── +// +// Codex T3 of the eng review: WS close codes alone can't distinguish +// "intentional close" (sidebar closed, browser quit, extension reload) +// from "transient blip" (wifi hiccup) reliably — Chrome routes the +// former through code 1001 (going-away) and the latter through 1006 +// (abnormal), but neither is a load-bearing contract across browsers +// and extension lifecycles. +// +// pagehide fires reliably for tab close, panel close, extension reload, +// and navigation-away. We use it to fire-and-forget a /pty-dispose POST +// so the server can synchronously dispose the PtySession instead of +// waiting for the 60s detach window (Commit 3) to time out. Zombie +// claude processes lingering for 60s on every browser quit was the +// codex-flagged failure mode. +// +// sendBeacon is the only fetch primitive that survives a closing page — +// it doesn't accept custom headers, which is why the server's +// /pty-dispose route accepts the auth token in the BODY (see +// server-pty-lease-routes.test.ts test 4). +window.addEventListener('pagehide', () => { + const sessionId = window.gstackPtySession; + const authToken = window.gstackAuthToken; + const port = window.gstackServerPort; + if (!sessionId || !authToken || !port) return; + try { + const blob = new Blob([JSON.stringify({ sessionId, authToken })], { + type: 'application/json', + }); + navigator.sendBeacon(`http://127.0.0.1:${port}/pty-dispose`, blob); + } catch { + // Best-effort — the 60s detach timer will catch any session we miss. + } +});