From 33edda40476d712849931c6a68db5cc6208cbc27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 2 Oct 2026 22:29:26 +0200 Subject: [PATCH 1/7] fix: return explicit daemon termination outcomes from one stop operation --- scripts/clean-daemon.ts | 26 ++- src/__tests__/daemon-exit-wait.test.ts | 120 +++++++++-- src/__tests__/daemon-process-takeover.test.ts | 46 +++-- .../daemon-client-startup-race.test.ts | 14 +- src/daemon-client/daemon-client-metadata.ts | 36 ++-- src/daemon-process.ts | 91 ++++++--- src/daemon/__tests__/daemon-stop.test.ts | 188 ++++++++---------- src/daemon/daemon-stop.ts | 65 ++---- .../daemon-replace-exit-flush.test.ts | 24 +-- test/integration/smoke-daemon-clean.test.ts | 39 +++- test/integration/smoke-daemon-http.test.ts | 11 +- test/integration/smoke-web-platform.test.ts | 17 +- .../support/daemon-leak-model.test.ts | 2 +- test/integration/support/daemon-leak-model.ts | 7 +- 14 files changed, 411 insertions(+), 275 deletions(-) diff --git a/scripts/clean-daemon.ts b/scripts/clean-daemon.ts index bfb1e1a2eb..659afeb6ad 100644 --- a/scripts/clean-daemon.ts +++ b/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 }); diff --git a/src/__tests__/daemon-exit-wait.test.ts b/src/__tests__/daemon-exit-wait.test.ts index b6c65943d6..2a6c375d1d 100644 --- a/src/__tests__/daemon-exit-wait.test.ts +++ b/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'; @@ -92,26 +92,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, ''); + 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' }); +}); diff --git a/src/__tests__/daemon-process-takeover.test.ts b/src/__tests__/daemon-process-takeover.test.ts index ca39cc8496..2be1d815d0 100644 --- a/src/__tests__/daemon-process-takeover.test.ts +++ b/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 }[] = []; 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((resolve) => child.once('exit', () => resolve())), + }); return { pid, entryPath }; } @@ -55,7 +54,11 @@ 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'); assert.equal(isProcessAlive(pid), false); }); @@ -65,7 +68,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 +85,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); }); diff --git a/src/daemon-client/__tests__/daemon-client-startup-race.test.ts b/src/daemon-client/__tests__/daemon-client-startup-race.test.ts index edd2c418a5..d52ba0e6c2 100644 --- a/src/daemon-client/__tests__/daemon-client-startup-race.test.ts +++ b/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[1]) => { - if (pid !== winner.pid) return await actual.stopProcessForTakeover(pid, options); + stopDaemonProcess: vi.fn( + async ( + identity: Parameters[0], + options: Parameters[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, + }; }, ), }; diff --git a/src/daemon-client/daemon-client-metadata.ts b/src/daemon-client/daemon-client-metadata.ts index 3935ba6ba2..bac92e3197 100644 --- a/src/daemon-client/daemon-client-metadata.ts +++ b/src/daemon-client/daemon-client-metadata.ts @@ -1,7 +1,11 @@ import fs from 'node:fs'; import { shellQuote } from '@agent-device/kernel/device-shell'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; -import { isAgentDeviceDaemonProcess, stopProcessForTakeover } from '../daemon-process.ts'; +import { + isAgentDeviceDaemonProcess, + stopDaemonProcess, + type DaemonTerminationResult, +} from '../daemon-process.ts'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; @@ -198,11 +202,14 @@ export async function cleanupFailedDaemonStartupMetadata( result.retainedLockProcess = true; } else { if (liveLockProcess) { - await stopProcessForTakeover(lockInfo.pid, { - termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, - killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, - expectedStartTime: lockInfo.processStartTime, - }); + await stopDaemonProcess( + { pid: lockInfo.pid, startTime: lockInfo.processStartTime ?? null }, + { + mode: 'graceful', + termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, + killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, + }, + ); result.stoppedLockProcess = true; } removeDaemonLock(paths.lockPath); @@ -246,12 +253,17 @@ export async function recoverDaemonLockHolder(paths: DaemonPaths): Promise { - await stopProcessForTakeover(info.pid, { - termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, - killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, - expectedStartTime: info.processStartTime, - }); +export async function stopDaemonProcessForTakeover( + info: DaemonInfo, +): Promise { + return await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { + mode: 'graceful', + termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, + killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, + }, + ); } export function isRemoteDaemon(info: DaemonInfo): boolean { diff --git a/src/daemon-process.ts b/src/daemon-process.ts index c42205a9c1..ddc9f610c4 100644 --- a/src/daemon-process.ts +++ b/src/daemon-process.ts @@ -3,6 +3,7 @@ import { readHostProcessIdentityObservations, readProcessCommand, readProcessStartTime, + type OwnerIdentity, } from '@agent-device/host-kit/process'; import { sleep } from '@agent-device/host-kit/retry'; @@ -50,10 +51,10 @@ export function trySignalProcess(pid: number, signal: NodeJS.Signals): boolean { } /** A daemon pinned to one process lifetime, never a bare pid. */ -export type DaemonProcessIdentity = { +export type DaemonProcessIdentity = Readonly<{ pid: number; startTime: string; -}; +}>; export type DaemonExitWait = { /** The pid was released, or the host handed it to a different process. */ @@ -63,20 +64,6 @@ export type DaemonExitWait = { const DAEMON_EXIT_POLL_MS = 100; -type DaemonPidState = 'ours' | 'exiting' | 'released' | 'recycled'; - -function classifyDaemonPid(identity: DaemonProcessIdentity): DaemonPidState { - if (!isProcessAlive(identity.pid)) return 'released'; - // A terminated pid awaiting reap answers kill(pid, 0), keeps its start time, and - // reports its command as ``; only the process state distinguishes it. - const observed = readHostProcessIdentityObservations([identity.pid]).get(identity.pid); - if (!observed || observed.state.startsWith('Z')) return 'exiting'; - if (observed.startTime !== identity.startTime) return 'recycled'; - const command = readProcessCommand(identity.pid); - if (!command) return 'exiting'; - return isAgentDeviceDaemonCommand(command) ? 'ours' : 'recycled'; -} - /** * Resolves once `identity` has left the host — released or recycled. A pid still * being torn down is neither, so the wait continues until the number is free. @@ -89,34 +76,72 @@ export async function waitForDaemonExit( const deadline = startedAt + options.timeoutMs; const pollMs = options.pollMs ?? DAEMON_EXIT_POLL_MS; const hasExited = (): boolean => { - const state = classifyDaemonPid(identity); - return state === 'released' || state === 'recycled'; + if (!isProcessAlive(identity.pid)) return true; + const observed = readHostProcessIdentityObservations([identity.pid]).get(identity.pid); + return Boolean(observed?.startTime && observed.startTime !== identity.startTime); }; let exited = hasExited(); while (!exited && Date.now() < deadline) { - await sleep(pollMs); + await sleep(Math.min(pollMs, Math.max(0, deadline - Date.now()))); exited = hasExited(); } return { exited, elapsedMs: Date.now() - startedAt }; } -function signalDaemonIdentity(identity: DaemonProcessIdentity, signal: NodeJS.Signals): boolean { - if (!isAgentDeviceDaemonProcess(identity.pid, identity.startTime)) return false; - return trySignalProcess(identity.pid, signal); -} +export type DaemonTerminationResult = + /** A released bare PID, without lifetime proof or metadata cleanup authority. */ + | Readonly<{ status: 'not-running' }> + | Readonly<{ + status: 'exited'; + identity: DaemonProcessIdentity; + mode: 'already-exited' | 'graceful' | 'forced'; + }> + | Readonly<{ + status: 'retained'; + reason: 'missing-start-time' | 'identity-unverified' | 'signal-failed' | 'exit-timeout'; + signal?: NodeJS.Signals; + }>; -export async function stopProcessForTakeover( - pid: number, +/** Owns every signal and exit wait for one observed daemon lifetime. */ +export async function stopDaemonProcess( + observed: Readonly, options: { + mode: 'graceful' | 'force'; termTimeoutMs: number; killTimeoutMs: number; - expectedStartTime: string | undefined; }, -): Promise { - if (!options.expectedStartTime) return; - const identity: DaemonProcessIdentity = { pid, startTime: options.expectedStartTime }; - if (!signalDaemonIdentity(identity, 'SIGTERM')) return; - if ((await waitForDaemonExit(identity, { timeoutMs: options.termTimeoutMs })).exited) return; - if (!signalDaemonIdentity(identity, 'SIGKILL')) return; - await waitForDaemonExit(identity, { timeoutMs: options.killTimeoutMs }); +): Promise { + if (!observed.startTime?.trim()) { + if (!isProcessAlive(observed.pid)) return { status: 'not-running' }; + return { status: 'retained', reason: 'missing-start-time' }; + } + const identity: DaemonProcessIdentity = { pid: observed.pid, startTime: observed.startTime }; + let mode: 'already-exited' | 'graceful' | 'forced' = 'already-exited'; + const confirmed = (): DaemonTerminationResult => ({ + status: 'exited', + identity, + mode, + }); + if ((await waitForDaemonExit(identity, { timeoutMs: 0 })).exited) return confirmed(); + for (const signal of options.mode === 'force' + ? (['SIGKILL'] as const) + : (['SIGTERM', 'SIGKILL'] as const)) { + const verified = isAgentDeviceDaemonProcess(identity.pid, identity.startTime); + const signaled = verified && trySignalProcess(identity.pid, signal); + if (signaled) mode = signal === 'SIGTERM' ? 'graceful' : 'forced'; + const timeoutMs = signal === 'SIGTERM' ? options.termTimeoutMs : options.killTimeoutMs; + if ( + (await waitForDaemonExit(identity, { timeoutMs: mode === 'already-exited' ? 0 : timeoutMs })) + .exited + ) { + return confirmed(); + } + if (!signaled) + return { + status: 'retained', + signal, + reason: verified ? 'signal-failed' : 'identity-unverified', + }; + } + return { status: 'retained', reason: 'exit-timeout' }; } diff --git a/src/daemon/__tests__/daemon-stop.test.ts b/src/daemon/__tests__/daemon-stop.test.ts index 5a524ecf40..8a294910b6 100644 --- a/src/daemon/__tests__/daemon-stop.test.ts +++ b/src/daemon/__tests__/daemon-stop.test.ts @@ -1,25 +1,13 @@ -import assert from 'node:assert/strict'; import fs from 'node:fs'; import { afterEach, expect, test, vi } from 'vitest'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; const mocks = vi.hoisted(() => ({ - isAgentDeviceDaemonProcess: vi.fn(), - isProcessAlive: vi.fn(), + stopDaemonProcess: vi.fn(), sleep: vi.fn(async () => undefined), - trySignalProcess: vi.fn(), - waitForDaemonExit: vi.fn(), })); -vi.mock('../../daemon-process.ts', () => ({ - isAgentDeviceDaemonProcess: mocks.isAgentDeviceDaemonProcess, - trySignalProcess: mocks.trySignalProcess, - waitForDaemonExit: mocks.waitForDaemonExit, -})); -vi.mock('@agent-device/host-kit/process', async (importOriginal) => ({ - ...(await importOriginal()), - isProcessAlive: mocks.isProcessAlive, -})); +vi.mock('../../daemon-process.ts', () => ({ stopDaemonProcess: mocks.stopDaemonProcess })); vi.mock('@agent-device/host-kit/retry', async (importOriginal) => ({ ...(await importOriginal()), sleep: mocks.sleep, @@ -29,7 +17,7 @@ import { resolveDaemonPaths } from '../../daemon-resolution.ts'; import { stopDaemon } from '../daemon-stop.ts'; afterEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); }); function createDaemonPaths(): ReturnType { @@ -55,112 +43,100 @@ test('reports not-running when daemon metadata is absent', async () => { } }); -test('refuses to signal a live process whose daemon identity cannot be verified', async () => { +test('retained identity verification is reported as failure without known cleanup', async () => { const paths = createDaemonPaths(); - mocks.isAgentDeviceDaemonProcess.mockReturnValue(false); - mocks.isProcessAlive.mockReturnValue(true); - - try { - await assert.rejects( - async () => await stopDaemon({ paths }), - (error: { code?: string }) => error.code === 'COMMAND_FAILED', - ); - expect(mocks.trySignalProcess).not.toHaveBeenCalled(); - } finally { - removeDaemonPaths(paths); - } + mocks.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason: 'identity-unverified' }); + await expect(stopDaemon({ paths })).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + details: { reason: 'daemon_exit_unconfirmed', terminationReason: 'identity-unverified' }, + }); + expect(mocks.stopDaemonProcess).toHaveBeenCalledWith( + { pid: 123, startTime: 'start-time' }, + { + mode: 'graceful', + termTimeoutMs: 10_000, + killTimeoutMs: 2_000, + }, + ); }); -test('refuses to signal a live process when daemon metadata lacks a non-empty start-time identity', async () => { +test('missing start-time identity is passed to the owning termination operation', async () => { const paths = createDaemonPaths(); fs.writeFileSync(paths.infoPath, JSON.stringify({ pid: 123, processStartTime: ' ' })); - mocks.isProcessAlive.mockReturnValue(true); - - try { - await assert.rejects( - async () => await stopDaemon({ paths }), - (error: { code?: string }) => error.code === 'COMMAND_FAILED', - ); - expect(mocks.isAgentDeviceDaemonProcess).not.toHaveBeenCalled(); - expect(mocks.trySignalProcess).not.toHaveBeenCalled(); - } finally { - removeDaemonPaths(paths); - } + mocks.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason: 'missing-start-time' }); + await expect(stopDaemon({ paths })).rejects.toMatchObject({ + details: { terminationReason: 'missing-start-time' }, + }); + expect(mocks.stopDaemonProcess).toHaveBeenCalledWith( + { pid: 123, startTime: null }, + expect.anything(), + ); }); -test('treats an exited daemon between identity verification and SIGTERM as not-running', async () => { - const paths = createDaemonPaths(); - mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); - mocks.trySignalProcess.mockReturnValue(false); - mocks.isProcessAlive.mockReturnValue(false); +test('a previously exited verified lifetime is reported as not-running', async () => { + mocks.stopDaemonProcess.mockResolvedValue({ + status: 'exited', + mode: 'already-exited', + identity: { pid: 123, startTime: 'start-time' }, + }); + expect(await stopDaemon({ paths: createDaemonPaths() })).toMatchObject({ + stopped: false, + mode: 'not-running', + }); +}); - try { - const result = await stopDaemon({ paths }); - expect(result).toMatchObject({ stopped: false, mode: 'not-running' }); - } finally { - removeDaemonPaths(paths); - } +test('an already released pid without start time remains not-running without cleanup proof', async () => { + mocks.stopDaemonProcess.mockResolvedValue({ status: 'not-running' }); + expect(await stopDaemon({ paths: createDaemonPaths() })).toMatchObject({ + stopped: false, + mode: 'not-running', + }); }); -test('reports graceful cleanup after SIGTERM exits the verified daemon', async () => { +test('confirmed TERM exit preserves graceful report behavior and configured budgets', async () => { const paths = createDaemonPaths(); - mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); - mocks.trySignalProcess.mockReturnValue(true); - mocks.waitForDaemonExit.mockImplementation(async () => { + mocks.stopDaemonProcess.mockImplementation(async () => { fs.rmSync(paths.infoPath, { force: true }); - return { exited: true, elapsedMs: 0 }; + return { status: 'exited', mode: 'graceful', identity: { pid: 123, startTime: 'start-time' } }; }); - - try { - const result = await stopDaemon({ paths }); - expect(result).toMatchObject({ - stopped: true, + expect(await stopDaemon({ paths, graceTimeoutMs: 11, killTimeoutMs: 7 })).toMatchObject({ + stopped: true, + mode: 'graceful', + cleanupConfidence: 'known', + providerReleases: { pending: [] }, + }); + expect(mocks.stopDaemonProcess).toHaveBeenCalledWith( + { pid: 123, startTime: 'start-time' }, + { mode: 'graceful', - cleanupConfidence: 'known', - providerReleases: { pending: [] }, - }); - expect(mocks.trySignalProcess).toHaveBeenCalledWith(123, 'SIGTERM'); - } finally { - removeDaemonPaths(paths); - } + termTimeoutMs: 11, + killTimeoutMs: 7, + }, + ); }); -test('re-verifies identity before SIGKILL and reports forced cleanup as unknown', async () => { - const paths = createDaemonPaths(); - mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); - mocks.trySignalProcess.mockReturnValue(true); - mocks.waitForDaemonExit - .mockResolvedValueOnce({ exited: false, elapsedMs: 0 }) - .mockResolvedValueOnce({ exited: true, elapsedMs: 0 }); - - try { - const result = await stopDaemon({ paths }); - expect(result).toMatchObject({ - stopped: true, - mode: 'forced', - cleanupConfidence: 'unknown', - providerReleases: { pending: null }, - }); - expect(mocks.trySignalProcess).toHaveBeenNthCalledWith(1, 123, 'SIGTERM'); - expect(mocks.trySignalProcess).toHaveBeenNthCalledWith(2, 123, 'SIGKILL'); - } finally { - removeDaemonPaths(paths); - } +test('confirmed KILL exit preserves unknown provider cleanup', async () => { + mocks.stopDaemonProcess.mockResolvedValue({ + status: 'exited', + mode: 'forced', + identity: { pid: 123, startTime: 'start-time' }, + }); + expect(await stopDaemon({ paths: createDaemonPaths() })).toMatchObject({ + stopped: true, + mode: 'forced', + cleanupConfidence: 'unknown', + providerReleases: { status: 'unknown', pending: null }, + warnings: [expect.stringContaining('force-killed')], + }); }); -test('does not send SIGKILL if the daemon identity changes during the graceful wait', async () => { - const paths = createDaemonPaths(); - mocks.isAgentDeviceDaemonProcess.mockReturnValueOnce(true).mockReturnValueOnce(false); - mocks.trySignalProcess.mockReturnValue(true); - mocks.waitForDaemonExit - .mockResolvedValueOnce({ exited: false, elapsedMs: 0 }) - .mockResolvedValueOnce({ exited: true, elapsedMs: 0 }); - - try { - await stopDaemon({ paths }); - expect(mocks.trySignalProcess).toHaveBeenCalledTimes(1); - expect(mocks.trySignalProcess).toHaveBeenCalledWith(123, 'SIGTERM'); - } finally { - removeDaemonPaths(paths); - } -}); +test.each(['signal-failed', 'exit-timeout'])( + '%s cannot become a successful stop', + async (reason) => { + mocks.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason }); + await expect(stopDaemon({ paths: createDaemonPaths() })).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + details: { reason: 'daemon_exit_unconfirmed', terminationReason: reason }, + }); + }, +); diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index 0aaf2ac99b..bf5be1697b 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -1,12 +1,6 @@ import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; -import { - isAgentDeviceDaemonProcess, - trySignalProcess, - waitForDaemonExit, - type DaemonProcessIdentity, -} from '../daemon-process.ts'; -import { isProcessAlive } from '@agent-device/host-kit/process'; +import { stopDaemonProcess } from '../daemon-process.ts'; import { sleep } from '@agent-device/host-kit/retry'; import type { DaemonPaths } from '../daemon-resolution.ts'; @@ -51,29 +45,24 @@ export async function stopDaemon(params: { }): Promise { const info = readRegisteredDaemonIdentity(params.paths.infoPath); if (!info) return notRunningResult(); - if (!info.startTime) { - if (!isProcessAlive(info.pid)) return notRunningResult(); - throw new AppError( - 'COMMAND_FAILED', - 'Refusing to stop a daemon without a verified process start-time identity.', - { pid: info.pid }, - ); + const termination = await stopDaemonProcess(info, { + mode: 'graceful', + termTimeoutMs: params.graceTimeoutMs ?? DAEMON_STOP_GRACE_TIMEOUT_MS, + killTimeoutMs: params.killTimeoutMs ?? DAEMON_STOP_KILL_TIMEOUT_MS, + }); + if (termination.status === 'retained') { + throw new AppError('COMMAND_FAILED', 'Daemon termination could not be confirmed.', { + pid: info.pid, + processStartTime: info.startTime, + reason: 'daemon_exit_unconfirmed', + terminationReason: termination.reason, + signal: termination.signal, + }); } - if (!isAgentDeviceDaemonProcess(info.pid, info.startTime)) { - if (!isProcessAlive(info.pid)) return notRunningResult(); - throw new AppError( - 'COMMAND_FAILED', - 'Refusing to stop a daemon whose PID or start-time identity could not be verified.', - { pid: info.pid, processStartTime: info.startTime }, - ); + if (termination.status === 'not-running' || termination.mode === 'already-exited') { + return notRunningResult(); } - - const identity: DaemonProcessIdentity = { pid: info.pid, startTime: info.startTime }; - if (!signalDaemonProcess(info.pid, 'SIGTERM')) return notRunningResult(); - const { exited: graceful } = await waitForDaemonExit(identity, { - timeoutMs: params.graceTimeoutMs ?? DAEMON_STOP_GRACE_TIMEOUT_MS, - }); - if (graceful) { + if (termination.mode === 'graceful') { await waitForDaemonMetadataRemoval(params.paths, DAEMON_STOP_METADATA_WAIT_MS); return { stopped: true, @@ -88,17 +77,6 @@ export async function stopDaemon(params: { }; } - // Re-verify immediately before escalation so a PID cannot be reused between - // the graceful wait and SIGKILL. - if (isAgentDeviceDaemonProcess(info.pid, info.startTime)) { - signalDaemonProcess(info.pid, 'SIGKILL'); - } - const { exited: stopped } = await waitForDaemonExit(identity, { - timeoutMs: params.killTimeoutMs ?? DAEMON_STOP_KILL_TIMEOUT_MS, - }); - if (!stopped) { - throw new AppError('COMMAND_FAILED', 'Daemon did not exit after SIGKILL.', { pid: info.pid }); - } return { stopped: true, mode: 'forced', @@ -114,15 +92,6 @@ export async function stopDaemon(params: { }; } -function signalDaemonProcess(pid: number, signal: NodeJS.Signals): boolean { - if (trySignalProcess(pid, signal)) return true; - if (!isProcessAlive(pid)) return false; - throw new AppError('COMMAND_FAILED', `Daemon could not be signaled with ${signal}.`, { - pid, - signal, - }); -} - export function readDaemonStopIdentity( infoPath: string, ): { pid: number; processStartTime: string } | null { diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index 7a58a970b3..3aa7c437c3 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -4,7 +4,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; -import { stopProcessForTakeover } from '../../src/daemon-process.ts'; +import { stopDaemonProcess } from '../../src/daemon-process.ts'; import { isProcessAlive } from '@agent-device/host-kit/process'; import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; @@ -73,25 +73,23 @@ 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, - }); + await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); // #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 + // stays set until this passes: an unconfirmed 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, { - termTimeoutMs: 1_500, - killTimeoutMs: 1_500, - expectedStartTime: info.processStartTime, - }); + await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); } fs.rmSync(stateDir, { recursive: true, force: true }); } diff --git a/test/integration/smoke-daemon-clean.test.ts b/test/integration/smoke-daemon-clean.test.ts index ac62ccd2af..0bf94aab8f 100644 --- a/test/integration/smoke-daemon-clean.test.ts +++ b/test/integration/smoke-daemon-clean.test.ts @@ -5,8 +5,8 @@ import os from 'node:os'; import path from 'node:path'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; import { runCmdSync } from '@agent-device/host-kit/command'; -import { isProcessAlive } from '@agent-device/host-kit/process'; -import { stopProcessForTakeover } from '../../src/daemon-process.ts'; +import { isProcessAlive, readProcessStartTime } from '@agent-device/host-kit/process'; +import { stopDaemonProcess } from '../../src/daemon-process.ts'; import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; @@ -16,6 +16,32 @@ type DaemonInfo = { processStartTime?: string; }; +test('clean daemon retains metadata when a live recorded process is not a verified daemon', () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-clean-retained-')); + const identity = { pid: process.pid, processStartTime: readProcessStartTime(process.pid) }; + assert.ok(identity.processStartTime); + const contents = JSON.stringify(identity); + fs.writeFileSync(path.join(stateDir, 'daemon.json'), contents); + fs.writeFileSync(path.join(stateDir, 'daemon.lock'), contents); + try { + const cleanup = runCmdSync( + process.execPath, + ['--experimental-strip-types', 'scripts/clean-daemon.ts'], + { + env: { ...process.env, AGENT_DEVICE_STATE_DIR: stateDir }, + timeoutMs: 5_000, + allowFailure: true, + }, + ); + assert.notEqual(cleanup.exitCode, 0, 'unconfirmed exit cannot complete cleanup'); + assert.equal(fs.readFileSync(path.join(stateDir, 'daemon.json'), 'utf8'), contents); + assert.equal(fs.readFileSync(path.join(stateDir, 'daemon.lock'), 'utf8'), contents); + assert.equal(isProcessAlive(process.pid), true); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + test('clean daemon script stops a live daemon before removing metadata', async (t) => { if (await skipWhenLoopbackUnavailable(t)) { return; @@ -48,11 +74,10 @@ test('clean daemon script stops a live daemon before removing metadata', async ( await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { if (info) { - await stopProcessForTakeover(info.pid, { - termTimeoutMs: 1_500, - killTimeoutMs: 1_500, - expectedStartTime: info.processStartTime, - }); + await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); } fs.rmSync(stateDir, { recursive: true, force: true }); } diff --git a/test/integration/smoke-daemon-http.test.ts b/test/integration/smoke-daemon-http.test.ts index 3a3544c391..a2c66e3b5d 100644 --- a/test/integration/smoke-daemon-http.test.ts +++ b/test/integration/smoke-daemon-http.test.ts @@ -5,7 +5,7 @@ import os from 'node:os'; import path from 'node:path'; import { DAEMON_RPC_PROTOCOL_VERSION } from '@agent-device/contracts/daemon-http'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; -import { stopProcessForTakeover } from '../../src/daemon-process.ts'; +import { stopDaemonProcess } from '../../src/daemon-process.ts'; import { formatResultDebug } from './cli-json.ts'; import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; @@ -113,9 +113,8 @@ async function callCommandRpc( 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, - }); + await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1500, killTimeoutMs: 1500 }, + ); } diff --git a/test/integration/smoke-web-platform.test.ts b/test/integration/smoke-web-platform.test.ts index 746908c05a..285e86d229 100644 --- a/test/integration/smoke-web-platform.test.ts +++ b/test/integration/smoke-web-platform.test.ts @@ -17,7 +17,7 @@ import { type AgentBrowserToolStatus, } from '@agent-device/platform-web'; import { - stopProcessForTakeover, + stopDaemonProcess, waitForDaemonExit, type DaemonProcessIdentity, } from '../../src/daemon-process.ts'; @@ -227,7 +227,7 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise { } // Best-effort and independent of how far the `try` block got: a daemon that survived SIGTERM -// (stopProcessForTakeover escalates to SIGKILL) and a Chrome fleet that outlived it (a forceful +// (stopDaemonProcess escalates to SIGKILL) and a Chrome fleet that outlived it (a forceful // reap, not cleanupManagedAgentBrowserOrphans — see forceKillManagedBrowserProcesses) are reaped // here regardless of which assertion above failed, or whether none did. Mirrors cleanupWebSmoke's // AggregateError shape so a cleanup failure never silently swallows the assertion failure it ran @@ -244,11 +244,14 @@ async function cleanupWebShutdownSmoke( const errors: unknown[] = []; if (daemonIdentity !== undefined) { try { - await stopProcessForTakeover(daemonIdentity.pid, { - termTimeoutMs: timeouts.termTimeoutMs, - killTimeoutMs: timeouts.killTimeoutMs, - expectedStartTime: daemonIdentity.startTime, - }); + await stopDaemonProcess( + { pid: daemonIdentity.pid, startTime: daemonIdentity.startTime ?? null }, + { + mode: 'graceful', + termTimeoutMs: timeouts.termTimeoutMs, + killTimeoutMs: timeouts.killTimeoutMs, + }, + ); } catch (error) { errors.push(error); } diff --git a/test/integration/support/daemon-leak-model.test.ts b/test/integration/support/daemon-leak-model.test.ts index b32c50313a..7c3c535590 100644 --- a/test/integration/support/daemon-leak-model.test.ts +++ b/test/integration/support/daemon-leak-model.test.ts @@ -158,7 +158,7 @@ describe('recorded owned-process rules', () => { }); describe('surviving-daemon rule', () => { - // stopProcessForTakeover is best-effort void: it returns silently on identity + // A retained stopDaemonProcess result is not proof of exit: 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( diff --git a/test/integration/support/daemon-leak-model.ts b/test/integration/support/daemon-leak-model.ts index a3f9cc4a8f..0a47f1c80a 100644 --- a/test/integration/support/daemon-leak-model.ts +++ b/test/integration/support/daemon-leak-model.ts @@ -6,10 +6,9 @@ // Three 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. +// the leak. `stopDaemonProcess` returns a retained result when exit +// cannot be confirmed. The oracle still checks actual +// liveness rather than accepting an attempted stop. // 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 From 7c28b80884c5609350ebbe9d278fabb178884050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 2 Oct 2026 22:42:10 +0200 Subject: [PATCH 2/7] refactor: compose verified signal and exit waits without a public signaling primitive --- src/daemon-process.ts | 71 ++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/daemon-process.ts b/src/daemon-process.ts index ddc9f610c4..862d1eac4f 100644 --- a/src/daemon-process.ts +++ b/src/daemon-process.ts @@ -39,7 +39,7 @@ export function isAgentDeviceDaemonProcess( return isAgentDeviceDaemonCommand(command); } -export function trySignalProcess(pid: number, signal: NodeJS.Signals): boolean { +function trySignalProcess(pid: number, signal: NodeJS.Signals): boolean { try { process.kill(pid, signal); return true; @@ -116,32 +116,47 @@ export async function stopDaemonProcess( return { status: 'retained', reason: 'missing-start-time' }; } const identity: DaemonProcessIdentity = { pid: observed.pid, startTime: observed.startTime }; - let mode: 'already-exited' | 'graceful' | 'forced' = 'already-exited'; - const confirmed = (): DaemonTerminationResult => ({ - status: 'exited', - identity, - mode, - }); - if ((await waitForDaemonExit(identity, { timeoutMs: 0 })).exited) return confirmed(); - for (const signal of options.mode === 'force' - ? (['SIGKILL'] as const) - : (['SIGTERM', 'SIGKILL'] as const)) { - const verified = isAgentDeviceDaemonProcess(identity.pid, identity.startTime); - const signaled = verified && trySignalProcess(identity.pid, signal); - if (signaled) mode = signal === 'SIGTERM' ? 'graceful' : 'forced'; - const timeoutMs = signal === 'SIGTERM' ? options.termTimeoutMs : options.killTimeoutMs; - if ( - (await waitForDaemonExit(identity, { timeoutMs: mode === 'already-exited' ? 0 : timeoutMs })) - .exited - ) { - return confirmed(); - } - if (!signaled) - return { - status: 'retained', - signal, - reason: verified ? 'signal-failed' : 'identity-unverified', - }; + if ((await waitForDaemonExit(identity, { timeoutMs: 0 })).exited) { + return { status: 'exited', identity, mode: 'already-exited' }; + } + let previousSignal: 'SIGTERM' | undefined; + if (options.mode === 'graceful') { + const result = await signalAndWaitForDaemonExit(identity, { + signal: 'SIGTERM', + timeoutMs: options.termTimeoutMs, + }); + if (result.status !== 'survived') return result; + previousSignal = 'SIGTERM'; } - return { status: 'retained', reason: 'exit-timeout' }; + const result = await signalAndWaitForDaemonExit(identity, { + signal: 'SIGKILL', + timeoutMs: options.killTimeoutMs, + previousSignal, + }); + return result.status === 'survived' ? { status: 'retained', reason: 'exit-timeout' } : result; +} + +async function signalAndWaitForDaemonExit( + identity: DaemonProcessIdentity, + options: { signal: 'SIGTERM' | 'SIGKILL'; timeoutMs: number; previousSignal?: 'SIGTERM' }, +): Promise> { + const verified = isAgentDeviceDaemonProcess(identity.pid, identity.startTime); + const signaled = verified && trySignalProcess(identity.pid, options.signal); + const mode = signaled + ? options.signal === 'SIGTERM' + ? 'graceful' + : 'forced' + : options.previousSignal + ? 'graceful' + : 'already-exited'; + const wait = await waitForDaemonExit(identity, { + timeoutMs: mode === 'already-exited' ? 0 : options.timeoutMs, + }); + if (wait.exited) return { status: 'exited', identity, mode }; + if (signaled) return { status: 'survived' }; + return { + status: 'retained', + signal: options.signal, + reason: verified ? 'signal-failed' : 'identity-unverified', + }; } From 89efd123a454a4f31fecec8d9058a1ceeaf727d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 2 Oct 2026 23:32:35 +0200 Subject: [PATCH 3/7] fix: preserve retained daemon termination outcomes --- .../__tests__/daemon-client-metadata.test.ts | 37 ++++++++++++++++++- src/daemon-client/daemon-client-metadata.ts | 15 +++++++- .../daemon-replace-exit-flush.test.ts | 6 ++- test/integration/smoke-daemon-clean.test.ts | 7 +++- test/integration/smoke-daemon-http.test.ts | 3 +- test/integration/smoke-web-platform.test.ts | 5 ++- 6 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/daemon-client/__tests__/daemon-client-metadata.test.ts b/src/daemon-client/__tests__/daemon-client-metadata.test.ts index ec743577cd..777696f35b 100644 --- a/src/daemon-client/__tests__/daemon-client-metadata.test.ts +++ b/src/daemon-client/__tests__/daemon-client-metadata.test.ts @@ -1,11 +1,20 @@ import assert from 'node:assert/strict'; 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 } 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()), + 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 +56,27 @@ 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/); + }); +} diff --git a/src/daemon-client/daemon-client-metadata.ts b/src/daemon-client/daemon-client-metadata.ts index bac92e3197..155bd14f85 100644 --- a/src/daemon-client/daemon-client-metadata.ts +++ b/src/daemon-client/daemon-client-metadata.ts @@ -1,4 +1,5 @@ import fs from 'node:fs'; +import { AppError } from '@agent-device/kernel/errors'; import { shellQuote } from '@agent-device/kernel/device-shell'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { @@ -202,7 +203,7 @@ export async function cleanupFailedDaemonStartupMetadata( result.retainedLockProcess = true; } else { if (liveLockProcess) { - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: lockInfo.pid, startTime: lockInfo.processStartTime ?? null }, { mode: 'graceful', @@ -210,6 +211,7 @@ export async function cleanupFailedDaemonStartupMetadata( killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, }, ); + requireDaemonExit(termination); result.stoppedLockProcess = true; } removeDaemonLock(paths.lockPath); @@ -256,7 +258,7 @@ export async function recoverDaemonLockHolder(paths: DaemonPaths): Promise { - return await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: info.pid, startTime: info.processStartTime ?? null }, { mode: 'graceful', @@ -264,6 +266,15 @@ export async function stopDaemonProcessForTakeover( killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, }, ); + requireDaemonExit(termination); + return termination; +} + +function requireDaemonExit(termination: DaemonTerminationResult): void { + if (termination.status !== 'retained') return; + throw new AppError('COMMAND_FAILED', 'Daemon exit could not be confirmed.', { + details: { reason: 'daemon_exit_unconfirmed', termination }, + }); } export function isRemoteDaemon(info: DaemonInfo): boolean { diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index 3aa7c437c3..638fd3fc74 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -73,10 +73,11 @@ test('daemon replace mid-command returns a structured, parseable error and exits info = readDaemonInfo(stateDir); daemonPids.push(info.pid); - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: info.pid, startTime: info.processStartTime ?? null }, { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); // #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: an unconfirmed stop must still reach the @@ -86,10 +87,11 @@ test('daemon replace mid-command returns a structured, parseable error and exits info = null; } finally { if (info) { - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: info.pid, startTime: info.processStartTime ?? null }, { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); } fs.rmSync(stateDir, { recursive: true, force: true }); } diff --git a/test/integration/smoke-daemon-clean.test.ts b/test/integration/smoke-daemon-clean.test.ts index 0bf94aab8f..3e2897ea2d 100644 --- a/test/integration/smoke-daemon-clean.test.ts +++ b/test/integration/smoke-daemon-clean.test.ts @@ -34,6 +34,10 @@ test('clean daemon retains metadata when a live recorded process is not a verifi }, ); assert.notEqual(cleanup.exitCode, 0, 'unconfirmed exit cannot complete cleanup'); + assert.match( + cleanup.stderr, + /Daemon cleanup retained state because exit could not be confirmed/, + ); assert.equal(fs.readFileSync(path.join(stateDir, 'daemon.json'), 'utf8'), contents); assert.equal(fs.readFileSync(path.join(stateDir, 'daemon.lock'), 'utf8'), contents); assert.equal(isProcessAlive(process.pid), true); @@ -74,10 +78,11 @@ test('clean daemon script stops a live daemon before removing metadata', async ( await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { if (info) { - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: info.pid, startTime: info.processStartTime ?? null }, { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); } fs.rmSync(stateDir, { recursive: true, force: true }); } diff --git a/test/integration/smoke-daemon-http.test.ts b/test/integration/smoke-daemon-http.test.ts index a2c66e3b5d..6b49acd042 100644 --- a/test/integration/smoke-daemon-http.test.ts +++ b/test/integration/smoke-daemon-http.test.ts @@ -113,8 +113,9 @@ async function callCommandRpc( async function stopDaemon(info: DaemonInfo): Promise { if (!Number.isInteger(info.pid) || info.pid <= 0) return; - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: info.pid, startTime: info.processStartTime ?? null }, { mode: 'graceful', termTimeoutMs: 1500, killTimeoutMs: 1500 }, ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); } diff --git a/test/integration/smoke-web-platform.test.ts b/test/integration/smoke-web-platform.test.ts index 285e86d229..fbf2b83e4e 100644 --- a/test/integration/smoke-web-platform.test.ts +++ b/test/integration/smoke-web-platform.test.ts @@ -244,7 +244,7 @@ async function cleanupWebShutdownSmoke( const errors: unknown[] = []; if (daemonIdentity !== undefined) { try { - await stopDaemonProcess( + const termination = await stopDaemonProcess( { pid: daemonIdentity.pid, startTime: daemonIdentity.startTime ?? null }, { mode: 'graceful', @@ -252,6 +252,9 @@ async function cleanupWebShutdownSmoke( killTimeoutMs: timeouts.killTimeoutMs, }, ); + if (termination.status === 'retained') { + errors.push(new Error(`Daemon cleanup retained the process: ${termination.reason}`)); + } } catch (error) { errors.push(error); } From fe45ce9441a7372171a1fbb6f507e246c759e8d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 2 Oct 2026 23:33:52 +0200 Subject: [PATCH 4/7] refactor: keep web smoke termination checks together --- test/integration/smoke-web-platform.test.ts | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/integration/smoke-web-platform.test.ts b/test/integration/smoke-web-platform.test.ts index fbf2b83e4e..a9a3751eb2 100644 --- a/test/integration/smoke-web-platform.test.ts +++ b/test/integration/smoke-web-platform.test.ts @@ -244,17 +244,7 @@ async function cleanupWebShutdownSmoke( const errors: unknown[] = []; if (daemonIdentity !== undefined) { try { - const termination = await stopDaemonProcess( - { pid: daemonIdentity.pid, startTime: daemonIdentity.startTime ?? null }, - { - mode: 'graceful', - termTimeoutMs: timeouts.termTimeoutMs, - killTimeoutMs: timeouts.killTimeoutMs, - }, - ); - if (termination.status === 'retained') { - errors.push(new Error(`Daemon cleanup retained the process: ${termination.reason}`)); - } + await stopWebSmokeDaemon(daemonIdentity, timeouts); } catch (error) { errors.push(error); } @@ -275,6 +265,16 @@ async function cleanupWebShutdownSmoke( if (errors.length > 1) throw new AggregateError(errors, 'web shutdown smoke cleanup failed'); } +async function stopWebSmokeDaemon( + identity: DaemonProcessIdentity, + timeouts: { termTimeoutMs: number; killTimeoutMs: number }, +): Promise { + const termination = await stopDaemonProcess(identity, { mode: 'graceful', ...timeouts }); + if (termination.status === 'retained') { + throw new Error(`Daemon cleanup retained the process: ${termination.reason}`); + } +} + // Deliberately NOT cleanupManagedAgentBrowserOrphans: that function exists to leave an actively // used fleet alone, and skips killing anything once it sees activity inside the (here, minutes- // long) idle window — precisely the state this test's own fleet is always in. A forced safety-net From 5d551071a26c8afdf07341516ddc0205e512aa1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 3 Oct 2026 00:32:57 +0200 Subject: [PATCH 5/7] fix: recognize verified zombie daemon termination --- src/__tests__/daemon-exit-wait.test.ts | 12 ++++++++++-- src/__tests__/daemon-process-takeover.test.ts | 1 + src/daemon-process.ts | 11 +++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/__tests__/daemon-exit-wait.test.ts b/src/__tests__/daemon-exit-wait.test.ts index 2a6c375d1d..2111bc2f3b 100644 --- a/src/__tests__/daemon-exit-wait.test.ts +++ b/src/__tests__/daemon-exit-wait.test.ts @@ -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, ''); 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( diff --git a/src/__tests__/daemon-process-takeover.test.ts b/src/__tests__/daemon-process-takeover.test.ts index 2be1d815d0..78156ffc64 100644 --- a/src/__tests__/daemon-process-takeover.test.ts +++ b/src/__tests__/daemon-process-takeover.test.ts @@ -59,6 +59,7 @@ test('stops a branch-named daemon before replacement can strand its session', as { mode: 'graceful', ...TAKEOVER_TIMEOUTS }, ); assert.equal(result.status, 'exited'); + await spawnedChildren.at(-1)!.exited; assert.equal(isProcessAlive(pid), false); }); diff --git a/src/daemon-process.ts b/src/daemon-process.ts index 862d1eac4f..fee88d3622 100644 --- a/src/daemon-process.ts +++ b/src/daemon-process.ts @@ -57,7 +57,7 @@ export type DaemonProcessIdentity = Readonly<{ }>; export type DaemonExitWait = { - /** The pid was released, or the host handed it to a different process. */ + /** The process terminated, or the host handed its pid to a different process. */ exited: boolean; elapsedMs: number; }; @@ -65,8 +65,8 @@ export type DaemonExitWait = { const DAEMON_EXIT_POLL_MS = 100; /** - * Resolves once `identity` has left the host — released or recycled. A pid still - * being torn down is neither, so the wait continues until the number is free. + * Confirms termination from a released pid, a verified zombie, or a different + * readable process lifetime. An unreadable process observation is not proof. */ export async function waitForDaemonExit( identity: DaemonProcessIdentity, @@ -78,7 +78,10 @@ export async function waitForDaemonExit( const hasExited = (): boolean => { if (!isProcessAlive(identity.pid)) return true; const observed = readHostProcessIdentityObservations([identity.pid]).get(identity.pid); - return Boolean(observed?.startTime && observed.startTime !== identity.startTime); + return Boolean( + observed?.state.startsWith('Z') || + (observed?.startTime && observed.startTime !== identity.startTime), + ); }; let exited = hasExited(); while (!exited && Date.now() < deadline) { From 33a577d1b80053b1025cf673090787287c1eb0b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 3 Oct 2026 00:42:56 +0200 Subject: [PATCH 6/7] fix: preserve primary errors during refused daemon cleanup --- .../__tests__/daemon-client-metadata.test.ts | 19 +++++++- .../daemon-client-timeout-route.test.ts | 46 ++++++++++++++++++- src/daemon-client/daemon-client-metadata.ts | 3 +- src/daemon-client/daemon-client-timeout.ts | 10 +++- .../daemon-replace-exit-flush.test.ts | 18 +++++--- test/integration/smoke-daemon-clean.test.ts | 18 +++++--- test/integration/smoke-daemon-http.test.ts | 10 ++-- 7 files changed, 101 insertions(+), 23 deletions(-) diff --git a/src/daemon-client/__tests__/daemon-client-metadata.test.ts b/src/daemon-client/__tests__/daemon-client-metadata.test.ts index 777696f35b..2ecfce7cae 100644 --- a/src/daemon-client/__tests__/daemon-client-metadata.test.ts +++ b/src/daemon-client/__tests__/daemon-client-metadata.test.ts @@ -1,11 +1,16 @@ 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 { 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, cleanupFailedDaemonStartupMetadata } 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'; @@ -80,3 +85,15 @@ for (const artifact of ['daemon.json', 'daemon.lock']) { 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; + }, + ); +}); diff --git a/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts b/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts index 195960e862..f02582c40e 100644 --- a/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts +++ b/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts @@ -27,9 +27,18 @@ import http from 'node:http'; import path from 'node:path'; import assert from 'node:assert/strict'; -import { beforeEach, test, vi } from 'vitest'; +import { beforeEach, afterEach, test, vi } from 'vitest'; -const { mockRunCmdSync } = vi.hoisted(() => ({ mockRunCmdSync: vi.fn() })); +const { mockRunCmdSync, mockIsDaemon, mockStop } = vi.hoisted(() => ({ + mockRunCmdSync: vi.fn(), + mockIsDaemon: vi.fn(), + mockStop: vi.fn(), +})); +vi.mock('../../daemon-process.ts', async (importOriginal) => ({ + ...(await importOriginal()), + isAgentDeviceDaemonProcess: mockIsDaemon, + stopDaemonProcess: mockStop, +})); vi.mock('@agent-device/host-kit/command', async () => { const actual = await vi.importActual( @@ -120,7 +129,10 @@ function startHangingHttpServer(): Promise<{ server: http.Server; port: number } beforeEach(() => { mockRunCmdSync.mockReset(); + mockIsDaemon.mockReset(); + mockStop.mockReset(); }); +afterEach(() => vi.restoreAllMocks()); test('socket timeout: pkill cleanup still runs for a declared non-Apple platform that actually terminates a runner (rebound-session case), and the hint claims Apple on that evidence', async () => { // Simulates --session-lock strip silently rebinding this request onto an @@ -266,3 +278,33 @@ test('remote HTTP timeout never runs the Apple pkill cleanup and uses the remote assert.equal(mockRunCmdSync.mock.calls.length, 0); }); + +test('a refused timeout fallback preserves the timeout without an unhandled rejection', async () => { + mockRunCmdSync.mockReturnValue({ exitCode: 1, stdout: '', stderr: '' }); + mockIsDaemon.mockReturnValue(true); + mockStop.mockResolvedValue({ status: 'retained', reason: 'exit-timeout' }); + vi.spyOn(process, 'kill').mockImplementation(() => { + throw Object.assign(new Error('refused'), { code: 'EPERM' }); + }); + const { server, port } = await startHangingSocketServer(); + try { + await assert.rejects( + sendRequest( + { port, pid: 7, token: 'test-token', processStartTime: 'start' }, + { ...buildRequest(undefined), command: 'open' }, + 'socket', + dummyStatePaths(), + TIMEOUT_MS, + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, 'daemon_transport_timeout'); + return true; + }, + ); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(mockStop.mock.calls.length, 1); + } finally { + server.close(); + } +}); diff --git a/src/daemon-client/daemon-client-metadata.ts b/src/daemon-client/daemon-client-metadata.ts index 155bd14f85..3a448b099f 100644 --- a/src/daemon-client/daemon-client-metadata.ts +++ b/src/daemon-client/daemon-client-metadata.ts @@ -273,7 +273,8 @@ export async function stopDaemonProcessForTakeover( function requireDaemonExit(termination: DaemonTerminationResult): void { if (termination.status !== 'retained') return; throw new AppError('COMMAND_FAILED', 'Daemon exit could not be confirmed.', { - details: { reason: 'daemon_exit_unconfirmed', termination }, + reason: 'daemon_exit_unconfirmed', + termination, }); } diff --git a/src/daemon-client/daemon-client-timeout.ts b/src/daemon-client/daemon-client-timeout.ts index 3043b49ff5..d338f74d02 100644 --- a/src/daemon-client/daemon-client-timeout.ts +++ b/src/daemon-client/daemon-client-timeout.ts @@ -1,4 +1,4 @@ -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; import { runCmdSync } from '@agent-device/host-kit/command'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; @@ -186,7 +186,13 @@ function resetDaemonAfterTimeout(info: DaemonInfo, paths: DaemonPaths): { forced forcedKill = true; } } catch { - void stopDaemonProcessForTakeover(info); + void stopDaemonProcessForTakeover(info).catch((error: unknown) => { + emitDiagnostic({ + level: 'warn', + phase: 'daemon_timeout_stop_failed', + data: { error: normalizeError(error) }, + }); + }); } finally { removeDaemonInfo(paths.infoPath); removeDaemonLock(paths.lockPath); diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index 638fd3fc74..0b47d6a491 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -86,14 +86,18 @@ test('daemon replace mid-command returns a structured, parseable error and exits await assertNoDaemonLeaks({ stateDir, daemonPids, phase: 'after-shutdown' }); info = null; } finally { - if (info) { - const termination = await stopDaemonProcess( - { pid: info.pid, startTime: info.processStartTime ?? null }, - { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, - ); - assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + try { + if (info) { + const termination = await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + } + fs.rmSync(stateDir, { recursive: true, force: true }); + } catch (error) { + console.warn('Daemon test cleanup retained state:', stateDir, error); } - fs.rmSync(stateDir, { recursive: true, force: true }); } }); diff --git a/test/integration/smoke-daemon-clean.test.ts b/test/integration/smoke-daemon-clean.test.ts index 3e2897ea2d..6fc26fffe5 100644 --- a/test/integration/smoke-daemon-clean.test.ts +++ b/test/integration/smoke-daemon-clean.test.ts @@ -77,14 +77,18 @@ test('clean daemon script stops a live daemon before removing metadata', async ( // leave only classified artifacts in its state dir. await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { - if (info) { - const termination = await stopDaemonProcess( - { pid: info.pid, startTime: info.processStartTime ?? null }, - { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, - ); - assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + try { + if (info) { + const termination = await stopDaemonProcess( + { pid: info.pid, startTime: info.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + } + fs.rmSync(stateDir, { recursive: true, force: true }); + } catch (error) { + console.warn('Daemon test cleanup retained state:', stateDir, error); } - fs.rmSync(stateDir, { recursive: true, force: true }); } }); diff --git a/test/integration/smoke-daemon-http.test.ts b/test/integration/smoke-daemon-http.test.ts index 6b49acd042..bbc084da7b 100644 --- a/test/integration/smoke-daemon-http.test.ts +++ b/test/integration/smoke-daemon-http.test.ts @@ -73,10 +73,14 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t await stopDaemon(info); await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { - if (fs.existsSync(path.join(stateDir, 'daemon.json'))) { - await stopDaemon(readDaemonInfo(stateDir)); + try { + if (fs.existsSync(path.join(stateDir, 'daemon.json'))) { + await stopDaemon(readDaemonInfo(stateDir)); + } + fs.rmSync(stateDir, { recursive: true, force: true }); + } catch (error) { + console.warn('Daemon test cleanup retained state:', stateDir, error); } - fs.rmSync(stateDir, { recursive: true, force: true }); } }); From 77d86161c7de77c02ffe77f95a8f7ab4adb5a91a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 3 Oct 2026 00:47:34 +0200 Subject: [PATCH 7/7] refactor: share best-effort daemon test cleanup --- .../daemon-replace-exit-flush.test.ts | 15 +------ test/integration/smoke-daemon-clean.test.ts | 15 +------ test/integration/smoke-daemon-http.test.ts | 13 ++----- .../support/daemon-test-cleanup.ts | 39 +++++++++++++++++++ 4 files changed, 47 insertions(+), 35 deletions(-) create mode 100644 test/integration/support/daemon-test-cleanup.ts diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index 0b47d6a491..a1034da385 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -1,3 +1,4 @@ +import { cleanupDaemonTestState } from './support/daemon-test-cleanup.ts'; import test from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; @@ -84,20 +85,8 @@ test('daemon replace mid-command returns a structured, parseable error and exits // `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 { - try { - if (info) { - const termination = await stopDaemonProcess( - { pid: info.pid, startTime: info.processStartTime ?? null }, - { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, - ); - assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); - } - fs.rmSync(stateDir, { recursive: true, force: true }); - } catch (error) { - console.warn('Daemon test cleanup retained state:', stateDir, error); - } + await cleanupDaemonTestState(stateDir, info); } }); diff --git a/test/integration/smoke-daemon-clean.test.ts b/test/integration/smoke-daemon-clean.test.ts index 6fc26fffe5..4d36c81823 100644 --- a/test/integration/smoke-daemon-clean.test.ts +++ b/test/integration/smoke-daemon-clean.test.ts @@ -1,3 +1,4 @@ +import { cleanupDaemonTestState } from './support/daemon-test-cleanup.ts'; import test from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; @@ -6,7 +7,6 @@ import path from 'node:path'; import { skipWhenLoopbackUnavailable } from '../../src/__tests__/test-utils/loopback.ts'; import { runCmdSync } from '@agent-device/host-kit/command'; import { isProcessAlive, readProcessStartTime } from '@agent-device/host-kit/process'; -import { stopDaemonProcess } from '../../src/daemon-process.ts'; import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; @@ -77,18 +77,7 @@ test('clean daemon script stops a live daemon before removing metadata', async ( // leave only classified artifacts in its state dir. await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { - try { - if (info) { - const termination = await stopDaemonProcess( - { pid: info.pid, startTime: info.processStartTime ?? null }, - { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, - ); - assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); - } - fs.rmSync(stateDir, { recursive: true, force: true }); - } catch (error) { - console.warn('Daemon test cleanup retained state:', stateDir, error); - } + await cleanupDaemonTestState(stateDir, info); } }); diff --git a/test/integration/smoke-daemon-http.test.ts b/test/integration/smoke-daemon-http.test.ts index bbc084da7b..f0052464ed 100644 --- a/test/integration/smoke-daemon-http.test.ts +++ b/test/integration/smoke-daemon-http.test.ts @@ -1,3 +1,4 @@ +import { cleanupDaemonTestState } from './support/daemon-test-cleanup.ts'; import test from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; @@ -24,6 +25,7 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t } const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-http-smoke-')); + let info: DaemonInfo | null = null; try { const args = [ 'session', @@ -46,7 +48,7 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t assert.equal(cli.status, 0, formatResultDebug('start HTTP daemon', ['session', 'list'], cli)); assert.equal(cli.json?.success, true, JSON.stringify(cli.json)); - const info = readDaemonInfo(stateDir); + info = readDaemonInfo(stateDir); assert.equal(info.transport, 'http'); assert.equal(typeof info.httpPort, 'number'); assert.ok((info.httpPort ?? 0) > 0); @@ -73,14 +75,7 @@ test('daemon HTTP transport starts from CLI and accepts a command RPC', async (t await stopDaemon(info); await assertNoDaemonLeaks({ stateDir, daemonPids: [info.pid], phase: 'after-shutdown' }); } finally { - try { - if (fs.existsSync(path.join(stateDir, 'daemon.json'))) { - await stopDaemon(readDaemonInfo(stateDir)); - } - fs.rmSync(stateDir, { recursive: true, force: true }); - } catch (error) { - console.warn('Daemon test cleanup retained state:', stateDir, error); - } + await cleanupDaemonTestState(stateDir, info); } }); diff --git a/test/integration/support/daemon-test-cleanup.ts b/test/integration/support/daemon-test-cleanup.ts new file mode 100644 index 0000000000..371159c521 --- /dev/null +++ b/test/integration/support/daemon-test-cleanup.ts @@ -0,0 +1,39 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { normalizeError } from '@agent-device/kernel/errors'; +import { stopDaemonProcess } from '../../../src/daemon-process.ts'; + +type TestDaemonIdentity = { pid: number; processStartTime?: string }; + +/** Best-effort cleanup keeps primary test failures and unconfirmed daemon state. */ +export async function cleanupDaemonTestState( + stateDir: string, + observed: TestDaemonIdentity | null, +): Promise { + try { + const identity = observed ?? readIdentity(stateDir); + if (!identity) throw new Error('No daemon lifetime was observed'); + const termination = await stopDaemonProcess( + { pid: identity.pid, startTime: identity.processStartTime ?? null }, + { mode: 'graceful', termTimeoutMs: 1_500, killTimeoutMs: 1_500 }, + ); + if (termination.status !== 'exited') { + console.warn('Daemon test cleanup retained state:', stateDir, termination); + return; + } + fs.rmSync(stateDir, { recursive: true, force: true }); + } catch (error) { + console.warn('Daemon test cleanup retained state:', stateDir, normalizeError(error)); + } +} + +function readIdentity(stateDir: string): TestDaemonIdentity | null { + try { + return JSON.parse( + fs.readFileSync(path.join(stateDir, 'daemon.json'), 'utf8'), + ) as TestDaemonIdentity; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + throw error; + } +}