mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-14 18:20:28 +02:00
feat(browse): terminal-agent watchdog with PID liveness + crash-loop guard
terminal-agent could die independently of the server — SIGKILL from the OS
OOM killer, an uncaught exception under PTY churn, an external `pkill` from
a sibling debugging session. Pre-v1.44 the sidebar would observe the broken
connection and stay broken until the user reloaded the sidebar. Now a 60s
ticker checks the recorded agent PID and respawns via the shared
spawnTerminalAgent helper when dead.
Identity-based liveness (T4 from the eng review):
* Uses readAgentRecord + isProcessAlive (signal 0 probe), not a name match.
* Slow-but-alive agents intentionally fall through — respawning around a
living agent would create split-brain (two agents writing the port
file, tokens diverging between them, mystery upgrade 401s).
* Pairs with the v1.44 generation counter in /internal/* loopback calls:
if a stale agent does come back to life mid-cycle, its X-Browse-Gen
no longer matches and the parent's calls 409 cleanly.
Crash-loop guard:
* 3 respawn attempts inside a rolling 60s window → stop trying. A daemon
up for a week with one crash a day shouldn't trip the guard.
* On trip: one-line error to console (`respawn guard tripped`) and the
watchdog goes dormant. Manual restart via the sidebar Restart button
is the explicit signal to re-arm (added in Commit 2 of the larger PR).
Shared spawn path (refactor):
* New spawnTerminalAgent(opts) in terminal-agent-control.ts handles:
prior-PID cleanup → spawn → record stash. Both the CLI cold-start path
in cli.ts and the new server.ts watchdog route through it. Removes the
copy-paste between them; future env wiring lands in one place.
Gated on cfg.ownsTerminalAgent — embedders that pre-launch their own PTY
server (gbrowser phoenix overlay) still own the full lifecycle.
GSTACK_AGENT_WATCHDOG_TICK_MS env knob compresses the 60s tick for e2e
tests without 60s waits per assertion.
Tests:
* browse/test/terminal-agent-watchdog.test.ts — 7 static-grep tripwires
for the load-bearing invariants (ownsTerminalAgent gate, PID-based
liveness, crash-loop guard with window pruning, shutdown cleanup,
CLI cold-start uses the same helper, env knob exists).
* Live process-kill tests belong in the e2e tier; cheaper invariants
here catch refactor regressions in ~1ms each.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
ad669b238a
commit
f42d7bac6d
@@ -19,6 +19,69 @@ import * as path from 'path';
|
||||
import { safeUnlink, safeKill, isProcessAlive } from './error-handling';
|
||||
import { writeSecureFile, mkdirSecure } from './file-permissions';
|
||||
|
||||
/**
|
||||
* Locate the terminal-agent script on disk. In dev (cli.ts running via
|
||||
* `bun run`), it lives next to this file in browse/src. In a compiled
|
||||
* binary, Bun's --compile bakes the source into the executable and
|
||||
* exposes it relative to process.execPath. Either path must work or
|
||||
* the agent can't be spawned at all.
|
||||
*/
|
||||
export function resolveTerminalAgentScript(searchHints: { metaDir?: string; execPath?: string } = {}): string | null {
|
||||
const meta = searchHints.metaDir || __dirname;
|
||||
const exec = searchHints.execPath || process.execPath;
|
||||
const candidates = [
|
||||
path.resolve(meta, 'terminal-agent.ts'),
|
||||
path.resolve(path.dirname(exec), '..', 'src', 'terminal-agent.ts'),
|
||||
];
|
||||
for (const c of candidates) {
|
||||
if (fs.existsSync(c)) return c;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a fresh terminal-agent as a detached child. Handles the standard
|
||||
* three steps: kill any prior agent recorded at `<stateDir>/terminal-agent-pid`,
|
||||
* clear the stale record, then `Bun.spawn(['bun', 'run', script], ...)` with
|
||||
* env wiring. Returns the PID of the new agent on success, null when the
|
||||
* agent script can't be located.
|
||||
*
|
||||
* Used by both the CLI cold-start path (cli.ts) and the v1.44 watchdog in
|
||||
* server.ts. Centralizing here removes a copy-paste between them and means
|
||||
* future spawn-env additions (e.g. BROWSE_OWNER_PID for the generation
|
||||
* counter rollout) land in one place.
|
||||
*/
|
||||
export function spawnTerminalAgent(opts: {
|
||||
stateFile: string;
|
||||
serverPort: number;
|
||||
cwd?: string;
|
||||
/** Optional extra env vars to add to the agent's process env. */
|
||||
extraEnv?: Record<string, string>;
|
||||
/** Override script lookup for tests. */
|
||||
scriptPath?: string;
|
||||
}): number | null {
|
||||
const stateDir = path.dirname(opts.stateFile);
|
||||
const prior = readAgentRecord(stateDir);
|
||||
if (prior) {
|
||||
killAgentByRecord(prior, 'SIGTERM');
|
||||
clearAgentRecord(stateDir);
|
||||
}
|
||||
const script = opts.scriptPath || resolveTerminalAgentScript();
|
||||
if (!script || !fs.existsSync(script)) return null;
|
||||
const proc = (Bun as any).spawn(['bun', 'run', script], {
|
||||
cwd: opts.cwd || process.cwd(),
|
||||
env: {
|
||||
...process.env,
|
||||
BROWSE_STATE_FILE: opts.stateFile,
|
||||
BROWSE_SERVER_PORT: String(opts.serverPort),
|
||||
...(opts.extraEnv || {}),
|
||||
},
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
});
|
||||
proc.unref?.();
|
||||
return proc.pid ?? null;
|
||||
}
|
||||
|
||||
export interface AgentRecord {
|
||||
pid: number;
|
||||
/** Random per-boot identifier. Loopback /internal/* sees X-Browse-Gen: <gen>. */
|
||||
|
||||
Reference in New Issue
Block a user