mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-31 18:30:39 +02:00
fix(test): kill the four worst fixed sleeps (300s/30s/30s/20s)
- watchdog.test: the 20s blind wait for one production parent-watchdog tick becomes BROWSE_PARENT_WATCHDOG_INTERVAL_MS=250 (new env knob in server.ts, NaN-safe, production default unchanged) + polls for the boot line and the tick's stay-alive log — strictly stronger (the old form never proved a tick observed the parent death). 24s → 3.6s. - stop-dead-daemon / terminal-agent-owner-watchdog: the 300s/30s stand-in child lifetimes become stdin-EOF-bound — the child can never self-exit mid-test on a slow runner (spurious-failure class) and self-reaps instantly if the test dies (no 300s orphans). Node-compat stdin APIs (owner-watchdog runs on the Windows lane). - browser-skill-commands: the sleeper fixture's 30s self-time becomes 8s (no stdin pipe exists in runToFiles) — far above the 1s product timeout it must outlive, below the test ceiling, so a timeout-kill regression fails on clean assertions instead of an opaque bun timeout; added: stdout must NOT contain 'done'. 45/45 green across the four files + server tripwires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
73fe950fbf
commit
5c081a355a
+10
-1
@@ -812,8 +812,17 @@ function parentWatchdogTick(parentPid: number = BROWSE_PARENT_PID): void {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Poll cadence. Env-overridable as a test seam: watchdog.test.ts shrinks it
|
||||||
|
// (250ms) so a free-tier test can observe a real tick deciding on a dead
|
||||||
|
// parent instead of sleeping through the 15s production cadence. Production
|
||||||
|
// launchers never set this; unparsable or non-positive values fall back to 15s.
|
||||||
|
const rawWatchdogIntervalMs = parseInt(process.env.BROWSE_PARENT_WATCHDOG_INTERVAL_MS || '', 10);
|
||||||
|
const PARENT_WATCHDOG_INTERVAL_MS =
|
||||||
|
Number.isFinite(rawWatchdogIntervalMs) && rawWatchdogIntervalMs > 0
|
||||||
|
? rawWatchdogIntervalMs
|
||||||
|
: 15_000;
|
||||||
if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) {
|
if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) {
|
||||||
setInterval(parentWatchdogTick, 15_000);
|
setInterval(parentWatchdogTick, PARENT_WATCHDOG_INTERVAL_MS);
|
||||||
} else if (IS_HEADED_WATCHDOG) {
|
} else if (IS_HEADED_WATCHDOG) {
|
||||||
console.log('[browse] Parent-process watchdog disabled (headed mode)');
|
console.log('[browse] Parent-process watchdog disabled (headed mode)');
|
||||||
} else if (BROWSE_PARENT_PID === 0) {
|
} else if (BROWSE_PARENT_PID === 0) {
|
||||||
|
|||||||
@@ -342,8 +342,15 @@ describe.skipIf(SKIP_SPAWN)('spawnSkill: lifecycle', () => {
|
|||||||
it('timeout fires, exit code 124, token revoked', async () => {
|
it('timeout fires, exit code 124, token revoked', async () => {
|
||||||
const dir = makeSkillDir(tiers.bundled, 'sleeper',
|
const dir = makeSkillDir(tiers.bundled, 'sleeper',
|
||||||
'name: sleeper\nhost: x.com\ntrusted: true',
|
'name: sleeper\nhost: x.com\ntrusted: true',
|
||||||
// Sleep longer than the test timeout; the spawn should kill us.
|
// The child's self-lifetime is a bound, not a wait — the test blocks
|
||||||
`await new Promise(r => setTimeout(r, 30000)); console.log("done");`,
|
// only for the 1s spawn timeout that kills it. 8s is sized to be far
|
||||||
|
// above that 1s (the kill always lands first) but below this test's
|
||||||
|
// 10s ceiling: if the timeout-kill ever regresses, the child completes,
|
||||||
|
// prints "done", and the assertions below fail cleanly in-budget
|
||||||
|
// instead of the test opaquely timing out while the child lingers.
|
||||||
|
// (runToFiles gives skill children no stdin pipe, so a parent-death
|
||||||
|
// EOF lifetime isn't available here — self-timing is required.)
|
||||||
|
`await new Promise(r => setTimeout(r, 8000)); console.log("done");`,
|
||||||
);
|
);
|
||||||
const skill = readBrowserSkill('sleeper', tiers)!;
|
const skill = readBrowserSkill('sleeper', tiers)!;
|
||||||
const result = await spawnSkill({
|
const result = await spawnSkill({
|
||||||
@@ -351,6 +358,9 @@ describe.skipIf(SKIP_SPAWN)('spawnSkill: lifecycle', () => {
|
|||||||
});
|
});
|
||||||
expect(result.timedOut).toBe(true);
|
expect(result.timedOut).toBe(true);
|
||||||
expect(result.exitCode).toBe(124);
|
expect(result.exitCode).toBe(124);
|
||||||
|
// The kill must land before the script completes — "done" ever appearing
|
||||||
|
// means the child outlived its timeout.
|
||||||
|
expect(result.stdout).not.toContain('done');
|
||||||
expect(listTokens().filter(t => t.clientId.startsWith('skill:sleeper:'))).toEqual([]);
|
expect(listTokens().filter(t => t.clientId.startsWith('skill:sleeper:'))).toEqual([]);
|
||||||
}, 10_000);
|
}, 10_000);
|
||||||
|
|
||||||
|
|||||||
@@ -109,7 +109,16 @@ describe('stop --force-restart on a LIVE daemon', () => {
|
|||||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-stop-force-'));
|
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-stop-force-'));
|
||||||
const stateFile = path.join(tmpDir, 'browse.json');
|
const stateFile = path.join(tmpDir, 'browse.json');
|
||||||
// Portable long-lived child standing in for the wedged daemon process.
|
// Portable long-lived child standing in for the wedged daemon process.
|
||||||
const wedged = spawn('bun', ['-e', 'await Bun.sleep(300000)'], { stdio: 'ignore' });
|
// Its lifetime is tied to this test process instead of a fixed sleep: it
|
||||||
|
// blocks until its stdin (a pipe we hold open) hits EOF. That means it
|
||||||
|
// can never self-exit mid-test — which would let the "pid is dead"
|
||||||
|
// assertion below pass without the CLI having killed anything — and it
|
||||||
|
// reaps itself the moment the test process dies, even on a hard kill
|
||||||
|
// where the finally block never runs.
|
||||||
|
const wedged = spawn('bun', ['-e',
|
||||||
|
"process.stdin.resume(); const bye = () => process.exit(0); "
|
||||||
|
+ "process.stdin.on('end', bye); process.stdin.on('error', bye); process.stdin.on('close', bye);",
|
||||||
|
], { stdio: ['pipe', 'ignore', 'ignore'] });
|
||||||
try {
|
try {
|
||||||
const port = await closedPort();
|
const port = await closedPort();
|
||||||
fs.writeFileSync(stateFile, JSON.stringify({
|
fs.writeFileSync(stateFile, JSON.stringify({
|
||||||
|
|||||||
@@ -44,9 +44,16 @@ describe('terminal-agent owner lifecycle', () => {
|
|||||||
// process.execPath (the running bun) instead of `sleep`: coreutils are
|
// process.execPath (the running bun) instead of `sleep`: coreutils are
|
||||||
// not guaranteed on a bare windows-latest runner, and this test is on the
|
// not guaranteed on a bare windows-latest runner, and this test is on the
|
||||||
// Windows CI curated list — the owner-orphan leak it pins is a Windows bug.
|
// Windows CI curated list — the owner-orphan leak it pins is a Windows bug.
|
||||||
|
// The owner's lifetime is tied to this test process instead of a fixed
|
||||||
|
// 30s sleep: it blocks until its stdin (a pipe we hold open) hits EOF, so
|
||||||
|
// it is guaranteed alive until the SIGTERM below no matter how slow the
|
||||||
|
// runner is, and it reaps itself if the test process dies without running
|
||||||
|
// afterEach. Node-compatible stdin APIs, not Bun.stdin — Windows-portable.
|
||||||
const owner = Bun.spawn(
|
const owner = Bun.spawn(
|
||||||
[process.execPath, '-e', 'await Bun.sleep(30000)'],
|
[process.execPath, '-e',
|
||||||
{ stdio: ['ignore', 'ignore', 'ignore'] },
|
"process.stdin.resume(); const bye = () => process.exit(0); "
|
||||||
|
+ "process.stdin.on('end', bye); process.stdin.on('error', bye); process.stdin.on('close', bye);"],
|
||||||
|
{ stdio: ['pipe', 'ignore', 'ignore'] },
|
||||||
);
|
);
|
||||||
spawned.push(owner);
|
spawned.push(owner);
|
||||||
const agent = Bun.spawn(['bun', 'run', AGENT_SCRIPT], {
|
const agent = Bun.spawn(['bun', 'run', AGENT_SCRIPT], {
|
||||||
|
|||||||
@@ -27,8 +27,10 @@ import { resolveConfig } from '../src/config';
|
|||||||
// seam as idleCheckTick) and tunnelActive is simulated via setTunnelActive.
|
// seam as idleCheckTick) and tunnelActive is simulated via setTunnelActive.
|
||||||
//
|
//
|
||||||
// Each test spawns the real server.ts. Tests 1 and 2 verify behavior via
|
// Each test spawns the real server.ts. Tests 1 and 2 verify behavior via
|
||||||
// stdout log line (fast). Test 3 waits for the watchdog poll cycle to confirm
|
// stdout log line (fast). Test 3 shrinks the poll cadence via
|
||||||
// the server REMAINS alive after parent death (slow — ~20s observation window).
|
// BROWSE_PARENT_WATCHDOG_INTERVAL_MS (test seam in server.ts), waits for the
|
||||||
|
// tick's one-time "parent exited (server stays alive" log to prove a tick
|
||||||
|
// observed the death, then confirms the server REMAINS alive.
|
||||||
|
|
||||||
const ROOT = path.resolve(import.meta.dir, '..');
|
const ROOT = path.resolve(import.meta.dir, '..');
|
||||||
const SERVER_SCRIPT = path.join(ROOT, 'src', 'server.ts');
|
const SERVER_SCRIPT = path.join(ROOT, 'src', 'server.ts');
|
||||||
@@ -139,21 +141,37 @@ describe('parent-process watchdog (v0.18.1.0)', () => {
|
|||||||
const parentPid = parentProc.pid!;
|
const parentPid = parentProc.pid!;
|
||||||
|
|
||||||
// Default headless: no BROWSE_HEADED, real parent PID — watchdog active.
|
// Default headless: no BROWSE_HEADED, real parent PID — watchdog active.
|
||||||
serverProc = spawnServer({ BROWSE_PARENT_PID: String(parentPid) }, 34903);
|
// The poll cadence is shrunk via the server's env seam (250ms instead of
|
||||||
|
// the production 15s) so observing a real tick doesn't cost a 20s sleep.
|
||||||
|
serverProc = spawnServer({
|
||||||
|
BROWSE_PARENT_PID: String(parentPid),
|
||||||
|
BROWSE_PARENT_WATCHDOG_INTERVAL_MS: '250',
|
||||||
|
}, 34903);
|
||||||
const serverPid = serverProc.pid!;
|
const serverPid = serverProc.pid!;
|
||||||
|
|
||||||
// Give the server a moment to start and register the watchdog interval.
|
// Startup barrier: poll stdout for the listen line instead of a fixed 2s
|
||||||
await Bun.sleep(2000);
|
// sleep. The watchdog interval is registered at module load, before this
|
||||||
|
// line prints, so once we see it the ticks are running.
|
||||||
|
const bootOut = await readStdoutUntil(serverProc, 'Server running on', 15_000);
|
||||||
|
expect(bootOut).toContain('Server running on');
|
||||||
expect(isProcessAlive(serverPid)).toBe(true);
|
expect(isProcessAlive(serverPid)).toBe(true);
|
||||||
|
|
||||||
// Kill the parent. The watchdog polls every 15s, so first tick after
|
// Kill the parent, then wait for a tick to OBSERVE the death: the
|
||||||
// parent death lands within ~15s. Pre-#994 the server would shutdown
|
// stay-alive branch logs a one-time latched line. Seeing it proves a tick
|
||||||
// here. Post-#994 the server logs the parent exit and stays alive.
|
// ran after parent death and chose NOT to shut down — pre-#994 the same
|
||||||
|
// tick called shutdown instead. (Await exited first so the PID is reaped
|
||||||
|
// and the tick's kill(pid, 0) probe sees ESRCH, not a zombie.)
|
||||||
parentProc.kill('SIGKILL');
|
parentProc.kill('SIGKILL');
|
||||||
|
await parentProc.exited;
|
||||||
|
const marker = `Parent process ${parentPid} exited (server stays alive`;
|
||||||
|
const out = await readStdoutUntil(serverProc, marker, 15_000);
|
||||||
|
expect(out).toContain(marker);
|
||||||
|
expect(out).not.toContain('shutting down');
|
||||||
|
|
||||||
// Wait long enough for at least one watchdog tick (15s) plus margin.
|
// Let several more ticks land (4+ at 250ms — the old fixed 20s sleep
|
||||||
// Server should still be alive — that's the whole point of #994.
|
// covered ~1 production tick) and confirm the server is still alive —
|
||||||
await Bun.sleep(20_000);
|
// that's the whole point of #994.
|
||||||
|
await Bun.sleep(1_000);
|
||||||
expect(isProcessAlive(serverPid)).toBe(true);
|
expect(isProcessAlive(serverPid)).toBe(true);
|
||||||
}, 45_000);
|
}, 45_000);
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user