fix: preserve interactive terminal across tab switches and project changes (#3055)

Two root causes destroyed terminal state during normal navigation:

1. The pagehide handler fired a shutdown beacon unconditionally, but on
   mobile/Safari tab switches pagehide fires with event.persisted=true
   (bfcache entry). This killed the server and all PTY sessions when the
   user merely switched browser tabs. Fix: check event.persisted and skip
   the beacon when the page is being cached, not unloaded.

2. ShellTerminal used project-agnostic session IDs ("default"), so
   switching projects and switching back either collided with the old
   session or spawned a new one, losing terminal state. Fix: scope session
   IDs by project path (e.g. "default:/path/to/project") so the server's
   getOrCreateSession returns the existing live PTY on reconnect.

Closes #2701

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 16:46:09 -04:00 committed by GitHub
parent fb10141e9b
commit dfb4fbecef
3 changed files with 306 additions and 3 deletions

View file

@ -0,0 +1,264 @@
import test from "node:test";
import assert from "node:assert/strict";
// ---------------------------------------------------------------------------
// Constants mirrored from the shutdown-gate and app-shell
// ---------------------------------------------------------------------------
const SHUTDOWN_DELAY_MS = 3_000;
// ---------------------------------------------------------------------------
// Test 1: pagehide handler must NOT fire shutdown beacon on tab switches
// ---------------------------------------------------------------------------
// The bug: `pagehide` fires both on actual page unload AND on mobile/Safari
// tab switches (where event.persisted === true because the page enters bfcache).
// The current handler does not check event.persisted, so it fires shutdown
// beacons on tab switches — killing the server and all PTY sessions.
/**
* Mirrors the pagehide handler logic from app-shell.tsx's
* ProjectAwareWorkspace component. The BUGGY version sends a shutdown
* beacon unconditionally.
*/
function buggyPageHideHandler(_event: { persisted: boolean }): boolean {
// Current code (buggy): always sends beacon regardless of event.persisted
return true; // true = beacon was sent
}
/**
* Fixed version: only send shutdown beacon when the page is truly being
* unloaded (event.persisted === false). When persisted is true the page
* is being put into bfcache (tab switch, app backgrounding) and the
* server should stay alive.
*/
function fixedPageHideHandler(event: { persisted: boolean }): boolean {
if (event.persisted) {
// Page is entering bfcache (tab switch) — do NOT shut down
return false;
}
return true; // true = beacon was sent
}
test("pagehide: buggy handler sends shutdown beacon on tab switch (persisted=true)", () => {
// This test documents the bug — the buggy handler fires on tab switches
const beaconSent = buggyPageHideHandler({ persisted: true });
assert.equal(beaconSent, true, "Buggy handler sends beacon even on tab switch");
});
test("pagehide: fixed handler skips shutdown beacon on tab switch (persisted=true)", () => {
const beaconSent = fixedPageHideHandler({ persisted: true });
assert.equal(beaconSent, false, "Fixed handler must NOT send beacon on tab switch");
});
test("pagehide: fixed handler still sends shutdown beacon on real page unload (persisted=false)", () => {
const beaconSent = fixedPageHideHandler({ persisted: false });
assert.equal(beaconSent, true, "Fixed handler must send beacon on real unload");
});
// ---------------------------------------------------------------------------
// Test 2: Project switching must NOT destroy PTY sessions
// ---------------------------------------------------------------------------
// The bug: ProjectStoreManager.switchProject() changes the active store,
// which causes React to unmount the entire WorkspaceChrome tree (including
// ShellTerminal). The PTY processes survive server-side, but the client
// loses all xterm state and SSE connections. When the user switches back,
// a NEW terminal is created instead of reconnecting to the existing one.
/**
* Mirrors the session-id generation logic used by ShellTerminal.
* The BUGGY version generates a project-agnostic session ID, so switching
* projects and switching back creates a collision or a fresh session.
*
* The FIXED version namespaces session IDs by project so switching back
* reconnects to the same server-side PTY session via its stable ID.
*/
interface TerminalSessionTracker {
/** Active PTY session IDs on the server (survives client unmount) */
serverSessions: Map<string, { alive: boolean; projectCwd: string }>;
/** Client-side session IDs (destroyed on unmount) */
clientSessions: Set<string>;
}
function createTracker(): TerminalSessionTracker {
return {
serverSessions: new Map(),
clientSessions: new Set(),
};
}
/**
* Simulates what happens when ShellTerminal mounts for a project.
* The BUGGY version uses a plain default ID with no project namespace.
*/
function buggyMountTerminal(tracker: TerminalSessionTracker, _projectCwd: string): string {
const sessionId = "default"; // No project namespace — always the same ID
tracker.serverSessions.set(sessionId, { alive: true, projectCwd: _projectCwd });
tracker.clientSessions.add(sessionId);
return sessionId;
}
/**
* Simulates what happens when ShellTerminal unmounts (project switch).
* Client-side state is destroyed but server session stays alive.
*/
function unmountTerminal(tracker: TerminalSessionTracker, sessionId: string): void {
tracker.clientSessions.delete(sessionId);
// Server session stays alive — this is the correct behavior
}
/**
* FIXED mount: uses a project-scoped session ID so switching back to
* a project reconnects to the same server-side PTY.
*/
function fixedMountTerminal(tracker: TerminalSessionTracker, projectCwd: string): string {
const sessionId = `shell:${projectCwd}:default`;
// getOrCreateSession on the server: if alive, returns existing; if dead, creates new
if (!tracker.serverSessions.has(sessionId) || !tracker.serverSessions.get(sessionId)!.alive) {
tracker.serverSessions.set(sessionId, { alive: true, projectCwd });
}
tracker.clientSessions.add(sessionId);
return sessionId;
}
test("project switch: buggy flow reuses same session ID for different projects", () => {
const tracker = createTracker();
// Mount terminal for project A
const sessionA = buggyMountTerminal(tracker, "/projects/alpha");
assert.equal(sessionA, "default");
assert.equal(tracker.serverSessions.get("default")?.projectCwd, "/projects/alpha");
// Switch to project B — unmount A, mount B
unmountTerminal(tracker, sessionA);
const sessionB = buggyMountTerminal(tracker, "/projects/beta");
// Bug: same session ID, but now points to a different project
assert.equal(sessionB, "default");
assert.equal(
tracker.serverSessions.get("default")?.projectCwd,
"/projects/beta",
"Buggy: server session is overwritten with new project",
);
});
test("project switch: fixed flow preserves per-project session identity", () => {
const tracker = createTracker();
// Mount terminal for project A
const sessionA = fixedMountTerminal(tracker, "/projects/alpha");
assert.ok(sessionA.includes("/projects/alpha"), "Session ID includes project path");
// Switch to project B — unmount A, mount B
unmountTerminal(tracker, sessionA);
const sessionB = fixedMountTerminal(tracker, "/projects/beta");
// Session IDs are different — no collision
assert.notEqual(sessionA, sessionB, "Different projects get different session IDs");
// Both server sessions exist independently
assert.equal(tracker.serverSessions.get(sessionA)?.alive, true);
assert.equal(tracker.serverSessions.get(sessionB)?.alive, true);
// Switch back to project A — should reconnect to same session
unmountTerminal(tracker, sessionB);
const sessionA2 = fixedMountTerminal(tracker, "/projects/alpha");
assert.equal(sessionA2, sessionA, "Switching back reconnects to the same session ID");
assert.equal(tracker.serverSessions.get(sessionA)?.alive, true, "Original server session is still alive");
});
// ---------------------------------------------------------------------------
// Test 3: Shutdown gate must differentiate tab-switch from real unload
// ---------------------------------------------------------------------------
// The shutdown gate has a 3s delay to allow page refreshes to cancel the
// shutdown. But on mobile tab switches that fire pagehide, the 3s timer
// starts — and if the user doesn't switch back within 3s, the server dies.
// The fix is to never start the timer on persisted pagehide events.
interface ShutdownGateState {
timerScheduled: boolean;
shutdownExecuted: boolean;
}
function createShutdownGate(): ShutdownGateState {
return { timerScheduled: false, shutdownExecuted: false };
}
function scheduleShutdownIfAllowed(gate: ShutdownGateState, event: { persisted: boolean }): void {
// Fixed: only schedule shutdown when the page is truly unloading
if (event.persisted) return;
gate.timerScheduled = true;
}
function cancelShutdown(gate: ShutdownGateState): void {
gate.timerScheduled = false;
}
test("shutdown gate: tab switch (persisted=true) must not schedule shutdown", () => {
const gate = createShutdownGate();
scheduleShutdownIfAllowed(gate, { persisted: true });
assert.equal(gate.timerScheduled, false, "No shutdown timer on tab switch");
});
test("shutdown gate: real page unload (persisted=false) must schedule shutdown", () => {
const gate = createShutdownGate();
scheduleShutdownIfAllowed(gate, { persisted: false });
assert.equal(gate.timerScheduled, true, "Shutdown timer on real unload");
});
test("shutdown gate: scheduled shutdown can still be cancelled by page refresh", () => {
const gate = createShutdownGate();
scheduleShutdownIfAllowed(gate, { persisted: false });
assert.equal(gate.timerScheduled, true);
cancelShutdown(gate);
assert.equal(gate.timerScheduled, false, "Timer cancelled on refresh");
});
// ---------------------------------------------------------------------------
// Test 4: Shell terminal session ID must be project-scoped
// ---------------------------------------------------------------------------
/**
* Mirrors the session ID derivation that ShellTerminal should use.
* The default session ID (when no sessionPrefix is given) must incorporate
* the project path so that different projects get different PTY sessions.
*/
function deriveSessionId(
projectCwd: string | undefined,
sessionPrefix?: string,
command?: string,
): string {
const base = sessionPrefix ?? (command ? "gsd-default" : "default");
if (!projectCwd) return base;
// Stable hash-like key from the project path — keeps IDs short but unique
return `${base}:${projectCwd}`;
}
test("session ID derivation: different projects produce different IDs", () => {
const idA = deriveSessionId("/projects/alpha");
const idB = deriveSessionId("/projects/beta");
assert.notEqual(idA, idB);
});
test("session ID derivation: same project produces stable ID", () => {
const id1 = deriveSessionId("/projects/alpha");
const id2 = deriveSessionId("/projects/alpha");
assert.equal(id1, id2);
});
test("session ID derivation: explicit sessionPrefix is preserved with project scope", () => {
const id = deriveSessionId("/projects/alpha", "my-prefix");
assert.ok(id.includes("my-prefix"), "Prefix included");
assert.ok(id.includes("/projects/alpha"), "Project path included");
});
test("session ID derivation: command sessions are also project-scoped", () => {
const idA = deriveSessionId("/projects/alpha", undefined, "gsd");
const idB = deriveSessionId("/projects/beta", undefined, "gsd");
assert.notEqual(idA, idB);
assert.ok(idA.includes("gsd-default"), "Uses gsd-default base for command sessions");
});
test("session ID derivation: no projectCwd falls back to plain base ID", () => {
const id = deriveSessionId(undefined);
assert.equal(id, "default");
});

View file

@ -564,9 +564,18 @@ function ProjectAwareWorkspace() {
const activeProjectCwd = useSyncExternalStore(manager.subscribe, manager.getSnapshot, manager.getSnapshot)
const activeStore = activeProjectCwd ? manager.getActiveStore() : null
// Shut down all projects when the tab actually closes
// Shut down all projects when the tab actually closes.
// IMPORTANT: pagehide fires both on real page unload AND on mobile/Safari
// tab switches (bfcache entry). When event.persisted is true the page is
// being cached for later reuse — the server must stay alive. Only send
// the shutdown beacon when the page is truly being discarded.
useEffect(() => {
const handlePageHide = () => {
const handlePageHide = (event: PageTransitionEvent) => {
if (event.persisted) {
// Page is entering bfcache (tab switch, app backgrounding) — keep
// the server alive so PTY sessions survive.
return
}
// sendBeacon cannot set custom headers, so pass the auth token as a
// query parameter instead (the proxy accepts `_token` as a fallback).
const token = getAuthToken()

View file

@ -465,6 +465,23 @@ async function uploadAndInjectImage(file: File, sessionId: string, projectCwd?:
// ─── Multi-instance terminal panel ────────────────────────────────────────────
/**
* Derive a session ID that is scoped to the project path. This ensures
* that switching projects creates a separate PTY session per project, and
* switching back reconnects to the *same* server-side PTY instead of
* spawning a new one (the server's getOrCreateSession returns the existing
* live session when the ID matches).
*/
function deriveProjectScopedSessionId(
projectCwd: string | undefined,
sessionPrefix?: string,
command?: string,
): string {
const base = sessionPrefix ?? (command ? "gsd-default" : "default")
if (!projectCwd) return base
return `${base}:${projectCwd}`
}
export function ShellTerminal({
className,
command,
@ -477,7 +494,7 @@ export function ShellTerminal({
}: ShellTerminalProps) {
const { resolvedTheme } = useTheme()
const isDark = resolvedTheme !== "light"
const defaultId = sessionPrefix ?? (command ? "gsd-default" : "default")
const defaultId = deriveProjectScopedSessionId(projectCwd, sessionPrefix, command)
const commandLabel = deriveCommandLabel(command)
const [tabs, setTabs] = useState<TerminalTab[]>([
{ id: defaultId, label: commandLabel, connected: false },
@ -486,6 +503,19 @@ export function ShellTerminal({
const [isDragOver, setIsDragOver] = useState(false)
const terminalAreaRef = useRef<HTMLDivElement>(null)
// When the project changes, the defaultId changes. Reset tabs so the
// terminal reconnects to the project-scoped PTY session on the server.
// The server's getOrCreateSession will return the existing live session
// when the session ID matches, preserving terminal state.
const prevDefaultIdRef = useRef(defaultId)
useEffect(() => {
if (prevDefaultIdRef.current !== defaultId) {
prevDefaultIdRef.current = defaultId
setTabs([{ id: defaultId, label: commandLabel, connected: false }])
setActiveTabId(defaultId)
}
}, [defaultId, commandLabel])
// ── Drag-and-drop handlers (native DOM, capture phase) ──────────────────
// React synthetic events don't reliably fire through xterm's internal DOM.
// Native capture-phase listeners intercept before xterm can swallow them —