Skip to content

Commit f37f17d

Browse files
committed
test: fail the daemon leak oracle on a surviving daemon and pin its rules
Review of #1859 found the oracle reported "clean" when the daemon itself outlived shutdown: daemon pids were excluded from ownership, liveDaemonPids never reached hasDaemonLeaks, and a live daemon even flipped daemon.json/lock from stray to expected. stopProcessForTakeover is best-effort void, so only smoke-daemon-clean independently asserted death. - a live daemon pid at phase 'after-shutdown' is now itself a leak, and its metadata files stay stray; a live daemon remains legitimate at 'after-close' - split the pure ownership/residue rules into daemon-leak-model.ts and pin them with daemon-leak-model.test.ts, using the real ps rows captured during the #1109 and #1324 red-proofs (the lanes' daemons own no children, so the fixture test is what guards those shapes in CI) - exempt the managed tools/ install tree before the .tmp rule, so agent-browser's own download temporaries are no longer a false LEAK - report empty directories as residue (an unswept session scaffold leaves no file) - reuse src/utils/host-process.ts (expandProcessTree, uniquePositivePids) and its /bin/ps convention instead of re-deriving them - assert in smoke-daemon-http on the success path, not in finally, so the settle window cannot replace a primary assertion's diagnostic Refs #1781 #1431
1 parent b3d8a8d commit f37f17d

5 files changed

Lines changed: 582 additions & 244 deletions

File tree

test/integration/smoke-daemon-http.test.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,17 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t
6666
const unauthorized = await callCommandRpc({ ...info, token: 'wrong-token' }, 'session_list');
6767
assert.equal(unauthorized.status, 401);
6868
assert.equal(unauthorized.body.error?.data?.code, 'UNAUTHORIZED');
69+
// #1781 B1: the HTTP-mode daemon must exit — leaving nothing it owns and no
70+
// unclassified state-dir residue. Asserted on the success path so the
71+
// oracle's settle window can never replace a primary assertion's
72+
// diagnostic; the `finally` below stays best-effort cleanup.
73+
await stopDaemon(info);
74+
await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' });
6975
} finally {
70-
await stopDaemonForStateDir(stateDir);
76+
if (fs.existsSync(path.join(stateDir, 'daemon.json'))) {
77+
await stopDaemon(readDaemonInfo(stateDir));
78+
}
79+
fs.rmSync(stateDir, { recursive: true, force: true });
7180
}
7281
});
7382

@@ -102,21 +111,11 @@ async function callCommandRpc(
102111
};
103112
}
104113

105-
async function stopDaemonForStateDir(stateDir: string): Promise<void> {
106-
try {
107-
const infoPath = path.join(stateDir, 'daemon.json');
108-
if (!fs.existsSync(infoPath)) return;
109-
const info = readDaemonInfo(stateDir);
110-
if (!Number.isInteger(info.pid) || info.pid <= 0) return;
111-
await stopProcessForTakeover(info.pid, {
112-
termTimeoutMs: 1500,
113-
killTimeoutMs: 1500,
114-
expectedStartTime: info.processStartTime,
115-
});
116-
// #1781 B1: the HTTP-mode daemon must exit without owned processes or
117-
// unclassified state-dir residue.
118-
await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' });
119-
} finally {
120-
fs.rmSync(stateDir, { recursive: true, force: true });
121-
}
114+
async function stopDaemon(info: DaemonInfo): Promise<void> {
115+
if (!Number.isInteger(info.pid) || info.pid <= 0) return;
116+
await stopProcessForTakeover(info.pid, {
117+
termTimeoutMs: 1500,
118+
killTimeoutMs: 1500,
119+
expectedStartTime: info.processStartTime,
120+
});
122121
}
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
import { describe, expect, test } from 'vitest';
2+
import {
3+
evaluateDaemonLeaks,
4+
formatDaemonLeakReport,
5+
hasDaemonLeaks,
6+
type DaemonLeakObservation,
7+
type DaemonLeakPhase,
8+
type HostProcess,
9+
type StateEntry,
10+
} from './daemon-leak-model.ts';
11+
12+
// The lanes that call the oracle run device-free daemons, so the ownership rules
13+
// would otherwise only ever see an empty process set. These fixtures are the
14+
// real `ps` shapes captured during the #1109 and #1324 red-proofs (SHAs in the
15+
// #1781 B1 PR), so a regex or ordering edit that stops catching either leak
16+
// fails here instead of silently going quiet in CI.
17+
const STATE_DIR = '/tmp/agent-device-lane-abc';
18+
const DAEMON_PID = 4340;
19+
const OBSERVER_PID = 999;
20+
21+
function proc(overrides: Partial<HostProcess> & Pick<HostProcess, 'pid'>): HostProcess {
22+
return { ppid: 1, pgid: overrides.pid, command: 'unrelated', env: '', ...overrides };
23+
}
24+
25+
function observe(overrides: Partial<DaemonLeakObservation> = {}): DaemonLeakObservation {
26+
return {
27+
stateDir: STATE_DIR,
28+
daemonPids: [DAEMON_PID],
29+
livePids: [],
30+
phase: 'after-shutdown',
31+
processes: [],
32+
excludedPids: [OBSERVER_PID],
33+
stateEntries: [],
34+
...overrides,
35+
};
36+
}
37+
38+
function file(entryPath: string, descriptorLifecycle?: string): StateEntry {
39+
return { path: entryPath, kind: 'file', ...(descriptorLifecycle ? { descriptorLifecycle } : {}) };
40+
}
41+
42+
describe('owned-process rules', () => {
43+
// #1324: `simctl io … recordVideo` reparents to launchd (ppid 1) but keeps the
44+
// dead daemon's process group, and simctl only finalizes the mp4 on SIGINT.
45+
test('flags a recorder orphaned into the dead daemon process group', () => {
46+
const recorder = proc({
47+
pid: 52420,
48+
ppid: 1,
49+
pgid: DAEMON_PID,
50+
command: '/…/simctl io 416440AE recordVideo /tmp/out.mp4',
51+
});
52+
const snapshot = evaluateDaemonLeaks(observe({ processes: [recorder] }));
53+
54+
expect(snapshot.ownedProcesses).toEqual([
55+
expect.objectContaining({ pid: 52420, reasons: ['process-group'] }),
56+
]);
57+
expect(hasDaemonLeaks(snapshot)).toBe(true);
58+
});
59+
60+
// #1109: the agent-browser daemon setsids away from the daemon's group, so
61+
// only the inherited state-dir environment and its argv identify the fleet.
62+
test('flags an agent-browser fleet by inherited state dir, including its children', () => {
63+
const browserDaemon = proc({
64+
pid: 47515,
65+
ppid: 1,
66+
pgid: 47515,
67+
command: `${STATE_DIR}/tools/agent-browser/0.27.1/package/…/agent-browser-darwin-arm64`,
68+
env: `HOME=/tmp AGENT_DEVICE_STATE_DIR=${STATE_DIR}`,
69+
});
70+
const chrome = proc({
71+
pid: 47586,
72+
ppid: 47515,
73+
pgid: 47586,
74+
command: 'Google Chrome for Testing',
75+
});
76+
const renderer = proc({
77+
pid: 47953,
78+
ppid: 47586,
79+
pgid: 47586,
80+
command: 'Chrome Helper (Renderer)',
81+
});
82+
const snapshot = evaluateDaemonLeaks(observe({ processes: [browserDaemon, chrome, renderer] }));
83+
84+
expect(snapshot.ownedProcesses.map((owned) => owned.pid)).toEqual([47515, 47586, 47953]);
85+
expect(snapshot.ownedProcesses[0]?.reasons).toEqual(['state-dir-env', 'state-dir-argv']);
86+
// The fleet below the matched root is owned transitively, not by its own argv.
87+
expect(snapshot.ownedProcesses[1]?.reasons).toEqual(['descendant']);
88+
});
89+
90+
test('ignores foreign processes, the observer chain, and the daemons themselves', () => {
91+
const simulator = proc({ pid: 700, command: '/…/CoreSimulator … SimulatorTrampoline' });
92+
const neighbourStateDir = proc({
93+
pid: 701,
94+
command: `node --state-dir ${STATE_DIR}-other/daemon.ts`,
95+
env: `AGENT_DEVICE_STATE_DIR=${STATE_DIR}-other`,
96+
});
97+
const observer = proc({ pid: OBSERVER_PID, pgid: DAEMON_PID });
98+
const daemon = proc({ pid: DAEMON_PID, pgid: DAEMON_PID });
99+
const snapshot = evaluateDaemonLeaks(
100+
observe({ processes: [simulator, neighbourStateDir, observer, daemon] }),
101+
);
102+
103+
expect(snapshot.ownedProcesses).toEqual([]);
104+
expect(hasDaemonLeaks(snapshot)).toBe(false);
105+
});
106+
});
107+
108+
describe('surviving-daemon rule', () => {
109+
// stopProcessForTakeover is best-effort void: it returns silently on identity
110+
// mismatch, signal failure, or kill timeout, so a daemon can outlive the stop.
111+
test('a daemon still alive after shutdown is itself a leak', () => {
112+
const snapshot = evaluateDaemonLeaks(
113+
observe({ livePids: [DAEMON_PID], stateEntries: [file('daemon.log')] }),
114+
);
115+
116+
expect(snapshot.liveDaemonPids).toEqual([DAEMON_PID]);
117+
expect(hasDaemonLeaks(snapshot)).toBe(true);
118+
expect(formatDaemonLeakReport(snapshot)).toContain('daemons that outlived shutdown: 1');
119+
});
120+
121+
test('its metadata files stay stray rather than being excused by its own survival', () => {
122+
const snapshot = evaluateDaemonLeaks(
123+
observe({ livePids: [DAEMON_PID], stateEntries: [file('daemon.json'), file('daemon.lock')] }),
124+
);
125+
126+
expect(snapshot.strayStateEntries).toEqual(['daemon.json', 'daemon.lock']);
127+
});
128+
129+
test('a live daemon is expected while a session merely closed', () => {
130+
const snapshot = evaluateDaemonLeaks(
131+
observe({
132+
phase: 'after-close',
133+
livePids: [DAEMON_PID],
134+
stateEntries: [file('daemon.json'), file('daemon.lock')],
135+
}),
136+
);
137+
138+
expect(hasDaemonLeaks(snapshot)).toBe(false);
139+
});
140+
});
141+
142+
describe('state-dir residue rules', () => {
143+
test.each<[string, StateEntry, DaemonLeakPhase, 'expected' | 'stray']>([
144+
['session event log', file('sessions/default/events.ndjson'), 'after-shutdown', 'expected'],
145+
['request diagnostics', file('sessions/d/requests/abc.ndjson'), 'after-shutdown', 'expected'],
146+
['shutdown report', file('daemon-shutdown.json'), 'after-shutdown', 'expected'],
147+
['torn publish temporary', file('device-claims/a.json.55.tmp'), 'after-shutdown', 'stray'],
148+
[
149+
'managed tool download',
150+
file('tools/agent-browser/0.27.1/dl.tmp'),
151+
'after-shutdown',
152+
'expected',
153+
],
154+
[
155+
'unswept artifact scaffold',
156+
{ path: 'sessions/d/artifacts/pending/', kind: 'empty-directory' },
157+
'after-shutdown',
158+
'stray',
159+
],
160+
[
161+
'unswept session scaffold',
162+
{ path: 'sessions/leaked/requests/', kind: 'empty-directory' },
163+
'after-shutdown',
164+
'stray',
165+
],
166+
['unknown artifact', file('sessions/d/mystery.bin'), 'after-shutdown', 'stray'],
167+
[
168+
'open capture descriptor',
169+
file('sessions/d/screen-recording.resource.json', 'open'),
170+
'after-shutdown',
171+
'stray',
172+
],
173+
[
174+
'completed capture descriptor',
175+
file('sessions/d/screen-recording.resource.json', 'completed'),
176+
'after-shutdown',
177+
'expected',
178+
],
179+
[
180+
'open capture during close',
181+
file('sessions/d/screen-recording.resource.json', 'open'),
182+
'after-close',
183+
'expected',
184+
],
185+
['legacy app-log marker', file('sessions/d/app-log.pid'), 'after-shutdown', 'stray'],
186+
])('%s is %s at %s', (_name, entry, phase, verdict) => {
187+
const snapshot = evaluateDaemonLeaks(observe({ phase, stateEntries: [entry] }));
188+
189+
expect(snapshot.strayStateEntries).toEqual(verdict === 'stray' ? [entry.path] : []);
190+
});
191+
192+
// A managed install tree is third-party output the daemon neither writes nor
193+
// owns, so its own temporaries must not be read as our torn publish.
194+
test('the managed tools exemption does not leak into daemon-written paths', () => {
195+
const snapshot = evaluateDaemonLeaks(
196+
observe({ stateEntries: [file('sessions/d/tools/pending.tmp')] }),
197+
);
198+
199+
expect(snapshot.strayStateEntries).toEqual(['sessions/d/tools/pending.tmp']);
200+
});
201+
});
202+
203+
test('a clean shutdown reports no leak', () => {
204+
const snapshot = evaluateDaemonLeaks(
205+
observe({
206+
processes: [proc({ pid: 700, command: 'unrelated' })],
207+
stateEntries: [file('daemon.log'), file('sessions/default/events.ndjson')],
208+
}),
209+
);
210+
211+
expect(hasDaemonLeaks(snapshot)).toBe(false);
212+
expect(formatDaemonLeakReport(snapshot)).toContain('daemon leak oracle: clean (after-shutdown)');
213+
});

0 commit comments

Comments
 (0)