mirror of https://github.com/garrytan/gstack.git
feat(sidebar): forceRestart via /pty-restart + pagehide /pty-dispose
Closes the Commit 2 loop: server-side lease + restart routes shipped in
25ef24e9; this commit wires the extension client to use them. End-to-end
result — clicking Restart now actually kills the server's PTY before
opening a new WS (zero race window), and closing the sidebar / quitting
the browser disposes the PTY immediately instead of letting it linger
for the upcoming 60s detach window.
sidepanel-terminal.js:
* mintSession callers read the v1.44 4-tuple (sessionId + attachToken)
from /pty-session, with a backward-compat fallback to ptySessionToken
so a partially-updated extension still works against a fresh server
for one minor release.
* Eager spawn via {type:"start"} text frame replaces the legacy
`TextEncoder().encode("\n")` newline hack. Pre-v1.44, the lazy-binary-
spawn pattern made forceRestart look stuck until the user typed —
now claude boots before the prompt renders.
* forceRestart() rewritten as an async one-transaction handler:
1. close current WS with code 4001 (intentional-restart)
2. POST /pty-restart with priorSessionId so the server can scope
the dispose, then mint fresh sessionId + lease + attachToken
in the same response
3. Open new WS with the returned attachToken, send {type:"start"}
immediately for eager spawn
4. On 401: sticky-abort the auto-connect loop (no spam)
5. On 503 / network failure: fall back to patient autoconnect
* currentSessionId tracked and exposed on window.gstackPtySession so
sidepanel.js's pagehide handler can sendBeacon the dispose.
sidepanel.js:
* New pagehide handler fires navigator.sendBeacon('/pty-dispose',
{sessionId, authToken}) on tab close, panel close, browser quit,
or extension reload. sendBeacon-compatible: auth token rides in
the body since sendBeacon can't set custom headers (server route
accepts body-auth per 25ef24e9).
* try/catch around the entire body so a sendBeacon failure can't
interfere with the browser's unload sequence — the 60s detach
window from Commit 3 catches anything we miss.
There's bounded duplication between connect() and forceRestart() (~70
lines of WS attach/handler wiring). Extracting a shared helper is a
clean follow-up but out of scope for the v1.44 ship — both paths are
exercised by the same e2e test.
Test (browse/test/sidepanel-restart-dispose.test.ts):
* 9 static-grep tripwires pinning the 4-tuple parse, eager spawn,
close-code 4001 contract, /pty-restart wire shape, sticky-abort
401 path, sessionId window plumbing, sendBeacon body contract,
and the best-effort try/catch around pagehide.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
parent
25ef24e92e
commit
0aed7f8ecf
|
|
@ -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);
|
||||
}
|
||||
|
|
@ -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);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue