fix(terminal-agent): tie agent lifetime to its owning browse server PID

The terminal agent is intentionally detached so it survives the
short-lived CLI launcher, but its real owner is the persistent browse
server. If that server crashed or was killed before running normal
shutdown, the agent was adopted by PID 1 and lived forever (#2019).

spawnTerminalAgent now requires an ownerPid and exports it to the agent
as BROWSE_OWNER_PID; all three spawn sites pass the server PID (cli.ts
cold-start, cli.ts supervisor respawn, server.ts watchdog). The agent
polls the owner with signal 0 every 15s (GSTACK_TERMINAL_OWNER_WATCHDOG_MS
to tune) on an unref'd timer and, when the owner disappears, exits
through the SAME cleanup path as an intentional SIGTERM shutdown — now
re-entrancy-guarded and also removing the terminal-internal-token file
alongside the port file and agent record.

Runtime test spawns a real agent tied to a throwaway owner process,
kills the owner, and asserts the agent exits and its discovery files
(terminal-agent-pid, terminal-port) are gone.

Reconciled with the watchdog commit's spawnTerminalAgent contract test
(process-liveness-windows.test.ts now passes ownerPid and pins the
BROWSE_OWNER_PID env forwarding).

Closes #2019.

Contributed by @csarigoz (PR #2530).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-08-14 19:11:39 -07:00
parent bbaf5068b0
commit 5f6266e012
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
6 changed files with 104 additions and 2 deletions

View File

@ -1130,6 +1130,7 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
const newPid = spawnTerminalAgent({
stateFile: config.stateFile,
serverPort: newState.port,
ownerPid: newState.pid,
cwd: config.projectDir,
});
if (newPid) {
@ -1222,6 +1223,7 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
spawnTerminalAgent({
stateFile: config.stateFile,
serverPort: respawned.port,
ownerPid: respawned.pid,
cwd: config.projectDir,
});
} catch (err: any) {

View File

@ -1573,6 +1573,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
const pid = spawnTerminalAgent({
stateFile: cfg.config.stateFile,
serverPort: cfg.browsePort,
ownerPid: process.pid,
cwd: cfg.config.projectDir,
});
if (pid) {

View File

@ -48,12 +48,13 @@ export function resolveTerminalAgentScript(searchHints: { metaDir?: string; exec
*
* 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.
* spawn-env additions (BROWSE_OWNER_PID being the first) land in one place.
*/
export function spawnTerminalAgent(opts: {
stateFile: string;
serverPort: number;
/** PID of the browse server that owns this agent. */
ownerPid: number;
cwd?: string;
/** Optional extra env vars to add to the agent's process env. */
extraEnv?: Record<string, string>;
@ -74,6 +75,7 @@ export function spawnTerminalAgent(opts: {
...process.env,
BROWSE_STATE_FILE: opts.stateFile,
BROWSE_SERVER_PORT: String(opts.serverPort),
BROWSE_OWNER_PID: String(opts.ownerPid),
...(opts.extraEnv || {}),
},
stdio: ['ignore', 'ignore', 'ignore'],

View File

@ -30,6 +30,11 @@ import { writeAgentRecord, clearAgentRecord } from './terminal-agent-control';
const STATE_FILE = process.env.BROWSE_STATE_FILE || path.join(process.env.HOME || '/tmp', '.gstack', 'browse.json');
const PORT_FILE = path.join(path.dirname(STATE_FILE), 'terminal-port');
const BROWSE_SERVER_PORT = parseInt(process.env.BROWSE_SERVER_PORT || '0', 10);
const BROWSE_OWNER_PID = parseInt(process.env.BROWSE_OWNER_PID || '0', 10);
const OWNER_WATCHDOG_MS = parseInt(
process.env.GSTACK_TERMINAL_OWNER_WATCHDOG_MS || '15000',
10,
);
const EXTENSION_ID = process.env.BROWSE_EXTENSION_ID || ''; // optional: tighten Origin check
const INTERNAL_TOKEN = crypto.randomBytes(32).toString('base64url'); // shared with parent server via env at spawn
/**
@ -987,13 +992,33 @@ function main() {
console.log(`[terminal-agent] listening on 127.0.0.1:${port} pid=${process.pid} gen=${CURRENT_GEN}`);
// Cleanup port file + agent record on exit.
let cleaningUp = false;
const cleanup = () => {
if (cleaningUp) return;
cleaningUp = true;
safeUnlink(PORT_FILE);
safeUnlink(INTERNAL_TOKEN_FILE);
clearAgentRecord(dir);
process.exit(0);
};
process.on('SIGTERM', cleanup);
process.on('SIGINT', cleanup);
// The terminal agent is intentionally detached so it survives the short-lived
// CLI launcher, but its real owner is the persistent browse server. If that
// server crashes or is killed before running normal shutdown, the agent would
// otherwise be adopted by PID 1 and live forever. Poll the server PID and use
// the same cleanup path as an intentional shutdown when it disappears.
if (BROWSE_OWNER_PID > 0) {
const ownerWatchdog = setInterval(() => {
try {
process.kill(BROWSE_OWNER_PID, 0);
} catch {
cleanup();
}
}, OWNER_WATCHDOG_MS);
(ownerWatchdog as any)?.unref?.();
}
}
// Export the internal token so cli.ts can pass the SAME value to the parent

View File

@ -100,12 +100,16 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
const pid = spawnTerminalAgent({
stateFile: path.join(tmpDir, 'state.json'),
serverPort: 12345,
ownerPid: process.pid,
cwd: tmpDir,
scriptPath: script,
});
expect(pid).toBe(4242);
expect(captured).not.toBeNull();
expect(captured.windowsHide).toBe(true);
// Owner-PID lifetime tie (#2019): the agent polls this and exits when
// its owning browse server dies, so it can't be adopted by PID 1.
expect(captured.env.BROWSE_OWNER_PID).toBe(String(process.pid));
// Detached background daemon — must not inherit a terminal either.
expect(captured.stdio).toEqual(['ignore', 'ignore', 'ignore']);
} finally {

View File

@ -0,0 +1,68 @@
import { afterEach, describe, expect, test } from 'bun:test';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
const AGENT_SCRIPT = path.join(import.meta.dir, '../src/terminal-agent.ts');
const spawned: any[] = [];
const tempDirs: string[] = [];
function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
async function waitFor(predicate: () => boolean, timeoutMs = 5_000): Promise<boolean> {
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
if (predicate()) return true;
await Bun.sleep(25);
}
return predicate();
}
afterEach(() => {
for (const proc of spawned.splice(0)) {
try { proc.kill?.('SIGKILL'); } catch {}
}
for (const dir of tempDirs.splice(0)) {
try { fs.rmSync(dir, { recursive: true, force: true }); } catch {}
}
});
describe('terminal-agent owner lifecycle', () => {
test('exits after its owning browse server process exits', async () => {
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-term-owner-'));
tempDirs.push(stateDir);
const stateFile = path.join(stateDir, 'browse.json');
fs.writeFileSync(stateFile, JSON.stringify({ token: 'test-token' }));
const owner = Bun.spawn(['sleep', '30'], { stdio: ['ignore', 'ignore', 'ignore'] });
spawned.push(owner);
const agent = Bun.spawn(['bun', 'run', AGENT_SCRIPT], {
env: {
...process.env,
BROWSE_STATE_FILE: stateFile,
BROWSE_SERVER_PORT: '0',
BROWSE_OWNER_PID: String(owner.pid),
GSTACK_TERMINAL_OWNER_WATCHDOG_MS: '25',
},
stdio: ['ignore', 'ignore', 'ignore'],
});
spawned.push(agent);
expect(await waitFor(() => fs.existsSync(path.join(stateDir, 'terminal-agent-pid')))).toBe(true);
expect(isAlive(agent.pid)).toBe(true);
owner.kill('SIGTERM');
await owner.exited;
expect(await waitFor(() => !isAlive(agent.pid))).toBe(true);
expect(fs.existsSync(path.join(stateDir, 'terminal-agent-pid'))).toBe(false);
expect(fs.existsSync(path.join(stateDir, 'terminal-port'))).toBe(false);
});
});