diff --git a/test/integration/daemon-leak-oracle-cli.test.ts b/test/integration/daemon-leak-oracle-cli.test.ts new file mode 100644 index 000000000..6a7810569 --- /dev/null +++ b/test/integration/daemon-leak-oracle-cli.test.ts @@ -0,0 +1,61 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runCmdSync } from '../../src/utils/exec.ts'; + +// #1781 B1: the oracle's `after-close` checkpoint is only meaningful when it can +// name the sessions that closed — without them it would accept every unfinalized +// capture handle and report clean, which is the vacuous mode the type system now +// refuses in code. The standalone CLI is the one caller that builds options from +// strings rather than types, so it must refuse the same invocation at runtime +// instead of silently degrading to that mode. + +const ORACLE_PATH = 'test/integration/support/daemon-leak-oracle.ts'; + +function runOracle(args: string[]) { + return runCmdSync(process.execPath, ['--experimental-strip-types', ORACLE_PATH, ...args], { + allowFailure: true, + timeoutMs: 60_000, + }); +} + +test('the leak oracle CLI refuses an after-close checkpoint that names no session', (t) => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-leak-oracle-cli-')); + // A closed session that left an unfinalized capture handle: the exact residue + // the refused invocation would have reported clean. + const sessionDir = path.join(stateDir, 'sessions', 'closed-one'); + fs.mkdirSync(sessionDir, { recursive: true }); + fs.writeFileSync( + path.join(sessionDir, 'screen-recording.resource.json'), + `${JSON.stringify({ lifecycle: 'open' })}\n`, + ); + t.after(() => fs.rmSync(stateDir, { recursive: true, force: true })); + + const refused = runOracle(['--state-dir', stateDir, '--phase', 'after-close']); + assert.notEqual(refused.exitCode, 0, `expected a refusal, got:\n${refused.stdout}`); + assert.match(refused.stderr, /--phase after-close requires at least one --closed-session/); + // A refusal, not a leak report: the checkpoint never ran. + assert.doesNotMatch(refused.stdout, /daemon leak oracle:/); + + // The same invocation, once it names the session, runs and finds the handle. + const named = runOracle([ + '--state-dir', + stateDir, + '--phase', + 'after-close', + '--closed-session', + 'closed-one', + '--settle-ms', + '0', + ]); + assert.equal(named.exitCode, 1, `expected a leak report, got:\n${named.stdout}${named.stderr}`); + assert.match(named.stdout, /LEAK \(after-close\)/); + assert.match(named.stdout, /sessions\/closed-one\/screen-recording\.resource\.json/); + + // `after-shutdown` needs no session identity and is unaffected by the guard. + const shutdown = runOracle(['--state-dir', stateDir, '--settle-ms', '0']); + assert.equal(shutdown.exitCode, 1, shutdown.stderr); + assert.match(shutdown.stdout, /LEAK \(after-shutdown\)/); +}); diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index 7333068e4..b3009a408 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -6,6 +6,7 @@ import path from 'node:path'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; import { stopProcessForTakeover } from '../../src/daemon/daemon-process.ts'; import { isProcessAlive } from '../../src/utils/host-process.ts'; +import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; // #1596: a CLI command that finds its recorded daemon unreachable replaces it @@ -28,6 +29,7 @@ test('daemon replace mid-command returns a structured, parseable error and exits const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replace-exit-flush-')); let info: DaemonInfo | null = null; + const daemonPids: number[] = []; try { // A real daemon, started by this codebase, so its recorded version/code // signature legitimately match — the only way to reach the "unreachable" @@ -36,6 +38,7 @@ test('daemon replace mid-command returns a structured, parseable error and exits assert.equal(started.status, 0, `${started.stderr}\n${started.stdout}`); info = readDaemonInfo(stateDir); + daemonPids.push(info.pid); assert.equal(isProcessAlive(info.pid), true, 'expected the started daemon to be alive'); // Kill it out from under its own metadata: daemon.json stays put and @@ -69,6 +72,19 @@ test('daemon replace mid-command returns a structured, parseable error and exits ); info = readDaemonInfo(stateDir); + daemonPids.push(info.pid); + await stopProcessForTakeover(info.pid, { + termTimeoutMs: 1_500, + killTimeoutMs: 1_500, + expectedStartTime: info.processStartTime, + }); + // #1781 B1: neither the SIGKILLed daemon nor its replacement may leave owned + // processes or unclassified state-dir residue once both are gone. `info` + // stays set until this passes: `stopProcessForTakeover` is best-effort, so a + // failed stop must still reach the `finally` retry below rather than have + // the state dir removed out from under a daemon that is still running. + await assertNoDaemonLeaks({ stateDir, daemonPids, phase: 'after-shutdown' }); + info = null; } finally { if (info) { await stopProcessForTakeover(info.pid, { diff --git a/test/integration/provider-scenarios/session-close-leak-oracle.test.ts b/test/integration/provider-scenarios/session-close-leak-oracle.test.ts new file mode 100644 index 000000000..f9ebe0c19 --- /dev/null +++ b/test/integration/provider-scenarios/session-close-leak-oracle.test.ts @@ -0,0 +1,76 @@ +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import { assertRpcOk } from './assertions.ts'; +import { + createAndroidRecordingScenarioHarness, + withAndroidRecordingScenario, +} from './android-recording-fixtures.ts'; +import { createAndroidRecordingProvider } from './android-recording-provider-fixtures.ts'; +import { PROVIDER_SCENARIO_ANDROID } from './fixtures.ts'; +import { PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS } from './test-timeouts.ts'; +import { screenRecordingResourceStore } from '../../../src/daemon/screen-recording-resource-store.ts'; +import { assertNoDaemonLeaks } from '../support/daemon-leak-oracle.ts'; + +// The `after-close` half of the #1781 B1 leak oracle, on a real daemon route. +// The three CLI daemon lanes only ever shut a daemon down, so this is the one +// place a closing session actually owns a resource: `record start` opens a +// record-only session with a live screen-recording handle, and the session-close +// teardown that #1325 added must finalize it. Removing that teardown step leaves +// the descriptor `lifecycle: "open"` and turns this test red. +test( + 'Provider-backed integration closing a recording session leaves no unfinalized capture handle', + async () => { + await withAndroidRecordingScenario( + 'agent-device-provider-scenario-close-leak-', + async (tmpDir) => { + const calls: string[][] = []; + const outputPath = path.join(tmpDir, 'close-leak.mp4'); + const daemon = await createAndroidRecordingScenarioHarness({ + androidAdbProvider: () => createAndroidRecordingProvider({ calls }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + try { + const started = await daemon.callCommand('record', ['start', outputPath], { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + recordingScope: 'device', + }); + assert.equal(assertRpcOk<{ recording?: unknown }>(started).recording, 'started'); + + const sessionDir = daemon.sessionDir('default'); + // The live handle is durable state: the oracle must be able to see the + // unfinalized descriptor it will later refuse. + assert.equal( + screenRecordingResourceStore.read(screenRecordingResourceStore.resolvePath(sessionDir)) + .status, + 'decoded', + ); + + await daemon.callCommand('close', [], { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + }); + + // `daemonPids: []` on purpose: this harness runs the daemon route + // in-process, so there is no daemon process to own children and the + // state-dir arm is the whole check here. The process arm is exercised by + // the CLI lanes and pinned by daemon-leak-model.test.ts. + await expect( + assertNoDaemonLeaks({ + stateDir: path.dirname(sessionDir), + sessionsDir: path.dirname(sessionDir), + daemonPids: [], + phase: 'after-close', + closedSessions: [path.basename(sessionDir)], + settleMs: 0, + }), + ).resolves.toBeUndefined(); + } finally { + await daemon.close(); + } + }, + ); + }, + PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS, +); diff --git a/test/integration/smoke-daemon-clean.test.ts b/test/integration/smoke-daemon-clean.test.ts index 6da6a55a7..2d39ff29b 100644 --- a/test/integration/smoke-daemon-clean.test.ts +++ b/test/integration/smoke-daemon-clean.test.ts @@ -7,6 +7,7 @@ import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loop import { runCmdSync } from '../../src/utils/exec.ts'; import { stopProcessForTakeover } from '../../src/daemon/daemon-process.ts'; import { isProcessAlive } from '../../src/utils/host-process.ts'; +import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; type DaemonInfo = { @@ -41,6 +42,9 @@ test('clean daemon script stops a live daemon before removing metadata', async ( assert.equal(isProcessAlive(info.pid), false); assert.equal(fs.existsSync(path.join(stateDir, 'daemon.json')), false); assert.equal(fs.existsSync(path.join(stateDir, 'daemon.lock')), false); + // #1781 B1: the stopped daemon must take every process it owned with it and + // leave only classified artifacts in its state dir. + await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { if (info) { await stopProcessForTakeover(info.pid, { diff --git a/test/integration/smoke-daemon-http.test.ts b/test/integration/smoke-daemon-http.test.ts index 3e03998e8..53a7e941c 100644 --- a/test/integration/smoke-daemon-http.test.ts +++ b/test/integration/smoke-daemon-http.test.ts @@ -7,6 +7,7 @@ import { DAEMON_RPC_PROTOCOL_VERSION } from '../../src/daemon/http-health.ts'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; import { stopProcessForTakeover } from '../../src/daemon/daemon-process.ts'; import { formatResultDebug } from './cli-json.ts'; +import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; type DaemonInfo = { @@ -65,8 +66,17 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t const unauthorized = await callCommandRpc({ ...info, token: 'wrong-token' }, 'session_list'); assert.equal(unauthorized.status, 401); assert.equal(unauthorized.body.error?.data?.code, 'UNAUTHORIZED'); + // #1781 B1: the HTTP-mode daemon must exit — leaving nothing it owns and no + // unclassified state-dir residue. Asserted on the success path so the + // oracle's settle window can never replace a primary assertion's + // diagnostic; the `finally` below stays best-effort cleanup. + await stopDaemon(info); + await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { - await stopDaemonForStateDir(stateDir); + if (fs.existsSync(path.join(stateDir, 'daemon.json'))) { + await stopDaemon(readDaemonInfo(stateDir)); + } + fs.rmSync(stateDir, { recursive: true, force: true }); } }); @@ -101,18 +111,11 @@ async function callCommandRpc( }; } -async function stopDaemonForStateDir(stateDir: string): Promise { - try { - const infoPath = path.join(stateDir, 'daemon.json'); - if (!fs.existsSync(infoPath)) return; - const info = readDaemonInfo(stateDir); - if (!Number.isInteger(info.pid) || info.pid <= 0) return; - await stopProcessForTakeover(info.pid, { - termTimeoutMs: 1500, - killTimeoutMs: 1500, - expectedStartTime: info.processStartTime, - }); - } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); - } +async function stopDaemon(info: DaemonInfo): Promise { + if (!Number.isInteger(info.pid) || info.pid <= 0) return; + await stopProcessForTakeover(info.pid, { + termTimeoutMs: 1500, + killTimeoutMs: 1500, + expectedStartTime: info.processStartTime, + }); } diff --git a/test/integration/support/daemon-leak-model.test.ts b/test/integration/support/daemon-leak-model.test.ts new file mode 100644 index 000000000..c4c511ca2 --- /dev/null +++ b/test/integration/support/daemon-leak-model.test.ts @@ -0,0 +1,198 @@ +import { describe, expect, test } from 'vitest'; +import { + evaluateDaemonLeaks, + formatDaemonLeakReport, + hasDaemonLeaks, + type DaemonLeakObservation, + type DaemonLeakObservationBase, + type DaemonLeakPhaseSelection, + type NonEmpty, + type StateEntry, +} from './daemon-leak-model.ts'; + +const STATE_DIR = '/tmp/agent-device-lane-abc'; +const DAEMON_PID = 4340; + +// The phase and its identity arrive together: `after-close` cannot be requested +// without naming the sessions that closed, so these helpers cannot construct the +// vacuous checkpoint either. +function observe( + overrides: Partial = {}, + selection: DaemonLeakPhaseSelection = { phase: 'after-shutdown' }, +): DaemonLeakObservation { + return { + stateDir: STATE_DIR, + daemonPids: [DAEMON_PID], + livePids: [], + stateEntries: [], + ...overrides, + ...selection, + }; +} + +function observeAfterClose( + closedSessions: NonEmpty, + overrides: Partial = {}, +): DaemonLeakObservation { + return observe( + { livePids: [DAEMON_PID], ...overrides }, + { phase: 'after-close', closedSessions }, + ); +} + +function file(entryPath: string, descriptorLifecycle?: string): StateEntry { + return { path: entryPath, kind: 'file', ...(descriptorLifecycle ? { descriptorLifecycle } : {}) }; +} + +describe('surviving-daemon rule', () => { + // stopProcessForTakeover is best-effort void: it returns silently on identity + // mismatch, signal failure, or kill timeout, so a daemon can outlive the stop. + test('a daemon still alive after shutdown is itself a leak', () => { + const snapshot = evaluateDaemonLeaks( + observe({ livePids: [DAEMON_PID], stateEntries: [file('daemon.log')] }), + ); + + expect(snapshot.liveDaemonPids).toEqual([DAEMON_PID]); + expect(hasDaemonLeaks(snapshot)).toBe(true); + expect(formatDaemonLeakReport(snapshot)).toContain('daemons that outlived shutdown: 1'); + }); + + test('its metadata files stay stray rather than being excused by its own survival', () => { + const snapshot = evaluateDaemonLeaks( + observe({ livePids: [DAEMON_PID], stateEntries: [file('daemon.json'), file('daemon.lock')] }), + ); + + expect(snapshot.strayStateEntries).toEqual(['daemon.json', 'daemon.lock']); + }); + + test('a live daemon is expected while a session merely closed', () => { + const snapshot = evaluateDaemonLeaks( + observeAfterClose(['closed-one'], { + stateEntries: [file('daemon.json'), file('daemon.lock')], + }), + ); + + expect(hasDaemonLeaks(snapshot)).toBe(false); + }); +}); + +describe('state-dir residue rules', () => { + const AFTER_SHUTDOWN: DaemonLeakPhaseSelection = { phase: 'after-shutdown' }; + // A different session from the one this entry belongs to, so the row proves the + // phase rule rather than the closed-session rule. + const AFTER_CLOSE: DaemonLeakPhaseSelection = { + phase: 'after-close', + closedSessions: ['closed-one'], + }; + + test.each<[string, StateEntry, DaemonLeakPhaseSelection, 'expected' | 'stray']>([ + ['session event log', file('sessions/default/events.ndjson'), AFTER_SHUTDOWN, 'expected'], + ['request diagnostics', file('sessions/d/requests/abc.ndjson'), AFTER_SHUTDOWN, 'expected'], + ['shutdown report', file('daemon-shutdown.json'), AFTER_SHUTDOWN, 'expected'], + ['torn publish temporary', file('device-claims/a.json.55.tmp'), AFTER_SHUTDOWN, 'stray'], + [ + 'managed tool download', + file('tools/agent-browser/0.27.1/dl.tmp'), + AFTER_SHUTDOWN, + 'expected', + ], + [ + 'unswept artifact scaffold', + { path: 'sessions/d/artifacts/pending/', kind: 'empty-directory' }, + AFTER_SHUTDOWN, + 'stray', + ], + [ + 'unswept session scaffold', + { path: 'sessions/leaked/requests/', kind: 'empty-directory' }, + AFTER_SHUTDOWN, + 'stray', + ], + ['unknown artifact', file('sessions/d/mystery.bin'), AFTER_SHUTDOWN, 'stray'], + [ + 'open capture descriptor', + file('sessions/d/screen-recording.resource.json', 'open'), + AFTER_SHUTDOWN, + 'stray', + ], + [ + 'completed capture descriptor', + file('sessions/d/screen-recording.resource.json', 'completed'), + AFTER_SHUTDOWN, + 'expected', + ], + [ + "another session's open capture during close", + file('sessions/other/screen-recording.resource.json', 'open'), + AFTER_CLOSE, + 'expected', + ], + ['legacy app-log marker', file('sessions/d/app-log.pid'), AFTER_SHUTDOWN, 'stray'], + ])('%s is %s', (_name, entry, selection, verdict) => { + const snapshot = evaluateDaemonLeaks(observe({ stateEntries: [entry] }, selection)); + + expect(snapshot.strayStateEntries).toEqual(verdict === 'stray' ? [entry.path] : []); + }); + + // A managed install tree is third-party output the daemon neither writes nor + // owns, so its own temporaries must not be read as our torn publish. + test('the managed tools exemption does not leak into daemon-written paths', () => { + const snapshot = evaluateDaemonLeaks( + observe({ stateEntries: [file('sessions/d/tools/pending.tmp')] }), + ); + + expect(snapshot.strayStateEntries).toEqual(['sessions/d/tools/pending.tmp']); + }); +}); + +// The phase only means something if it can name the session that closed: without +// that, the closed session's unfinalized capture handle is indistinguishable +// from another session's legitimately live one, and `after-close` certifies +// nothing. Wired through by the session-close route regression in +// test/integration/provider-scenarios/session-close-leak-oracle.test.ts. +describe('closed-session capture handles', () => { + const closedSessionCapture = (lifecycle: string) => + file('sessions/closed-one/screen-recording.resource.json', lifecycle); + const afterClose = (stateEntries: StateEntry[]): DaemonLeakObservation => + observeAfterClose(['closed-one'], { stateEntries }); + + test('the closed session must have finalized its capture handle', () => { + const snapshot = evaluateDaemonLeaks(afterClose([closedSessionCapture('open')])); + + expect(snapshot.strayStateEntries).toEqual([ + 'sessions/closed-one/screen-recording.resource.json', + ]); + expect(hasDaemonLeaks(snapshot)).toBe(true); + }); + + test('a session that did not close may still hold a live capture handle', () => { + const snapshot = evaluateDaemonLeaks( + afterClose([file('sessions/still-open/screen-recording.resource.json', 'open')]), + ); + + expect(hasDaemonLeaks(snapshot)).toBe(false); + }); + + test('a finalized handle from the closed session is its finish record', () => { + const snapshot = evaluateDaemonLeaks(afterClose([closedSessionCapture('completed')])); + + expect(hasDaemonLeaks(snapshot)).toBe(false); + }); + + test('a legacy pid marker is never a finish record for the closed session', () => { + const snapshot = evaluateDaemonLeaks(afterClose([file('sessions/closed-one/app-log.pid')])); + + expect(snapshot.strayStateEntries).toEqual(['sessions/closed-one/app-log.pid']); + }); +}); + +test('a clean shutdown reports no leak', () => { + const snapshot = evaluateDaemonLeaks( + observe({ + stateEntries: [file('daemon.log'), file('sessions/default/events.ndjson')], + }), + ); + + expect(hasDaemonLeaks(snapshot)).toBe(false); + expect(formatDaemonLeakReport(snapshot)).toContain('daemon leak oracle: clean (after-shutdown)'); +}); diff --git a/test/integration/support/daemon-leak-model.ts b/test/integration/support/daemon-leak-model.ts new file mode 100644 index 000000000..7fdda099d --- /dev/null +++ b/test/integration/support/daemon-leak-model.ts @@ -0,0 +1,182 @@ +// Daemon leak rules (#1781 B1, feeds #1431): given one observation of an +// isolated state dir and the daemon pids a lane observed, decide whether daemon +// lifecycle or durable state leaked. Pure — `daemon-leak-oracle.ts` gathers the +// observation and asserts on it; `daemon-leak-model.test.ts` pins the rules. +// +// Two leak classes: +// +// surviving daemon at `after-shutdown` a daemon pid that is still alive IS +// the leak. `stopProcessForTakeover` is best-effort and +// returns silently on identity mismatch, signal failure, or +// kill timeout, so a lane that only stops the daemon never +// learns it survived. +// state-dir residue every entry must match EXPECTED_STATE_DIR_ENTRIES +// (unknown ⇒ classify the new artifact, do not widen the +// matcher): `*.tmp` write-then-publish temporaries are torn +// publishes, an empty directory is an unswept session +// scaffold, daemon.json/daemon.lock may exist only while a +// daemon legitimately lives, and a capture descriptor still +// `lifecycle: "open"` (or a legacy `app-log.pid` marker) +// after shutdown is a capture handle that outlived its +// owner — a `completed` descriptor is the finish record +// ADR 0019 keeps. `tools/` holds managed third-party +// installs (agent-browser and its Chrome), whose own +// download temporaries and scaffolding this daemon neither +// writes nor owns. +import { uniquePositivePids } from '../../../src/utils/host-process.ts'; + +export type DaemonLeakPhase = 'after-close' | 'after-shutdown'; + +/** One state-dir path, pre-read so the rules stay free of filesystem access. */ +export type StateEntry = { + /** Relative to the state dir, `/`-separated; directories keep a trailing `/`. */ + path: string; + kind: 'file' | 'empty-directory'; + /** `lifecycle` of a durable capture descriptor, when this entry is one. */ + descriptorLifecycle?: string; +}; + +export type NonEmpty = readonly [T, ...T[]]; + +/** + * The phase and the identity it needs, as one shape. An `after-close` + * checkpoint that cannot name the sessions that closed cannot tell their + * unfinalized capture handles from another session's legitimately live one, so + * it would accept every open handle and certify nothing. The identity is + * therefore part of the phase rather than an option a caller may omit, and the + * typechecker refuses the empty case. + */ +export type DaemonLeakPhaseSelection = + | { phase: 'after-shutdown' } + | { phase: 'after-close'; closedSessions: NonEmpty }; + +export type DaemonLeakObservationBase = { + stateDir: string; + daemonPids: readonly number[]; + livePids: readonly number[]; + stateEntries: readonly StateEntry[]; +}; + +export type DaemonLeakObservation = DaemonLeakPhaseSelection & DaemonLeakObservationBase; + +export type DaemonLeakSnapshot = { + stateDir: string; + daemonPids: number[]; + phase: DaemonLeakPhase; + liveDaemonPids: number[]; + strayStateEntries: string[]; +}; + +// `tools/` is a managed third-party install tree (agent-browser + Chrome), not +// daemon write-then-publish output, so it is exempted before the generic rules. +const MANAGED_TOOLS_ENTRY = /^tools\//; +const EXPECTED_STATE_DIR_ENTRIES: readonly RegExp[] = [ + /^daemon\.json$/, + /^daemon\.lock$/, + /^daemon\.log$/, + /^daemon-shutdown\.json$/, + /^sessions\/[^/]+\/events\.ndjson$/, + /^sessions\/[^/]+\/requests\/[^/]+\.ndjson$/, + /^sessions\/[^/]+\/(?:app|runner)\.log$/, + /^sessions\/[^/]+\/repair-tombstone\.json$/, + /^sessions\/[^/]+\/artifacts\/.+$/, + /^device-claims\/.+$/, +]; +const CAPTURE_DESCRIPTOR_ENTRY = /^sessions\/[^/]+\/[^/]+\.resource\.json$/; +const LEGACY_APP_LOG_MARKER_ENTRY = /^sessions\/[^/]+\/app-log\.pid$/; +const DAEMON_LIVENESS_ENTRY = /^daemon\.(?:json|lock)$/; + +export function evaluateDaemonLeaks(observation: DaemonLeakObservation): DaemonLeakSnapshot { + const daemonPids = uniquePositivePids(observation.daemonPids); + const liveDaemonPids = daemonPids.filter((pid) => observation.livePids.includes(pid)); + const daemonLegitimatelyAlive = observation.phase === 'after-close' && liveDaemonPids.length > 0; + return { + stateDir: observation.stateDir, + daemonPids, + phase: observation.phase, + liveDaemonPids, + strayStateEntries: observation.stateEntries + .filter( + (entry) => + classifyStateEntry(entry, { + phase: observation.phase, + daemonLegitimatelyAlive, + closedSessions: closedSessionsOf(observation), + }) === 'stray', + ) + .map((entry) => entry.path) + .sort(), + }; +} + +/** + * A daemon that outlived its own shutdown is itself the leak — not a detail of + * the report — so it fails alongside owned children and state-dir residue. + */ +export function hasDaemonLeaks(snapshot: DaemonLeakSnapshot): boolean { + return survivingDaemonPids(snapshot).length > 0 || snapshot.strayStateEntries.length > 0; +} + +function survivingDaemonPids(snapshot: DaemonLeakSnapshot): number[] { + return snapshot.phase === 'after-shutdown' ? snapshot.liveDaemonPids : []; +} + +type StateEntryContext = { + phase: DaemonLeakPhase; + daemonLegitimatelyAlive: boolean; + closedSessions: readonly string[]; +}; + +function classifyStateEntry(entry: StateEntry, context: StateEntryContext): 'expected' | 'stray' { + if (MANAGED_TOOLS_ENTRY.test(entry.path)) return 'expected'; + if (entry.kind === 'empty-directory') return 'stray'; + if (entry.path.endsWith('.tmp')) return 'stray'; + if (DAEMON_LIVENESS_ENTRY.test(entry.path)) { + return context.daemonLegitimatelyAlive ? 'expected' : 'stray'; + } + if (CAPTURE_DESCRIPTOR_ENTRY.test(entry.path) || LEGACY_APP_LOG_MARKER_ENTRY.test(entry.path)) { + return classifyCaptureEntry(entry, context); + } + return EXPECTED_STATE_DIR_ENTRIES.some((matcher) => matcher.test(entry.path)) + ? 'expected' + : 'stray'; +} + +// A capture handle must be finalized once its owning session is gone: after +// shutdown that is every session, after close only the sessions that closed. +// Another session's live capture stays expected, and a legacy pid marker is +// never a finish record, so it is stray whenever its session is gone. +function classifyCaptureEntry(entry: StateEntry, context: StateEntryContext): 'expected' | 'stray' { + const owningSessionGone = + context.phase === 'after-shutdown' || + context.closedSessions.includes(sessionDirectoryOf(entry.path) ?? ''); + if (!owningSessionGone) return 'expected'; + if (!CAPTURE_DESCRIPTOR_ENTRY.test(entry.path)) return 'stray'; + return entry.descriptorLifecycle === 'completed' ? 'expected' : 'stray'; +} + +function closedSessionsOf(observation: DaemonLeakObservation): readonly string[] { + return observation.phase === 'after-close' ? observation.closedSessions : []; +} + +function sessionDirectoryOf(entryPath: string): string | undefined { + return /^sessions\/([^/]+)\//.exec(entryPath)?.[1]; +} + +export function formatDaemonLeakReport(snapshot: DaemonLeakSnapshot): string { + const surviving = survivingDaemonPids(snapshot); + return [ + `daemon leak oracle: ${hasDaemonLeaks(snapshot) ? 'LEAK' : 'clean'} (${snapshot.phase})`, + ` state dir: ${snapshot.stateDir}`, + ` daemon pids: ${formatPids(snapshot.daemonPids)}; live: ${formatPids(snapshot.liveDaemonPids)}`, + ` daemons that outlived shutdown: ${surviving.length}${ + surviving.length > 0 ? ` (${formatPids(surviving)})` : '' + }`, + ` stray state-dir entries: ${snapshot.strayStateEntries.length}`, + ...snapshot.strayStateEntries.map((entry) => ` ${entry}`), + ].join('\n'); +} + +function formatPids(pids: readonly number[]): string { + return pids.join(', ') || '(none)'; +} diff --git a/test/integration/support/daemon-leak-oracle.ts b/test/integration/support/daemon-leak-oracle.ts new file mode 100644 index 000000000..75758b9f9 --- /dev/null +++ b/test/integration/support/daemon-leak-oracle.ts @@ -0,0 +1,182 @@ +// Daemon leak oracle (#1781 B1, feeds #1431): observes daemon liveness and an +// isolated state dir, then applies the lifecycle/residue rules in +// `daemon-leak-model.ts` (which documents them and is where they are tested). +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { isProcessAlive } from '../../../src/utils/host-process.ts'; +import { AppError } from '@agent-device/kernel/errors'; +import { + evaluateDaemonLeaks, + formatDaemonLeakReport, + hasDaemonLeaks, + type DaemonLeakPhaseSelection, + type DaemonLeakSnapshot, + type StateEntry, +} from './daemon-leak-model.ts'; + +const DEFAULT_SETTLE_MS = 5_000; +const SETTLE_POLL_MS = 250; + +/** + * `phase` carries the identity that phase needs: `after-close` cannot be + * requested without naming at least one closed session (see + * DaemonLeakPhaseSelection), so the vacuous checkpoint that accepts every open + * capture handle is not expressible. `after-shutdown` takes nothing extra. + * + * Closed sessions are directory names, not paths — the basename of + * `sessionStore.resolveSessionDir(name)`, or of the `sessionStateDir` an + * `open`/`close` response reports. + */ +export type DaemonLeakOracleOptions = DaemonLeakPhaseSelection & { + stateDir: string; + /** Every daemon pid the lane observed for this state dir (from daemon.json). */ + daemonPids: readonly number[]; + /** + * Where sessions live, when that is not `/sessions` — an in-process + * scenario harness roots its SessionStore directly at its own temp dir. + * Entries below it are normalized to the canonical `sessions//…` shape + * so the rules stay layout-independent. + */ + sessionsDir?: string; + /** How long stragglers may take to exit before they count as leaked. */ + settleMs?: number; +}; + +async function captureDaemonLeakSnapshot( + options: DaemonLeakOracleOptions, +): Promise { + const stateDir = path.resolve(options.stateDir); + return evaluateDaemonLeaks({ + ...phaseSelectionOf(options), + stateDir, + daemonPids: options.daemonPids, + livePids: options.daemonPids.filter((pid) => isProcessAlive(pid)), + stateEntries: readStateEntries(stateDir, options.sessionsDir), + }); +} + +/** + * Re-snapshots until clean or `settleMs` elapses. Stragglers that exit on their + * own inside the window are not leaks. + */ +async function settleDaemonLeakSnapshot( + options: DaemonLeakOracleOptions, +): Promise { + const deadline = Date.now() + (options.settleMs ?? DEFAULT_SETTLE_MS); + let snapshot = await captureDaemonLeakSnapshot(options); + while (hasDaemonLeaks(snapshot) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, SETTLE_POLL_MS)); + snapshot = await captureDaemonLeakSnapshot(options); + } + return snapshot; +} + +/** Throws with the full report if anything daemon-owned survives the settle window. */ +export async function assertNoDaemonLeaks(options: DaemonLeakOracleOptions): Promise { + const snapshot = await settleDaemonLeakSnapshot(options); + if (hasDaemonLeaks(snapshot)) throw new Error(formatDaemonLeakReport(snapshot)); +} + +function phaseSelectionOf(options: DaemonLeakOracleOptions): DaemonLeakPhaseSelection { + return options.phase === 'after-close' + ? { phase: 'after-close', closedSessions: options.closedSessions } + : { phase: 'after-shutdown' }; +} + +// Files, plus empty directories as their own entries (an unswept session +// scaffold leaves no file behind), plus the `lifecycle` of durable capture +// descriptors so the rules stay free of filesystem access. +function readStateEntries(stateDir: string, sessionsDir?: string): StateEntry[] { + if (!fs.existsSync(stateDir)) return []; + const sessionsRoot = path.resolve(sessionsDir ?? path.join(stateDir, 'sessions')); + const entries: StateEntry[] = []; + for (const dirent of fs.readdirSync(stateDir, { recursive: true, withFileTypes: true })) { + const absolute = path.join(dirent.parentPath, dirent.name); + const relative = toCanonicalEntryPath(absolute, stateDir, sessionsRoot); + if (!dirent.isDirectory()) { + entries.push({ + path: relative, + kind: 'file', + ...(relative.endsWith('.resource.json') + ? { descriptorLifecycle: readDescriptorLifecycle(absolute) } + : {}), + }); + } else if (isEmptyDirectory(absolute)) { + entries.push({ path: `${relative}/`, kind: 'empty-directory' }); + } + } + return entries; +} + +// `sessions//…` regardless of where the SessionStore is rooted. +function toCanonicalEntryPath(absolute: string, stateDir: string, sessionsRoot: string): string { + const withinSessions = path.relative(sessionsRoot, absolute); + const relative = + withinSessions.startsWith('..') || path.isAbsolute(withinSessions) + ? path.relative(stateDir, absolute) + : path.join('sessions', withinSessions); + return relative.split(path.sep).join('/'); +} + +function isEmptyDirectory(directoryPath: string): boolean { + try { + return fs.readdirSync(directoryPath).length === 0; + } catch { + return false; + } +} + +function readDescriptorLifecycle(filePath: string): string | undefined { + try { + const parsed = JSON.parse(fs.readFileSync(filePath, 'utf8')) as { lifecycle?: unknown }; + return typeof parsed.lifecycle === 'string' ? parsed.lifecycle : undefined; + } catch { + return undefined; + } +} + +/** + * Parses the standalone CLI's argv into oracle options, refusing the invocation + * the type system already refuses in code: `--phase after-close` without a + * `--closed-session` would accept every open capture handle and report clean. + */ +function parseDaemonLeakOracleArgs(argv: readonly string[]): DaemonLeakOracleOptions { + const readFlag = (name: string): string[] => + argv.flatMap((arg, index) => (arg === name && argv[index + 1] ? [argv[index + 1]!] : [])); + const stateDir = readFlag('--state-dir')[0]; + if (!stateDir) { + throw new AppError('INVALID_ARGS', 'daemon leak oracle: --state-dir is required'); + } + const common = { + stateDir, + daemonPids: readFlag('--daemon-pid').map(Number), + ...(readFlag('--sessions-dir')[0] ? { sessionsDir: readFlag('--sessions-dir')[0] } : {}), + settleMs: Number(readFlag('--settle-ms')[0] ?? DEFAULT_SETTLE_MS), + }; + const phase = readFlag('--phase')[0] ?? 'after-shutdown'; + if (phase !== 'after-close') return { ...common, phase: 'after-shutdown' }; + const [first, ...rest] = readFlag('--closed-session'); + if (!first) { + throw new AppError( + 'INVALID_ARGS', + 'daemon leak oracle: --phase after-close requires at least one --closed-session', + { + hint: 'Pass the closed session directory name (the basename of the session state dir). Without it the checkpoint would accept every unfinalized capture handle and report clean.', + }, + ); + } + return { ...common, phase: 'after-close', closedSessions: [first, ...rest] }; +} + +// Standalone use (red-proofs, manual triage): +// node --experimental-strip-types test/integration/support/daemon-leak-oracle.ts \ +// --state-dir --daemon-pid [--daemon-pid …] \ +// [--phase after-shutdown | --phase after-close --closed-session …] +// [--sessions-dir ] [--settle-ms ] +// Prints the report and exits 1 on a leak. +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + const snapshot = await settleDaemonLeakSnapshot(parseDaemonLeakOracleArgs(process.argv.slice(2))); + process.stdout.write(`${formatDaemonLeakReport(snapshot)}\n`); + process.exitCode = hasDaemonLeaks(snapshot) ? 1 : 0; +} diff --git a/vitest.config.ts b/vitest.config.ts index e15533161..1645984c5 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -89,6 +89,10 @@ export default defineConfig({ // #1781 A9: pins the root-doc paths-ignore entries directly against the // real workflow YAML, parse-only like its sibling above. 'test/ci/root-docs-paths-ignore.test.ts', + // The daemon leak oracle's lifecycle/residue rules (#1781 B1): pure + // decisions over fixture state-dir listings, so they need no daemon, + // device, or subprocess. + 'test/integration/support/daemon-leak-model.test.ts', // The frozen replay-compat corpus (#1417): parse-only, no device or // subprocess work, so it belongs in the fast lane next to the // grammar it guards.