fix: return confirmed daemon termination outcomes

scripts/clean-daemon.ts

@@ -1,8 +1,9 @@
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
+import { AppError } from '@agent-device/kernel/errors';
 import { resolveDaemonPaths } from '../src/daemon-resolution.ts';
-import { isAgentDeviceDaemonProcess, stopProcessForTakeover } from '../src/daemon-process.ts';
+import { isAgentDeviceDaemonProcess, stopDaemonProcess } from '../src/daemon-process.ts';
 
 const DAEMON_TERM_TIMEOUT_MS = 15_000;
 const DAEMON_KILL_TIMEOUT_MS = 2_000;
@@ -19,11 +20,24 @@ const info = readDaemonInfo(paths.infoPath);
 const daemonPid = readPositivePid(info?.pid);
 
 if (daemonPid !== null) {
-  await stopProcessForTakeover(daemonPid, {
-    termTimeoutMs: DAEMON_TERM_TIMEOUT_MS,
-    killTimeoutMs: DAEMON_KILL_TIMEOUT_MS,
-    expectedStartTime: info?.processStartTime,
-  });
+  const termination = await stopDaemonProcess(
+    { pid: daemonPid, startTime: info?.processStartTime ?? null },
+    {
+      mode: 'graceful',
+      termTimeoutMs: DAEMON_TERM_TIMEOUT_MS,
+      killTimeoutMs: DAEMON_KILL_TIMEOUT_MS,
+    },
+  );
+  if (termination.status !== 'exited') {
+    throw new AppError(
+      'COMMAND_FAILED',
+      'Daemon cleanup retained state because exit could not be confirmed.',
+      {
+        reason: 'daemon_exit_unconfirmed',
+        termination,
+      },
+    );
+  }
   const { cleanupRunnerLeasesForOwner } =
     await import('@agent-device/platform-apple/runner/operations');
   await cleanupRunnerLeasesForOwner({ pid: daemonPid, startTime: info?.processStartTime });

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

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, expect, test, vi } from 'vitest';
-import { stopProcessForTakeover, waitForDaemonExit } from '../daemon-process.ts';
+import { stopDaemonProcess, waitForDaemonExit } from '../daemon-process.ts';
 
 const DAEMON_COMMAND = '/opt/checkout/dist/src/internal/daemon.js';
 const OURS = 'Mon Aug 24 10:00:00 2026';
@@ -75,14 +75,22 @@ test('waitForDaemonExit reports a daemon that keeps its identity as not exited',
   expect(wait.exited).toBe(false);
 });
 
-test('waitForDaemonExit keeps waiting through a zombie until the pid is reaped', async () => {
+test('a verified zombie proves exit before its pid is reaped', async () => {
   state.states.set(PID, 'Z+');
   state.commands.set(PID, '<defunct>');
   const stillTaken = await waitForDaemonExit(
     { pid: PID, startTime: OURS },
     { timeoutMs: 40, pollMs: POLL_MS },
   );
-  expect(stillTaken.exited).toBe(false);
+  expect(stillTaken.exited).toBe(true);
+  expect(stillTaken.elapsedMs).toBeLessThan(40);
+  expect(
+    await stopDaemonProcess(
+      { pid: PID, startTime: OURS },
+      { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 0 },
+    ),
+  ).toMatchObject({ status: 'exited', mode: 'already-exited' });
+  expect(signals).toEqual([]);
 
   state.alive.set(PID, false);
   const reaped = await waitForDaemonExit(
@@ -92,26 +100,122 @@ test('waitForDaemonExit keeps waiting through a zombie until the pid is reaped',
   expect(reaped.exited).toBe(true);
 });
 
-test('stopProcessForTakeover does not SIGKILL a pid recycled during the grace wait', async () => {
+test('stopDaemonProcess does not SIGKILL a pid recycled during the grace wait', async () => {
   onSignal = (signal) => {
     if (signal === 'SIGTERM') state.starts.set(PID, RECYCLED);
   };
-  await stopProcessForTakeover(PID, {
-    termTimeoutMs: TIMEOUT_MS,
-    killTimeoutMs: 40,
-    expectedStartTime: OURS,
-  });
+  await stopDaemonProcess(
+    { pid: PID, startTime: OURS },
+    { mode: 'graceful', termTimeoutMs: TIMEOUT_MS, killTimeoutMs: 40 },
+  );
   expect(signals).toEqual(['SIGTERM']);
 });
 
-test('stopProcessForTakeover still escalates to SIGKILL for a daemon that survives SIGTERM', async () => {
+test('stopDaemonProcess still escalates to SIGKILL for a daemon that survives SIGTERM', async () => {
   onSignal = (signal) => {
     if (signal === 'SIGKILL') state.alive.set(PID, false);
   };
-  await stopProcessForTakeover(PID, {
-    termTimeoutMs: 40,
-    killTimeoutMs: 40,
-    expectedStartTime: OURS,
+  await stopDaemonProcess(
+    { pid: PID, startTime: OURS },
+    { mode: 'graceful', termTimeoutMs: 40, killTimeoutMs: 40 },
+  );
+  expect(signals).toEqual(['SIGTERM', 'SIGKILL']);
+});
+
+test('a changed command in the same process lifetime does not prove exit', async () => {
+  state.commands.set(PID, '/usr/bin/another-command');
+  expect(await waitForDaemonExit({ pid: PID, startTime: OURS }, { timeoutMs: 0 })).toMatchObject({
+    exited: false,
   });
+});
+
+test('an unreadable process start time does not prove exit', async () => {
+  state.starts.delete(PID);
+  expect(await waitForDaemonExit({ pid: PID, startTime: OURS }, { timeoutMs: 0 })).toMatchObject({
+    exited: false,
+  });
+});
+
+test('a recycled pid proves the original exit even when its successor is a zombie', async () => {
+  state.starts.set(PID, RECYCLED);
+  state.states.set(PID, 'Z');
+  expect(await waitForDaemonExit({ pid: PID, startTime: OURS }, { timeoutMs: 0 })).toMatchObject({
+    exited: true,
+  });
+});
+
+test('missing start-time identity retains a live daemon without signaling', async () => {
+  expect(
+    await stopDaemonProcess(
+      { pid: PID, startTime: null },
+      { mode: 'graceful', termTimeoutMs: 0, killTimeoutMs: 0 },
+    ),
+  ).toMatchObject({ status: 'retained', reason: 'missing-start-time' });
+  expect(signals).toEqual([]);
+});
+
+test('an unidentified released pid has no lifetime cleanup proof', async () => {
+  state.alive.set(PID, false);
+  expect(
+    await stopDaemonProcess(
+      { pid: PID, startTime: null },
+      {
+        mode: 'graceful',
+        termTimeoutMs: 0,
+        killTimeoutMs: 0,
+      },
+    ),
+  ).toEqual({ status: 'not-running' });
+  expect(signals).toEqual([]);
+});
+
+test('an exhausted kill wait returns retained rather than silently completing', async () => {
+  expect(
+    await stopDaemonProcess(
+      { pid: PID, startTime: OURS },
+      { mode: 'graceful', termTimeoutMs: 0, killTimeoutMs: 0 },
+    ),
+  ).toMatchObject({ status: 'retained', reason: 'exit-timeout' });
   expect(signals).toEqual(['SIGTERM', 'SIGKILL']);
 });
+
+test('failed signaling is retained when the same daemon is still alive', async () => {
+  vi.mocked(process.kill).mockImplementation(() => {
+    throw Object.assign(new Error('signal refused'), { code: 'EPERM' });
+  });
+  expect(
+    await stopDaemonProcess(
+      { pid: PID, startTime: OURS },
+      { mode: 'graceful', termTimeoutMs: 0, killTimeoutMs: 0 },
+    ),
+  ).toMatchObject({ status: 'retained', reason: 'signal-failed', signal: 'SIGTERM' });
+});
+
+test('force termination delivers KILL first and returns its confirmed exit', async () => {
+  onSignal = (signal) => {
+    if (signal === 'SIGKILL') state.alive.set(PID, false);
+  };
+  const options = { mode: 'force' as const, termTimeoutMs: 0, killTimeoutMs: 0 };
+  const result = await stopDaemonProcess({ pid: PID, startTime: OURS }, options);
+  expect(signals).toEqual(['SIGKILL']);
+  expect(result).toMatchObject({
+    status: 'exited',
+    identity: { pid: PID, startTime: OURS },
+    mode: 'forced',
+  });
+});
+
+test('a daemon reaped after the TERM budget retains graceful mode without signaling its zombie', async () => {
+  onSignal = (signal) => {
+    if (signal !== 'SIGTERM') return;
+    state.states.set(PID, 'Z');
+    state.commands.set(PID, '<defunct>');
+    setTimeout(() => state.alive.set(PID, false), 10);
+  };
+  const result = await stopDaemonProcess(
+    { pid: PID, startTime: OURS },
+    { mode: 'graceful', termTimeoutMs: 0, killTimeoutMs: 40 },
+  );
+  expect(signals).toEqual(['SIGTERM']);
+  expect(result).toMatchObject({ status: 'exited', mode: 'graceful' });
+});

src/__tests__/daemon-process-takeover.test.ts

@@ -1,25 +1,21 @@
 import assert from 'node:assert/strict';
-import { spawn } from 'node:child_process';
+import { spawn, type ChildProcess } from 'node:child_process';
 import fs from 'node:fs';
 // oxlint-disable-next-line no-restricted-imports -- real /tmp socket path within the 104-char limit
 import os from 'node:os';
 import path from 'node:path';
 import { afterEach, test } from 'vitest';
 import { isProcessAlive, readProcessStartTime } from '@agent-device/host-kit/process';
-import { isAgentDeviceDaemonProcess, stopProcessForTakeover } from '../daemon-process.ts';
+import { isAgentDeviceDaemonProcess, stopDaemonProcess } from '../daemon-process.ts';
 
 const TAKEOVER_TIMEOUTS = { termTimeoutMs: 5_000, killTimeoutMs: 2_000 };
-const spawnedPids: number[] = [];
+const spawnedChildren: { child: ChildProcess; exited: Promise<void> }[] = [];
 const spawnedRoots: string[] = [];
 
-afterEach(() => {
-  for (const pid of spawnedPids.splice(0)) {
-    if (!isProcessAlive(pid)) continue;
-    try {
-      process.kill(pid, 'SIGKILL');
-    } catch {
-      // The test's assertion already observed that the child exited.
-    }
+afterEach(async () => {
+  for (const { child, exited } of spawnedChildren.splice(0)) {
+    if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL');
+    await exited;
   }
   for (const root of spawnedRoots.splice(0)) {
     fs.rmSync(root, { recursive: true, force: true });
@@ -40,7 +36,10 @@ function spawnFakeDaemonFromBranchNamedCheckout(): { pid: number; entryPath: str
   const child = spawn(process.execPath, [entryPath], { stdio: 'ignore' });
   const pid = child.pid ?? 0;
   assert.ok(pid > 0, 'expected the fake daemon to have a pid');
-  spawnedPids.push(pid);
+  spawnedChildren.push({
+    child,
+    exited: new Promise<void>((resolve) => child.once('exit', () => resolve())),
+  });
   return { pid, entryPath };
 }
 
@@ -55,7 +54,12 @@ test('stops a branch-named daemon before replacement can strand its session', as
   assert.ok(startTime, 'expected the spawned daemon to report a start time');
   assert.equal(isAgentDeviceDaemonProcess(pid, startTime), true);
 
-  await stopProcessForTakeover(pid, { ...TAKEOVER_TIMEOUTS, expectedStartTime: startTime });
+  const result = await stopDaemonProcess(
+    { pid, startTime },
+    { mode: 'graceful', ...TAKEOVER_TIMEOUTS },
+  );
+  assert.equal(result.status, 'exited');
+  await spawnedChildren.at(-1)!.exited;
   assert.equal(isProcessAlive(pid), false);
 });
 
@@ -65,7 +69,11 @@ test('does not stop a branch-named daemon when process identity is missing', asy
 
   assert.equal(isAgentDeviceDaemonProcess(pid, undefined), false);
 
-  await stopProcessForTakeover(pid, { ...TAKEOVER_TIMEOUTS, expectedStartTime: undefined });
+  const result = await stopDaemonProcess(
+    { pid, startTime: null },
+    { mode: 'graceful', ...TAKEOVER_TIMEOUTS },
+  );
+  assert.deepEqual(result, { status: 'retained', reason: 'missing-start-time' });
   assert.equal(isProcessAlive(pid), true);
 });
 
@@ -78,9 +86,14 @@ test('does not stop a branch-named daemon when the pid belongs to a different pr
   const staleStartTime = `${actualStartTime}-previous-lifetime`;
   assert.equal(isAgentDeviceDaemonProcess(pid, staleStartTime), false);
 
-  await stopProcessForTakeover(pid, {
-    ...TAKEOVER_TIMEOUTS,
-    expectedStartTime: staleStartTime,
+  const result = await stopDaemonProcess(
+    { pid, startTime: staleStartTime },
+    { mode: 'graceful', ...TAKEOVER_TIMEOUTS },
+  );
+  assert.deepEqual(result, {
+    status: 'exited',
+    mode: 'already-exited',
+    identity: { pid, startTime: staleStartTime },
   });
   assert.equal(isProcessAlive(pid), true);
 });

src/daemon-client/__tests__/daemon-client-metadata.test.ts

@@ -1,11 +1,25 @@
 import assert from 'node:assert/strict';
+import { AppError, normalizeError } from '@agent-device/kernel/errors';
 import fs from 'node:fs';
 import path from 'node:path';
-import { test } from 'vitest';
+import { afterEach, test, vi } from 'vitest';
 import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
 import { writeInfo } from '../../daemon/server/server-lifecycle.ts';
-import { readDaemonInfo } from '../daemon-client-metadata.ts';
+import {
+  readDaemonInfo,
+  cleanupFailedDaemonStartupMetadata,
+  stopDaemonProcessForTakeover,
+} from '../daemon-client-metadata.ts';
+import { isAgentDeviceDaemonProcess, stopDaemonProcess } from '../../daemon-process.ts';
+import { resolveDaemonPaths } from '../../daemon-resolution.ts';
+
+vi.mock('../../daemon-process.ts', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('../../daemon-process.ts')>()),
+  isAgentDeviceDaemonProcess: vi.fn(),
+  stopDaemonProcess: vi.fn(),
+}));
+afterEach(() => vi.resetAllMocks());
 
 // The reuse decision is only as good as the identity that survives the round trip
 // through `daemon.json`: a client cannot compare what the file lost (#2458).
@@ -47,3 +61,39 @@ test('a registration this version did not write reads back unreported', () => {
     assert.equal(readDaemonInfo(infoPath)?.codeOrigin, undefined);
   }
 });
+
+for (const artifact of ['daemon.json', 'daemon.lock']) {
+  test(`unconfirmed startup stop retains ${artifact} without claiming cleanup`, async () => {
+    const [stateDir] = scratchStateDir();
+    const paths = resolveDaemonPaths(stateDir);
+    const file = path.join(stateDir, artifact);
+    const contents = JSON.stringify({
+      pid: 7,
+      processStartTime: 'start',
+      port: 1234,
+      token: 'secret',
+    });
+    fs.writeFileSync(file, contents);
+    vi.mocked(isAgentDeviceDaemonProcess).mockReturnValue(true);
+    vi.mocked(stopDaemonProcess).mockResolvedValue({ status: 'retained', reason: 'exit-timeout' });
+    const result = await cleanupFailedDaemonStartupMetadata(paths, 'start_error');
+    assert.equal(fs.readFileSync(file, 'utf8'), contents);
+    assert.equal(result.removedInfo, false);
+    assert.equal(result.removedLock, false);
+    assert.equal(result.stoppedInfoProcess, false);
+    assert.equal(result.stoppedLockProcess, false);
+    assert.match(result.error ?? '', /exit could not be confirmed/);
+  });
+}
+
+test('a retained takeover keeps its reason at the normalized error boundary', async () => {
+  vi.mocked(stopDaemonProcess).mockResolvedValue({ status: 'retained', reason: 'exit-timeout' });
+  await assert.rejects(
+    stopDaemonProcessForTakeover({ pid: 7, token: 'secret', processStartTime: 'start' }),
+    (error: unknown) => {
+      assert.ok(error instanceof AppError);
+      assert.equal(normalizeError(error).details?.reason, 'daemon_exit_unconfirmed');
+      return true;
+    },
+  );
+});

src/daemon-client/__tests__/daemon-client-startup-race.test.ts

@@ -19,10 +19,18 @@ vi.mock('../../daemon-process.ts', async (importOriginal) => {
     isAgentDeviceDaemonProcess: vi.fn((pid: number, startTime: string | undefined) =>
       pid === winner.pid ? winner.alive : actual.isAgentDeviceDaemonProcess(pid, startTime),
     ),
-    stopProcessForTakeover: vi.fn(
-      async (pid: number, options: Parameters<typeof actual.stopProcessForTakeover>[1]) => {
-        if (pid !== winner.pid) return await actual.stopProcessForTakeover(pid, options);
+    stopDaemonProcess: vi.fn(
+      async (
+        identity: Parameters<typeof actual.stopDaemonProcess>[0],
+        options: Parameters<typeof actual.stopDaemonProcess>[1],
+      ) => {
+        if (identity.pid !== winner.pid) return await actual.stopDaemonProcess(identity, options);
         winner.alive = false;
+        return {
+          status: 'exited' as const,
+          identity: { pid: identity.pid, startTime: identity.startTime! },
+          mode: 'graceful' as const,
+        };
       },
     ),
   };