Skip to content

Commit f6bf3e2

Browse files
committed
refactor(daemon): drop implementation narration from the daemon-exit change
AGENTS.md allows only public API docs, tool directives, and a brief citation to an external constraint. The identity type, the pid-state union, the single wait helper and the planted-red assertions carry the invariant; the prose restating them, and the review history behind them, belong in the PR. Kept: the exported identity/result types and the wait helper's contract, plus a two-line citation for the one thing code cannot encode — a terminated pid awaiting reap answers kill(pid, 0), keeps its start time, and reports `<defunct>`.
1 parent 73ad6cf commit f6bf3e2

3 files changed

Lines changed: 5 additions & 47 deletions

File tree

‎src/daemon/__tests__/daemon-exit-wait.test.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,9 @@ const DAEMON_COMMAND = '/opt/checkout/dist/src/internal/daemon.js';
55
const OURS = 'Mon Aug 24 10:00:00 2026';
66
const RECYCLED = 'Mon Aug 24 10:00:07 2026';
77
const PID = 4242;
8-
// Small enough that a regression that waits the budget out still lands inside the
9-
// unit lane's wall-clock gate and fails on its assertion rather than on the clock.
108
const TIMEOUT_MS = 1_000;
119
const POLL_MS = 5;
1210

13-
// A live process cannot be made to change identity mid-wait, so the host reads are
14-
// mocked: a recycled pid is the same live pid reporting a different start time,
15-
// which is what the host presents after reuse.
1611
const state = vi.hoisted(() => ({
1712
alive: new Map<number, boolean>(),
1813
starts: new Map<number, string>(),
@@ -36,7 +31,6 @@ vi.mock('../../utils/host-process.ts', async (importOriginal) => ({
3631
},
3732
}));
3833

39-
/** Delivered signals only; a `0` probe is a liveness read, not a write. */
4034
const signals: NodeJS.Signals[] = [];
4135
let onSignal: (signal: NodeJS.Signals) => void = () => {};
4236

@@ -81,10 +75,6 @@ test('waitForDaemonExit reports a daemon that keeps its identity as not exited',
8175
expect(wait.exited).toBe(false);
8276
});
8377

84-
// A daemon killed by its own parent lingers as a zombie: kill(pid, 0) still succeeds
85-
// and the start time still matches, while ps reports the command as `<defunct>`.
86-
// Reading that as a recycled pid would hand the number back to callers while it is
87-
// still taken, so it must count as neither ending until the reap lands.
8878
test('waitForDaemonExit keeps waiting through a zombie until the pid is reaped', async () => {
8979
state.states.set(PID, 'Z+');
9080
state.commands.set(PID, '<defunct>');
@@ -102,9 +92,6 @@ test('waitForDaemonExit keeps waiting through a zombie until the pid is reaped',
10292
expect(reaped.exited).toBe(true);
10393
});
10494

105-
// Planted red for the defect this pair exists to prevent: the grace wait used to poll
106-
// the bare pid, so a daemon that exited and had its pid reused read as "still running"
107-
// and the takeover escalated SIGKILL onto whatever now held that number.
10895
test('stopProcessForTakeover does not SIGKILL a pid recycled during the grace wait', async () => {
10996
onSignal = (signal) => {
11097
if (signal === 'SIGTERM') state.starts.set(PID, RECYCLED);

‎src/daemon/daemon-process.ts‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -56,33 +56,19 @@ export type DaemonProcessIdentity = {
5656
};
5757

5858
export type DaemonExitWait = {
59-
/** The pid no longer belongs to the identity: it exited, or the host recycled it. */
59+
/** The pid was released, or the host handed it to a different process. */
6060
exited: boolean;
6161
elapsedMs: number;
6262
};
6363

64-
/**
65-
* Poll for {@link waitForDaemonExit}. {@link classifyDaemonPid} reads `ps` only
66-
* once the cheap liveness check says the pid is still taken, so a daemon that has
67-
* already gone costs no subprocess at all.
68-
*/
6964
const DAEMON_EXIT_POLL_MS = 100;
7065

71-
/**
72-
* What a pid is doing relative to the identity that claimed it. `exiting` is the
73-
* state a bare liveness read cannot express: `kill(pid, 0)` still succeeds for a
74-
* process that has died but has not been reaped yet, while `ps` has already
75-
* dropped its row — a pid on its way out, not a pid handed to somebody else.
76-
*/
7766
type DaemonPidState = 'ours' | 'exiting' | 'released' | 'recycled';
7867

7968
function classifyDaemonPid(identity: DaemonProcessIdentity): DaemonPidState {
8069
if (!isProcessAlive(identity.pid)) return 'released';
81-
// State and start time in one `ps` read. A process that has died but has not
82-
// been reaped answers kill(pid, 0) AND still reports its original start time,
83-
// so the state is the only field that separates it from one still running —
84-
// and its command reads `<defunct>`, which would otherwise look like a pid
85-
// handed to a different program.
70+
// A terminated pid awaiting reap answers kill(pid, 0), keeps its start time, and
71+
// reports its command as `<defunct>`; only the process state distinguishes it.
8672
const observed = readHostProcessIdentityObservations([identity.pid]).get(identity.pid);
8773
if (!observed || observed.state.startsWith('Z')) return 'exiting';
8874
if (observed.startTime !== identity.startTime) return 'recycled';
@@ -92,13 +78,8 @@ function classifyDaemonPid(identity: DaemonProcessIdentity): DaemonPidState {
9278
}
9379

9480
/**
95-
* Waits for a daemon identity to leave the host. Two endings count as exited: the
96-
* pid was released, or the host handed it to someone else. Recycling is the one a
97-
* bare liveness wait gets wrong — it reports a stranger's pid as "still running",
98-
* which is what lets a grace wait escalate a SIGKILL onto an unrelated process —
99-
* so escalating callers must branch on this result, never on bare liveness. A pid
100-
* still being torn down is neither ending, so the wait keeps polling and callers
101-
* retain the old guarantee that the number is free before they act on it.
81+
* Resolves once `identity` has left the host — released or recycled. A pid still
82+
* being torn down is neither, so the wait continues until the number is free.
10283
*/
10384
export async function waitForDaemonExit(
10485
identity: DaemonProcessIdentity,
@@ -119,12 +100,6 @@ export async function waitForDaemonExit(
119100
return { exited, elapsedMs: Date.now() - startedAt };
120101
}
121102

122-
/**
123-
* The only way this module signals a daemon: identity is re-read immediately
124-
* before the write, so a pid recycled since the last observation cannot be
125-
* signaled at all. Escalation safety is then a property of the call, not a rule
126-
* each call site has to remember.
127-
*/
128103
function signalDaemonIdentity(identity: DaemonProcessIdentity, signal: NodeJS.Signals): boolean {
129104
if (!isAgentDeviceDaemonProcess(identity.pid, identity.startTime)) return false;
130105
return trySignalProcess(identity.pid, signal);

‎test/integration/smoke-web-platform.test.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,6 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise<void> {
193193
// orchestrator shutting the container down would, rather than going through `close`.
194194
process.kill(daemonPid, 'SIGTERM');
195195

196-
// The fleet dying and the daemon exiting are unordered — the daemon awaits the
197-
// `agent-browser close` CLI's return, not the disappearance of the pids it reaps — so each
198-
// settles on its own deadline from this SIGTERM and neither waits the other out. That the
199-
// fleet closed BECAUSE the daemon closed it is held by WEB_SHUTDOWN_IDLE_TIMEOUT_MS above.
200196
const [after, daemonExit] = await Promise.all([
201197
settleManagedBrowserProcesses(status),
202198
waitForDaemonExit(daemonIdentity, {

0 commit comments

Comments
 (0)