mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-13 16:38:56 +02:00
Three-bug chain behind the Windows terminal-agent leak (console window strobing every 60s, one orphaned agent per watchdog tick until the box ran out of committable memory): 1. isProcessAlive shelled out to `tasklist /FI "PID eq <pid>"` on Windows with a 3s timeout. A Bun.spawnSync that hits its timeout still RETURNS with partial stdout, so the `.includes()` PID match read a LIVE agent as dead — killAgentByRecord skipped the kill, the watchdog respawned around the survivor, and every orphan slowed the next tasklist enough to produce the next false negative. Now: `process.kill(pid, 0)` on every platform (Node and Bun both map signal 0 to an OpenProcess existence check on Windows), with EPERM counted as alive. No subprocess, no timeout, no console window. 2. The respawn circuit-breaker was mathematically unreachable — verified in this tree: RESPAWN_GUARD_WINDOW_MS was a fixed 60_000 against a 60_000ms default tick, and each tick pushes at most one respawn timestamp, so three pushes span ~120s and can never coexist inside a 60s window (eviction is strict `>`, and setInterval drift plus per-tick work always ages the prior entry past the boundary). The guard could not fire at the default tick rate and a steady one-per-tick leak ran unbounded. The window now scales with the tick: max(60_000, tick * (RESPAWN_GUARD_MAX + 2)), so "3 crashes in quick succession → stop" holds at any tick value. 3. The tasklist probe popped a visible console per tick (no windowsHide). Removing the shell-out kills that site; the agent-spawn site itself already passes windowsHide: true (landed with the bun-polyfill windowsHide commit — PR #2414's terminal-agent-control.ts hunk is reconciled there rather than duplicated). New browse/test/process-liveness-windows.test.ts pins all three: no subprocess from the probe, a static tripwire against reintroducing `tasklist` + `PID eq` liveness checks in src/, the spawnTerminalAgent windowsHide + stdio contract, and the window-derived-from-tick arithmetic. terminal-agent-watchdog.test.ts test 4 now pins the window/tick relationship instead of the fixed literal that let this ship. Also converts `new URL(import.meta.url).pathname` to `import.meta.path` across the static-grep tests it touches — the pathname form yields /C:/... on Windows and breaks path.resolve. Contributed by @SYKhayyat (PR #2414). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
89 lines
4.1 KiB
TypeScript
89 lines
4.1 KiB
TypeScript
import { describe, test, expect } from 'bun:test';
|
|
import * as fs from 'fs';
|
|
import * as path from 'path';
|
|
|
|
// v1.44 WS keepalive — static-grep invariants for the protocol contract.
|
|
//
|
|
// terminal-agent.ts and sidepanel-terminal.js cooperate on a 25s ping/pong +
|
|
// keepalive cycle so long-idle PTY connections survive NAT idle timeouts and
|
|
// Chromium's MV3 panel suspension heuristics. The wiring is invisible to
|
|
// integration tests (you'd have to wait 25s to observe a ping) but trivially
|
|
// regressed by a refactor. These tests fail CI if either side stops sending
|
|
// or stops accepting the protocol frames.
|
|
|
|
const AGENT_TS = path.resolve(import.meta.path, '..', '..', 'src', 'terminal-agent.ts');
|
|
const CLIENT_JS = path.resolve(import.meta.path, '..', '..', '..', 'extension', 'sidepanel-terminal.js');
|
|
|
|
describe('terminal-agent WS keepalive (v1.44+)', () => {
|
|
test('1. agent has a KEEPALIVE_INTERVAL_MS env knob, default 25000', () => {
|
|
const src = fs.readFileSync(AGENT_TS, 'utf-8');
|
|
expect(src).toContain('GSTACK_PTY_KEEPALIVE_INTERVAL_MS');
|
|
expect(src).toMatch(/KEEPALIVE_INTERVAL_MS\s*=\s*parseInt\(/);
|
|
// Default constant present so the env knob has a fallback.
|
|
expect(src).toContain("'25000'");
|
|
});
|
|
|
|
test('2. WS open handler starts a ping interval on the session', () => {
|
|
const src = fs.readFileSync(AGENT_TS, 'utf-8');
|
|
// The open(ws) handler in the websocket: { ... } block must call
|
|
// setInterval to drive the ping cadence and store the handle.
|
|
const wsBlock = sliceBetween(src, 'websocket: {', 'function handleTabState');
|
|
expect(wsBlock).toMatch(/open\s*\(\s*ws\s*\)/);
|
|
expect(wsBlock).toContain('setInterval');
|
|
expect(wsBlock).toContain("type: 'ping'");
|
|
expect(wsBlock).toContain('pingInterval');
|
|
});
|
|
|
|
test('3. WS close handler clears the ping interval', () => {
|
|
const src = fs.readFileSync(AGENT_TS, 'utf-8');
|
|
const wsBlock = sliceBetween(src, 'websocket: {', 'function handleTabState');
|
|
// close(ws, code?, reason?) MUST clearInterval the pingInterval —
|
|
// otherwise we leak timers across reconnects and the ping handler
|
|
// captures a dead ws ref. Signature widened in Commit 3 to include
|
|
// the close code for the detach state machine, hence the loose match.
|
|
expect(wsBlock).toMatch(/close\s*\(\s*ws/);
|
|
expect(wsBlock).toContain('clearInterval(session.pingInterval)');
|
|
});
|
|
|
|
test('4. message handler accepts pong / keepalive frames silently', () => {
|
|
const src = fs.readFileSync(AGENT_TS, 'utf-8');
|
|
// The text-frame router must recognize the keepalive vocabulary —
|
|
// if a future refactor strips this branch, unknown-text-frame
|
|
// suppression would still drop them but we lose intent.
|
|
expect(src).toMatch(/msg\?\.type === 'pong'/);
|
|
expect(src).toMatch(/msg\?\.type === 'keepalive'/);
|
|
});
|
|
|
|
test('5. client sends keepalive every 25s on ws.open', () => {
|
|
const src = fs.readFileSync(CLIENT_JS, 'utf-8');
|
|
expect(src).toContain('keepaliveInterval');
|
|
expect(src).toMatch(/setInterval\(/);
|
|
expect(src).toContain("type: 'keepalive'");
|
|
expect(src).toContain('KEEPALIVE_INTERVAL_MS = 25000');
|
|
});
|
|
|
|
test('6. client replies pong to server ping', () => {
|
|
const src = fs.readFileSync(CLIENT_JS, 'utf-8');
|
|
// The ws.message handler must short-circuit on msg.type === 'ping'
|
|
// and reply with {type: 'pong', ts: msg.ts}.
|
|
expect(src).toMatch(/msg\.type === 'ping'/);
|
|
expect(src).toMatch(/type: 'pong'/);
|
|
});
|
|
|
|
test('7. client clears keepalive in close + teardown + forceRestart', () => {
|
|
const src = fs.readFileSync(CLIENT_JS, 'utf-8');
|
|
// Three teardown paths exist; all three must drop the interval to
|
|
// avoid leaking timers across reconnect attempts.
|
|
const occurrences = (src.match(/clearInterval\(keepaliveInterval\)/g) || []).length;
|
|
expect(occurrences).toBeGreaterThanOrEqual(3);
|
|
});
|
|
});
|
|
|
|
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);
|
|
}
|