From ea3e1c3b0c70669ed5569305d7c667d3d0e0fd7f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 3 Nov 2026 23:03:50 +0000 Subject: [PATCH] test: keep the lifecycle tests stable on a slow runner and cut fixed waits - destroy --force before the host names the program: a held host never answers create, which gave up after about 5 s and sent the host SIGTERM. On a slow macOS runner that came before the program's destroy, which then read SESSION_ALREADY_DESTROYED or died with the program. The test now kills create once the program runs and holds the host's write until the destroy kills the host. - replay-hold.mjs: REPLAY_RELEASE= ends a hold early. The create give-up tests release the host's write once create has exited instead of after a fixed 10 s. - Tests wait for the crashed host to end instead of a fixed sleep, and the offline snapshot test waits for the program's output in the log. - The file's per-test time-out is 60 s: a runner three times slower than usual took 33 s for the stale host recovery test. --- .../scripts/replay-hold.mjs | 20 ++++- test/helpers.ts | 23 +++++- test/integration/lifecycle.test.ts | 79 +++++++++++++------ 3 files changed, 93 insertions(+), 29 deletions(-) diff --git a/.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs b/.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs index e829d878..38274dad 100644 --- a/.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs +++ b/.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs @@ -13,8 +13,9 @@ // hold the nth matching call (1 = the first) in that process // // The detached host inherits NODE_OPTIONS and REPLAY_HOLD from `create`. -// Set REPLAY_LOG= to log each hold. -import { appendFileSync } from 'node:fs'; +// Set REPLAY_LOG= to log each hold. Set REPLAY_RELEASE= to end +// every hold as soon as that file exists, so is only a cap. +import { appendFileSync, existsSync } from 'node:fs'; import fsp from 'node:fs/promises'; import { syncBuiltinESMExports } from 'node:module'; import process from 'node:process'; @@ -42,6 +43,17 @@ const holds = (process.env.REPLAY_HOLD ?? '') const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +async function holdFor(ms) { + const release = process.env.REPLAY_RELEASE; + const end = Date.now() + ms; + while (Date.now() < end) { + if (release !== undefined && existsSync(release)) { + return; + } + await sleep(Math.min(release === undefined ? ms : 50, end - Date.now())); + } +} + for (const call of new Set(holds.map((hold) => hold.call))) { const original = fsp[call]; const mine = holds.filter((hold) => hold.call === call); @@ -61,12 +73,12 @@ for (const call of new Set(holds.map((hold) => hold.call))) { } log(`hold ${call} ${path} for ${String(hold.ms)} ms`); if (HOLD_BEFORE.has(call)) { - await sleep(hold.ms); + await holdFor(hold.ms); log(`release ${call} ${path}`); return original(...args); } const result = await original(...args); - await sleep(hold.ms); + await holdFor(hold.ms); log(`release ${call} ${path}`); return result; }; diff --git a/test/helpers.ts b/test/helpers.ts index c0fb5749..49c3a685 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -247,7 +247,8 @@ export function destroySession(testHome: string, sessionId: string): void { }); } -export function crashSession(testHome: string, sessionId: string): void { +// Returns the pid of the host that it killed. +export function crashSession(testHome: string, sessionId: string): number { const session = inspectSession(testHome, sessionId); const hostPid = session.hostPid; expect(hostPid).toBeTypeOf('number'); @@ -262,6 +263,26 @@ export function crashSession(testHome: string, sessionId: string): void { } catch { // Process may already be dead } + return hostPid; +} + +// A killed process stays a zombie until its parent, init here, reaps it. +export async function processEnds(pid: number): Promise { + const deadline = Date.now() + 10_000; + for (;;) { + try { + process.kill(pid, 0); + } catch (error) { + if (hasErrorCode(error, 'ESRCH')) { + return true; + } + throw error; + } + if (Date.now() > deadline) { + return false; + } + await sleep(50); + } } export function inspectSession( diff --git a/test/integration/lifecycle.test.ts b/test/integration/lifecycle.test.ts index 39377cd6..d7ad5e77 100644 --- a/test/integration/lifecycle.test.ts +++ b/test/integration/lifecycle.test.ts @@ -39,6 +39,7 @@ import { inspectSession, isGroupAlive, killGroupIfAlive, + processEnds, readEvents, runCli, sleep, @@ -101,7 +102,9 @@ interface GcResult { let testHome = ''; -describe('lifecycle integration', { timeout: 30000 }, () => { +// Many tests spend 5 to 15 s in real waits and CLI starts, and a loaded +// runner takes up to three times as long. +describe('lifecycle integration', { timeout: 60_000 }, () => { beforeEach(async () => { // oxfmt-ignore testHome = await realpath(await mkdtemp(join(tmpdir(), 'agent-tty-home-'))); @@ -656,8 +659,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { const beforeCrash = inspectSession(testHome, sessionId); expect(beforeCrash.status).toBe('running'); - crashSession(testHome, sessionId); - await sleep(500); + expect(await processEnds(crashSession(testHome, sessionId))).toBe(true); const afterCrash = inspectSession(testHome, sessionId); expect(afterCrash.status).toBe('failed'); @@ -698,8 +700,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { 'session directory should exist on disk before stale-host reconciliation', ).resolves.toBeDefined(); - crashSession(testHome, sessionId); - await sleep(1000); + expect(await processEnds(crashSession(testHome, sessionId))).toBe(true); const listResult = runCli(['list', '--json'], { AGENT_TTY_HOME: testHome, @@ -1106,8 +1107,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { const sessionId = await createProgram( 'trap "" TERM HUP; sleep 300 & touch "$0"; wait', ); - crashSession(testHome, sessionId); - await sleep(500); + expect(await processEnds(crashSession(testHome, sessionId))).toBe(true); expect(isGroupAlive(pgid)).toBe(true); expect(inspectSession(testHome, sessionId).status).toBe('failed'); @@ -1667,6 +1667,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { // The held start-up write keeps the host from listening past create's // readiness loop, as a host that starts slowly on a loaded machine does. + // The hold ends once create has given up. // Without a session token, program.json names no group that a CLI can // prove, so only the host can stop the program. describe.each([ @@ -1689,6 +1690,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { it('lets its host stop the program and record destroyed', async () => { const holdLog = join(testHome, 'hold.log'); + const release = join(testHome, 'release'); const creator = spawn( process.execPath, [ @@ -1708,7 +1710,8 @@ describe('lifecycle integration', { timeout: 30000 }, () => { ...process.env, AGENT_TTY_HOME: testHome, REPLAY_LOG: holdLog, - REPLAY_HOLD: '_host:rename:/session.json:1:10000', + REPLAY_RELEASE: release, + REPLAY_HOLD: '_host:rename:/session.json:1:60000', NODE_OPTIONS: [ process.env.NODE_OPTIONS ?? '', `--import=${pathToFileURL(resolve('.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs')).href}`, @@ -1747,6 +1750,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { expect(isGroupAlive(pgid)).toBe(true); const [code] = (await creatorExit) as [number | null]; + await writeFile(release, ''); expect(code).toBe(6); expect(JSON.parse(stdout)).toMatchObject({ @@ -1754,7 +1758,7 @@ describe('lifecycle integration', { timeout: 30000 }, () => { error: { code: 'HOST_UNREACHABLE', details: { sessionId } }, }); // The host hangs the program up, then kills its group after the - // shutdown grace, and exits once its held write has landed. + // shutdown grace, and exits once its released write has landed. const stopDeadline = Date.now() + 20_000; while (isGroupAlive(pgid) || isProcessAlive(hostPid)) { expect(Date.now()).toBeLessThan(stopDeadline); @@ -1883,38 +1887,61 @@ describe('lifecycle integration', { timeout: 30000 }, () => { // The program can run destroy before the host's start-up write names its // pid. Holding that write keeps create's manifest, with no childPid, for // the whole destroy. The hold matches the host's argv only, so the inner - // CLI, which inherits it, runs at full speed. + // CLI, which inherits it, runs at full speed. A held host never answers + // create, which gives up after about 5 s and sends it SIGTERM. On a slow + // runner that came before the inner destroy, so create is killed once the + // program runs, and the destroy kills the held host. it('destroy --force before the host names the program prints its result', async () => { const out = join(testHome, 'early-destroy.json'); - runCli( + const started = `${out}.started`; + const create = spawn( + process.execPath, [ + '--import', + 'tsx', + './src/cli/main.ts', 'create', '--json', '--', '/bin/sh', '-c', - '"$@" destroy "$AGENT_TTY_SESSION_ID" --force --json > "$0"', + ': > "$0.started"; "$@" destroy "$AGENT_TTY_SESSION_ID" --force --json > "$0"', out, ...cli, ], { - AGENT_TTY_HOME: testHome, - NODE_OPTIONS: [ - process.env.NODE_OPTIONS ?? '', - `--import=${pathToFileURL(resolve('.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs')).href}`, - ].join(' '), - REPLAY_HOLD: '_host:rename:/session.json:1:4000', + cwd: process.cwd(), + env: { + ...process.env, + AGENT_TTY_HOME: testHome, + NODE_OPTIONS: [ + process.env.NODE_OPTIONS ?? '', + `--import=${pathToFileURL(resolve('.agents/skills/verify-session-lifecycle/scripts/replay-hold.mjs')).href}`, + ].join(' '), + REPLAY_HOLD: '_host:rename:/session.json:1:120000', + }, + stdio: 'ignore', }, ); + try { + const deadline = Date.now() + 20_000; + while (!existsSync(started)) { + expect(Date.now()).toBeLessThan(deadline); + await sleep(50); + } + } finally { + create.kill('SIGKILL'); + } const [sessionId] = await readdir(join(testHome, 'sessions')); expect(await readEnvelopeOnceWritten(out)).toMatchObject({ ok: true, result: { sessionId, destroyed: true }, }); - expect(inspectSession(testHome, sessionId as string).status).toBe( - 'destroyed', - ); + expect(inspectSession(testHome, sessionId as string)).toMatchObject({ + status: 'destroyed', + childPid: null, + }); }, 60_000); // Bash runs a typed command as a job in its own process group, so only @@ -2092,10 +2119,14 @@ describe('lifecycle integration', { timeout: 30000 }, () => { 'echo offline-test-data; exec cat', ]); - await sleep(1000); + const events = join(testHome, 'sessions', sessionId, 'events.jsonl'); + const deadline = Date.now() + 10_000; + while (!(await readFile(events, 'utf8')).includes('offline-test-data')) { + expect(Date.now()).toBeLessThan(deadline); + await sleep(50); + } - crashSession(testHome, sessionId); - await sleep(500); + expect(await processEnds(crashSession(testHome, sessionId))).toBe(true); const session = inspectSession(testHome, sessionId); expect(session.status).toBe('failed');