From 29745b5a8762e0814a9f7fbabe4cffe1d1b50356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 15:39:02 +0200 Subject: [PATCH 1/6] fix: centralize daemon startup and retirement ownership --- docs/adr/0030-process-lock-exclusion.md | 44 +- .../internal/owner-identity-liveness.test.ts | 10 + .../host-kit/src/internal/owner-identity.ts | 11 +- .../src/internal/process-lock.fixtures.ts | 50 +- .../src/internal/process-lock.test.ts | 185 ++--- .../host-kit/src/internal/process-lock.ts | 22 +- packages/host-kit/src/process.ts | 1 + src/__tests__/daemon-exit-wait.test.ts | 15 +- src/__tests__/daemon-process-takeover.test.ts | 13 + .../daemon-registration-owner.test.ts | 530 +++++++++++- .../test-utils/daemon-http-fixture.ts | 3 +- .../test-utils/legacy-daemon-fixture.ts | 92 +++ .../registered-daemon-fixture.test.ts | 43 + .../test-utils/registered-daemon-fixture.ts | 115 +++ .../daemon-client-address-hints.test.ts | 59 ++ .../__tests__/daemon-client-lifecycle.test.ts | 359 ++++----- .../__tests__/daemon-client-metadata.test.ts | 54 +- .../daemon-client-startup-race.test.ts | 752 ++++++++++++++---- .../daemon-client-timeout-route.test.ts | 287 +++++-- .../__tests__/daemon-client-transport.test.ts | 108 +++ .../__tests__/daemon-client.test.ts | 231 +----- .../daemon-client-address-hints.ts | 44 + src/daemon-client/daemon-client-lifecycle.ts | 617 ++++++++------ src/daemon-client/daemon-client-metadata.ts | 221 +---- src/daemon-client/daemon-client-timeout.ts | 84 +- src/daemon-client/daemon-client-transport.ts | 164 ++-- src/daemon-client/daemon-client.ts | 63 +- src/daemon-process.ts | 3 + src/daemon-registration-owner.ts | 307 ++++++- src/daemon-registration.ts | 3 +- src/daemon/__tests__/daemon-stop.test.ts | 129 +-- src/daemon/daemon-stop.ts | 57 +- src/session-repair-tombstone.test.ts | 75 ++ src/session-repair-tombstone.ts | 18 +- .../managed-runtime-automation.fixtures.ts | 13 + .../support/daemon-test-cleanup.test.ts | 176 ++++ .../support/daemon-test-cleanup.ts | 85 +- test/wire-compat/ledger.json | 6 +- vitest.config.ts | 2 + website/docs/docs/installation.md | 27 +- 40 files changed, 3521 insertions(+), 1557 deletions(-) create mode 100644 src/__tests__/test-utils/legacy-daemon-fixture.ts create mode 100644 src/__tests__/test-utils/registered-daemon-fixture.test.ts create mode 100644 src/__tests__/test-utils/registered-daemon-fixture.ts create mode 100644 src/daemon-client/__tests__/daemon-client-address-hints.test.ts create mode 100644 src/daemon-client/daemon-client-address-hints.ts create mode 100644 src/session-repair-tombstone.test.ts create mode 100644 test/integration/support/daemon-test-cleanup.test.ts diff --git a/docs/adr/0030-process-lock-exclusion.md b/docs/adr/0030-process-lock-exclusion.md index 5aac07b2df..324c9f4c57 100644 --- a/docs/adr/0030-process-lock-exclusion.md +++ b/docs/adr/0030-process-lock-exclusion.md @@ -40,6 +40,44 @@ guard cannot constrain legacy code after its final check. The support boundary t requires deployment control; host-kit does not claim to detect or evict every legacy user. Daemon registration has a separate cutover boundary: keep the same `daemon.lock` path and -refuse legacy files rather than automatically reclaiming them. Its startup tests must cover -an already-running older daemon and concurrent old/new startup. This does not expand the -host-kit mixed-protocol support contract. +refuse legacy files rather than automatically reclaiming them. A legacy daemon creates an +exclusive file; a hardened daemon creates a directory at that same path. The old acquisition +cannot unlink a directory, and the new acquisition retains an existing file. + +The [registration tests](../../src/__tests__/daemon-registration-owner.test.ts) exercise real +children using the c237027737 legacy acquisition and the current owner. They cover an older +daemon already running, a hardened owner waiting to publish metadata, and concurrent startup. +The client refuses an older registration before signaling or changing it. This proves the daemon +cutover; it does not expand the host-kit mixed-protocol support contract. Older clients still +require the deployment controls above. + +## Recovering retained legacy daemon state + +Stop every daemon and client using the affected state directory, including older versions, +and verify that those processes have exited. Keep them stopped throughout recovery. +Only then remove the legacy `daemon.lock` file and any retained `daemon.reclaim.lock` guard +from that directory. Preserve the other state, logs and repair evidence. + +Start one supported version using that directory and prevent older clients or daemons from +returning. If every user cannot be identified and stopped, retain the files and use a separate +state directory. This procedure applies to daemon registration; shared build and cache locks +require the same confirmation for every process using their own paths. + +## Registration operations + +[Shared retirement](../../src/daemon-registration-owner.ts) owns verified termination, protected +metadata inspection and removal, and release. Takeover, failed startup, replay cleanup, timeout +reset and manual stop await its result. Abandoned recovery uses the same protected retirement +sequence without signaling a live process. Daemon publication and shutdown use functions bound +to their acquired claim. + +```mermaid +flowchart LR + C[Client lifecycle and timeout] --> R[Shared retirement] + M[Manual stop] --> R + P[Abandoned recovery and pruning] --> R + R --> L[Acquired registration claim] + D[Daemon startup and shutdown] --> O[Functions bound to own claim] + O --> L + L --> F[Protected metadata and reports] +``` diff --git a/packages/host-kit/src/internal/owner-identity-liveness.test.ts b/packages/host-kit/src/internal/owner-identity-liveness.test.ts index fd5278afa8..511da64644 100644 --- a/packages/host-kit/src/internal/owner-identity-liveness.test.ts +++ b/packages/host-kit/src/internal/owner-identity-liveness.test.ts @@ -59,3 +59,13 @@ test('a missing entry in a completed process snapshot stays fail-closed without assert.equal(mockIsProcessZombie.mock.calls.length, 0); assert.equal(mockReadProcessStartTime.mock.calls.length, 0); }); + +test('a pid outside the native range is unknown without a liveness probe', () => { + assert.equal( + classifyOwnerLiveness({ owner: { pid: 2_147_483_648, startTime: 'start-a' } }), + 'unknown', + ); + assert.equal(mockIsProcessAlive.mock.calls.length, 0); + assert.equal(mockIsProcessZombie.mock.calls.length, 0); + assert.equal(mockReadProcessStartTime.mock.calls.length, 0); +}); diff --git a/packages/host-kit/src/internal/owner-identity.ts b/packages/host-kit/src/internal/owner-identity.ts index a7b1352b35..552a030ea7 100644 --- a/packages/host-kit/src/internal/owner-identity.ts +++ b/packages/host-kit/src/internal/owner-identity.ts @@ -11,6 +11,11 @@ export type OwnerIdentity = { startTime: string | null; }; +/** A process id accepted by Node's native signal API. */ +export function isProcessPid(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0 && value <= 0x7fff_ffff; +} + export type OwnerLiveness = | 'live' | 'owner-process-dead' @@ -75,6 +80,7 @@ export function classifyOwnerLivenessFromObservation( observation?: HostProcessIdentityObservation | null, ): OwnerLiveness { const { owner, stateDir } = params; + if (!isProcessPid(owner.pid)) return 'unknown'; if (!isProcessAlive(owner.pid)) return 'owner-process-dead'; if (observation !== undefined ? observation?.state.startsWith('Z') : isProcessZombie(owner.pid)) { return 'owner-process-dead'; @@ -88,7 +94,10 @@ export function classifyOwnerLivenessFromObservation( return 'owner-process-reused'; } } - if (!stateDir) return 'live'; + return stateDir ? classifyOwnerStateDirectory(stateDir) : 'live'; +} + +function classifyOwnerStateDirectory(stateDir: string): OwnerLiveness { try { fs.statSync(stateDir); return 'live'; diff --git a/packages/host-kit/src/internal/process-lock.fixtures.ts b/packages/host-kit/src/internal/process-lock.fixtures.ts index 64259a5ad6..33fbc988a4 100644 --- a/packages/host-kit/src/internal/process-lock.fixtures.ts +++ b/packages/host-kit/src/internal/process-lock.fixtures.ts @@ -1,7 +1,28 @@ import fs from 'node:fs'; +import path from 'node:path'; import { vi } from 'vitest'; import { readProcessStartTime } from './host-process.ts'; -import type { ProcessLockOwner } from './process-lock.ts'; +import type { ProcessLockOwner, ProcessLockOwnerRecord } from './process-lock.ts'; + +export function writeLockOwnerFixture( + lockDirPath: string, + owner: ProcessLockOwner | ProcessLockOwnerRecord, +): void { + fs.mkdirSync(lockDirPath); + fs.writeFileSync(path.join(lockDirPath, 'owner.json'), JSON.stringify(owner)); +} + +export function writeDeadLockFixture(lockDirPath: string, acquiredAtMs = Date.now()): void { + writeLockOwnerFixture(lockDirPath, { pid: 999_999_999, startTime: null, acquiredAtMs }); +} + +export function failUnlinkForPath(filePath: string, error: Error) { + const unlink = fs.unlinkSync; + return vi.spyOn(fs, 'unlinkSync').mockImplementation((target) => { + if (String(target) === filePath) throw error; + return unlink(target); + }); +} export function currentProcessOwner(): ProcessLockOwner { return { @@ -41,3 +62,30 @@ export function onFirstGuardOpen(mutexPath: string, action: () => void) { }) as typeof fs.openSync); return { fired: () => fired, restore: () => spy.mockRestore() }; } + +export const UNINFORMATIVE_OWNER_RECORDS = [ + '{ pid: ', + 'null', + '"999999999"', + '{"pid":"999999999","startTime":null,"acquiredAtMs":1}', + '{"pid":0,"startTime":null,"acquiredAtMs":1}', + '{"pid":999999999,"startTime":7,"acquiredAtMs":1}', + '{"pid":999999999,"startTime":null}', +] as const; + +function failRenameForPath(filePath: string, error: Error) { + const rename = fs.renameSync; + return vi.spyOn(fs, 'renameSync').mockImplementation((source, destination) => { + if (String(destination) === filePath) throw error; + return rename(source, destination); + }); +} + +export function failLockOwnerPublication(lockDirPath: string, releaseFails: boolean) { + const primary = Object.assign(new Error('publication failed'), { code: 'EIO' }); + const releaseError = Object.assign(new Error('guard unlink refused'), { code: 'EPERM' }); + const renameSpy = failRenameForPath(path.join(lockDirPath, 'owner.json'), primary); + const guardPath = lockDirPath.replace(/\.lock$/, '.reclaim.lock'); + const unlinkSpy = releaseFails ? failUnlinkForPath(guardPath, releaseError) : undefined; + return { primary, renameSpy, unlinkSpy, guardPath }; +} diff --git a/packages/host-kit/src/internal/process-lock.test.ts b/packages/host-kit/src/internal/process-lock.test.ts index a55e7a113c..0e277c3235 100644 --- a/packages/host-kit/src/internal/process-lock.test.ts +++ b/packages/host-kit/src/internal/process-lock.test.ts @@ -2,7 +2,8 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, beforeEach, test, vi } from 'vitest'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import * as diagnostics from './diagnostics.ts'; const { zombiePids, processProbe } = vi.hoisted(() => ({ zombiePids: new Set(), @@ -36,9 +37,14 @@ import { mkdtempForTestSync } from './tmp-dir.fixtures.ts'; import { holdLegacyReclaimMutex } from './legacy-process-lock.fixtures.ts'; import { currentProcessOwner, + failLockOwnerPublication, + UNINFORMATIVE_OWNER_RECORDS, + failUnlinkForPath, listReclaimSiblings, onFirstGuardOpen, stampDirectoryAbandoned, + writeDeadLockFixture, + writeLockOwnerFixture, } from './process-lock.fixtures.ts'; let tmpDir: string; @@ -67,15 +73,7 @@ test('acquireProcessLock creates and releases a lock directory', async () => { test('acquireProcessLock reclaims locks owned by dead processes', async () => { const lockDirPath = path.join(tmpDir, 'stale.lock'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ - pid: 999_999_999, - startTime: null, - acquiredAtMs: Date.now() - 10_000, - }), - ); + writeDeadLockFixture(lockDirPath, Date.now() - 10_000); const release = await acquireProcessLock({ lockDirPath, @@ -113,18 +111,14 @@ test('acquireProcessLock reclaims locks owned by zombie processes', async () => test('acquireProcessLock never steals a null-start-time lock from an alive pid', async () => { const lockDirPath = path.join(tmpDir, 'null-start.lock'); - fs.mkdirSync(lockDirPath); // An acquiredAtMs far older than this process simulates what a wall-clock // step makes a live null-start owner look like; age is not proof of death, // so the waiter must time out instead of reclaiming the held lock. - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ - pid: process.pid, - startTime: null, - acquiredAtMs: Date.now() - 365 * 24 * 60 * 60_000, - }), - ); + writeLockOwnerFixture(lockDirPath, { + pid: process.pid, + startTime: null, + acquiredAtMs: Date.now() - 365 * 24 * 60 * 60_000, + }); await assert.rejects( () => @@ -144,9 +138,8 @@ test('acquireProcessLock never steals a null-start-time lock from an alive pid', test('acquireProcessLock reports live lock owner details on timeout', async () => { const lockDirPath = path.join(tmpDir, 'busy.lock'); - fs.mkdirSync(lockDirPath); const owner = currentProcessOwner(); - fs.writeFileSync(path.join(lockDirPath, 'owner.json'), JSON.stringify(owner)); + writeLockOwnerFixture(lockDirPath, owner); await assert.rejects( () => @@ -268,15 +261,7 @@ test('acquireProcessLock retains an owner whose record was never written', async test('one abandoned lock offered to two contenders is held by exactly one of them', async () => { const lockDirPath = path.join(tmpDir, 'contended.lock'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ - pid: 999_999_999, - startTime: null, - acquiredAtMs: Date.now(), - }), - ); + writeDeadLockFixture(lockDirPath); stampDirectoryAbandoned(lockDirPath); const attempts = await Promise.allSettled([ @@ -305,16 +290,6 @@ test('one abandoned lock offered to two contenders is held by exactly one of the assert.deepEqual(listReclaimSiblings(tmpDir), []); }); -const UNINFORMATIVE_OWNER_RECORDS = [ - '{ pid: ', - 'null', - '"999999999"', - '{"pid":"999999999","startTime":null,"acquiredAtMs":1}', - '{"pid":0,"startTime":null,"acquiredAtMs":1}', - '{"pid":999999999,"startTime":7,"acquiredAtMs":1}', - '{"pid":999999999,"startTime":null}', -] as const; - for (const [index, record] of UNINFORMATIVE_OWNER_RECORDS.entries()) { test(`acquireProcessLock does not evict the lock behind the record ${record}`, async () => { const lockDirPath = path.join(tmpDir, `uninformative-${index}.lock`); @@ -371,13 +346,10 @@ test('a release that could not verify ownership does not wedge the next acquire const release = await acquireProcessLock({ lockDirPath, owner: currentProcessOwner() }); // The unlink the release needs is refused, which is what an EACCES or EMFILE looks like here. - const realUnlink = fs.unlinkSync; - const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(((target: fs.PathLike) => { - if (String(target) === ownerFilePath) { - throw Object.assign(new Error('EACCES: permission denied, unlink'), { code: 'EACCES' }); - } - return realUnlink(target as string); - }) as typeof fs.unlinkSync); + const unlinkSpy = failUnlinkForPath( + ownerFilePath, + Object.assign(new Error('EACCES: permission denied, unlink'), { code: 'EACCES' }), + ); try { await assert.rejects( () => release(), @@ -408,15 +380,11 @@ test('a release that could not verify ownership does not wedge the next acquire // somebody is holding, which is worse than the wait the rule exists to end. test('a claim issued by another loading of this module is not read as spent', async () => { const lockDirPath = path.join(tmpDir, 'other-issuer.lock'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ - ...currentProcessOwner(), - claimToken: 'a-token-this-loading-never-issued', - claimIssuerId: 'another-loading-of-this-module', - }), - ); + writeLockOwnerFixture(lockDirPath, { + ...currentProcessOwner(), + claimToken: 'a-token-this-loading-never-issued', + claimIssuerId: 'another-loading-of-this-module', + }); await assert.rejects( () => @@ -454,11 +422,7 @@ test('a contender that claims the path during a reclaim keeps its lock', async ( const lockDirPath = path.join(tmpDir, 'claimed-during-reclaim.lock'); const mutexPath = path.join(tmpDir, 'claimed-during-reclaim.reclaim.lock'); const ownerFilePath = path.join(lockDirPath, 'owner.json'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - ownerFilePath, - JSON.stringify({ pid: 999_999_999, startTime: null, acquiredAtMs: Date.now() }), - ); + writeDeadLockFixture(lockDirPath); stampDirectoryAbandoned(lockDirPath); // The moment a contender is admitted to judging this lock, another process clears the dead @@ -563,8 +527,7 @@ test('a lock directory made anew while a reclaim holds the mutex is not the one fs.mkdirSync(lockDirPath); stampDirectoryAbandoned(lockDirPath); - // Replacing the directory rather than filling it is what a contender that won the path looks - // like from the inside: same name, same emptiness, and an age that says it was never abandoned. + // A replacement directory has a new identity and a fresh age. let refilledAtMs = 0; const guard = onFirstGuardOpen(mutexPath, () => { fs.rmSync(lockDirPath, { recursive: true, force: true }); @@ -642,11 +605,7 @@ test('a reclaim mutex another contender holds leaves the abandoned lock standing test('age alone never authorizes taking a publication or reclaim mutex', async () => { const lockDirPath = path.join(tmpDir, 'dead-janitor.lock'); const mutexPath = path.join(tmpDir, 'dead-janitor.reclaim.lock'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ pid: 999_999_999, startTime: null, acquiredAtMs: Date.now() }), - ); + writeDeadLockFixture(lockDirPath); fs.mkdirSync(mutexPath); stampDirectoryAbandoned(lockDirPath); stampDirectoryAbandoned(mutexPath); @@ -665,11 +624,7 @@ test('age alone never authorizes taking a publication or reclaim mutex', async ( test('a reclaim that cannot clear the lock directory leaves the record it judged', async () => { const lockDirPath = path.join(tmpDir, 'immovable.lock'); const ownerFilePath = path.join(lockDirPath, 'owner.json'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - ownerFilePath, - JSON.stringify({ pid: 999_999_999, startTime: null, acquiredAtMs: Date.now() }), - ); + writeDeadLockFixture(lockDirPath); stampDirectoryAbandoned(lockDirPath); const realRemove = fs.rmdirSync; @@ -904,23 +859,73 @@ test('an acquisition cannot authorize a successor record', async () => { assert.deepEqual(JSON.parse(fs.readFileSync(ownerPath, 'utf8')), successor); }); -test('failed owner publication rolls back only the attempted empty directory', async () => { - const lockDirPath = path.join(tmpDir, 'failed-publication.lock'); - const realRename = fs.renameSync; - const renameSpy = vi.spyOn(fs, 'renameSync').mockImplementation((source, destination) => { - if (String(destination) === path.join(lockDirPath, 'owner.json')) { - throw Object.assign(new Error('publication failed'), { code: 'EIO' }); +for (const releaseFails of [false, true]) { + test(`failed owner publication remains primary when guard release fails: ${releaseFails}`, async () => { + const lockDirPath = path.join(tmpDir, 'failed-publication.lock'); + const faults = failLockOwnerPublication(lockDirPath, releaseFails); + const diagnosticSpy = vi.spyOn(diagnostics, 'emitDiagnostic'); + try { + assert.throws( + () => tryAcquireProcessLock({ lockDirPath, owner: currentProcessOwner() }), + (error) => error === faults.primary, + ); + if (releaseFails) { + const data = diagnosticSpy.mock.calls.find( + ([event]) => event.phase === 'process_lock_guard_release_failed', + )?.[0].data; + const failure = data?.error as ReturnType; + assert.equal(failure.cause?.code, 'EPERM'); + assert.equal(failure.details?.reason, 'process_lock_guard_release_failed'); + assert.match(failure.hint ?? '', /confirming all users/); + assert.equal(fs.existsSync(faults.guardPath), true); + } + } finally { + faults.renameSpy.mockRestore(); + faults.unlinkSpy?.mockRestore(); + diagnosticSpy.mockRestore(); } - return realRename(source, destination); + if (releaseFails) fs.unlinkSync(faults.guardPath); + const retry = tryAcquireProcessLock({ lockDirPath, owner: currentProcessOwner() }); + assert.equal(retry.status, 'acquired'); + if (retry.status === 'acquired') await retry.acquisition.release(); }); +} + +test('a failed guard release reports retained exclusion and supports verified manual recovery', async () => { + const lockDirPath = path.join(tmpDir, 'guard-release.lock'); + const guardPath = path.join(tmpDir, 'guard-release.reclaim.lock'); + const primary = Object.assign(new Error('guard unlink refused'), { code: 'EPERM' }); + const unlinkSpy = failUnlinkForPath(guardPath, primary); try { assert.throws( () => tryAcquireProcessLock({ lockDirPath, owner: currentProcessOwner() }), - /publication failed/, + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.reason, 'process_lock_guard_release_failed'); + assert.equal(error.details?.lockDirPath, lockDirPath); + assert.equal(error.cause, primary); + const failure = normalizeError(error); + assert.equal(failure.cause?.code, 'EPERM'); + assert.equal( + failure.hint, + `Restore process inspection or stop the verified owner, then retry. Remove ${lockDirPath} and ${guardPath} only after confirming all users of this state directory have stopped.`, + ); + return true; + }, + ); + assert.equal(fs.existsSync(guardPath), true); + const ownerRecord = fs.readFileSync(path.join(lockDirPath, 'owner.json'), 'utf8'); + assert.equal(JSON.parse(ownerRecord).pid, process.pid); + assert.equal( + tryAcquireProcessLock({ lockDirPath, owner: currentProcessOwner() }).status, + 'busy', ); + assert.equal(fs.readFileSync(path.join(lockDirPath, 'owner.json'), 'utf8'), ownerRecord); } finally { - renameSpy.mockRestore(); + unlinkSpy.mockRestore(); } + fs.unlinkSync(guardPath); const retry = tryAcquireProcessLock({ lockDirPath, owner: currentProcessOwner() }); assert.equal(retry.status, 'acquired'); if (retry.status === 'acquired') await retry.acquisition.release(); @@ -973,16 +978,12 @@ test('release waits for a guard another process holds for a filesystem step', as test('a contender judges a live owner without holding the guard', async () => { const lockDirPath = path.join(tmpDir, 'probed-outside-guard.lock'); const mutexPath = path.join(tmpDir, 'probed-outside-guard.reclaim.lock'); - fs.mkdirSync(lockDirPath); - fs.writeFileSync( - path.join(lockDirPath, 'owner.json'), - JSON.stringify({ - pid: process.ppid, - startTime: null, - acquiredAtMs: Date.now(), - claimToken: 'live-rival', - }), - ); + writeLockOwnerFixture(lockDirPath, { + pid: process.ppid, + startTime: null, + acquiredAtMs: Date.now(), + claimToken: 'live-rival', + }); let probes = 0; let probesUnderGuard = 0; processProbe.observe = () => { diff --git a/packages/host-kit/src/internal/process-lock.ts b/packages/host-kit/src/internal/process-lock.ts index 989b28a78d..d5352530b4 100644 --- a/packages/host-kit/src/internal/process-lock.ts +++ b/packages/host-kit/src/internal/process-lock.ts @@ -1,11 +1,12 @@ import crypto from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; import { publishFileSync } from './atomic-file.ts'; import { emitDiagnostic } from './diagnostics.ts'; import { classifyOwnerLiveness, + isProcessPid, ownerIdentityMatches, type OwnerLiveness, } from './owner-identity.ts'; @@ -179,7 +180,7 @@ function withMutationGuardHeld( emitDiagnostic({ level: 'warn', phase: 'process_lock_guard_release_failed', - data: { lockDirPath, error: String(releaseError) }, + data: { lockDirPath, error: normalizeError(releaseError) }, }); } throw error; @@ -504,7 +505,20 @@ async function waitForReclaimMutex(lockDirPath: string): Promise { } function releaseReclaimMutex(lockDirPath: string): void { - fs.unlinkSync(reclaimMutexPath(lockDirPath)); + try { + fs.unlinkSync(reclaimMutexPath(lockDirPath)); + } catch (error) { + throw new AppError( + 'COMMAND_FAILED', + 'Process lock mutation guard release could not be confirmed.', + { + reason: 'process_lock_guard_release_failed', + lockDirPath, + hint: staleLockHint(lockDirPath), + }, + error, + ); + } } function errorCode(error: unknown): string | undefined { @@ -553,7 +567,7 @@ function parseProcessLockOwner(contents: string): ProcessLockOwnerRecord | null const PROCESS_LOCK_OWNER_FIELD_SHAPES: Record boolean> = { - pid: (value) => typeof value === 'number' && Number.isInteger(value) && value > 0, + pid: isProcessPid, acquiredAtMs: (value) => typeof value === 'number' && Number.isFinite(value), startTime: (value) => value === undefined || value === null || typeof value === 'string', }; diff --git a/packages/host-kit/src/process.ts b/packages/host-kit/src/process.ts index 2ed4c2c06d..6c061dd11f 100644 --- a/packages/host-kit/src/process.ts +++ b/packages/host-kit/src/process.ts @@ -36,6 +36,7 @@ export { export { classifyOwnerLiveness, classifyOwnerLivenessFromObservation, + isProcessPid, type OwnerIdentity, ownerIdentityDiffers, ownerIdentityMatches, diff --git a/src/__tests__/daemon-exit-wait.test.ts b/src/__tests__/daemon-exit-wait.test.ts index 2111bc2f3b..deaa03e829 100644 --- a/src/__tests__/daemon-exit-wait.test.ts +++ b/src/__tests__/daemon-exit-wait.test.ts @@ -57,6 +57,16 @@ afterEach(() => { vi.restoreAllMocks(); }); +test.each([0, -1, 1.5, 2_147_483_648, Number.MAX_SAFE_INTEGER])( + 'an invalid native pid %s cannot prove exit during recovery', + async (pid) => { + expect(await waitForDaemonExit({ pid, startTime: OURS }, { timeoutMs: 0 })).toEqual({ + exited: false, + elapsedMs: 0, + }); + }, +); + test('waitForDaemonExit reports a pid recycled mid-wait as exited, without burning the deadline', async () => { setTimeout(() => state.starts.set(PID, RECYCLED), 20); const wait = await waitForDaemonExit( @@ -205,16 +215,15 @@ test('force termination delivers KILL first and returns its confirmed exit', asy }); }); -test('a daemon reaped after the TERM budget retains graceful mode without signaling its zombie', async () => { +test('a daemon that becomes a zombie on TERM retains graceful mode without SIGKILL', 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 }, + { mode: 'graceful', termTimeoutMs: 0, killTimeoutMs: 0 }, ); 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 78156ffc64..99292988f6 100644 --- a/src/__tests__/daemon-process-takeover.test.ts +++ b/src/__tests__/daemon-process-takeover.test.ts @@ -12,6 +12,19 @@ const TAKEOVER_TIMEOUTS = { termTimeoutMs: 5_000, killTimeoutMs: 2_000 }; const spawnedChildren: { child: ChildProcess; exited: Promise }[] = []; const spawnedRoots: string[] = []; +test.each([0, -1, 1.5, Number.NaN, '123', 2_147_483_648, Number.MAX_SAFE_INTEGER])( + 'an invalid daemon pid %s cannot prove exit', + async (pid) => { + assert.deepEqual( + await stopDaemonProcess( + { pid: pid as number, startTime: 'captured-birth' }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 0 }, + ), + { status: 'retained', reason: 'identity-unverified' }, + ); + }, +); + afterEach(async () => { for (const { child, exited } of spawnedChildren.splice(0)) { if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); diff --git a/src/__tests__/daemon-registration-owner.test.ts b/src/__tests__/daemon-registration-owner.test.ts index edb42abced..439b1a0a6c 100644 --- a/src/__tests__/daemon-registration-owner.test.ts +++ b/src/__tests__/daemon-registration-owner.test.ts @@ -1,16 +1,35 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; +import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; -import { readCurrentOwnerIdentity } from '@agent-device/host-kit/process'; +import { readCurrentOwnerIdentity, isProcessAlive } from '@agent-device/host-kit/process'; +import * as hostProcess from '@agent-device/host-kit/process'; import { tryAcquireDaemonRegistration, stopAndRetireDaemon, recoverAbandonedDaemonRegistration, + createOwnedReplayStateDir, + DAEMON_STARTUP_EXIT_CODES, + launchDaemonProcess, + type DaemonRetirementResult, + type OwnedReplayStateDir, } from '../daemon-registration-owner.ts'; import { resolveDaemonPaths, type DaemonPaths } from '../daemon-resolution.ts'; import { readRegisteredDaemonOwnership } from '../daemon-registration.ts'; import { readDaemonShutdownReport } from '../daemon-shutdown-report.ts'; import { mkdtempForTestSync } from './test-utils/tmp-dir.ts'; +import { + registeredDaemonFixtureArgs, + spawnRegisteredDaemonFixture, + finishRegisteredDaemonFixture, +} from './test-utils/registered-daemon-fixture.ts'; +import { spawnLegacyDaemonFixture } from './test-utils/legacy-daemon-fixture.ts'; +import { ensureDaemon, resolveClientSettings } from '../daemon-client/daemon-client-lifecycle.ts'; +import { inspectProcessLock } from '@agent-device/host-kit/file'; +import { AppError } from '@agent-device/kernel/errors'; +import { sleep } from '@agent-device/host-kit/retry'; +import { stopDaemonProcess } from '../daemon-process.ts'; +import { stopDaemon } from '../daemon/daemon-stop.ts'; const fields = { socketPort: 4210, @@ -305,3 +324,512 @@ test.skipIf(process.getuid?.() === 0)( } }, ); + +test('a forged private-directory capability cannot authorize even matching dead metadata removal', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-private-forgery-')); + replaceInfo(paths, deadIdentity.pid, deadIdentity.startTime); + const before = fs.readFileSync(paths.infoPath, 'utf8'); + const result = await stopAndRetireDaemon({ + paths, + observed: deadIdentity, + mode: 'force', + ownedStateDir: Object.freeze({ paths }) as OwnedReplayStateDir, + }); + assert.equal(result.status, 'retained'); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), before); +}); + +test('private retirement closes startup admission, joins the actual child and never recreates a removed directory', async () => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const args = registeredDaemonFixtureArgs(paths, fields); + const launch = launchDaemonProcess({ paths, args, serverMode: 'socket', ownedStateDir }); + let contender: ReturnType | undefined; + try { + assert.ok(launch.startTime); + await waitForFixtureFile(paths.infoPath); + contender = launchDaemonProcess({ paths, args, serverMode: 'socket', ownedStateDir }); + assert.equal((await contender.exited).exitCode, DAEMON_STARTUP_EXIT_CODES.busy); + const input = { + paths, + observed: { pid: launch.pid, startTime: launch.startTime }, + mode: 'graceful' as const, + ownedStateDir, + }; + const pending = stopAndRetireDaemon(input); + assert.throws( + () => launchDaemonProcess({ paths, args, serverMode: 'socket', ownedStateDir }), + (error: { details?: { reason?: string } }) => + error.details?.reason === 'daemon_startup_admission_closed', + ); + const result = await pending; + assert.equal(result.status, 'retired', JSON.stringify(result)); + if (result.status !== 'retired') assert.fail('retirement not confirmed'); + assert.equal(result.removedStateDir, true); + assert.equal((await launch.exited).exitCode, 0); + assert.equal(fs.existsSync(paths.baseDir), false); + assert.deepEqual(await stopAndRetireDaemon(input), result); + assert.equal(fs.existsSync(paths.baseDir), false); + } finally { + await finishPrivateTestDaemons(paths, launch, contender); + } +}); + +test('private retirement retains the directory while an earlier actual startup child is still paused', async () => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const args = registeredDaemonFixtureArgs(paths, fields); + const entry = args[1]!; + const ready = `${paths.baseDir}/paused-startup.ready`; + fs.writeFileSync( + entry, + `import fs from 'node:fs'; fs.writeFileSync(${JSON.stringify(ready)}, 'ready'); setInterval(() => {}, 1000);`, + ); + const first = launchDaemonProcess({ paths, args, serverMode: 'socket', ownedStateDir }); + let second: ReturnType | undefined; + try { + await waitForFixtureFile(ready); + second = launchDaemonProcess({ + paths, + args: registeredDaemonFixtureArgs(paths, fields), + serverMode: 'socket', + ownedStateDir, + }); + await waitForFixtureFile(paths.infoPath); + const result = await stopAndRetireDaemon({ + paths, + observed: { pid: second.pid, startTime: second.startTime ?? null }, + mode: 'graceful', + ownedStateDir, + startupJoinTimeoutMs: 0, + }); + assert.equal(result.status, 'retained', JSON.stringify(result)); + if (result.status !== 'retained') assert.fail('private state unexpectedly retired'); + assert.equal(result.reason, 'startup-unconfirmed'); + assert.equal(result.termination?.status, 'exited'); + assert.equal(fs.existsSync(paths.baseDir), true); + assert.equal(isProcessAlive(first.pid), true); + assert.equal((await second.exited).exitCode, 0); + } finally { + await finishPrivateTestDaemons(paths, first, second); + } +}); + +const privateStartupFailures = [ + ['missing-birth', 'ownership-unproven', []], + ['unverified-birth', 'exit-unconfirmed', []], + ['signal-error', 'stop-failed', ['SIGTERM']], +] as const; + +for (const [failure, reason, expectedSignals] of privateStartupFailures) { + test(`private startup completion is joined despite ${failure} proof`, async () => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const exitPath = path.join(paths.baseDir, 'exit-startup'); + const args = registeredDaemonFixtureArgs(paths, fields); + fs.appendFileSync( + args[1]!, + 'setInterval(() => { if (fs.existsSync(path.join(paths.baseDir, "exit-startup"))) process.exit(0); }, 10);', + ); + const birthProbe = + failure === 'missing-birth' + ? vi.spyOn(hostProcess, 'readProcessStartTime').mockReturnValueOnce(null) + : undefined; + const launch = launchDaemonProcess({ paths, args, serverMode: 'socket', ownedStateDir }); + birthProbe?.mockRestore(); + let actualStartTime: string | null = null; + let exitTimer: ReturnType | undefined; + try { + await waitForFixtureFile(paths.infoPath); + actualStartTime = hostProcess.readProcessStartTime(launch.pid); + assert.ok(actualStartTime); + if (failure === 'missing-birth') { + assert.equal(launch.startTime, undefined); + replaceInfo(paths, launch.pid); + } else assert.ok(launch.startTime); + const metadata = fs.readFileSync(paths.infoPath, 'utf8'); + let joined = false; + void launch.exited.then(() => { + joined = true; + }); + const signal = process.kill.bind(process); + const signals = vi.spyOn(process, 'kill'); + const primary = Object.assign(new Error('Cannot signal fixture'), { code: 'EIO' }); + if (failure === 'unverified-birth') + vi.spyOn(hostProcess, 'readProcessStartTime').mockReturnValue(null); + if (failure === 'signal-error') + signals.mockImplementation((pid, requested) => { + if (requested === 'SIGTERM') throw primary; + return signal(pid, requested); + }); + const pending = stopAndRetireDaemon({ + paths, + observed: { pid: launch.pid, startTime: launch.startTime ?? null }, + mode: 'graceful', + ownedStateDir, + startupJoinTimeoutMs: 500, + }); + exitTimer = setTimeout(() => fs.writeFileSync(exitPath, 'exit'), 10); + const result = await pending; + assert.equal(joined, true, 'retirement returned before its recorded child completed'); + assert.equal(result.status, 'retained', JSON.stringify(result)); + if (result.status !== 'retained') assert.fail('unproven startup unexpectedly retired'); + assert.equal(result.reason, reason); + assertPrivateStartupFailureDetails(result, failure, primary); + assert.deepEqual( + signals.mock.calls.filter(([, requested]) => requested !== 0), + expectedSignals.map((requested) => [launch.pid, requested]), + ); + assert.equal(result.removedInfo, false); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), metadata); + assert.equal(fs.existsSync(paths.lockPath), true); + assert.equal(fs.existsSync(paths.baseDir), true); + } finally { + clearTimeout(exitTimer); + vi.restoreAllMocks(); + await finishPrivateTestDaemons(paths, { + ...launch, + startTime: actualStartTime ?? hostProcess.readProcessStartTime(launch.pid) ?? undefined, + }); + } + }); +} + +function assertPrivateStartupFailureDetails( + result: Extract, + failure: (typeof privateStartupFailures)[number][0], + primary: Error, +): void { + switch (failure) { + case 'missing-birth': + assert.equal(result.error?.details?.reason, 'daemon_private_startup_unowned'); + break; + case 'unverified-birth': + assert.deepEqual(result.termination, { + status: 'retained', + reason: 'identity-unverified', + signal: 'SIGTERM', + }); + break; + case 'signal-error': + assert.equal(result.error?.message, primary.message); + assert.equal(result.error?.cause?.code, 'EIO'); + break; + } +} + +test('private startup completion stays bounded without published birth proof and retains the live child', async () => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const birthProbe = vi.spyOn(hostProcess, 'readProcessStartTime').mockReturnValueOnce(null); + const launch = launchDaemonProcess({ + paths, + args: registeredDaemonFixtureArgs(paths, fields), + serverMode: 'socket', + ownedStateDir, + }); + birthProbe.mockRestore(); + let actualStartTime: string | null = null; + try { + await waitForFixtureFile(paths.infoPath); + actualStartTime = hostProcess.readProcessStartTime(launch.pid); + assert.ok(actualStartTime); + assert.equal(launch.startTime, undefined); + replaceInfo(paths, launch.pid); + const metadata = fs.readFileSync(paths.infoPath, 'utf8'); + const signals = vi.spyOn(process, 'kill'); + const pending = stopAndRetireDaemon({ + paths, + observed: { pid: launch.pid, startTime: null }, + mode: 'force', + ownedStateDir, + startupJoinTimeoutMs: 0, + }); + assert.throws( + () => launchDaemonProcess({ paths, args: [], serverMode: 'socket', ownedStateDir }), + (error: { details?: { reason?: string } }) => + error.details?.reason === 'daemon_startup_admission_closed', + ); + const result = await pending; + assert.equal(result.status, 'retained', JSON.stringify(result)); + if (result.status !== 'retained') assert.fail('unproven live startup unexpectedly retired'); + assert.equal(result.reason, 'ownership-unproven'); + assert.equal(result.error?.details?.reason, 'daemon_private_startup_unowned'); + assert.equal(isProcessAlive(launch.pid), true); + assert.deepEqual( + signals.mock.calls.filter(([, requested]) => requested !== 0), + [], + ); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), metadata); + assert.equal(fs.existsSync(paths.lockPath), true); + assert.equal(fs.existsSync(paths.baseDir), true); + } finally { + vi.restoreAllMocks(); + await finishPrivateTestDaemons(paths, { + ...launch, + startTime: actualStartTime ?? hostProcess.readProcessStartTime(launch.pid) ?? undefined, + }); + } +}); + +for (const contents of [ + '{broken', + '{"owner":"default","reapedAt":1,"expiresAt":1e400}', + '{"owner":"default","reapedAt":1,"expiresAt":-1e400}', + ...[false, null, 0, {}].map((commitFailure) => + JSON.stringify({ + owner: 'default', + reapedAt: Date.now(), + expiresAt: Date.now() + 60_000, + commitFailure, + }), + ), +]) { + test(`malformed repair evidence (${contents}) retains private state after the actual child has exited`, async () => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const sessionDir = `${paths.sessionsDir}/default`; + fs.mkdirSync(sessionDir, { recursive: true }); + const evidencePath = `${sessionDir}/repair-tombstone.json`; + fs.writeFileSync(evidencePath, contents); + const launch = launchDaemonProcess({ + paths, + args: registeredDaemonFixtureArgs(paths, fields), + serverMode: 'socket', + ownedStateDir, + }); + try { + await waitForFixtureFile(paths.infoPath); + const result = await stopAndRetireDaemon({ + paths, + observed: { pid: launch.pid, startTime: launch.startTime ?? null }, + mode: 'graceful', + ownedStateDir, + }); + assert.equal(result.status, 'retained', JSON.stringify(result)); + if (result.status !== 'retained') assert.fail('repair evidence unexpectedly discarded'); + assert.equal(result.termination?.status, 'exited'); + assert.equal(result.error?.details?.reason, 'repair_evidence_invalid'); + assert.equal(fs.readFileSync(evidencePath, 'utf8'), contents); + await launch.exited; + } finally { + await finishPrivateTestDaemons(paths, launch); + } + }); +} + +async function waitForFixtureFile(filePath: string): Promise { + const deadline = Date.now() + 2_000; + while (!fs.existsSync(filePath) && Date.now() < deadline) await sleep(20); + assert.equal(fs.existsSync(filePath), true); +} + +async function finishPrivateTestDaemons( + paths: DaemonPaths, + ...launches: (ReturnType | undefined)[] +): Promise { + for (const launch of launches) { + if (!launch) continue; + const termination = await stopDaemonProcess( + { pid: launch.pid, startTime: launch.startTime ?? null }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 2_000 }, + ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + await launch.exited; + } + fs.rmSync(paths.baseDir, { recursive: true, force: true }); +} + +async function waitForCutoverFixture(ready: () => boolean): Promise { + for (let attempt = 0; attempt < 200; attempt += 1) { + if (ready()) return; + await sleep(10); + } + assert.fail('cutover fixture did not reach its barrier'); +} + +function legacyDisposition(paths: DaemonPaths): boolean | undefined { + try { + const parsed = JSON.parse( + fs.readFileSync(path.join(paths.baseDir, 'legacy-disposition.json'), 'utf8'), + ) as { acquired?: unknown }; + return typeof parsed.acquired === 'boolean' ? parsed.acquired : undefined; + } catch { + return undefined; + } +} + +test('cutover refuses an already-running legacy daemon before signaling or changing registration', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-old-daemon-')); + const legacy = spawnLegacyDaemonFixture(paths); + try { + await waitForCutoverFixture(() => fs.existsSync(paths.infoPath)); + const metadata = fs.readFileSync(paths.infoPath, 'utf8'); + const lock = fs.readFileSync(paths.lockPath, 'utf8'); + const contender = spawnRegisteredDaemonFixture(paths, fields, undefined); + let disposition: Awaited | undefined; + void contender.exited.then((result) => { + disposition = result; + }); + await waitForCutoverFixture( + () => Boolean(disposition) || fs.existsSync(path.join(paths.baseDir, 'registration-held')), + ); + assert.ok(disposition, 'a new daemon must refuse an occupied legacy file'); + assert.equal(disposition.exitCode, DAEMON_STARTUP_EXIT_CODES.unproven); + await assert.rejects( + ensureDaemon( + resolveClientSettings({ + session: 'default', + command: 'devices', + positionals: [], + flags: { stateDir: paths.baseDir }, + }), + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, 'daemon_registration_unproven'); + return true; + }, + ); + assert.equal(process.kill(legacy.pid, 0), true); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), metadata); + assert.equal(fs.readFileSync(paths.lockPath, 'utf8'), lock); + } finally { + await legacy.stop(); + await finishRegisteredDaemonFixture(paths.baseDir); + } +}); + +test('a legacy contender cannot unlink a hardened owner while it delays metadata publication', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-new-daemon-')); + const deferred = path.join(paths.baseDir, 'defer-publication'); + fs.writeFileSync(deferred, 'wait'); + const current = spawnRegisteredDaemonFixture(paths, fields, undefined); + let legacy: ReturnType | undefined; + try { + await waitForCutoverFixture(() => fs.existsSync(path.join(paths.baseDir, 'registration-held'))); + legacy = spawnLegacyDaemonFixture(paths); + await waitForCutoverFixture(() => legacyDisposition(paths) !== undefined); + assert.equal(legacyDisposition(paths), false); + await legacy.exited; + const claim = inspectProcessLock(paths.lockPath); + assert.equal(claim.state, 'held'); + if (claim.state !== 'held') throw new Error('current owner lost its claim'); + assert.equal(claim.owner.pid, current.pid); + assert.equal(process.kill(current.pid, 0), true); + assert.equal(fs.existsSync(paths.infoPath), false); + fs.unlinkSync(deferred); + await waitForCutoverFixture(() => fs.existsSync(paths.infoPath)); + assert.equal(JSON.parse(fs.readFileSync(paths.infoPath, 'utf8')).pid, current.pid); + } finally { + await legacy?.stop(); + await finishRegisteredDaemonFixture(paths.baseDir); + } +}); + +test('concurrent old and new daemon startup has one owner at the shared lock path', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-cutover-race-')); + const barrier = path.join(paths.baseDir, 'start'); + const legacy = spawnLegacyDaemonFixture(paths, barrier); + const current = spawnRegisteredDaemonFixture(paths, fields, undefined, barrier); + let currentExited = false; + void current.exited.then(() => { + currentExited = true; + }); + try { + await waitForCutoverFixture( + () => + fs.existsSync(`${barrier}.ready-${legacy.pid}`) && + fs.existsSync(`${barrier}.ready-${current.pid}`), + ); + fs.writeFileSync(barrier, 'start'); + await waitForCutoverFixture( + () => + legacyDisposition(paths) !== undefined && + (currentExited || fs.existsSync(path.join(paths.baseDir, 'registration-held'))), + ); + const oldAcquired = legacyDisposition(paths); + const claim = inspectProcessLock(paths.lockPath); + const newAcquired = fs.existsSync(path.join(paths.baseDir, 'registration-held')); + assert.equal(Number(oldAcquired) + Number(newAcquired), 1); + if (newAcquired) { + assert.ok(claim.state === 'held'); + assert.equal(claim.owner.pid, current.pid); + } + if (oldAcquired) + assert.equal((await current.exited).exitCode, DAEMON_STARTUP_EXIT_CODES.unproven); + else await legacy.exited; + await waitForCutoverFixture(() => fs.existsSync(paths.infoPath)); + assert.equal( + JSON.parse(fs.readFileSync(paths.infoPath, 'utf8')).pid, + newAcquired ? current.pid : legacy.pid, + ); + } finally { + await legacy.stop(); + await finishRegisteredDaemonFixture(paths.baseDir); + } +}); + +for (const mode of ['graceful', 'forced'] as const) { + test(`manual ${mode} stop awaits actual child exit and protected registration retirement`, async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-manual-stop-')); + if (mode === 'forced') fs.writeFileSync(path.join(paths.baseDir, 'ignore-sigterm'), 'hold'); + const child = spawnRegisteredDaemonFixture(paths, fields, undefined); + try { + await waitForFixtureFile(paths.infoPath); + const result = await stopDaemon({ paths, graceTimeoutMs: 30, killTimeoutMs: 1_000 }); + assert.equal(result.stopped, true); + assert.equal(result.mode, mode); + assert.equal(result.cleanupConfidence, mode === 'forced' ? 'unknown' : 'known'); + await child.exited; + assert.equal(fs.existsSync(paths.infoPath), false); + assert.equal(fs.existsSync(paths.lockPath), false); + assert.equal(fs.existsSync(paths.baseDir), true); + } finally { + await finishRegisteredDaemonFixture(paths.baseDir); + } + }); +} + +test.for(['different pid', 'different birth', 'joined child'] as const)( + 'startup birth recovery refuses $0 despite published metadata', + async (proof) => { + const ownedStateDir = createOwnedReplayStateDir(); + const paths = ownedStateDir.paths; + const probe = vi.spyOn(hostProcess, 'readProcessStartTime').mockReturnValueOnce(null); + const launch = launchDaemonProcess({ + paths, + args: registeredDaemonFixtureArgs(paths, fields), + serverMode: 'socket', + ownedStateDir, + }); + probe.mockRestore(); + let actualStartTime: string | null = null; + try { + await waitForFixtureFile(paths.infoPath); + actualStartTime = hostProcess.readProcessStartTime(launch.pid); + assert.ok(actualStartTime); + if (proof === 'joined child') { + const metadata = fs.readFileSync(paths.infoPath); + const savedLock = path.join(paths.baseDir, 'saved-lock'); + fs.cpSync(paths.lockPath, savedLock, { recursive: true }); + process.kill(launch.pid, 'SIGTERM'); + await launch.exited; + fs.writeFileSync(paths.infoPath, metadata); + fs.cpSync(savedLock, paths.lockPath, { recursive: true }); + } else { + replaceInfo( + paths, + proof === 'different pid' ? process.pid : launch.pid, + proof === 'different birth' ? 'different-birth' : actualStartTime, + ); + } + const metadata = fs.readFileSync(paths.infoPath, 'utf8'); + assert.deepEqual(launch.readIdentity(), { pid: launch.pid, startTime: null }); + assert.equal(launch.startTime, undefined); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), metadata); + } finally { + await finishPrivateTestDaemons(paths, { ...launch, startTime: actualStartTime ?? undefined }); + } + }, +); diff --git a/src/__tests__/test-utils/daemon-http-fixture.ts b/src/__tests__/test-utils/daemon-http-fixture.ts index 4712a049c6..485bba4e7e 100644 --- a/src/__tests__/test-utils/daemon-http-fixture.ts +++ b/src/__tests__/test-utils/daemon-http-fixture.ts @@ -16,6 +16,7 @@ export type HttpDaemonFixture = { export async function startHttpDaemonFixture( responseData: Record, + options: { ready?: () => boolean } = {}, ): Promise { const seenPaths: string[] = []; const rpcRequests: Record[] = []; @@ -24,7 +25,7 @@ export async function startHttpDaemonFixture( seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`); if (req.method === 'GET' && url.pathname === '/health') { - res.writeHead(200); + res.writeHead(options.ready?.() === false ? 503 : 200); res.end('ok'); return; } diff --git a/src/__tests__/test-utils/legacy-daemon-fixture.ts b/src/__tests__/test-utils/legacy-daemon-fixture.ts new file mode 100644 index 0000000000..b5b40ff06a --- /dev/null +++ b/src/__tests__/test-utils/legacy-daemon-fixture.ts @@ -0,0 +1,92 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { runCmdDetachedMonitored } from '@agent-device/host-kit/command'; +import { readProcessStartTime } from '@agent-device/host-kit/process'; +import { stopDaemonProcess } from '../../daemon-process.ts'; +import type { DaemonPaths } from '../../daemon-resolution.ts'; + +// c237027737: server-lifecycle.ts readLockInfo/acquireDaemonLock/releaseDaemonLock. +const legacyLockProtocol = ` +function readLockInfo(lockPath) { + if (!fs.existsSync(lockPath)) return null; + try { + const parsed = JSON.parse(fs.readFileSync(lockPath, 'utf8')); + if (!Number.isInteger(parsed.pid) || parsed.pid <= 0) return null; + return parsed; + } catch { + return null; + } +} +function acquireDaemonLock(baseDir, lockPath, lockData) { + if (!fs.existsSync(baseDir)) fs.mkdirSync(baseDir, { recursive: true }); + const payload = JSON.stringify(lockData, null, 2); + const tryWriteLock = () => { + try { + fs.writeFileSync(lockPath, payload, { flag: 'wx', mode: 0o600 }); + return true; + } catch (error) { + if (error.code === 'EEXIST') return false; + throw error; + } + }; + if (tryWriteLock()) return true; + const existing = readLockInfo(lockPath); + if (existing?.pid && existing.pid !== process.pid && + isAgentDeviceDaemonProcess(existing.pid, existing.processStartTime)) return false; + try { fs.unlinkSync(lockPath); } catch {} + return tryWriteLock(); +} +function releaseDaemonLock(lockPath) { + const existing = readLockInfo(lockPath); + if (existing && existing.pid !== process.pid) return; + try { if (fs.existsSync(lockPath)) fs.unlinkSync(lockPath); } catch {} +} +`; + +export function spawnLegacyDaemonFixture(paths: DaemonPaths, acquisitionBarrier?: string) { + const codeDir = path.join(paths.baseDir, 'legacy'); + const entry = path.join(codeDir, 'dist', 'src', 'internal', 'daemon.js'); + fs.mkdirSync(path.dirname(entry), { recursive: true }); + fs.writeFileSync(path.join(codeDir, 'package.json'), '{"type":"module"}'); + const processUrl = new URL('../../daemon-process.ts', import.meta.url).href; + const hostProcessUrl = new URL('../../../packages/host-kit/src/process.ts', import.meta.url).href; + fs.writeFileSync( + entry, + ` +import fs from 'node:fs'; +import { isAgentDeviceDaemonProcess } from ${JSON.stringify(processUrl)}; +import { readProcessStartTime } from ${JSON.stringify(hostProcessUrl)}; +${legacyLockProtocol} +const paths = ${JSON.stringify(paths)}; +const barrier = ${JSON.stringify(acquisitionBarrier)}; +if (barrier) { + fs.writeFileSync(barrier + '.ready-' + process.pid, 'ready'); + while (!fs.existsSync(barrier)) await new Promise(resolve => setTimeout(resolve, 10)); +} +const identity = { pid: process.pid, processStartTime: readProcessStartTime(process.pid) }; +const acquired = acquireDaemonLock(paths.baseDir, paths.lockPath, { + ...identity, version: '0.21.20', startedAt: Date.now(), +}); +fs.writeFileSync(paths.baseDir + '/legacy-disposition.json', JSON.stringify({ acquired })); +if (!acquired) process.exit(0); +fs.writeFileSync(paths.infoPath, JSON.stringify({ ...identity, port: 4210, token: 'legacy-token', version: '0.21.20' })); +process.on('SIGTERM', () => { releaseDaemonLock(paths.lockPath); process.exit(0); }); +setInterval(() => {}, 1000); +`, + ); + const child = runCmdDetachedMonitored(process.execPath, ['--experimental-strip-types', entry]); + const startTime = readProcessStartTime(child.pid); + return { + pid: child.pid, + exited: child.exited, + async stop() { + const result = await stopDaemonProcess( + { pid: child.pid, startTime }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 2_000 }, + ); + assert.notEqual(result.status, 'retained', JSON.stringify(result)); + await child.exited; + }, + }; +} diff --git a/src/__tests__/test-utils/registered-daemon-fixture.test.ts b/src/__tests__/test-utils/registered-daemon-fixture.test.ts new file mode 100644 index 0000000000..e4e265f4af --- /dev/null +++ b/src/__tests__/test-utils/registered-daemon-fixture.test.ts @@ -0,0 +1,43 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { test } from 'vitest'; +import { resolveDaemonPaths } from '../../daemon-resolution.ts'; +import { stopDaemonProcess } from '../../daemon-process.ts'; +import { mkdtempForTestSync } from './tmp-dir.ts'; +import { + spawnRegisteredDaemonFixture, + waitForRegisteredDaemonFixture, + finishRegisteredDaemonFixture, +} from './registered-daemon-fixture.ts'; + +test('a joined fixture exit refuses its remaining registration metadata', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-fixture-exited-publication-')); + const child = spawnRegisteredDaemonFixture( + paths, + { + httpPort: 4210, + token: 'fixture', + version: 'test', + codeOrigin: 'checkout', + codeSignature: 'fixture', + }, + undefined, + ); + try { + const observed = await waitForRegisteredDaemonFixture(paths, child); + assert.equal( + ( + await stopDaemonProcess( + { pid: child.pid, startTime: observed.processStartTime ?? null }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 1_000 }, + ) + ).status, + 'exited', + ); + await child.exited; + assert.equal(fs.existsSync(paths.infoPath), true); + await assert.rejects(waitForRegisteredDaemonFixture(paths, child), /exited before publication/); + } finally { + await finishRegisteredDaemonFixture(paths.baseDir); + } +}); diff --git a/src/__tests__/test-utils/registered-daemon-fixture.ts b/src/__tests__/test-utils/registered-daemon-fixture.ts new file mode 100644 index 0000000000..9131617410 --- /dev/null +++ b/src/__tests__/test-utils/registered-daemon-fixture.ts @@ -0,0 +1,115 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { vi } from 'vitest'; +import type { runCmdDetachedMonitored, ExecDetachedExit } from '@agent-device/host-kit/command'; +import { readDaemonInfo, type DaemonInfo } from '../../daemon-client/daemon-client-metadata.ts'; +import { readProcessStartTime } from '@agent-device/host-kit/process'; +import { stopDaemonProcess } from '../../daemon-process.ts'; +import type { DaemonPaths } from '../../daemon-resolution.ts'; +import type { DaemonRegistrationFields } from '../../daemon-registration-owner.ts'; + +const actualCommand = await vi.importActual( + '@agent-device/host-kit/command', +); +const children = new Map< + string, + Array<{ launch: ReturnType; startTime: string | null }> +>(); + +/** A real registration owner, advertising the caller's HTTP fixture and joining before deletion. */ +export function registeredDaemonFixtureArgs( + paths: DaemonPaths, + fields: DaemonRegistrationFields, + acquisitionBarrier?: string, +): string[] { + const entry = path.join(paths.baseDir, 'dist', 'src', 'internal', 'daemon.js'); + fs.mkdirSync(path.dirname(entry), { recursive: true }); + fs.writeFileSync(path.join(paths.baseDir, 'package.json'), '{"type":"module"}'); + const registrationUrl = new URL('../../daemon-registration-owner.ts', import.meta.url).href; + fs.writeFileSync( + entry, + `import fs from 'node:fs'; +import path from 'node:path'; +import { DAEMON_STARTUP_EXIT_CODES, tryAcquireDaemonRegistration } from ${JSON.stringify(registrationUrl)}; +const paths = ${JSON.stringify(paths)}; +const barrier = ${JSON.stringify(acquisitionBarrier)}; +if (barrier) { + fs.writeFileSync(barrier + '.ready-' + process.pid, 'ready'); + while (!fs.existsSync(barrier)) await new Promise(resolve => setTimeout(resolve, 10)); +} +const acquired = await tryAcquireDaemonRegistration(paths); +if (acquired.status !== 'acquired') process.exit(DAEMON_STARTUP_EXIT_CODES[acquired.status]); +process.on('SIGTERM', async () => { + if (fs.existsSync(path.join(paths.baseDir, 'ignore-sigterm'))) return; + const deferred = path.join(paths.baseDir, 'repair-on-shutdown.json'); + if (fs.existsSync(deferred)) { + const dir = path.join(paths.sessionsDir, 'default'); + fs.mkdirSync(dir, { recursive: true }); + fs.copyFileSync(deferred, path.join(dir, 'repair-tombstone.json')); + } + await acquired.owner.finish(); + process.exit(0); +}); +fs.writeFileSync(path.join(paths.baseDir, 'registration-held'), 'ready'); +while (fs.existsSync(path.join(paths.baseDir, 'defer-publication'))) await new Promise(resolve => setTimeout(resolve, 10)); +acquired.owner.publish(${JSON.stringify(fields)}); +setInterval(() => {}, 1000); +`, + ); + return ['--experimental-strip-types', entry]; +} + +export function spawnRegisteredDaemonFixture( + paths: DaemonPaths, + fields: DaemonRegistrationFields, + options: Parameters[2], + acquisitionBarrier?: string, +): ReturnType { + const child = actualCommand.runCmdDetachedMonitored( + process.execPath, + registeredDaemonFixtureArgs(paths, fields, acquisitionBarrier), + options, + ); + const owned = children.get(paths.baseDir) ?? []; + owned.push({ launch: child, startTime: readProcessStartTime(child.pid) }); + children.set(paths.baseDir, owned); + return child; +} + +export async function waitForRegisteredDaemonFixture( + paths: DaemonPaths, + child: ReturnType, +): Promise { + let exit: ExecDetachedExit | undefined; + void child.exited.then((result) => { + exit = result; + }); + await Promise.resolve(); + for (let attempt = 0; attempt < 400; attempt += 1) { + if (exit) + throw new Error(`Registered child exited before publication: ${JSON.stringify(exit)}`); + const info = readDaemonInfo(paths.infoPath); + if (info?.pid === child.pid) return info; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + throw new Error(`Registered child ${child.pid} did not publish ${paths.infoPath} within 4s`); +} + +export async function finishRegisteredDaemonFixture(stateDir: string): Promise { + for (const owned of children.get(stateDir) ?? []) { + const child = owned.launch; + const termination = await stopDaemonProcess( + { pid: child.pid, startTime: owned.startTime }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 2_000 }, + ); + assert.notEqual(termination.status, 'retained', JSON.stringify(termination)); + await child.exited; + } + children.delete(stateDir); + fs.rmSync(stateDir, { recursive: true, force: true }); +} + +export async function finishRegisteredDaemonFixtures(): Promise { + for (const stateDir of children.keys()) await finishRegisteredDaemonFixture(stateDir); +} diff --git a/src/daemon-client/__tests__/daemon-client-address-hints.test.ts b/src/daemon-client/__tests__/daemon-client-address-hints.test.ts new file mode 100644 index 0000000000..22cd5c004b --- /dev/null +++ b/src/daemon-client/__tests__/daemon-client-address-hints.test.ts @@ -0,0 +1,59 @@ +import type { DaemonResponse } from '../../daemon/daemon-request.ts'; +import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { attachActiveSessionAddressHint } from '../daemon-client-address-hints.ts'; + +test('remote active-session guidance retains the endpoint without URL credentials', () => { + const response = { ok: true as const, data: { session: 'cwd:abc:default', sessionActive: true } }; + const hinted = attachActiveSessionAddressHint( + response, + undefined, + 'https://operator:private-token@example.com/team?secret=hidden#private', + ); + assert.ok(String(hinted.data?.hint).includes('--daemon-base-url https://example.com/team')); + assert.ok(String(hinted.data?.hint).includes('--session cwd:abc:default')); + assert.ok(!JSON.stringify(hinted).includes('private')); + assert.ok(!JSON.stringify(hinted).includes('hidden')); + assert.ok(!String(hinted.data?.hint).includes('--state-dir')); + assert.ok(String(hinted.data?.hint).includes('provide or configure --daemon-auth-token')); +}); + +test('attachActiveSessionAddressHint shell-quotes a --state-dir/--session value containing spaces or shell metacharacters', () => { + const unsafeStateDir = '/tmp/state dir with $(danger)'; + const unsafeSession = 'cwd:abc123:my session; rm -rf /'; + const response: Extract = { + ok: true, + data: { session: unsafeSession, message: 'Replayed 1 step in 0.1s' }, + }; + + const hinted = attachActiveSessionAddressHint(response, unsafeStateDir); + + assert.equal( + hinted.data?.hint, + "This session's daemon was kept alive because its script left the session active; " + + `pass --state-dir ${shellQuoteIfNeeded(unsafeStateDir)} ` + + `--session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, + ); + // Both values actually needed quoting — this test would pass vacuously + // (raw interpolation indistinguishable from quoted) if they didn't. + assert.notEqual(shellQuoteIfNeeded(unsafeStateDir), unsafeStateDir); + assert.notEqual(shellQuoteIfNeeded(unsafeSession), unsafeSession); +}); + +test('attachActiveSessionAddressHint omits --state-dir but still quotes an unsafe --session-only value', () => { + const unsafeSession = "cwd:abc123:it's mine"; + const response: Extract = { + ok: true, + data: { session: unsafeSession, message: 'Replayed 1 step in 0.1s' }, + }; + + const hinted = attachActiveSessionAddressHint(response, undefined); + + assert.equal( + hinted.data?.hint, + "This session's daemon was kept alive because its script left the session active; " + + `pass --session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, + ); + assert.doesNotMatch(String(hinted.data?.hint), /--state-dir/); +}); diff --git a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts index 4ad2f6e8c8..39e80a6123 100644 --- a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts +++ b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts @@ -6,6 +6,12 @@ import net from 'node:net'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + spawnRegisteredDaemonFixture, + waitForRegisteredDaemonFixture, + finishRegisteredDaemonFixture, + finishRegisteredDaemonFixtures, +} from '../../__tests__/test-utils/registered-daemon-fixture.ts'; vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ ...(await importOriginal()), @@ -19,9 +25,9 @@ vi.mock('@agent-device/host-kit/retry', async (importOriginal) => ({ })); import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts'; -import { sendToDaemon, type DaemonRequest, type DaemonResponse } from '../daemon-client.ts'; -import { attachActiveSessionAddressHint } from '../daemon-client-lifecycle.ts'; +import { sendToDaemon, type DaemonRequest } from '../daemon-client.ts'; import { sendRequest } from '../daemon-client-transport.ts'; +import type { DaemonRetirementResult } from '../../daemon-registration-owner.ts'; import { closeLoopbackServer, listenOnLoopback, @@ -34,8 +40,8 @@ import { currentDaemonCodeSignature, } from '../../__tests__/test-utils/daemon-http-fixture.ts'; import { AppError } from '@agent-device/kernel/errors'; +import { tryAcquireProcessLock, inspectProcessLock } from '@agent-device/host-kit/file'; import { runCmdDetachedMonitored, runCmdSync } from '@agent-device/host-kit/command'; -import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { readProcessStartTime } from '@agent-device/host-kit/process'; import { sleep } from '@agent-device/host-kit/retry'; import { readVersion } from '@agent-device/host-kit/version'; @@ -51,14 +57,20 @@ type DaemonInfoFixture = { processStartTime?: string; }; +const actualRetry = await vi.importActual( + '@agent-device/host-kit/retry', +); const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored); const mockRunCmdSync = vi.mocked(runCmdSync); const mockSleep = vi.mocked(sleep); -afterEach(() => { +afterEach(async () => { + await finishRegisteredDaemonFixtures(); mockRunCmdDetached.mockReset(); mockRunCmdSync.mockClear(); - mockSleep.mockClear(); + mockSleep.mockReset(); + mockSleep.mockImplementation(async () => {}); + vi.unstubAllEnvs(); }); function makeTempStateDir(prefix: string): string { @@ -146,16 +158,22 @@ function installSpawnedHttpDaemonAtOwnedStateDir( httpPort: number, onStateDir: (stateDir: string) => void, ): void { + mockSleep.mockImplementation(actualRetry.sleep); mockRunCmdDetached.mockImplementation((_command, _args, options) => { const ownedStateDir = String(options?.env?.AGENT_DEVICE_STATE_DIR); onStateDir(ownedStateDir); const ownedPaths = resolveDaemonPaths(ownedStateDir); - writeDaemonInfo(ownedPaths, { httpPort, transport: 'http' }); - writeDaemonLock(ownedPaths, { - pid: process.pid, - processStartTime: readProcessStartTime(process.pid) ?? undefined, - }); - return { pid: process.pid, exited: new Promise(() => {}) }; + return spawnRegisteredDaemonFixture( + ownedPaths, + { + httpPort, + token: 'local-secret', + version: readVersion(), + codeOrigin: 'checkout', + codeSignature: currentDaemonCodeSignature(), + }, + options, + ); }); } @@ -191,14 +209,20 @@ async function startHangingHttpDaemonFixture(): Promise { } function installSpawnedHttpDaemon(paths: DaemonPaths, httpPort: number): void { + mockSleep.mockImplementation(actualRetry.sleep); mockRunCmdDetached.mockImplementation((_command, _args, options) => { assert.equal(options?.env?.AGENT_DEVICE_STATE_DIR, paths.baseDir); - writeDaemonInfo(paths, { httpPort, transport: 'http' }); - writeDaemonLock(paths, { - pid: process.pid, - processStartTime: readProcessStartTime(process.pid) ?? undefined, - }); - return { pid: process.pid, exited: new Promise(() => {}) }; + return spawnRegisteredDaemonFixture( + paths, + { + httpPort, + token: 'local-secret', + version: readVersion(), + codeOrigin: 'checkout', + codeSignature: currentDaemonCodeSignature(), + }, + options, + ); }); } @@ -297,7 +321,7 @@ function mockSocketErrorAfterWrite(failingPort: number): { }; } -test('sendToDaemon retries daemon spawn failures and cleans partial metadata on terminal failure', async () => { +test('sendToDaemon retains unknown metadata after a spawn failure', async () => { const stateDir = makeTempStateDir('agent-device-daemon-spawn-retry-'); const paths = resolveDaemonPaths(stateDir); vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); @@ -328,27 +352,19 @@ test('sendToDaemon retries daemon spawn failures and cleans partial metadata on assert.ok(thrown instanceof AppError); assert.equal(thrown.message, 'Failed to start daemon'); - assert.equal(thrown.details?.startError, 'spawn failed 2'); - assert.equal(thrown.details?.startupAttempts, 2); - const cleanupResults = thrown.details?.cleanupResults; - assert.ok(Array.isArray(cleanupResults)); - assert.deepEqual( - cleanupResults.map((result) => ({ - reason: result.reason, - removedInfo: result.removedInfo, - removedLock: result.removedLock, - })), - [ - { reason: 'start_error', removedInfo: true, removedLock: true }, - { reason: 'start_error', removedInfo: true, removedLock: true }, - ], - ); - assert.equal(attempts, 2); - assert.equal(mockSleep.mock.calls[0]?.[0], 150); - assert.equal(fs.existsSync(paths.infoPath), false); - assert.equal(fs.existsSync(paths.lockPath), false); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), '{"partial":true}\n'); + assert.equal(fs.readFileSync(paths.lockPath, 'utf8'), 'not-json\n'); + assert.equal(thrown.details?.startError, 'spawn failed 1'); + assert.equal(thrown.details?.startupAttempts, 1); + const results = thrown.details?.cleanupResults as Array<{ + status: string; + removedInfo: boolean; + }>; + assert.equal(results[0]?.status, 'retained'); + assert.equal(results[0]?.removedInfo, false); + assert.equal(attempts, 1); } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -393,11 +409,11 @@ test('sendToDaemon reports early daemon exit with log tail and startup paths', a assert.match(String(thrown.details?.daemonLogTail), /early daemon failure 2/); assert.equal(attempts, 2); } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); -test('sendToDaemon removes stale daemon lock before spawning a fresh daemon', async (t) => { +test('daemon acquisition reclaims a proven reused owner before publication', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); return; @@ -407,10 +423,11 @@ test('sendToDaemon removes stale daemon lock before spawning a fresh daemon', as const paths = resolveDaemonPaths(stateDir); const daemon = await startHttpDaemonFixture({ via: 'fresh-daemon' }); vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); - writeDaemonLock(paths, { - pid: process.pid, - processStartTime: 'stale-start-time', + const stale = tryAcquireProcessLock({ + lockDirPath: paths.lockPath, + owner: { pid: process.pid, startTime: 'stale-start-time', acquiredAtMs: Date.now() }, }); + assert.equal(stale.status, 'acquired'); installSpawnedHttpDaemon(paths, daemon.port); try { @@ -422,18 +439,16 @@ test('sendToDaemon removes stale daemon lock before spawning a fresh daemon', as meta: { requestId: 'req-stale-lock' }, }); - const freshLock = JSON.parse(fs.readFileSync(paths.lockPath, 'utf8')) as { - pid?: number; - processStartTime?: string; - }; + const freshLock = inspectProcessLock(paths.lockPath); assert.deepEqual(response, { ok: true, data: { via: 'fresh-daemon' } }); assert.equal(mockRunCmdDetached.mock.calls.length, 1); - assert.equal(freshLock.pid, process.pid); - assert.notEqual(freshLock.processStartTime, 'stale-start-time'); + assert.equal(freshLock.state, 'held'); + if (freshLock.state === 'held') assert.notEqual(freshLock.owner.startTime, 'stale-start-time'); assert.deepEqual(daemon.seenPaths, ['GET /health', 'POST /rpc']); } finally { + if (stale.status === 'acquired') await stale.acquisition.release(); await closeLoopbackServer(daemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -501,7 +516,8 @@ test('sendToDaemon does not reuse reachable daemon metadata with mismatched vers stderrCapture.restore(); await closeLoopbackServer(staleDaemon.server); await closeLoopbackServer(freshDaemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); + vi.unstubAllEnvs(); } } }); @@ -514,7 +530,6 @@ test('sendToDaemon prints a takeover notice before replacing an unreachable daem const stateDir = makeTempStateDir('agent-device-daemon-unreachable-takeover-'); const paths = resolveDaemonPaths(stateDir); - // Bind fresh BEFORE freeing the port below: a later bind can reclaim it and skip the takeover. const freshDaemon = await startHttpDaemonFixture({ via: 'fresh-daemon' }); const unreachable = await startHttpDaemonFixture({ via: 'unused' }); await closeLoopbackServer(unreachable.server); @@ -544,7 +559,7 @@ test('sendToDaemon prints a takeover notice before replacing an unreachable daem } finally { stderrCapture.restore(); await closeLoopbackServer(freshDaemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -585,7 +600,7 @@ test('sendToDaemon replaces socket-only daemon metadata when HTTP transport is r } finally { stderrCapture.restore(); await closeLoopbackServer(freshDaemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -600,12 +615,18 @@ test('sendRequest timeout cleanup uses resolved daemon paths instead of request const daemonPaths = resolveDaemonPaths(daemonStateDir); const requestFlagPaths = resolveDaemonPaths(requestFlagStateDir); const daemon = await startHangingHttpDaemonFixture(); - writeDaemonInfo(daemonPaths, { - httpPort: daemon.port, - transport: 'http', - pid: 999_999, - }); - writeDaemonLock(daemonPaths, { pid: 999_999 }); + mockSleep.mockImplementation(actualRetry.sleep); + const child = spawnRegisteredDaemonFixture( + daemonPaths, + { + httpPort: daemon.port, + token: 'local-secret', + version: readVersion(), + codeOrigin: 'checkout', + codeSignature: currentDaemonCodeSignature(), + }, + undefined, + ); writeDaemonInfo(requestFlagPaths, { httpPort: daemon.port, transport: 'http', @@ -623,26 +644,21 @@ test('sendRequest timeout cleanup uses resolved daemon paths instead of request }; try { + const info = await waitForRegisteredDaemonFixture(daemonPaths, child); let thrown: unknown; try { - await sendRequest( - { - token: 'local-secret', - pid: 999_999, - httpPort: daemon.port, - transport: 'http', - }, - request, - 'http', - daemonPaths, - 50, - ); + await sendRequest(info, request, 'http', daemonPaths, 50); } catch (error) { thrown = error; } assert.ok(thrown instanceof AppError); assert.equal(thrown.message, 'Daemon request timed out'); + assert.equal( + (thrown.details?.retirement as DaemonRetirementResult | undefined)?.status, + 'retired', + ); + await child.exited; assert.deepEqual(daemon.seenPaths, ['POST /rpc']); assert.equal(fs.existsSync(daemonPaths.infoPath), false); assert.equal(fs.existsSync(daemonPaths.lockPath), false); @@ -650,7 +666,7 @@ test('sendRequest timeout cleanup uses resolved daemon paths instead of request assert.equal(fs.existsSync(requestFlagPaths.lockPath), true); } finally { await closeLoopbackServer(daemon.server); - fs.rmSync(daemonStateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(daemonStateDir); fs.rmSync(requestFlagStateDir, { recursive: true, force: true }); } }); @@ -688,7 +704,7 @@ test('sendToDaemon falls back from failed socket transport to HTTP using daemon } finally { socketFailures.restore(); await closeLoopbackServer(daemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -732,16 +748,10 @@ test('sendToDaemon does not replay over HTTP after the socket request is written } finally { socket.restore(); await closeLoopbackServer(daemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); -// --- ADR 0012 decision 6, R7 (Fix 1, C1): a repair-armed `replay --save-script` -// that comes back as a HELD divergence (the daemon's `resume.repairSessionHeld` -// signal) must keep its owning (owned/ephemeral) daemon alive and addressable. -// The keep-alive keys on that signal — the REPAIR-ARMED condition — NOT on -// `resume.allowed`, which reports only plan-resumability. --- - function heldDivergenceError( resume: Record = { allowed: true, from: 3, planDigest: 'digest-abc' }, ): Record { @@ -803,15 +813,13 @@ test('sendToDaemon keeps an owned ephemeral daemon alive and hints its --state-d assert.match(String(response.error.hint), /--state-dir/); assert.ok(String(response.error.hint).includes(ownedStateDir)); - // The daemon was NOT torn down: metadata and the owned state dir itself - // are still on disk, addressable by a follow-up command's --state-dir. const ownedPaths = resolveDaemonPaths(ownedStateDir); assert.equal(fs.existsSync(ownedPaths.infoPath), true); assert.equal(fs.existsSync(ownedPaths.lockPath), true); assert.equal(fs.existsSync(ownedStateDir), true); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -821,8 +829,6 @@ test('C1: keep-alive keys on repairSessionHeld, NOT resume.allowed — a HELD di return; } - // resume.allowed:false (plan not resumable), but the daemon still HELD the - // repair session — the agent must be able to reach it to close/inspect. const daemon = await startHttpDaemonErrorFixture( heldDivergenceError({ allowed: false, @@ -852,7 +858,7 @@ test('C1: keep-alive keys on repairSessionHeld, NOT resume.allowed — a HELD di assert.equal(fs.existsSync(ownedStateDir), true); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -881,92 +887,76 @@ test('sendToDaemon tears down an owned ephemeral daemon on an UNHELD divergence if (response.ok) return; assert.equal(response.error.hint, undefined); assert.ok(ownedStateDir.length > 0); - // No held signal (`resume.allowed:true` alone is not the keep-alive key) — - // ordinary one-shot teardown still applies. assert.equal(fs.existsSync(ownedStateDir), false); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); -// --- ADR 0012 decision 6 (BLOCKER 2, third follow-up): a one-shot -// `replay --save-script` that completes with no divergence returns SUCCESS -// immediately — the actual healed-script commit is deferred to daemon -// teardown. If that deferred commit then fails, the daemon leaves a -// REPAIR_COMMIT_FAILED tombstone in the owned state dir before exiting. The -// client cleanup must discover it (after waiting for the daemon to actually -// exit) BEFORE deleting the owned state dir, and must surface it in the -// response the caller receives — never silently delete the only evidence of -// the failure while reporting the success already computed for the replay -// itself. --- - test('BLOCKER 2 (third follow-up): a shutdown-time repair commit failure is surfaced and the owned state dir survives', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); return; } - // The daemon's RPC response for the replay itself is a plain SUCCESS (the - // plan completed with no divergence) — exactly what a real daemon would - // return before its deferred, teardown-time commit has even attempted. - const daemon = await startHttpDaemonFixture({ session: 'default' }); - let ownedStateDir = ''; - installSpawnedHttpDaemonAtOwnedStateDir(daemon.port, (dir) => { - ownedStateDir = dir; - // Simulate the daemon's OWN shutdown handler (`finalizeRepairTeardown`) - // having already run and left a commit-failure tombstone before this - // fake process "exits" — the real ordering `stopDaemonProcessForTakeover` - // depends on (it waits for the process to exit, and the real daemon only - // exits after teardown finishes writing this file). - const ownedPaths = resolveDaemonPaths(dir); - const sessionDir = path.join(ownedPaths.sessionsDir, 'default'); - fs.mkdirSync(sessionDir, { recursive: true }); - fs.writeFileSync( - path.join(sessionDir, 'repair-tombstone.json'), - `${JSON.stringify({ - owner: 'default', - reapedAt: Date.now(), - expiresAt: Date.now() + 60_000, - sourcePath: '/tmp/flow.ad', - commitFailure: { - code: 'COMMAND_FAILED', - message: 'a prior healed script already exists at /tmp/flow.healed.ad', - }, - })}\n`, - ); - }); - - try { - const response = await sendToDaemon({ - session: 'default', - command: 'replay', - positionals: ['flow.ad'], - flags: { saveScript: true, daemonTransport: 'http' }, - meta: { requestId: 'req-repair-commit-fail-teardown' }, + for (const failRelease of [false, true]) { + const daemon = await startHttpDaemonFixture({ session: 'default' }); + let ownedStateDir = ''; + installSpawnedHttpDaemonAtOwnedStateDir(daemon.port, (dir) => { + ownedStateDir = dir; + fs.writeFileSync( + path.join(dir, 'repair-on-shutdown.json'), + `${JSON.stringify({ + owner: 'default', + reapedAt: Date.now(), + expiresAt: Date.now() + 60_000, + sourcePath: '/tmp/flow.ad', + commitFailure: { + code: 'COMMAND_FAILED', + message: 'a prior healed script already exists at /tmp/flow.healed.ad', + }, + })}\n`, + ); }); - // The client-visible response must surface the deferred commit failure — - // never the raw success the daemon returned for the replay itself, and - // never silently swallowed by cleanup. - assert.equal(response.ok, false); - if (response.ok) return; - assert.equal(response.error.code, 'REPAIR_COMMIT_FAILED'); - assert.match(response.error.message, /a prior healed script already exists/); - assert.ok(response.error.message.includes('replay /tmp/flow.ad --save-script')); + const originalRmdir = fs.rmdirSync; + const releaseSpy = vi.spyOn(fs, 'rmdirSync').mockImplementation((target, options) => { + if (failRelease && target === resolveDaemonPaths(ownedStateDir).lockPath) + throw Object.assign(new Error('release failed'), { code: 'EBUSY' }); + return originalRmdir(target, options); + }); + try { + const response = await sendToDaemon({ + session: 'default', + command: 'replay', + positionals: ['flow.ad'], + flags: { saveScript: true, daemonTransport: 'http' }, + meta: { requestId: 'req-repair-commit-fail-teardown' }, + }); - // The owned state dir — and the tombstone evidence inside it — must - // survive: never rmSync'd while an unrecovered commit failure is on record. - assert.ok(ownedStateDir.length > 0); - assert.equal(fs.existsSync(ownedStateDir), true); - const ownedPaths = resolveDaemonPaths(ownedStateDir); - assert.equal( - fs.existsSync(path.join(ownedPaths.sessionsDir, 'default', 'repair-tombstone.json')), - true, - ); - } finally { - await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + assert.ok(!response.ok); + assert.equal(response.error.code, 'REPAIR_COMMIT_FAILED'); + const secondary = response.error.details?.cleanupFailure as + | { details?: { ownerReleaseUnverified?: boolean }; hint?: string } + | undefined; + assert.equal(Boolean(secondary?.details?.ownerReleaseUnverified), failRelease); + assert.equal(Boolean(secondary?.hint?.startsWith('Restore process inspection')), failRelease); + assert.match(response.error.message, /a prior healed script already exists/); + assert.ok(response.error.message.includes('replay /tmp/flow.ad --save-script')); + + assert.ok(ownedStateDir.length > 0); + assert.equal(fs.existsSync(ownedStateDir), true); + const ownedPaths = resolveDaemonPaths(ownedStateDir); + assert.equal( + fs.existsSync(path.join(ownedPaths.sessionsDir, 'default', 'repair-tombstone.json')), + true, + ); + } finally { + releaseSpy.mockRestore(); + await closeLoopbackServer(daemon.server); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); + } } }); @@ -1001,7 +991,7 @@ test('continuation: sendToDaemon keeps the daemon alive on a held divergence eve assert.equal(fs.existsSync(ownedStateDir), true); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -1029,45 +1019,6 @@ function activeReplaySuccessData(overrides: Record = {}): Recor }; } -test('attachActiveSessionAddressHint shell-quotes a --state-dir/--session value containing spaces or shell metacharacters', () => { - const unsafeStateDir = '/tmp/state dir with $(danger)'; - const unsafeSession = 'cwd:abc123:my session; rm -rf /'; - const response: Extract = { - ok: true, - data: activeReplaySuccessData({ session: unsafeSession }), - }; - - const hinted = attachActiveSessionAddressHint(response, unsafeStateDir); - - assert.equal( - hinted.data?.hint, - "This session's daemon was kept alive because its script left the session active; " + - `pass --state-dir ${shellQuoteIfNeeded(unsafeStateDir)} ` + - `--session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, - ); - // Both values actually needed quoting — this test would pass vacuously - // (raw interpolation indistinguishable from quoted) if they didn't. - assert.notEqual(shellQuoteIfNeeded(unsafeStateDir), unsafeStateDir); - assert.notEqual(shellQuoteIfNeeded(unsafeSession), unsafeSession); -}); - -test('attachActiveSessionAddressHint omits --state-dir but still quotes an unsafe --session-only value', () => { - const unsafeSession = "cwd:abc123:it's mine"; - const response: Extract = { - ok: true, - data: activeReplaySuccessData({ session: unsafeSession }), - }; - - const hinted = attachActiveSessionAddressHint(response, undefined); - - assert.equal( - hinted.data?.hint, - "This session's daemon was kept alive because its script left the session active; " + - `pass --session ${shellQuoteIfNeeded(unsafeSession)} on your next command to reach it.`, - ); - assert.doesNotMatch(String(hinted.data?.hint), /--state-dir/); -}); - /** Issues a close-less `replay` against an owned ephemeral daemon spawned at `daemonPort`. */ async function replayLeavingSessionActive( daemonPort: number, @@ -1129,7 +1080,7 @@ test('sendToDaemon keeps an owned ephemeral daemon alive and hints its --state-d assert.equal(fs.existsSync(ownedStateDir), true); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -1168,7 +1119,7 @@ test('closes the loop: a follow-up sendToDaemon using the hinted --state-dir/--s assert.equal(daemon.rpcRequests[1]?.params?.command, 'press'); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -1200,7 +1151,7 @@ test('sendToDaemon tears down an owned ephemeral daemon when replay reports the assert.equal(fs.existsSync(ownedStateDir), false); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -1246,7 +1197,7 @@ test('ADR 0012 R7 x ADR 0016: a completed --save-script repair also keeps its ow assert.equal(fs.existsSync(ownedStateDir), true); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); @@ -1292,7 +1243,7 @@ test('issue #1384: sendToDaemon does not stop a client-started daemon at an expl assert.equal(fs.existsSync(paths.lockPath), true); } finally { await closeLoopbackServer(daemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -1326,6 +1277,6 @@ test('sendToDaemon still tears down a `test` command owned ephemeral daemon even assert.equal(fs.existsSync(ownedStateDir), false); } finally { await closeLoopbackServer(daemon.server); - if (ownedStateDir) fs.rmSync(ownedStateDir, { recursive: true, force: true }); + if (ownedStateDir) await finishRegisteredDaemonFixture(ownedStateDir); } }); diff --git a/src/daemon-client/__tests__/daemon-client-metadata.test.ts b/src/daemon-client/__tests__/daemon-client-metadata.test.ts index 5b46a91e8d..bae4084103 100644 --- a/src/daemon-client/__tests__/daemon-client-metadata.test.ts +++ b/src/daemon-client/__tests__/daemon-client-metadata.test.ts @@ -1,27 +1,13 @@ 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 { test } from 'vitest'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; import { tryAcquireDaemonRegistration } from '../../daemon-registration-owner.ts'; -import { - readDaemonInfo, - cleanupFailedDaemonStartupMetadata, - stopDaemonProcessForTakeover, - type DaemonInfo, -} from '../daemon-client-metadata.ts'; -import { isAgentDeviceDaemonProcess, stopDaemonProcess } from '../../daemon-process.ts'; +import { readDaemonInfo, type DaemonInfo } from '../daemon-client-metadata.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). @@ -66,39 +52,3 @@ 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; - }, - ); -}); 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 0a094c07ab..ea258d73a0 100644 --- a/src/daemon-client/__tests__/daemon-client-startup-race.test.ts +++ b/src/daemon-client/__tests__/daemon-client-startup-race.test.ts @@ -1,212 +1,636 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; -import { afterEach, beforeAll, test, vi } from 'vitest'; +import path from 'node:path'; +import { afterEach, test, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { tryAcquireProcessLock, inspectProcessLock } from '@agent-device/host-kit/file'; +import { + readCurrentOwnerIdentity, + isProcessAlive, + readProcessStartTime, +} from '@agent-device/host-kit/process'; +import { readVersion } from '@agent-device/host-kit/version'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + spawnRegisteredDaemonFixture, + finishRegisteredDaemonFixtures, +} from '../../__tests__/test-utils/registered-daemon-fixture.ts'; +import { + startHttpDaemonFixture, + currentDaemonCodeSignature, +} from '../../__tests__/test-utils/daemon-http-fixture.ts'; +import { closeLoopbackServer, supportsLoopbackBind } from '../../__tests__/test-utils/loopback.ts'; +import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts'; +import { sendToDaemon } from '../daemon-client.ts'; +import * as lifecycle from '../daemon-client-lifecycle.ts'; +import { DAEMON_STARTUP_EXIT_CODES } from '../../daemon-registration-owner.ts'; vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ ...(await importOriginal()), runCmdDetachedMonitored: vi.fn(), })); +vi.mock('@agent-device/host-kit/process', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readProcessStartTime: vi.fn(actual.readProcessStartTime) }; +}); vi.mock('@agent-device/host-kit/retry', async (importOriginal) => ({ ...(await importOriginal()), - sleep: vi.fn(async () => {}), + sleep: vi.fn(), })); -const winner = vi.hoisted(() => ({ pid: 43_300, alive: true })); -vi.mock('../../daemon-process.ts', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - isAgentDeviceDaemonProcess: vi.fn((pid: number, startTime: string | undefined) => - pid === winner.pid ? winner.alive : actual.isAgentDeviceDaemonProcess(pid, startTime), - ), - 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, - }; - }, - ), - }; -}); - -import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts'; -import { sendToDaemon } from '../daemon-client.ts'; import { runCmdDetachedMonitored, type ExecDetachedExit } from '@agent-device/host-kit/command'; import { sleep } from '@agent-device/host-kit/retry'; -import { readVersion } from '@agent-device/host-kit/version'; -import { resolveLocalDaemonCodeIdentity } from '../daemon-launch-spec.ts'; -import { - startHttpDaemonFixture, - type HttpDaemonFixture, -} from '../../__tests__/test-utils/daemon-http-fixture.ts'; -import { closeLoopbackServer, supportsLoopbackBind } from '../../__tests__/test-utils/loopback.ts'; - -// Two clients that find no daemon both launch one; the daemon that loses the startup lock exits -// cleanly. These pin that the losing client adopts the winner instead of tearing it down. +const actualRetry = await vi.importActual( + '@agent-device/host-kit/retry', +); +const spawn = vi.mocked(runCmdDetachedMonitored); +const pause = vi.mocked(sleep); +afterEach(async () => { + vi.restoreAllMocks(); + await finishRegisteredDaemonFixtures(); + spawn.mockReset(); + pause.mockReset(); +}); -const WINNER_PID = winner.pid; -const LOSER_PID = 43_301; +function request(paths: DaemonPaths, command = 'devices') { + return { + session: 'default', + command, + positionals: [], + flags: { stateDir: paths.baseDir, daemonTransport: 'http' as const }, + }; +} +function fields(httpPort: number, version = readVersion()) { + return { + httpPort, + token: 'secret', + version, + codeOrigin: 'checkout' as const, + codeSignature: currentDaemonCodeSignature(), + }; +} +async function awaitFile(file: string) { + for (let attempt = 0; !fs.existsSync(file); attempt += 1) { + assert.ok(attempt < 200, `fixture did not publish ${file}`); + await actualRetry.sleep(10); + } +} -const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored); -const mockSleep = vi.mocked(sleep); +const observationIdentity = readCurrentOwnerIdentity(); +test.for([ + { + proof: 'matching', + startTime: observationIdentity.startTime, + infoStart: observationIdentity.startTime, + expectedAllowed: true, + expectedState: 'held', + }, + { + proof: 'missing', + startTime: null, + infoStart: undefined, + expectedAllowed: false, + expectedState: 'held', + }, + { + proof: 'null', + startTime: null, + infoStart: null, + expectedAllowed: false, + expectedState: 'held', + }, + { + proof: 'empty', + startTime: '', + infoStart: '', + expectedAllowed: false, + expectedState: 'held', + }, + { + proof: 'blank', + startTime: ' \t ', + infoStart: ' \t ', + expectedAllowed: false, + expectedState: 'held', + }, + { + proof: 'absent', + startTime: null, + infoStart: undefined, + expectedAllowed: true, + expectedState: 'absent', + }, +])( + 'existing registration observation requires proved matching birth times ($proof)', + async ({ startTime, infoStart, expectedAllowed, expectedState }, t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-observation-proof-')); + const http = await startHttpDaemonFixture({ devices: [] }); + assert.ok(observationIdentity.startTime?.trim()); + const claim = + expectedState === 'absent' + ? undefined + : tryAcquireProcessLock({ + lockDirPath: paths.lockPath, + owner: { pid: observationIdentity.pid, startTime, acquiredAtMs: Date.now() }, + }); + assert.ok(!claim || claim.status === 'acquired'); + assert.equal(inspectProcessLock(paths.lockPath).state, expectedState); + fs.writeFileSync( + paths.infoPath, + JSON.stringify({ + ...fields(http.port), + pid: observationIdentity.pid, + processStartTime: infoStart, + }), + ); + const originalInfo = fs.readFileSync(paths.infoPath, 'utf8'); + spawn.mockImplementation(() => { + throw new Error('fixture refuses a replacement launch'); + }); + try { + if (expectedAllowed) { + assert.equal((await sendToDaemon(request(paths))).ok, true); + assert.equal(spawn.mock.calls.length, 0); + assert.equal(http.rpcRequests.length, 1); + } else { + await assert.rejects(sendToDaemon(request(paths)), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.kind, 'daemon_startup_failed'); + return true; + }); + assert.equal(spawn.mock.calls.length, 1); + assert.deepEqual(http.seenPaths, []); + } + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), originalInfo); + } finally { + if (claim?.status === 'acquired') await claim.acquisition.release(); + await closeLoopbackServer(http.server); + } + }, +); -afterEach(() => { - winner.alive = true; - mockRunCmdDetached.mockReset(); - mockSleep.mockReset(); - mockSleep.mockImplementation(async () => {}); +test('a synchronous private startup failure removes its never-started owned directory', async () => { + let paths: DaemonPaths | undefined; + spawn.mockImplementation((_command, _args, options) => { + paths = resolveDaemonPaths(String(options?.env?.AGENT_DEVICE_STATE_DIR)); + throw new Error('cannot launch fixture'); + }); + await assert.rejects( + sendToDaemon({ session: 'default', command: 'test', positionals: [], flags: {} }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.kind, 'daemon_startup_failed'); + const cleanup = error.details?.cleanupResults; + assert.ok(Array.isArray(cleanup)); + assert.equal(cleanup[0]?.removedStateDir, true); + return true; + }, + ); + assert.ok(paths); + assert.equal(fs.existsSync(paths.baseDir), false); + assert.equal(spawn.mock.calls.length, 1); }); -/** The code signature this client stamps on, and expects of, a daemon it may reuse. */ -let codeSignature: string | undefined; - -beforeAll(async () => { - const identity = await resolveLocalDaemonCodeIdentity(); - codeSignature = identity.origin === 'installed' ? undefined : identity.codeSignature; -}); +test.for(['early exit', 'timeout'] as const)( + 'a private startup $0 retires its owned directory after joining the child', + async (failure) => { + let paths: DaemonPaths | undefined; + let child: ReturnType | undefined; + let now = Date.now(); + let advanced = false; + vi.spyOn(Date, 'now').mockImplementation(() => now); + spawn.mockImplementation((_command, _args, options) => { + paths = resolveDaemonPaths(String(options?.env?.AGENT_DEVICE_STATE_DIR)); + fs.writeFileSync(path.join(paths.baseDir, 'defer-publication'), 'wait'); + child = spawnRegisteredDaemonFixture(paths, fields(1), options); + return child; + }); + pause.mockImplementation(async (ms) => { + if (advanced) { + now += ms; + return; + } + assert.ok(paths && child); + await awaitFile(path.join(paths.baseDir, 'registration-held')); + if (failure === 'early exit') { + process.kill(child.pid, 'SIGTERM'); + await child.exited; + now += 14_999; + } else now += 15_000; + advanced = true; + }); + await assert.rejects( + sendToDaemon({ session: 'default', command: 'test', positionals: [], flags: {} }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.kind, 'daemon_startup_failed'); + const cleanup = error.details?.cleanupResults as Array<{ + status: string; + removedStateDir?: boolean; + }>; + assert.equal(cleanup[0]?.status, 'retired'); + assert.equal(cleanup[0]?.removedStateDir, true); + return true; + }, + ); + assert.ok(paths && child); + await child.exited; + assert.equal(isProcessAlive(child.pid), false); + assert.equal(fs.existsSync(paths.baseDir), false); + assert.equal(spawn.mock.calls.length, 1); + }, +); -/** Records the winning daemon the way it would: the startup lock, then its reachable metadata. */ -function writeWinner( - paths: DaemonPaths, - fixture: HttpDaemonFixture, - parts: 'lock' | 'all', - version = readVersion(), -): void { - fs.mkdirSync(paths.baseDir, { recursive: true }); - fs.writeFileSync( - paths.lockPath, - JSON.stringify({ pid: WINNER_PID, processStartTime: 'winner', startedAt: Date.now() }), - ); - if (parts === 'lock') return; - fs.writeFileSync( - paths.infoPath, - JSON.stringify({ - token: 'winner-secret', - pid: WINNER_PID, - version, - codeSignature, - processStartTime: 'winner', - httpPort: fixture.port, - transport: 'http', - }), - ); +for (const command of ['devices', 'test']) { + test(`a joined busy contender adopts a real winner for ${command}`, async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-winner-')); + const http = await startHttpDaemonFixture({ devices: [] }); + const deferred = path.join(paths.baseDir, 'defer-publication'); + fs.writeFileSync(deferred, 'wait'); + const winner = spawnRegisteredDaemonFixture(paths, fields(http.port), { stdio: 'ignore' }); + await awaitFile(path.join(paths.baseDir, 'registration-held')); + fs.writeFileSync( + paths.infoPath, + JSON.stringify({ ...fields(http.port, '0.0.1'), pid: 999_999_999, processStartTime: 'old' }), + ); + let joined = false; + let genuineExit: ExecDetachedExit | undefined; + let contender: ReturnType | undefined; + let releaseJoin: (exit: ExecDetachedExit) => void = () => {}; + let pauses = 0; + spawn.mockImplementation((_command, _args, options) => { + contender = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + void contender.exited.then((exit) => { + assert.equal(exit.exitCode, DAEMON_STARTUP_EXIT_CODES.busy); + genuineExit = exit; + }); + return { + ...contender, + exited: new Promise((resolve) => { + releaseJoin = resolve; + }), + }; + }); + pause.mockImplementation(async (ms) => { + pauses += 1; + fs.rmSync(deferred, { force: true }); + await awaitFile(paths.infoPath); + await actualRetry.sleep(ms); + if (pauses >= 2 && genuineExit) { + joined = true; + releaseJoin(genuineExit); + } + }); + try { + const response = await sendToDaemon(request(paths, command)); + assert.equal(response.ok, true); + assert.equal(joined, true); + assert.equal(spawn.mock.calls.length, 1); + assert.equal(http.rpcRequests.length, 1); + assert.equal(isProcessAlive(winner.pid), true); + const claim = inspectProcessLock(paths.lockPath); + assert.equal(claim.state, 'held'); + if (claim.state === 'held') assert.equal(claim.owner.pid, winner.pid); + } finally { + if (contender) releaseJoin(await contender.exited); + await closeLoopbackServer(http.server); + } + }); } -test('a client whose daemon lost the startup lock uses the daemon that won it', async (t) => { - if (!(await supportsLoopbackBind())) { - t.skip('loopback listeners are not permitted in this environment'); - return; - } - const stateDir = mkdtempForTestSync('agent-device-daemon-start-race-'); - const paths = resolveDaemonPaths(stateDir); - vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); - const fixture = await startHttpDaemonFixture({ devices: [] }); - let launches = 0; - mockRunCmdDetached.mockImplementation(() => { - launches += 1; - writeWinner(paths, fixture, 'lock'); - const exit: ExecDetachedExit = { pid: LOSER_PID, exitCode: 0 }; - return { pid: LOSER_PID, exited: Promise.resolve(exit) }; +test('a client-held claim is waited out before a fresh daemon attempt', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-client-holder-')); + const http = await startHttpDaemonFixture({ devices: [] }); + const claim = tryAcquireProcessLock({ + lockDirPath: paths.lockPath, + owner: { ...readCurrentOwnerIdentity(), acquiredAtMs: Date.now() }, }); - mockSleep.mockImplementation(async () => { - if (!fs.existsSync(paths.infoPath)) writeWinner(paths, fixture, 'all'); + assert.equal(claim.status, 'acquired'); + if (claim.status !== 'acquired') throw new Error('fixture claim refused'); + let loserJoined = false; + let released = false; + spawn.mockImplementation((_command, _args, options) => { + const child = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + if (spawn.mock.calls.length === 1) + void child.exited.then((exit) => { + assert.equal(exit.exitCode, DAEMON_STARTUP_EXIT_CODES.busy); + loserJoined = true; + }); + else assert.equal(loserJoined, true); + return child; + }); + pause.mockImplementation(async (ms) => { + if (loserJoined && !released) { + await claim.acquisition.release(); + released = true; + } + await actualRetry.sleep(ms); }); - try { - const response = await sendToDaemon({ - session: 'default', - command: 'devices', - positionals: [], - flags: { stateDir }, - meta: { requestId: 'req-start-race' }, - }); - - assert.equal(response.ok, true); - assert.equal(launches, 1); - assert.equal(fixture.rpcRequests.length, 1); - assert.equal(fs.existsSync(paths.infoPath), true); - assert.equal(fs.existsSync(paths.lockPath), true); + assert.equal((await sendToDaemon(request(paths))).ok, true); + assert.equal(spawn.mock.calls.length, 2); + assert.equal(http.rpcRequests.length, 1); } finally { - await closeLoopbackServer(fixture.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + if (!released) await claim.acquisition.release(); + await closeLoopbackServer(http.server); } }); -test('a one-shot test run leaves a daemon another client started running', async (t) => { - if (!(await supportsLoopbackBind())) { - t.skip('loopback listeners are not permitted in this environment'); - return; - } - const stateDir = mkdtempForTestSync('agent-device-daemon-start-race-owner-'); - const paths = resolveDaemonPaths(stateDir); - vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); - const fixture = await startHttpDaemonFixture({ passed: 1, failed: 0 }); - mockRunCmdDetached.mockImplementation(() => { - writeWinner(paths, fixture, 'all'); - return { pid: LOSER_PID, exited: new Promise(() => {}) }; +for (const exit of [ + { exitCode: 0 }, + { exitCode: 1 }, + { exitCode: DAEMON_STARTUP_EXIT_CODES.unproven }, + { exitCode: DAEMON_STARTUP_EXIT_CODES.busy, error: 'spawn refused' }, + { exitCode: DAEMON_STARTUP_EXIT_CODES.busy, signal: 'SIGTERM' as const }, +]) { + test(`generic exit ${exit.error ?? exit.signal ?? exit.exitCode} cannot adopt or stop a foreign winner`, async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-generic-exit-')); + const http = await startHttpDaemonFixture({ devices: [] }); + const deferred = path.join(paths.baseDir, 'defer-publication'); + fs.writeFileSync(deferred, 'wait'); + const winner = spawnRegisteredDaemonFixture(paths, fields(http.port), { stdio: 'ignore' }); + await awaitFile(path.join(paths.baseDir, 'registration-held')); + spawn.mockImplementation(() => ({ + pid: 999_999, + exited: Promise.resolve({ pid: 999_999, ...exit }), + })); + pause.mockImplementation(async (ms) => { + fs.rmSync(deferred, { force: true }); + await actualRetry.sleep(ms); + }); + try { + await assert.rejects( + sendToDaemon(request(paths)), + (error: unknown) => + error instanceof AppError && error.details?.kind === 'daemon_startup_failed', + ); + assert.equal(spawn.mock.calls.length, 1); + assert.equal(http.rpcRequests.length, 0); + assert.equal(isProcessAlive(winner.pid), true); + assert.equal(inspectProcessLock(paths.lockPath).state, 'held'); + } finally { + await closeLoopbackServer(http.server); + } }); +} +test('a joined busy contender waits for a published winner to become ready without signaling it', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-delayed-ready-')); + let probes = 0; + const http = await startHttpDaemonFixture({ devices: [] }, { ready: () => ++probes >= 12 }); + const deferred = path.join(paths.baseDir, 'defer-publication'); + fs.writeFileSync(deferred, 'wait'); + const winner = spawnRegisteredDaemonFixture(paths, fields(http.port), { stdio: 'ignore' }); + await awaitFile(path.join(paths.baseDir, 'registration-held')); + let contenderExit: ExecDetachedExit | undefined; + spawn.mockImplementation((_command, _args, options) => { + const child = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + void child.exited.then((exit) => { + contenderExit = exit; + }); + return child; + }); + pause.mockImplementation(async () => { + if (contenderExit) fs.rmSync(deferred, { force: true }); + await actualRetry.sleep(10); + }); + const signal = vi.spyOn(process, 'kill'); try { - const response = await sendToDaemon({ - session: 'default', - command: 'test', - positionals: [], - flags: { stateDir }, - meta: { requestId: 'req-start-race-test' }, + assert.equal((await sendToDaemon(request(paths))).ok, true); + assert.equal(contenderExit?.exitCode, DAEMON_STARTUP_EXIT_CODES.busy); + assert.ok(probes >= 12); + assert.equal(spawn.mock.calls.length, 1); + assert.equal(http.rpcRequests.length, 1); + assert.equal(isProcessAlive(winner.pid), true); + assert.equal( + signal.mock.calls.some(([pid, kind]) => pid === winner.pid && kind !== 0), + false, + ); + } finally { + signal.mockRestore(); + await closeLoopbackServer(http.server); + } +}); + +for (const held of [true, false]) { + test(`startup uses one deadline when the claim is ${held ? 'held' : 'released for relaunch'}`, async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-budget-')); + const claim = tryAcquireProcessLock({ + lockDirPath: paths.lockPath, + owner: { ...readCurrentOwnerIdentity(), acquiredAtMs: Date.now() }, + }); + assert.equal(claim.status, 'acquired'); + if (claim.status !== 'acquired') throw new Error('fixture claim refused'); + let now = Date.now(); + const started = now; + let released = false; + let advanced = false; + let finishPending: () => void = () => {}; + const nativeTimeout = globalThis.setTimeout; + vi.spyOn(globalThis, 'setTimeout').mockImplementation((handler, ms, ...args) => + nativeTimeout(handler, ms === 1_000 ? 0 : ms, ...args), + ); + vi.spyOn(Date, 'now').mockImplementation(() => now); + spawn.mockImplementation(() => ({ + pid: 999_999, + exited: + spawn.mock.calls.length === 1 + ? Promise.resolve({ pid: 999_999, exitCode: DAEMON_STARTUP_EXIT_CODES.busy }) + : new Promise((resolve) => { + finishPending = () => resolve({ pid: 999_999, exitCode: 1 }); + }), + })); + pause.mockImplementation(async (ms) => { + if (!advanced) { + if (!held) { + await claim.acquisition.release(); + released = true; + } + advanced = true; + now += 14_750; + } else now += ms; }); + try { + await assert.rejects(sendToDaemon(request(paths)), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.startupAttempts, held ? 1 : 2); + assert.equal(error.details?.startupTimeoutMs, 15_000); + return true; + }); + assert.equal(now - started, 15_000); + assert.equal(inspectProcessLock(paths.lockPath).state, held ? 'held' : 'absent'); + } finally { + finishPending(); + vi.restoreAllMocks(); + if (!released) await claim.acquisition.release(); + } + }); +} - assert.equal(response.ok, true); - assert.equal(fs.existsSync(paths.infoPath), true); +test.for([ + { budget: 'ample', offset: 0, launches: 2, alive: false, rpcs: 1 }, + { budget: 'near deadline', offset: 11_000, launches: 1, alive: true, rpcs: 0 }, +])('an older winner is replaced only with enough startup time ($budget)', async (expected, t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-older-winner-')); + const http = await startHttpDaemonFixture({ devices: [] }); + const deferred = path.join(paths.baseDir, 'defer-publication'); + fs.writeFileSync(deferred, 'wait'); + const winner = spawnRegisteredDaemonFixture(paths, fields(http.port, '0.0.1'), { + stdio: 'ignore', + }); + await awaitFile(path.join(paths.baseDir, 'registration-held')); + const wallTime = Date.now; + let offset = 0; + const clock = vi.spyOn(Date, 'now').mockImplementation(() => wallTime() + offset); + let joined = false; + spawn.mockImplementation((_command, _args, options) => { + if (spawn.mock.calls.length > 1) assert.equal(joined, true); + const child = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + if (spawn.mock.calls.length === 1) + void child.exited.then((exit) => { + assert.equal(exit.exitCode, DAEMON_STARTUP_EXIT_CODES.busy); + joined = true; + }); + return child; + }); + pause.mockImplementation(async (ms) => { + if (joined) { + fs.rmSync(deferred, { force: true }); + if (expected.offset) offset = offset ? offset + ms : expected.offset; + } + await actualRetry.sleep(10); + }); + const notice = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + try { + const pending = sendToDaemon(request(paths)); + if (expected.alive) + await assert.rejects(pending, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.kind, 'daemon_startup_failed'); + assert.equal(error.details?.startupAttempts, 1); + assert.equal(error.details?.startupTimeoutMs, 15_000); + return true; + }); + else { + assert.equal((await pending).ok, true); + await winner.exited; + } + assert.equal(joined, true); + assert.equal(isProcessAlive(winner.pid), expected.alive); + assert.equal(spawn.mock.calls.length, expected.launches); + assert.equal(http.rpcRequests.length, expected.rpcs); + assert.equal( + notice.mock.calls.flat().join('').includes(`Replacing daemon (pid ${winner.pid}, v0.0.1)`), + !expected.alive, + ); } finally { - await closeLoopbackServer(fixture.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + clock.mockRestore(); + notice.mockRestore(); + await closeLoopbackServer(http.server); } }); -test('a start race won by an older daemon replaces it instead of adopting it', async (t) => { - if (!(await supportsLoopbackBind())) { - t.skip('loopback listeners are not permitted in this environment'); - return; - } - const stateDir = mkdtempForTestSync('agent-device-daemon-start-race-older-'); - const paths = resolveDaemonPaths(stateDir); - vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); - const fixture = await startHttpDaemonFixture({ devices: [] }); - let launches = 0; - mockRunCmdDetached.mockImplementation(() => { - launches += 1; - if (launches === 1) writeWinner(paths, fixture, 'all', '0.0.1'); - const exit: ExecDetachedExit = { pid: LOSER_PID, exitCode: 0 }; - return { pid: LOSER_PID, exited: Promise.resolve(exit) }; +test('a failed own transport probe retires and joins the private startup before rejecting', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const http = await startHttpDaemonFixture({ devices: [] }); + let paths: DaemonPaths | undefined; + let child: ReturnType | undefined; + let failure: AppError | undefined; + spawn.mockImplementation((_command, _args, options) => { + paths = resolveDaemonPaths(String(options?.env?.AGENT_DEVICE_STATE_DIR)); + child = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + return child; }); - const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); - + pause.mockImplementation(actualRetry.sleep); try { await assert.rejects( sendToDaemon({ session: 'default', - command: 'devices', + command: 'test', positionals: [], - flags: { stateDir }, - meta: { requestId: 'req-start-race-older' }, + flags: { daemonTransport: 'socket', daemonServerMode: 'http' }, }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Daemon socket endpoint is unavailable'); + assert.equal(error.details?.reason, 'daemon_endpoint_unavailable'); + failure = error; + return true; + }, ); - assert.equal(fixture.rpcRequests.length, 0); - assert.equal(launches, 2); - assert.match( - String(stderr.mock.calls.flat().join('')), - /Replacing daemon \(pid 43300, v0\.0\.1\)/, - ); + assert.ok(paths && child && failure); + assert.equal(isProcessAlive(child.pid), false); + assert.equal(fs.existsSync(paths.baseDir), false); + await child.exited; + assert.equal(failure.details?.startupJoined, true); + assert.equal(failure.details?.stateDir, paths.baseDir); + const results = failure.details?.cleanupResults as Array<{ + status: string; + removedStateDir?: boolean; + }>; + assert.equal(results[0]?.status, 'retired'); + assert.equal(results[0]?.removedStateDir, true); + assert.equal(http.rpcRequests.length, 0); } finally { - stderr.mockRestore(); - await closeLoopbackServer(fixture.server); - fs.rmSync(stateDir, { recursive: true, force: true }); + await closeLoopbackServer(http.server); } }); + +test.for(['default', 'private replay'] as const)( + 'a missed initial birth probe recovers the monitored child for $0', + async (route, t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable'); + const http = await startHttpDaemonFixture({ devices: [], sessionActive: false }); + const defaultPaths = resolveDaemonPaths(mkdtempForTestSync('daemon-missed-birth-')); + const ensure = vi.spyOn(lifecycle, 'ensureDaemon'); + let paths: DaemonPaths | undefined; + let child: ReturnType | undefined; + let now = Date.now(); + vi.spyOn(Date, 'now').mockImplementation(() => now); + spawn.mockImplementation((_command, _args, options) => { + paths = resolveDaemonPaths(String(options?.env?.AGENT_DEVICE_STATE_DIR)); + child = spawnRegisteredDaemonFixture(paths, fields(http.port), options); + vi.mocked(readProcessStartTime).mockReturnValueOnce(null); + return child; + }); + pause.mockImplementation(async () => { + assert.ok(paths); + if (http.rpcRequests.length === 0) { + await awaitFile(paths.infoPath); + now += 1_000; + } else { + await actualRetry.sleep(10); + now += 10; + } + }); + try { + const response = await sendToDaemon( + route === 'default' + ? request(defaultPaths) + : { session: 'default', command: 'replay', positionals: [], flags: {} }, + ); + assert.equal(response.ok, true); + assert.equal(ensure.mock.calls.length, 1); + assert.equal((await ensure.mock.results[0]!.value).startedByClient, true); + assert.equal(spawn.mock.calls.length, 1); + assert.equal(http.rpcRequests.length, 1); + assert.ok(paths && child); + if (route === 'private replay') { + await child.exited; + assert.equal(isProcessAlive(child.pid), false); + assert.equal(fs.existsSync(paths.baseDir), false); + } else { + assert.equal(isProcessAlive(child.pid), true); + assert.equal(inspectProcessLock(paths.lockPath).state, 'held'); + assert.equal(fs.existsSync(paths.infoPath), true); + } + } finally { + await closeLoopbackServer(http.server); + } + }, +); 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 acdd528f7e..4399bf2b3a 100644 --- a/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts +++ b/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts @@ -27,19 +27,11 @@ import net from 'node:net'; import http from 'node:http'; import path from 'node:path'; +import fs from 'node:fs'; import assert from 'node:assert/strict'; import { beforeEach, afterEach, test, vi } from 'vitest'; -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, -})); +const { mockRunCmdSync } = vi.hoisted(() => ({ mockRunCmdSync: vi.fn() })); vi.mock('@agent-device/host-kit/command', async () => { const actual = await vi.importActual( @@ -48,18 +40,31 @@ vi.mock('@agent-device/host-kit/command', async () => { return { ...actual, runCmdSync: mockRunCmdSync }; }); -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { sleep } from '@agent-device/host-kit/retry'; +import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; import { sendRequest } from '../daemon-client-transport.ts'; import type { DaemonRequest } from '../../daemon/daemon-request.ts'; import type { DaemonInfo } from '../daemon-client-metadata.ts'; -import type { DaemonPaths } from '../../daemon-resolution.ts'; +import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts'; +import type { DaemonRetirementResult } from '../../daemon-registration-owner.ts'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + spawnRegisteredDaemonFixture, + waitForRegisteredDaemonFixture, + finishRegisteredDaemonFixture, + finishRegisteredDaemonFixtures, +} from '../../__tests__/test-utils/registered-daemon-fixture.ts'; +import { + closeLoopbackServer, + skipWhenLoopbackUnavailable, +} from '../../__tests__/test-utils/loopback.ts'; const TIMEOUT_MS = 120; // `snapshot`'s timeout policy preserves the daemon (onTimeout !== -// 'reset-daemon'), so `handleRequestTimeout` never reaches -// `resetDaemonAfterTimeout` (`process.kill`) here — keeping this suite +// 'reset-daemon'), so `handleRequestTimeout` never signals the daemon +// in the hint controls — keeping them // side-effect-free outside the mocked pkill sweep. const SNAPSHOT_COMMAND = 'snapshot'; @@ -95,6 +100,7 @@ function startHangingSocketServer(): Promise<{ server: net.Server; port: number // Accept the connection but never write a response — forces the // client's own request-timeout envelope to fire. socket.on('error', () => {}); + socket.resume(); }); server.on('error', reject); server.listen(0, '127.0.0.1', () => { @@ -130,10 +136,11 @@ function startHangingHttpServer(): Promise<{ server: http.Server; port: number } beforeEach(() => { mockRunCmdSync.mockReset(); - mockIsDaemon.mockReset(); - mockStop.mockReset(); }); -afterEach(() => vi.restoreAllMocks()); +afterEach(async () => { + vi.restoreAllMocks(); + await finishRegisteredDaemonFixtures(); +}); 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 @@ -280,46 +287,232 @@ 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 () => { +async function waitForForceStop(requested: Promise): Promise { + let timer: ReturnType | undefined; + try { + await Promise.race([ + requested, + new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error('force stop was not requested')), 1_500); + }), + ]); + } finally { + clearTimeout(timer); + } +} + +function timeoutDiagnostic(paths: DaemonPaths): Record { + const events = fs + .readFileSync(path.join(paths.baseDir, 'timeout-diagnostics.ndjson'), 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + const event = events.find((entry) => entry.phase === 'daemon_request_timeout'); + assert.ok(event); + return event.data; +} + +function assertForcedRetirement( + retirement: DaemonRetirementResult | undefined, + removalFails: boolean, +): void { + if (removalFails) { + assert.ok(retirement?.status === 'retained'); + assert.equal(retirement.reason, 'retirement-unconfirmed'); + assert.ok(retirement.termination?.status === 'exited'); + assert.equal(retirement.termination.mode, 'forced'); + } else { + assert.ok(retirement?.status === 'retired'); + assert.equal(retirement.termination.mode, 'forced'); + } +} + +for (const transport of ['socket', 'http'] as const) { + for (const removalFails of [false, true]) { + test(`${transport} timeout awaits force exit when metadata removal ${removalFails ? 'fails' : 'succeeds'}`, async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + mockRunCmdSync.mockReturnValue({ exitCode: 1, stdout: '', stderr: '' }); + const endpoint = await (transport === 'socket' + ? startHangingSocketServer() + : startHangingHttpServer()); + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-timeout-owner-')); + const child = spawnRegisteredDaemonFixture( + paths, + { + ...(transport === 'socket' ? { socketPort: endpoint.port } : { httpPort: endpoint.port }), + token: 'test-token', + version: 'test', + codeOrigin: 'checkout', + codeSignature: 'test', + }, + undefined, + ); + let exited = false; + void child.exited.then(() => { + exited = true; + }); + let killRequested!: () => void; + const requested = new Promise((resolve) => { + killRequested = resolve; + }); + const actualKill = process.kill.bind(process); + let settled = false; + let outcome: Promise | undefined; + const kill = vi.spyOn(process, 'kill').mockImplementation((pid, signal) => { + if (pid === child.pid && signal === 'SIGKILL') { + killRequested(); + return true; + } + return actualKill(pid, signal); + }); + const actualUnlink = fs.unlinkSync.bind(fs); + const remove = vi.spyOn(fs, 'unlinkSync').mockImplementation((file) => { + if (removalFails && file === paths.infoPath) + throw Object.assign(new Error('retained registration control'), { code: 'EACCES' }); + actualUnlink(file); + }); + try { + const info = await waitForRegisteredDaemonFixture(paths, child); + outcome = withDiagnosticsScope( + { debug: true, logPath: path.join(paths.baseDir, 'timeout-diagnostics.ndjson') }, + () => + sendRequest( + info, + { ...buildRequest(undefined), command: 'open' }, + transport, + paths, + TIMEOUT_MS, + ), + ).then( + () => assert.fail('hanging request unexpectedly succeeded'), + (error: unknown) => { + settled = true; + return error; + }, + ); + await waitForForceStop(requested); + await sleep(30); + assert.equal(actualKill(child.pid, 0), true); + assert.equal(settled, false, 'request must remain pending while the daemon is alive'); + kill.mockRestore(); + actualKill(child.pid, 'SIGKILL'); + await child.exited; + const error = await outcome; + assert.ok(error instanceof AppError); + assert.equal(normalizeError(error).details?.reason, 'daemon_transport_timeout'); + assertForcedRetirement( + error.details?.retirement as DaemonRetirementResult | undefined, + removalFails, + ); + assert.equal(fs.existsSync(paths.infoPath), removalFails); + assert.equal(fs.existsSync(paths.lockPath), false); + assert.equal(fs.existsSync(paths.baseDir), true); + assert.equal(timeoutDiagnostic(paths).daemonPreservedAfterTimeout, false); + assert.equal(timeoutDiagnostic(paths).daemonPidForceKilled, true); + assert.equal(mockRunCmdSync.mock.calls.filter(([cmd]) => cmd === 'pkill').length, 3); + } finally { + remove.mockRestore(); + kill.mockRestore(); + if (!exited) actualKill(child.pid, 'SIGKILL'); + await child.exited; + await outcome; + await finishRegisteredDaemonFixture(paths.baseDir); + await closeLoopbackServer(endpoint.server); + } + }); + } +} + +test('timeout retains a live registration without captured birth proof and reports that outcome', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; 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(); + const endpoint = await startHangingHttpServer(); + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-timeout-retained-')); + const child = spawnRegisteredDaemonFixture( + paths, + { + httpPort: endpoint.port, + token: 'test-token', + version: 'test', + codeOrigin: 'checkout', + codeSignature: 'test', + }, + undefined, + ); + const kill = vi.spyOn(process, 'kill'); try { + const info = await waitForRegisteredDaemonFixture(paths, child); + const before = fs.readFileSync(paths.infoPath, 'utf8'); + const lockBefore = fs + .readdirSync(paths.lockPath) + .map((name) => [name, fs.readFileSync(path.join(paths.lockPath, name), 'utf8')]); await assert.rejects( - sendRequest( - { port, pid: 7, token: 'test-token', processStartTime: 'start' }, - { ...buildRequest(undefined), command: 'open' }, - 'socket', - dummyStatePaths(), - TIMEOUT_MS, + withDiagnosticsScope( + { debug: true, logPath: path.join(paths.baseDir, 'timeout-diagnostics.ndjson') }, + () => + sendRequest( + { ...info, processStartTime: undefined }, + { ...buildRequest(undefined), command: 'open' }, + 'http', + paths, + TIMEOUT_MS, + ), ), (error: unknown) => { assert.ok(error instanceof AppError); - assert.equal(error.details?.reason, 'daemon_transport_timeout'); + assert.equal(normalizeError(error).details?.reason, 'daemon_transport_timeout'); + const retirement = error.details?.retirement as DaemonRetirementResult | undefined; + assert.ok(retirement?.status === 'retained'); + assert.ok(retirement.termination?.status === 'retained'); + assert.equal(retirement.termination.reason, 'missing-start-time'); + assert.match(normalizeError(error).hint ?? '', /State was retained/); + assert.doesNotMatch(normalizeError(error).hint ?? '', /daemon was reset/); return true; }, ); - await new Promise((resolve) => setImmediate(resolve)); - assert.equal(mockStop.mock.calls.length, 1); + assert.equal(timeoutDiagnostic(paths).daemonPreservedAfterTimeout, true); + assert.equal(process.kill(child.pid, 0), true); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), before); + assert.deepEqual( + fs + .readdirSync(paths.lockPath) + .map((name) => [name, fs.readFileSync(path.join(paths.lockPath, name), 'utf8')]), + lockBefore, + ); + assert.equal( + kill.mock.calls.some(([pid, signal]) => pid === child.pid && signal !== 0), + false, + ); } finally { - server.close(); + kill.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + await closeLoopbackServer(endpoint.server); } }); -test('a local record stop timeout leaves the exporting daemon and runner alive and names the retry', async () => { - // A match would terminate the runner, which is the recorder on a physical iOS device or macOS. +test('a local record stop timeout leaves the exporting daemon and runner alive and names the retry', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; mockRunCmdSync.mockReturnValue({ exitCode: 0, stdout: '', stderr: '' }); - mockIsDaemon.mockReturnValue(true); - const kill = vi.spyOn(process, 'kill').mockImplementation(() => true); - const { server, port } = await startHangingSocketServer(); + const endpoint = await startHangingSocketServer(); + const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-record-timeout-owner-')); + const child = spawnRegisteredDaemonFixture( + paths, + { + socketPort: endpoint.port, + token: 'test-token', + version: 'test', + codeOrigin: 'checkout', + codeSignature: 'test', + }, + undefined, + ); + const kill = vi.spyOn(process, 'kill'); try { + const info = await waitForRegisteredDaemonFixture(paths, child); + const registration = fs.readFileSync(paths.infoPath, 'utf8'); await assert.rejects( sendRequest( - { port, pid: 7, token: 'test-token', processStartTime: 'start' }, + info, { ...buildRequest('ios'), session: 'e2e-ios-0', @@ -327,7 +520,7 @@ test('a local record stop timeout leaves the exporting daemon and runner alive a positionals: ['stop'], }, 'socket', - dummyStatePaths(), + paths, TIMEOUT_MS, ), (error: unknown) => { @@ -340,10 +533,14 @@ test('a local record stop timeout leaves the exporting daemon and runner alive a return true; }, ); + assert.equal(mockRunCmdSync.mock.calls.length, 0); + assert.equal(kill.mock.calls.length, 0); + assert.equal(process.kill(child.pid, 0), true); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), registration); + assert.equal(fs.existsSync(paths.lockPath), true); } finally { - server.close(); + kill.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + await closeLoopbackServer(endpoint.server); } - assert.equal(mockRunCmdSync.mock.calls.length, 0); - assert.equal(kill.mock.calls.length, 0); - assert.equal(mockStop.mock.calls.length, 0); }); diff --git a/src/daemon-client/__tests__/daemon-client-transport.test.ts b/src/daemon-client/__tests__/daemon-client-transport.test.ts index d9b60a8bd8..9f591f4cae 100644 --- a/src/daemon-client/__tests__/daemon-client-transport.test.ts +++ b/src/daemon-client/__tests__/daemon-client-transport.test.ts @@ -1,6 +1,13 @@ import assert from 'node:assert/strict'; import http from 'node:http'; +import net from 'node:net'; +import fs from 'node:fs'; +import path from 'node:path'; +import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; import { test, vi } from 'vitest'; +import * as hostTransport from '@agent-device/host-kit/transport'; +import { sleep } from '@agent-device/host-kit/retry'; import { AppError } from '@agent-device/kernel/errors'; import { DAEMON_HTTP_INSTANCE_HEADER, @@ -36,6 +43,64 @@ function sendWithStaleInstance(port: number, timeoutMs: number) { ); } +test('auto health probing reserves time for a healthy fallback when HTTP hangs', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + const httpServer = http.createServer(() => {}); + const socketServer = net.createServer((socket) => socket.on('error', () => {})); + try { + const httpPort = await listenOnLoopback(httpServer); + const port = await listenOnLoopback(socketServer); + assert.equal( + await canConnect({ token: 'secret', pid: 1, transport: 'http', httpPort, port }, 'auto', 120), + true, + ); + } finally { + await closeLoopbackServer(httpServer); + await closeLoopbackServer(socketServer); + } +}); + +test('the health deadline includes requester loading and forbids a late request', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + let requests = 0; + const server = http.createServer((_req, res) => { + requests += 1; + res.end('{}'); + }); + let release!: () => void; + const blocked = new Promise((resolve) => { + release = resolve; + }); + const actualLoad = hostTransport.loadNodeHttpRequester; + const load = vi + .spyOn(hostTransport, 'loadNodeHttpRequester') + .mockImplementation(async (protocol) => { + await blocked; + return actualLoad(protocol); + }); + let probing: Promise | undefined; + try { + const httpPort = await listenOnLoopback(server); + let settled = false; + probing = canConnect({ token: 'secret', pid: 1, httpPort }, 'http', 40).then((reachable) => { + settled = true; + return reachable; + }); + await sleep(90); + assert.equal(settled, true, 'loading must not extend the probe deadline'); + assert.equal(await probing, false); + release(); + await blocked; + await sleep(10); + assert.equal(requests, 0, 'a timed-out loader must not open a request later'); + } finally { + release(); + await probing; + load.mockRestore(); + await closeLoopbackServer(server); + } +}); + test('persistent remote client caches health and retries a refused stale instance before dispatch', async (t) => { if (await skipWhenLoopbackUnavailable(t)) return; const paths: string[] = []; @@ -412,3 +477,46 @@ test('the classifier ignores message text', () => { true, ); }); + +test.for(['timeout', 'reset'] as const)( + 'HTTP %s reports only its own transport outcome', + async (mode, t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + const server = http.createServer((request) => { + if (mode === 'reset') request.socket.destroy(); + }); + const paths = resolveDaemonPaths(mkdtempForTestSync('http-outcome-')); + const logPath = path.join(paths.baseDir, 'diagnostics.ndjson'); + try { + const port = await listenOnLoopback(server); + await assert.rejects( + withDiagnosticsScope({ debug: true, logPath }, () => + sendRequest( + { baseUrl: `http://127.0.0.1:${port}`, token: 'secret', pid: 1 }, + { token: 'secret', command: 'devices', session: 'default', positionals: [], flags: {} }, + 'http', + paths, + mode === 'timeout' ? 40 : 1_000, + ), + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason === 'daemon_transport_timeout', mode === 'timeout'); + return true; + }, + ); + await sleep(30); + const events = fs + .readFileSync(logPath, 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + assert.equal( + events.filter((event) => event.phase === 'daemon_request_socket_error').length, + mode === 'timeout' ? 0 : 1, + ); + } finally { + await closeLoopbackServer(server); + } + }, +); diff --git a/src/daemon-client/__tests__/daemon-client.test.ts b/src/daemon-client/__tests__/daemon-client.test.ts index f579a3c4a5..cca31e8a26 100644 --- a/src/daemon-client/__tests__/daemon-client.test.ts +++ b/src/daemon-client/__tests__/daemon-client.test.ts @@ -1,5 +1,5 @@ import type { RequestProgressEvent } from '@agent-device/contracts/progress'; -import { test, vi } from 'vitest'; +import { test } from 'vitest'; import assert from 'node:assert/strict'; import http from 'node:http'; import net from 'node:net'; @@ -11,57 +11,18 @@ import { listenOnLoopback, supportsLoopbackBind, } from '../../__tests__/test-utils/loopback.ts'; -import { runCmdBackground } from '@agent-device/host-kit/command'; -import { - isProcessAlive, - readProcessCommand, - readProcessStartTime, - waitForProcessExit, -} from '@agent-device/host-kit/process'; +import { readProcessStartTime } from '@agent-device/host-kit/process'; import { sendToDaemon } from '../daemon-client.ts'; import { currentDaemonCodeSignature } from '../../__tests__/test-utils/daemon-http-fixture.ts'; import { computeDaemonCodeSignature } from '@agent-device/host-kit/code-signature'; import { downloadRemoteArtifact } from '../../remote/daemon-artifacts.ts'; -import { - cleanupFailedDaemonStartupMetadata, - resolveDaemonStartupHint, -} from '../daemon-client-metadata.ts'; +import { resolveDaemonStartupHint } from '../daemon-client-metadata.ts'; import { canConnectSocket } from '../daemon-client-transport.ts'; import { DAEMON_RPC_PROTOCOL_VERSION } from '@agent-device/contracts/daemon-http'; import { resolveDaemonPaths } from '../../daemon-resolution.ts'; import { readVersion } from '@agent-device/host-kit/version'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; -// readProcessStartTime/readProcessCommand shell out to `ps` with a 1s -// timeout (see host-process.ts). isAgentDeviceDaemonProcess re-reads both for -// every liveness check, so a spawned-daemon fixture that is proven live once -// (a real read, right after the process starts) can still be misclassified -// as dead later if a *subsequent* `ps` call happens to miss its deadline -// under full-suite CPU contention. mockReadProcessStartTime/mockReadProcessCommand -// default to `undefined`, which falls through to the real implementation for -// every pid in every test in this file; only the one test below that needs a -// stable answer for its spawned pid configures an override, and clears it -// afterward. -const { mockReadProcessStartTime, mockReadProcessCommand } = vi.hoisted(() => ({ - mockReadProcessStartTime: vi.fn<(pid: number) => string | null | undefined>(), - mockReadProcessCommand: vi.fn<(pid: number) => string | null | undefined>(), -})); - -vi.mock('@agent-device/host-kit/process', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - readProcessStartTime: (pid: number) => { - const overridden = mockReadProcessStartTime(pid); - return overridden !== undefined ? overridden : actual.readProcessStartTime(pid); - }, - readProcessCommand: (pid: number) => { - const overridden = mockReadProcessCommand(pid); - return overridden !== undefined ? overridden : actual.readProcessCommand(pid); - }, - }; -}); - type MockHttpResponse = EventEmitter & { headers?: Record; statusCode?: number; @@ -179,181 +140,19 @@ function writeCurrentDaemonInfo( ); } -test('resolveDaemonStartupHint prefers stale lock guidance when lock exists without info', () => { - const hint = resolveDaemonStartupHint({ hasInfo: false, hasLock: true }); - assert.match(hint, /daemon\.lock/i); - assert.match(hint, /automatically/i); - assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); -}); - -test('resolveDaemonStartupHint covers stale info+lock pair', () => { - const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: true }); - assert.match(hint, /daemon\.json/i); - assert.match(hint, /daemon\.lock/i); - assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); -}); - -test('resolveDaemonStartupHint falls back to daemon.json guidance', () => { - const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: false }); - assert.match(hint, /daemon\.json/i); - assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); -}); - -test('resolveDaemonStartupHint includes configured state directory paths', () => { - const paths = resolveDaemonPaths('/tmp/ad-custom-state'); - const hint = resolveDaemonStartupHint({ hasInfo: false, hasLock: true }, paths); - assert.match(hint, /\/tmp\/ad-custom-state\/daemon\.lock/); - assert.match(hint, /\/tmp\/ad-custom-state\/daemon\.json/); - assert.match( - hint, - /rm -f '\/tmp\/ad-custom-state\/daemon\.json' '\/tmp\/ad-custom-state\/daemon\.lock'/, - ); -}); - -test('resolveDaemonStartupHint shell-quotes cleanup paths', () => { +test('startup recovery guidance retains configured paths and requires stopping every user', () => { const paths = resolveDaemonPaths("/tmp/ad custom's state"); - const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: true }, paths); - assert.match( - hint, - /rm -f '\/tmp\/ad custom'\\''s state\/daemon\.json' '\/tmp\/ad custom'\\''s state\/daemon\.lock'/, - ); -}); - -test('cleanupFailedDaemonStartupMetadata removes partial startup metadata', async () => { - const stateDir = mkdtempForTestSync('agent-device-daemon-cleanup-'); - const paths = resolveDaemonPaths(stateDir); - try { - fs.mkdirSync(paths.baseDir, { recursive: true }); - fs.writeFileSync(paths.infoPath, '{"invalid":true}\n', 'utf8'); - fs.writeFileSync(paths.lockPath, 'not-json\n', 'utf8'); - - const result = await cleanupFailedDaemonStartupMetadata(paths, 'startup_timeout'); - - assert.deepEqual(result, { - reason: 'startup_timeout', - removedInfo: true, - removedLock: true, - stoppedInfoProcess: false, - stoppedLockProcess: false, - }); - assert.equal(fs.existsSync(paths.infoPath), false); - assert.equal(fs.existsSync(paths.lockPath), false); - } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); - } -}); - -test('cleanupFailedDaemonStartupMetadata retains live startup daemon on timeout', async (t) => { - const stateDir = mkdtempForTestSync('agent-device-daemon-live-cleanup-'); - const root = mkdtempForTestSync('agent-device-live-daemon-'); - const daemonDir = path.join(root, 'agent-device', 'dist', 'src', 'internal'); - const daemonScriptPath = path.join(daemonDir, 'daemon.js'); - fs.mkdirSync(daemonDir, { recursive: true }); - fs.writeFileSync(daemonScriptPath, 'setInterval(() => {}, 1000);\n', 'utf8'); - const daemonProcess = runCmdBackground(process.execPath, [daemonScriptPath], { - stdio: 'ignore', - allowFailure: true, - captureOutput: false, - }); - void daemonProcess.wait.catch(() => {}); - const pid = daemonProcess.child.pid; - assert.ok(pid, 'spawned child should have a pid'); - - try { - await new Promise((resolve) => setTimeout(resolve, 50)); - // Read the spawned daemon's real identity once (ground truth: it is - // genuinely alive, with this real start time and command line), then - // pin readProcessStartTime/readProcessCommand to keep returning these - // same proven-real values for this pid. isAgentDeviceDaemonProcess reads - // both again internally on every call inside cleanupFailedDaemonStartupMetadata; - // without pinning, a second real `ps` call could miss its 1s timeout - // under load and misclassify this genuinely-live daemon as dead. - const processStartTime = readProcessStartTime(pid) ?? undefined; - const command = readProcessCommand(pid); - if (command === null || processStartTime === undefined) { - t.skip('process command/start inspection is unavailable in this environment'); - return; - } - mockReadProcessStartTime.mockImplementation((queriedPid: number) => - queriedPid === pid ? processStartTime : undefined, - ); - mockReadProcessCommand.mockImplementation((queriedPid: number) => - queriedPid === pid ? command : undefined, - ); - - const paths = resolveDaemonPaths(stateDir); - fs.mkdirSync(paths.baseDir, { recursive: true }); - fs.writeFileSync( - paths.infoPath, - `${JSON.stringify({ - token: 'startup-secret', - port: 65530, - transport: 'socket', - pid, - processStartTime, - })}\n`, - 'utf8', - ); - fs.writeFileSync( - paths.lockPath, - `${JSON.stringify({ pid, processStartTime, startedAt: Date.now() })}\n`, - 'utf8', - ); - - const result = await cleanupFailedDaemonStartupMetadata(paths, 'startup_timeout', { - stopLiveProcesses: false, - }); - - assert.equal(result.retainedInfoProcess, true); - assert.equal(result.retainedLockProcess, true); - assert.equal(result.removedInfo, false); - assert.equal(result.removedLock, false); - assert.equal(isProcessAlive(pid), true); - assert.equal(fs.existsSync(paths.infoPath), true); - assert.equal(fs.existsSync(paths.lockPath), true); - } finally { - mockReadProcessStartTime.mockReset(); - mockReadProcessCommand.mockReset(); - if (isProcessAlive(pid)) { - process.kill(pid, 'SIGKILL'); - await waitForProcessExit(pid, 1_500); - } - fs.rmSync(stateDir, { recursive: true, force: true }); - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('cleanupFailedDaemonStartupMetadata removes stale daemon metadata on timeout', async () => { - const stateDir = mkdtempForTestSync('agent-device-daemon-stale-cleanup-'); - const paths = resolveDaemonPaths(stateDir); - try { - fs.mkdirSync(paths.baseDir, { recursive: true }); - fs.writeFileSync( - paths.infoPath, - `${JSON.stringify({ - token: 'startup-secret', - port: 65530, - transport: 'socket', - pid: 999_999, - })}\n`, - 'utf8', - ); - fs.writeFileSync( - paths.lockPath, - `${JSON.stringify({ pid: 999_999, startedAt: Date.now() })}\n`, - 'utf8', - ); - - const result = await cleanupFailedDaemonStartupMetadata(paths, 'startup_timeout', { - stopLiveProcesses: false, - }); - - assert.equal(result.removedInfo, true); - assert.equal(result.removedLock, true); - assert.equal(fs.existsSync(paths.infoPath), false); - assert.equal(fs.existsSync(paths.lockPath), false); - } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); + for (const state of [ + { hasInfo: false, hasLock: true }, + { hasInfo: true, hasLock: true }, + { hasInfo: true, hasLock: false }, + { hasInfo: false, hasLock: false }, + ]) { + const hint = resolveDaemonStartupHint(state, paths); + if (state.hasInfo) assert.ok(hint.includes(paths.infoPath)); + if (state.hasLock) assert.ok(hint.includes(paths.lockPath)); + assert.match(hint, /stop all older clients and daemons/); + assert.doesNotMatch(hint, /rm -f/); } }); diff --git a/src/daemon-client/daemon-client-address-hints.ts b/src/daemon-client/daemon-client-address-hints.ts new file mode 100644 index 0000000000..731dd8b180 --- /dev/null +++ b/src/daemon-client/daemon-client-address-hints.ts @@ -0,0 +1,44 @@ +import type { DaemonResponse } from './daemon-client.ts'; +import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; + +/** Adds copyable addressing for a kept-alive replay session, without URL credentials. */ +export function attachActiveSessionAddressHint( + response: Extract, + stateDir: string | undefined, + remoteBaseUrl?: string, +): Extract { + const data = response.data ?? {}; + const sessionName = typeof data.session === 'string' ? data.session : undefined; + const addressFlags = [ + ...(remoteBaseUrl + ? [`--daemon-base-url ${shellQuoteIfNeeded(publicRemoteEndpoint(remoteBaseUrl))}`] + : []), + ...(stateDir ? [`--state-dir ${shellQuoteIfNeeded(stateDir)}`] : []), + ...(sessionName ? [`--session ${shellQuoteIfNeeded(sessionName)}`] : []), + ]; + if (addressFlags.length === 0) return response; + const addressHint = + `This session's daemon was kept alive because its script left the session active; ` + + `pass ${addressFlags.join(' ')} on your next command to reach it.` + + (remoteBaseUrl + ? ' If authentication is required, provide or configure --daemon-auth-token.' + : ''); + const existingMessage = typeof data.message === 'string' ? data.message : undefined; + return { + ...response, + data: { + ...data, + hint: addressHint, + message: existingMessage ? `${existingMessage} ${addressHint}` : addressHint, + }, + }; +} + +function publicRemoteEndpoint(baseUrl: string): string { + const endpoint = new URL(baseUrl); + endpoint.username = ''; + endpoint.password = ''; + endpoint.search = ''; + endpoint.hash = ''; + return endpoint.toString().replace(/\/+$/, ''); +} diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index 43a6556781..88249fa579 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -1,17 +1,27 @@ +import { attachActiveSessionAddressHint } from './daemon-client-address-hints.ts'; import fs from 'node:fs'; import net from 'node:net'; -import os from 'node:os'; -import path from 'node:path'; -import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { AppError, normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; import { readReplayDivergenceResume } from '@agent-device/ad-replay/divergence'; import type { DaemonRequest, DaemonResponse } from '../daemon/daemon-request.ts'; -import { runCmdDetachedMonitored, type ExecDetachedExit } from '@agent-device/host-kit/command'; +import { type ExecDetachedExit } from '@agent-device/host-kit/command'; import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; -import { isProcessAlive, readProcessStartTime } from '@agent-device/host-kit/process'; +import { isProcessAlive } from '@agent-device/host-kit/process'; import { sleep } from '@agent-device/host-kit/retry'; +import { inspectProcessLock, type ProcessLockInspection } from '@agent-device/host-kit/file'; -import { findUnrecoveredRepairCommitFailure } from '../session-repair-tombstone.ts'; +import type { findUnrecoveredRepairCommitFailure } from '../session-repair-tombstone.ts'; +import { + DAEMON_STARTUP_EXIT_CODES, + createOwnedReplayStateDir, + recoverAbandonedDaemonRegistration, + type DaemonRetirementResult, + launchDaemonProcess, + stopAndRetireDaemon, + type OwnedReplayStateDir, + type DaemonStartupLaunch, +} from '../daemon-registration-owner.ts'; import { resolveDaemonPaths, resolveDaemonServerMode, @@ -28,19 +38,11 @@ import { import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import { - cleanupFailedDaemonStartupMetadata, - cleanupStaleDaemonLockIfSafe, getDaemonMetadataState, - isDaemonLockHeldByAnotherDaemon, isRemoteDaemon, readDaemonInfo, - recoverDaemonLockHolder, - removeDaemonInfo, - removeDaemonLock, resolveDaemonStartupHint, - stopDaemonProcessForTakeover, type DaemonInfo, - type DaemonStartupCleanupResult, } from './daemon-client-metadata.ts'; import { canConnect, @@ -52,7 +54,7 @@ export type DaemonClientSettings = { paths: DaemonPaths; transportPreference: DaemonTransportPreference; serverMode: DaemonServerMode; - ownedStateDir?: boolean; + ownedStateDir?: OwnedReplayStateDir; remoteBaseUrl?: string; remoteAuthToken?: string; }; @@ -62,19 +64,14 @@ export type EnsuredDaemon = { startedByClient: boolean; }; -type DaemonStartupLaunch = { - pid: number; - /** The launched process's start time, so a reused pid is never taken for it. */ - startTime?: string; - exited: Promise; -}; - type DaemonStartupWaitResult = | { kind: 'ready'; daemon: EnsuredDaemon } | { kind: 'early_exit'; exit: ExecDetachedExit } - | { kind: 'timeout' }; + | { kind: 'unproven'; error: AppError } + | { kind: 'retry' | 'timeout' }; const DAEMON_STARTUP_TIMEOUT_MS = 15_000; +const MINIMUM_DAEMON_TAKEOVER_BUDGET_MS = 5_000; const LIVE_DAEMON_PROBE_RETRIES = 3; const LIVE_DAEMON_PROBE_RETRY_DELAY_MS = 200; const DAEMON_STARTUP_ATTEMPTS = 2; @@ -91,10 +88,11 @@ export function resolveClientSettings( const explicitStateDir = resolveExplicitStateDir(req); const remote = resolveRemoteClientSettings(req, suppliedAuthToken); const transport = resolveTransportClientSettings(req, remote.remoteBaseUrl); - const ownedStateDir = shouldUseOwnedReplayStateDir(req, explicitStateDir, remote.rawBaseUrl); - const stateDir = ownedStateDir ? createOwnedReplayStateDir() : explicitStateDir; + const ownedStateDir = shouldUseOwnedReplayStateDir(req, explicitStateDir, remote.rawBaseUrl) + ? createOwnedReplayStateDir() + : undefined; return { - paths: resolveDaemonPaths(stateDir), + paths: ownedStateDir?.paths ?? resolveDaemonPaths(explicitStateDir), transportPreference: transport.preference, serverMode: transport.serverMode, ownedStateDir, @@ -153,10 +151,6 @@ function shouldUseOwnedReplayStateDir( return isOneShotReplayCommand(req.command) && !explicitStateDir && !rawRemoteBaseUrl; } -function createOwnedReplayStateDir(): string { - return fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-daemon-')); -} - export async function ensureDaemon(settings: DaemonClientSettings): Promise { if (settings.remoteBaseUrl) { return await ensureRemoteDaemon(settings); @@ -172,7 +166,6 @@ async function ensureLocalDaemon(settings: DaemonClientSettings): Promise { +async function readReusableLocalDaemon( + settings: DaemonClientSettings, + options: { deadline?: number; waitForLiveStartup?: boolean } = {}, +): Promise { + const { deadline } = options; + const inspection = inspectProcessLock(settings.paths.lockPath); + if (inspection.state === 'unproven') throw daemonRegistrationUnprovenError(settings, inspection); const existing = readDaemonInfo(settings.paths.infoPath); if (!existing) return null; + if (!registrationAllowsDaemonObservation(inspection, existing)) return null; + if (options.waitForLiveStartup && (await isAwaitingDaemonStartup(existing, deadline))) + return null; const decision = await resolveDaemonTakeover(existing, { - onClientTransport: () => canReachReusableDaemon(existing, settings.transportPreference), - onAnyAdvertisedTransport: () => canReachReusableDaemon(existing, 'auto'), + onClientTransport: () => + canReachReusableDaemon(existing, settings.transportPreference, deadline), + onAnyAdvertisedTransport: () => canReachReusableDaemon(existing, 'auto', deadline), }); if (decision.kind === 'reuse') return existing; if (decision.kind === 'refuseNewer') { throw newerDaemonRefusedError(existing, decision, settings.paths.baseDir); } + if (remainingStartupBudget(deadline) < MINIMUM_DAEMON_TAKEOVER_BUDGET_MS) return null; emitDaemonTakeoverNotice(existing, decision.reason, settings.paths.baseDir); - await stopDaemonProcessForTakeover(existing); - removeDaemonInfo(settings.paths.infoPath); + await retireDaemonForTakeover(existing, settings.paths); return null; } +async function isAwaitingDaemonStartup( + info: DaemonInfo, + deadline: number | undefined, +): Promise { + return ( + isProcessAlive(info.pid) && !(await canConnect(info, 'auto', remainingStartupBudget(deadline))) + ); +} + +function registrationAllowsDaemonObservation( + inspection: ProcessLockInspection, + info: DaemonInfo, +): boolean { + return ( + inspection.state === 'absent' || + (inspection.state === 'held' && + inspection.owner.pid === info.pid && + inspection.owner.startTime !== null && + inspection.owner.startTime.trim().length > 0 && + inspection.owner.startTime === info.processStartTime) + ); +} + +function daemonRegistrationUnprovenError( + settings: DaemonClientSettings, + inspection?: ProcessLockInspection, +): AppError { + return new AppError('COMMAND_FAILED', 'Daemon registration ownership could not be verified.', { + reason: 'daemon_registration_unproven', + inspection, + stateDir: settings.paths.baseDir, + hint: resolveDaemonStartupHint(getDaemonMetadataState(settings.paths), settings.paths), + }); +} + +async function retireDaemonForTakeover(existing: DaemonInfo, paths: DaemonPaths): Promise { + const retirement = await stopAndRetireDaemon({ + paths: paths, + observed: { pid: existing.pid, startTime: existing.processStartTime ?? null }, + mode: 'graceful', + }); + if (retirement.status === 'retained') { + throw new AppError('COMMAND_FAILED', 'Daemon replacement could not be confirmed.', { + reason: 'daemon_retirement_unconfirmed', + retirement, + hint: + retirement.error?.hint ?? resolveDaemonStartupHint(getDaemonMetadataState(paths), paths), + }); + } +} + /** * A daemon whose pid is still alive is probed again before it can be judged unreachable. A probe's * budget is wall-clock time on this client's event loop, so a client that stalls past it (a large @@ -226,12 +280,14 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise< async function canReachReusableDaemon( info: DaemonInfo, preference: DaemonTransportPreference, + deadline?: number, ): Promise { - if (await canConnectReusableDaemon(info, preference)) return true; + if (await canConnectReusableDaemon(info, preference, deadline)) return true; for (let retry = 1; retry <= LIVE_DAEMON_PROBE_RETRIES; retry += 1) { - if (!isProcessAlive(info.pid)) return false; - await sleep(LIVE_DAEMON_PROBE_RETRY_DELAY_MS); - if (await canConnectReusableDaemon(info, preference)) { + if (!isProcessAlive(info.pid) || (deadline !== undefined && Date.now() >= deadline)) + return false; + await sleep(Math.min(LIVE_DAEMON_PROBE_RETRY_DELAY_MS, remainingStartupBudget(deadline))); + if (await canConnectReusableDaemon(info, preference, deadline)) { emitDiagnostic({ level: 'warn', phase: 'daemon_probe_recovered', @@ -267,9 +323,10 @@ async function assertDaemonPolicyMatches(existing: DaemonInfo, stateDir: string) async function canConnectReusableDaemon( info: DaemonInfo, preference: DaemonTransportPreference, + deadline?: number, ): Promise { try { - return await canConnect(info, preference); + return await canConnect(info, preference, remainingStartupBudget(deadline)); } catch (error) { if (isDaemonTransportUnavailableError(error)) return false; throw error; @@ -303,65 +360,29 @@ function emitDaemonTakeoverNotice(info: DaemonInfo, reason: string, stateDir: st } } -async function startLocalDaemon(settings: DaemonClientSettings): Promise { - let lockRecoveryCount = 0; - const cleanupResults: DaemonStartupCleanupResult[] = []; - let startError: string | undefined; - let daemonProcess: ExecDetachedExit | { pid: number } | undefined; - for (let attempt = 1; attempt <= DAEMON_STARTUP_ATTEMPTS; attempt += 1) { - let launch: DaemonStartupLaunch; - try { - launch = startDaemon(settings); - daemonProcess = { pid: launch.pid }; - } catch (error) { - startError = error instanceof Error ? error.message : String(error); - cleanupResults.push(await cleanupFailedDaemonStartupMetadata(settings.paths, 'start_error')); - if (attempt < DAEMON_STARTUP_ATTEMPTS) { - await sleep(150); - continue; - } - break; - } - - const startup = await waitForDaemonStartup(DAEMON_STARTUP_TIMEOUT_MS, settings, launch); - if (startup.kind === 'ready') return startup.daemon; - if (startup.kind === 'early_exit') { - daemonProcess = startup.exit; - startError = describeDaemonEarlyExit(startup.exit); - cleanupResults.push(await cleanupFailedDaemonStartupMetadata(settings.paths, 'start_error')); - if (attempt < DAEMON_STARTUP_ATTEMPTS) { - await sleep(150); - continue; - } - break; - } - - if (await recoverDaemonLockHolder(settings.paths)) { - lockRecoveryCount += 1; - continue; - } - - const metadataState = getDaemonMetadataState(settings.paths); - const hasAnotherAttempt = attempt < DAEMON_STARTUP_ATTEMPTS; - const cleanup = await cleanupFailedDaemonStartupMetadata(settings.paths, 'startup_timeout', { - stopLiveProcesses: false, - }); - cleanupResults.push(cleanup); - if (cleanup.retainedInfoProcess || cleanup.retainedLockProcess) { - const extended = await waitForDaemonStartup(DAEMON_STARTUP_TIMEOUT_MS, settings, launch); - if (extended.kind === 'ready') return extended.daemon; - if (extended.kind === 'early_exit') { - daemonProcess = extended.exit; - startError = describeDaemonEarlyExit(extended.exit); - } - break; - } - if (!hasAnotherAttempt) break; +type FailedDaemonStartup = { + cleanup?: DaemonRetirementResult; + startError?: string; + startupError?: NormalizedError; + daemonProcess?: ExecDetachedExit | { pid: number }; + retry: boolean; +}; - // Detached daemon startup can race on busy CI hosts; retry when no metadata exists yet. - if (!metadataState.hasInfo && !metadataState.hasLock) await sleep(150); +async function startLocalDaemon(settings: DaemonClientSettings): Promise { + const deadline = Date.now() + DAEMON_STARTUP_TIMEOUT_MS; + const cleanupResults: DaemonRetirementResult[] = []; + let failure: FailedDaemonStartup | undefined; + let attempts = 0; + while (attempts < DAEMON_STARTUP_ATTEMPTS && Date.now() < deadline) { + attempts += 1; + const result = await attemptLocalDaemonStartup(settings, deadline); + if ('daemon' in result) return result.daemon; + failure = result; + if (result.cleanup) cleanupResults.push(result.cleanup); + if (!result.retry) break; + await sleep(Math.min(150, Math.max(0, deadline - Date.now()))); } - + const { startError, startupError, daemonProcess }: Partial = failure ?? {}; const state = getDaemonMetadataState(settings.paths); const daemonLogTail = readRecentLogTail(settings.paths.logPath); throw new AppError('COMMAND_FAILED', 'Failed to start daemon', { @@ -371,10 +392,10 @@ async function startLocalDaemon(settings: DaemonClientSettings): Promise { + let launch: DaemonStartupLaunch; + try { + launch = startDaemon(settings); + } catch (error) { + return await recoverFailedDaemonLaunch(settings, error); + } + const startup = await waitForDaemonStartup(deadline, settings, launch); + if (startup.kind === 'ready') return { daemon: startup.daemon }; + if (startup.kind === 'retry') return { retry: true }; + if (startup.kind === 'unproven') { + const startupError = normalizeError(startup.error); + return { + retry: false, + startError: startupError.message, + startupError, + daemonProcess: { pid: launch.pid }, + }; + } + const { cleanup, joined } = await retireStartupAttempt(settings, launch, deadline); + const available = isRegistrationAvailable(inspectProcessLock(settings.paths.lockPath)); + return { + cleanup, + retry: joined && startup.kind === 'early_exit' && available, + startError: startup.kind === 'early_exit' ? describeDaemonEarlyExit(startup.exit) : undefined, + daemonProcess: startup.kind === 'early_exit' ? startup.exit : { pid: launch.pid }, + }; +} + +async function recoverFailedDaemonLaunch( + settings: DaemonClientSettings, + error: unknown, +): Promise { + const cleanup = settings.ownedStateDir + ? await stopAndRetireDaemon({ + paths: settings.paths, + observed: null, + mode: 'graceful', + ownedStateDir: settings.ownedStateDir, + lockTimeoutMs: 0, + }) + : await recoverAbandonedDaemonRegistration({ + paths: settings.paths, + observed: null, + lockTimeoutMs: 0, + }); + return { + cleanup, + startError: normalizeError(error).message, + retry: !settings.ownedStateDir && cleanup.status !== 'retained', + }; +} + +async function retireStartupAttempt( + settings: DaemonClientSettings, + launch: DaemonStartupLaunch, + deadline: number, +): Promise<{ cleanup: DaemonRetirementResult; joined: boolean }> { + const cleanup = await stopAndRetireDaemon({ + paths: settings.paths, + observed: launch.readIdentity(), + mode: 'graceful', + ownedStateDir: settings.ownedStateDir, + termTimeoutMs: Math.min(3_000, remainingStartupBudget(deadline)), + killTimeoutMs: 1_000, + lockTimeoutMs: 0, + }); + const joined = await joinStartup(launch); + return { cleanup, joined }; +} + +async function joinStartup(launch: DaemonStartupLaunch): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + launch.exited.then(() => true), + new Promise((resolve) => { + timer = setTimeout(() => resolve(false), 1_000); + }), + ]); + } finally { + clearTimeout(timer); + } +} + +function remainingStartupBudget(deadline?: number): number { + return deadline === undefined ? Number.POSITIVE_INFINITY : Math.max(0, deadline - Date.now()); +} + +function isRegistrationAvailable(inspection: ProcessLockInspection): boolean { + return ( + inspection.state === 'absent' || + (inspection.state === 'held' && + (inspection.liveness === 'owner-process-dead' || + inspection.liveness === 'owner-process-reused')) + ); +} + /** * ADR 0012 decision 6 (BLOCKER 2, third follow-up): a one-shot repair * (`replay --save-script`) that COMPLETES without diverging returns SUCCESS * here — the actual healed-script COMMIT is deferred to daemon teardown * (`finalizeRepairTeardown`, run inside the daemon process's own shutdown - * handler, triggered by `stopDaemonProcessForTakeover` below). If that + * handler, triggered by `stopAndRetireDaemon`). If that * deferred commit then FAILS, the daemon leaves a `REPAIR_COMMIT_FAILED` * tombstone in this owned state dir — the only surviving record of the * failure, since the daemon process (and its in-memory session) is gone by @@ -435,49 +557,44 @@ export async function cleanupDaemonAfterRequest( return response; } - const result = { - pid: daemon.info.pid, - removedInfo: false, - removedLock: false, - removedStateDir: false, - error: undefined as string | undefined, - }; - let surfacedResponse = response; - - try { - await stopDaemonProcessForTakeover(daemon.info); - } catch (error) { - result.error = error instanceof Error ? error.message : String(error); - } finally { - const infoExists = fs.existsSync(settings.paths.infoPath); - removeDaemonInfo(settings.paths.infoPath); - result.removedInfo = infoExists && !fs.existsSync(settings.paths.infoPath); - - const lockExists = fs.existsSync(settings.paths.lockPath); - removeDaemonLock(settings.paths.lockPath); - result.removedLock = lockExists && !fs.existsSync(settings.paths.lockPath); - - if (settings.ownedStateDir) { - // `stopDaemonProcessForTakeover` above waits for the (real) daemon - // process to actually exit, which only happens AFTER its shutdown - // handler finishes `finalizeRepairTeardown` for every session — so by - // now any commit-failure tombstone it would leave is already on disk. - const unrecovered = findUnrecoveredRepairCommitFailure(settings.paths.sessionsDir); - if (unrecovered) { - surfacedResponse = surfaceUnrecoveredRepairCommitFailure(response, unrecovered); - } else { - fs.rmSync(settings.paths.baseDir, { recursive: true, force: true }); - result.removedStateDir = !fs.existsSync(settings.paths.baseDir); - } - } - } - + const result = await stopAndRetireDaemon({ + paths: settings.paths, + observed: { pid: daemon.info.pid, startTime: daemon.info.processStartTime ?? null }, + mode: 'graceful', + ownedStateDir: settings.ownedStateDir, + }); emitDiagnostic({ - level: result.error ? 'warn' : 'info', + level: result.status === 'retained' ? 'warn' : 'info', phase: 'daemon_replay_cleanup', - data: result, + data: { pid: daemon.info.pid, ...result }, }); - return surfacedResponse; + if (result.status !== 'absent' && result.repairCommitFailure) { + return surfaceUnrecoveredRepairCommitFailure( + response, + result.repairCommitFailure, + result.status === 'retained' ? result.error : undefined, + ); + } + if (result.status === 'retained' && response?.ok) { + return { + ok: false, + error: normalizeError( + new AppError( + 'COMMAND_FAILED', + 'Replay completed, but daemon cleanup could not be confirmed.', + { + reason: 'daemon_retirement_unconfirmed', + retirement: result, + stateDir: settings.paths.baseDir, + hint: + result.error?.hint ?? + `State and diagnostics were retained at ${settings.paths.baseDir}. Resolve the reported cleanup failure before retrying.`, + }, + ), + ), + }; + } + return response; } /** @@ -498,6 +615,7 @@ export async function cleanupDaemonAfterRequest( function surfaceUnrecoveredRepairCommitFailure( response: DaemonResponse | undefined, unrecovered: NonNullable>, + cleanupFailure?: NormalizedError, ): DaemonResponse { if (response && !response.ok) return response; const { sessionName, tombstone } = unrecovered; @@ -507,7 +625,16 @@ function surfaceUnrecoveredRepairCommitFailure( const message = `The repair transaction for session "${sessionName}" completed, but committing its ` + `healed script failed at teardown: ${tombstone.commitFailure.message}. ${reRun}.`; - return { ok: false, error: normalizeError(new AppError('REPAIR_COMMIT_FAILED', message)) }; + return { + ok: false, + error: normalizeError( + new AppError( + 'REPAIR_COMMIT_FAILED', + message, + cleanupFailure ? { cleanupFailure } : undefined, + ), + ), + }; } /** @@ -524,7 +651,7 @@ function surfaceUnrecoveredRepairCommitFailure( * `resume.allowed` (plan-resumability): a held divergence with `allowed: false` * still holds the session so the agent can inspect and `close` cleanly. */ -export function isHeldRepairDivergence(response: DaemonResponse | undefined): boolean { +function isHeldRepairDivergence(response: DaemonResponse | undefined): boolean { if (!response || response.ok) return false; if (response.error.code !== 'REPLAY_DIVERGENCE') return false; const resume = readReplayDivergenceResume(response.error.details?.divergence); @@ -539,7 +666,7 @@ export function isHeldRepairDivergence(response: DaemonResponse | undefined): bo * selector-miss's own guidance) so the agent's next command knows to target * the SAME daemon instead of resolving to the default one. */ -export function attachRepairSessionAddressHint( +function attachRepairSessionAddressHint( response: Extract, stateDir: string, ): Extract { @@ -571,7 +698,7 @@ function isOneShotReplayCommand(command: string | undefined): boolean { * anyway, but the explicit command check keeps that carve-out a decision * rather than an accident of the response shape. */ -export function isActiveReplaySessionResponse( +function isActiveReplaySessionResponse( req: Omit, response: DaemonResponse | undefined, ): boolean { @@ -580,117 +707,118 @@ export function isActiveReplaySessionResponse( return response.data?.sessionActive === true; } -/** - * ADR 0016 counterpart to `attachRepairSessionAddressHint`: a still-active - * replay session is only unaddressable by `--state-dir` when it lives on an - * OWNED, randomly generated one (`stateDir` undefined otherwise — an explicit - * `--state-dir`/`AGENT_DEVICE_STATE_DIR` caller already knows it). But the - * SESSION name is always cwd-qualified (`cwd::default`) and, per #1394, - * `session list` cannot rediscover it either — so `--session` is always - * emitted when a name is available, explicit state dir or not. Attached to - * both a structured `hint` field (for `--json` consumers) and appended to - * `message` — the only field the default text renderer surfaces - * (`@agent-device/kernel/success-text`) — so the hint reaches a caller in either mode. - * - * `data.session` is used verbatim, never reconstructed as `default`: an - * EXPLICIT `--session ` is used as-is by `resolveEffectiveSessionName`, - * skipping cwd-scoping entirely (`hasExplicitSessionFlag`), so passing the - * qualified name back unchanged is what actually reaches the same session - * from any cwd — a bare `--session default` would only match by coincidence - * (an implicit, no-`--session` follow-up run from the identical cwd). Both - * the state dir and the session name are shell-quoted (only when needed) so - * the hint stays literally copy-pasteable even if either contains spaces or - * shell metacharacters. - */ -export function attachActiveSessionAddressHint( - response: Extract, - stateDir: string | undefined, -): Extract { - const data = response.data ?? {}; - const sessionName = typeof data.session === 'string' ? data.session : undefined; - const addressFlags = [ - ...(stateDir ? [`--state-dir ${shellQuoteIfNeeded(stateDir)}`] : []), - ...(sessionName ? [`--session ${shellQuoteIfNeeded(sessionName)}`] : []), - ]; - if (addressFlags.length === 0) return response; - const addressHint = - `This session's daemon was kept alive because its script left the session active; ` + - `pass ${addressFlags.join(' ')} on your next command to reach it.`; - const existingMessage = typeof data.message === 'string' ? data.message : undefined; - return { - ...response, - data: { - ...data, - hint: addressHint, - message: existingMessage ? `${existingMessage} ${addressHint}` : addressHint, - }, - }; -} - async function waitForDaemonStartup( - timeoutMs: number, + deadline: number, settings: DaemonClientSettings, launch: DaemonStartupLaunch, ): Promise { - const start = Date.now(); let earlyExit: ExecDetachedExit | undefined; void launch.exited.then((exit) => { earlyExit = exit; }); - - while (Date.now() - start < timeoutMs) { - const info = readDaemonInfo(settings.paths.infoPath); - if (info && (await canConnect(info, settings.transportPreference))) { - if (isLaunchedDaemon(info, launch)) { - return { kind: 'ready', daemon: { info, startedByClient: true } }; - } - // Another client's daemon won the start: adopt it only as a reusable daemon would be. An - // incompatible one is replaced, and this wait then sees its own daemon's early exit. - const winner = await readReusableLocalDaemon(settings); - if (winner) return { kind: 'ready', daemon: { info: winner, startedByClient: false } }; + while (Date.now() < deadline) { + if (earlyExit) { + const kind = classifyDaemonStartupExit(earlyExit); + if (kind === 'unproven') + return { kind: 'unproven', error: daemonRegistrationUnprovenError(settings) }; + if (kind === 'failed') return { kind: 'early_exit', exit: earlyExit }; + const contender = await observeContendingDaemon(settings, deadline); + if (contender) return contender; + } else { + const info = await readReadyLaunchedDaemon(settings, launch, deadline); + if (info && !earlyExit) return { kind: 'ready', daemon: { info, startedByClient: true } }; } - // A daemon that lost the startup lock exits cleanly; the daemon that won it is still starting. - if (earlyExit && !isDaemonLockHeldByAnotherDaemon(settings.paths, earlyExit.pid)) { - return { kind: 'early_exit', exit: earlyExit }; - } - await sleep(100); + await sleep(Math.min(100, remainingStartupBudget(deadline))); } return { kind: 'timeout' }; } +function classifyDaemonStartupExit(exit: ExecDetachedExit): 'busy' | 'unproven' | 'failed' { + if (exit.error || exit.signal) return 'failed'; + switch (exit.exitCode) { + case DAEMON_STARTUP_EXIT_CODES.busy: + return 'busy'; + case DAEMON_STARTUP_EXIT_CODES.unproven: + return 'unproven'; + default: + return 'failed'; + } +} + +async function observeContendingDaemon( + settings: DaemonClientSettings, + deadline: number, +): Promise { + let winner: DaemonInfo | null; + try { + winner = await readReusableLocalDaemon(settings, { deadline, waitForLiveStartup: true }); + } catch (error) { + if (error instanceof AppError && error.details?.reason === 'daemon_registration_unproven') + return { kind: 'unproven', error }; + throw error; + } + if (Date.now() >= deadline) return null; + if (winner) return { kind: 'ready', daemon: { info: winner, startedByClient: false } }; + const inspection = inspectProcessLock(settings.paths.lockPath); + if (inspection.state === 'unproven') + return { kind: 'unproven', error: daemonRegistrationUnprovenError(settings, inspection) }; + return isRegistrationAvailable(inspection) ? { kind: 'retry' } : null; +} + +async function readReadyLaunchedDaemon( + settings: DaemonClientSettings, + launch: DaemonStartupLaunch, + deadline: number, +): Promise { + const info = readDaemonInfo(settings.paths.infoPath); + if (!info || !isLaunchedDaemon(info, launch)) return null; + try { + return (await canConnect( + info, + settings.transportPreference, + remainingStartupBudget(deadline), + )) && Date.now() < deadline + ? info + : null; + } catch (error) { + const { cleanup, joined } = await retireStartupAttempt(settings, launch, deadline); + emitDiagnostic({ + level: 'warn', + phase: 'daemon_startup_observation_failed', + data: { stateDir: settings.paths.baseDir, cleanup, joined, error: normalizeError(error) }, + }); + if (error instanceof AppError) { + error.details = { + ...error.details, + stateDir: settings.paths.baseDir, + cleanupResults: [cleanup], + startupJoined: joined, + }; + } + throw error; + } +} + /** Whether `info` names the daemon process this client launched: same pid and start time. */ function isLaunchedDaemon(info: DaemonInfo, launch: DaemonStartupLaunch): boolean { + const identity = launch.readIdentity(); return ( - info.pid === launch.pid && - launch.startTime !== undefined && - info.processStartTime === launch.startTime + info.pid === identity.pid && + identity.startTime !== null && + info.processStartTime === identity.startTime ); } function startDaemon(settings: DaemonClientSettings): DaemonStartupLaunch { const launchSpec = resolveDaemonLaunchSpec(); - const args = launchSpec.useSrc - ? ['--experimental-strip-types', launchSpec.srcPath] - : [launchSpec.distPath]; - const env = { - ...process.env, - AGENT_DEVICE_STATE_DIR: settings.paths.baseDir, - AGENT_DEVICE_DAEMON_SERVER_MODE: settings.serverMode, - }; - - fs.mkdirSync(settings.paths.baseDir, { recursive: true }); - const stdoutFd = fs.openSync(settings.paths.logPath, 'a'); - const stderrFd = fs.openSync(settings.paths.logPath, 'a'); - try { - const launched = runCmdDetachedMonitored(process.execPath, args, { - env, - stdio: ['ignore', stdoutFd, stderrFd], - }); - return { ...launched, startTime: readProcessStartTime(launched.pid) ?? undefined }; - } finally { - fs.closeSync(stdoutFd); - fs.closeSync(stderrFd); - } + return launchDaemonProcess({ + paths: settings.paths, + serverMode: settings.serverMode, + ownedStateDir: settings.ownedStateDir, + args: launchSpec.useSrc + ? ['--experimental-strip-types', launchSpec.srcPath] + : [launchSpec.distPath], + }); } function describeDaemonEarlyExit(exit: ExecDetachedExit): string { @@ -771,3 +899,22 @@ function isLoopbackHostname(hostname: string): boolean { if (net.isIPv6(normalized)) return LOOPBACK_BLOCK_LIST.check(normalized, 'ipv6'); return false; } + +export function attachSessionAddressHints( + response: DaemonResponse, + req: Omit, + settings: DaemonClientSettings, +): DaemonResponse { + if (!response.ok) { + return settings.ownedStateDir && isHeldRepairDivergence(response) + ? attachRepairSessionAddressHint(response, settings.paths.baseDir) + : response; + } + return isActiveReplaySessionResponse(req, response) + ? attachActiveSessionAddressHint( + response, + settings.ownedStateDir ? settings.paths.baseDir : undefined, + settings.remoteBaseUrl, + ) + : response; +} diff --git a/src/daemon-client/daemon-client-metadata.ts b/src/daemon-client/daemon-client-metadata.ts index 3a448b099f..718ed7b1cc 100644 --- a/src/daemon-client/daemon-client-metadata.ts +++ b/src/daemon-client/daemon-client-metadata.ts @@ -1,12 +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 { - isAgentDeviceDaemonProcess, - stopDaemonProcess, - type DaemonTerminationResult, -} from '../daemon-process.ts'; +import { isProcessPid } from '@agent-device/host-kit/process'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; @@ -33,33 +26,11 @@ export type DaemonInfo = { remoteUpstreamInstanceId?: string; }; -type DaemonLockInfo = { - pid: number; - processStartTime?: string; - startedAt?: number; -}; - export type DaemonMetadataState = { hasInfo: boolean; hasLock: boolean; }; -type DaemonStartupCleanupReason = 'start_error' | 'startup_timeout'; - -export type DaemonStartupCleanupResult = { - reason: DaemonStartupCleanupReason; - removedInfo: boolean; - removedLock: boolean; - stoppedInfoProcess: boolean; - stoppedLockProcess: boolean; - retainedInfoProcess?: boolean; - retainedLockProcess?: boolean; - error?: string; -}; - -const DAEMON_TAKEOVER_TERM_TIMEOUT_MS = 3000; -const DAEMON_TAKEOVER_KILL_TIMEOUT_MS = 1000; - export function readDaemonInfo(infoPath: string): DaemonInfo | null { const data = readJsonFile(infoPath); if (!data || typeof data !== 'object') return null; @@ -72,7 +43,7 @@ export function readDaemonInfo(infoPath: string): DaemonInfo | null { token, ...ports, transport: readDaemonInfoTransport(parsed.transport), - pid: readPositiveInteger(parsed.pid) ?? 0, + pid: isProcessPid(parsed.pid) ? parsed.pid : 0, version: readOptionalString(parsed.version), codeOrigin: readDaemonInfoCodeOrigin(parsed.codeOrigin), codeSignature: readOptionalString(parsed.codeSignature), @@ -110,129 +81,6 @@ function readPositiveInteger(value: unknown): number | undefined { return Number.isInteger(value) && Number(value) > 0 ? Number(value) : undefined; } -function readDaemonLockInfo(lockPath: string): DaemonLockInfo | null { - const data = readJsonFile(lockPath); - if (!data || typeof data !== 'object') return null; - const parsed = data as Partial; - const hasPid = Number.isInteger(parsed.pid) && Number(parsed.pid) > 0; - if (!hasPid) { - return null; - } - return { - pid: Number(parsed.pid), - processStartTime: - typeof parsed.processStartTime === 'string' ? parsed.processStartTime : undefined, - startedAt: typeof parsed.startedAt === 'number' ? parsed.startedAt : undefined, - }; -} - -/** - * Whether a live daemon other than `pid` holds the startup lock: another client's daemon won the - * start, and the daemon at `pid` exited because it lost the lock. - */ -export function isDaemonLockHeldByAnotherDaemon(paths: DaemonPaths, pid: number): boolean { - const lockInfo = readDaemonLockInfo(paths.lockPath); - return ( - lockInfo !== null && - lockInfo.pid !== pid && - isAgentDeviceDaemonProcess(lockInfo.pid, lockInfo.processStartTime) - ); -} - -export function removeDaemonInfo(infoPath: string): void { - removeFileIfExists(infoPath); -} - -export function removeDaemonLock(lockPath: string): void { - removeFileIfExists(lockPath); -} - -export function cleanupStaleDaemonLockIfSafe(paths: DaemonPaths): void { - const state = getDaemonMetadataState(paths); - if (!state.hasLock || state.hasInfo) return; - const lockInfo = readDaemonLockInfo(paths.lockPath); - if (!lockInfo) { - removeDaemonLock(paths.lockPath); - return; - } - if (isAgentDeviceDaemonProcess(lockInfo.pid, lockInfo.processStartTime)) { - return; - } - removeDaemonLock(paths.lockPath); -} - -export async function cleanupFailedDaemonStartupMetadata( - paths: DaemonPaths, - reason: DaemonStartupCleanupReason, - options: { stopLiveProcesses?: boolean } = {}, -): Promise { - const stopLiveProcesses = options.stopLiveProcesses ?? true; - const result: DaemonStartupCleanupResult = { - reason, - removedInfo: false, - removedLock: false, - stoppedInfoProcess: false, - stoppedLockProcess: false, - }; - - try { - const infoExists = fs.existsSync(paths.infoPath); - const info = readDaemonInfo(paths.infoPath); - if (info) { - const liveInfoProcess = isAgentDeviceDaemonProcess(info.pid, info.processStartTime); - if (liveInfoProcess && !stopLiveProcesses) { - result.retainedInfoProcess = true; - } else { - if (liveInfoProcess) { - await stopDaemonProcessForTakeover(info); - result.stoppedInfoProcess = true; - } - removeDaemonInfo(paths.infoPath); - result.removedInfo = true; - } - } else if (infoExists) { - removeDaemonInfo(paths.infoPath); - result.removedInfo = true; - } - - const lockExists = fs.existsSync(paths.lockPath); - const lockInfo = readDaemonLockInfo(paths.lockPath); - if (lockInfo) { - const liveLockProcess = isAgentDeviceDaemonProcess(lockInfo.pid, lockInfo.processStartTime); - if (liveLockProcess && !stopLiveProcesses) { - result.retainedLockProcess = true; - } else { - if (liveLockProcess) { - const termination = await stopDaemonProcess( - { pid: lockInfo.pid, startTime: lockInfo.processStartTime ?? null }, - { - mode: 'graceful', - termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, - killTimeoutMs: DAEMON_TAKEOVER_KILL_TIMEOUT_MS, - }, - ); - requireDaemonExit(termination); - result.stoppedLockProcess = true; - } - removeDaemonLock(paths.lockPath); - result.removedLock = true; - } - } else if (lockExists) { - removeDaemonLock(paths.lockPath); - result.removedLock = true; - } - } catch (error) { - result.error = error instanceof Error ? error.message : String(error); - } - - emitDiagnostic({ - level: result.error ? 'warn' : 'info', - phase: 'daemon_startup_metadata_cleanup', - data: result, - }); - return result; -} - export function getDaemonMetadataState(paths: DaemonPaths): DaemonMetadataState { return { hasInfo: fs.existsSync(paths.infoPath), @@ -240,44 +88,6 @@ export function getDaemonMetadataState(paths: DaemonPaths): DaemonMetadataState }; } -export async function recoverDaemonLockHolder(paths: DaemonPaths): Promise { - const state = getDaemonMetadataState(paths); - if (!state.hasLock || state.hasInfo) return false; - const lockInfo = readDaemonLockInfo(paths.lockPath); - if (!lockInfo) { - removeDaemonLock(paths.lockPath); - return true; - } - if (!isAgentDeviceDaemonProcess(lockInfo.pid, lockInfo.processStartTime)) { - removeDaemonLock(paths.lockPath); - return true; - } - return false; -} - -export async function stopDaemonProcessForTakeover( - info: DaemonInfo, -): Promise { - const termination = await stopDaemonProcess( - { pid: info.pid, startTime: info.processStartTime ?? null }, - { - mode: 'graceful', - termTimeoutMs: DAEMON_TAKEOVER_TERM_TIMEOUT_MS, - 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.', { - reason: 'daemon_exit_unconfirmed', - termination, - }); -} - export function isRemoteDaemon(info: DaemonInfo): boolean { return typeof info.baseUrl === 'string' && info.baseUrl.length > 0; } @@ -288,21 +98,10 @@ export function resolveDaemonStartupHint( process.env.AGENT_DEVICE_STATE_DIR, ), ): string { - const cleanupCommand = buildDaemonMetadataCleanupCommand(paths); - if (state.hasLock && !state.hasInfo) { - return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.lockPath} still exists without ${paths.infoPath}. Retry with --debug; if this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`; - } - if (state.hasLock && state.hasInfo) { - return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.infoPath} and ${paths.lockPath} still remain. Retry with --debug; if this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`; - } - if (state.hasInfo) { - return `agent-device did not observe reachable daemon metadata after retrying, and ${paths.infoPath} still remains. Stale metadata was cleaned automatically when safe; retry with --debug. If this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`; - } - return `agent-device did not observe reachable daemon metadata after retrying. Stale metadata was cleaned automatically when safe; retry with --debug and check daemon diagnostics logs. If stale metadata returns after confirming no agent-device daemon process is running, run: ${cleanupCommand}`; -} - -function buildDaemonMetadataCleanupCommand(paths: Pick) { - return `rm -f ${shellQuote(paths.infoPath)} ${shellQuote(paths.lockPath)}`; + const artifacts = [state.hasInfo ? paths.infoPath : null, state.hasLock ? paths.lockPath : null] + .filter(Boolean) + .join(' and '); + return `Daemon startup did not establish a reachable owner. ${artifacts ? `State was retained at ${artifacts}. ` : ''}Retry with --debug and inspect daemon diagnostics. Before upgrading, stop all older clients and daemons with their original CLI and prevent them from returning to this state directory. Unverified lock state requires confirming every user stopped before manual recovery; deleting metadata alone is not a safe reset.`; } function readJsonFile(filePath: string): unknown | null { @@ -313,11 +112,3 @@ function readJsonFile(filePath: string): unknown | null { return null; } } - -function removeFileIfExists(filePath: string): void { - try { - if (fs.existsSync(filePath)) fs.unlinkSync(filePath); - } catch { - // Best-effort cleanup only. - } -} diff --git a/src/daemon-client/daemon-client-timeout.ts b/src/daemon-client/daemon-client-timeout.ts index d442b24b3e..547c5f2799 100644 --- a/src/daemon-client/daemon-client-timeout.ts +++ b/src/daemon-client/daemon-client-timeout.ts @@ -1,18 +1,13 @@ -import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { AppError } from '@agent-device/kernel/errors'; import { runCmdSync } from '@agent-device/host-kit/command'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; -import { isAgentDeviceDaemonProcess } from '../daemon-process.ts'; +import type { DaemonRetirementResult } from '../daemon-registration-owner.ts'; import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import { resolveCommandTimeoutPolicy } from '@agent-device/command-registry/registry'; import type { DaemonPaths } from '../daemon-resolution.ts'; import type { PlatformSelector } from '@agent-device/kernel/device'; -import { - removeDaemonInfo, - removeDaemonLock, - stopDaemonProcessForTakeover, - type DaemonInfo, -} from './daemon-client-metadata.ts'; +import type { DaemonInfo } from './daemon-client-metadata.ts'; const IOS_RUNNER_XCODEBUILD_KILL_PATTERNS = [ 'xcodebuild .*AgentDeviceRunnerUITests/RunnerTests/testCommand', @@ -40,7 +35,7 @@ function isAffirmativelyApplePlatform(platform: PlatformSelector | undefined): b return platform !== undefined && AFFIRMATIVE_APPLE_PLATFORM_SELECTORS.has(platform); } -export function handleRequestTimeout( +export async function handleRequestTimeout( params: Readonly<{ info: DaemonInfo; statePaths: DaemonPaths; @@ -53,7 +48,7 @@ export function handleRequestTimeout( session?: string; action?: string; }>, -): AppError { +): Promise { const { info, statePaths, remote, timeoutMs, requestId, command, platform, session, action } = params; // Cleanup eligibility never depends on the declared platform, on purpose: @@ -72,9 +67,17 @@ export function handleRequestTimeout( const sweepRunnerBuilds = !remote && command !== PUBLIC_COMMANDS.record; const cleanup = sweepRunnerBuilds ? cleanupTimedOutIosRunnerBuilds() : { terminated: 0 }; const resetDaemon = !remote && shouldResetDaemonAfterRequestTimeout(command); - const daemonReset = resetDaemon - ? resetDaemonAfterTimeout(info, statePaths) - : { forcedKill: false }; + let retirement: DaemonRetirementResult | undefined; + if (resetDaemon) { + const { stopAndRetireDaemon } = await import('../daemon-registration-owner.ts'); + retirement = await stopAndRetireDaemon({ + paths: statePaths, + observed: { pid: info.pid, startTime: info.processStartTime ?? null }, + mode: 'force', + }); + } + const preserved = + retirement?.status === 'retained' && retirement.termination?.status !== 'exited'; // The HINT, unlike cleanup, may only name Apple-runner involvement on // evidence this call site actually has: an explicitly declared Apple // platform selector, or the cleanup itself having terminated a matching @@ -92,9 +95,10 @@ export function handleRequestTimeout( command, timedOutRunnerPidsTerminated: cleanup.terminated, timedOutRunnerCleanupError: cleanup.error, - daemonPidReset: resetDaemon ? info.pid : undefined, - daemonPidForceKilled: resetDaemon ? daemonReset.forcedKill : undefined, - daemonPreservedAfterTimeout: !remote && !resetDaemon, + daemonPidReset: retirement?.status === 'retired' ? info.pid : undefined, + daemonPidForceKilled: resetDaemon ? daemonWasForceKilled(retirement) : undefined, + daemonRetirement: retirement, + daemonPreservedAfterTimeout: preserved || (!remote && !resetDaemon), daemonBaseUrl: info.baseUrl, }, }); @@ -102,17 +106,27 @@ export function handleRequestTimeout( timeoutMs, requestId, reason: 'daemon_transport_timeout', - hint: resolveRequestTimeoutHint({ - remote, - resetDaemon, - command, - appleCleanupEvidence, - session, - action, - }), + ...(retirement ? { retirement, stateDir: statePaths.baseDir } : {}), + hint: + retirement?.status === 'retained' + ? `The daemon could not be safely retired. State was retained at ${statePaths.baseDir}. ${retirement.error?.hint ?? 'Retry with --debug and inspect daemon diagnostics before retrying.'}` + : resolveRequestTimeoutHint({ + remote, + resetDaemon, + command, + appleCleanupEvidence, + session, + action, + }), }); } +function daemonWasForceKilled(retirement: DaemonRetirementResult | undefined): boolean { + if (!retirement || retirement.status === 'absent') return false; + const termination = retirement.termination; + return termination?.status === 'exited' && termination.mode === 'forced'; +} + // Whether a timed-out request tears down the local daemon is declared on the // command's descriptor (ADR 0008, `timeoutPolicy.onTimeout`): read-only // capture/polling commands preserve the daemon so sessions survive and evidence @@ -180,25 +194,3 @@ function cleanupTimedOutIosRunnerBuilds(): { terminated: number; error?: string }; } } - -function resetDaemonAfterTimeout(info: DaemonInfo, paths: DaemonPaths): { forcedKill: boolean } { - let forcedKill = false; - try { - if (isAgentDeviceDaemonProcess(info.pid, info.processStartTime)) { - process.kill(info.pid, 'SIGKILL'); - forcedKill = true; - } - } catch { - 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); - } - return { forcedKill }; -} diff --git a/src/daemon-client/daemon-client-transport.ts b/src/daemon-client/daemon-client-transport.ts index e1ac9b1bb4..3f54b59615 100644 --- a/src/daemon-client/daemon-client-transport.ts +++ b/src/daemon-client/daemon-client-transport.ts @@ -166,23 +166,34 @@ type RemoteDaemonHealthLink = Pick< export async function canConnect( info: DaemonInfo, preference: DaemonTransportPreference, + probeTimeoutMs?: number, ): Promise { + const deadline = Date.now() + (probeTimeoutMs ?? Number.POSITIVE_INFINITY); const transport = chooseTransport(info, preference); - if (await canConnectWithTransport(info, transport)) return true; - const fallback = chooseAutoFallbackTransport(info, preference, transport); - return fallback ? await canConnectWithTransport(info, fallback) : false; + const firstBudget = (deadline - Date.now()) / (fallback ? 2 : 1); + if (await canConnectWithTransport(info, transport, firstBudget)) return Date.now() < deadline; + return fallback + ? (await canConnectWithTransport(info, fallback, deadline - Date.now())) && + Date.now() < deadline + : false; } async function canConnectWithTransport( info: DaemonInfo, transport: ResolvedDaemonTransport, + timeoutMs: number, ): Promise { - return transport === 'http' ? await canConnectHttp(info) : await canConnectSocket(info.port); + return transport === 'http' + ? await canConnectHttp(info, timeoutMs) + : await canConnectSocket(info.port, timeoutMs); } -export function canConnectSocket(port: number | undefined): Promise { - if (!port) return Promise.resolve(false); +export function canConnectSocket( + port: number | undefined, + timeoutMs = LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS, +): Promise { + if (!port || timeoutMs <= 0) return Promise.resolve(false); return new Promise((resolve) => { let settled = false; const socket = net.createConnection({ host: '127.0.0.1', port }, () => { @@ -194,7 +205,7 @@ export function canConnectSocket(port: number | undefined): Promise { socket.destroy(); resolve(reachable); }; - socket.setTimeout(LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS); + socket.setTimeout(Math.min(LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS, Math.ceil(timeoutMs))); socket.on('timeout', () => { finish(false); }); @@ -204,8 +215,8 @@ export function canConnectSocket(port: number | undefined): Promise { }); } -function canConnectHttp(info: DaemonInfo): Promise { - return readDaemonHttpHealth(info).then((health) => health.reachable); +function canConnectHttp(info: DaemonInfo, timeoutMs: number): Promise { + return readDaemonHttpHealth(info, timeoutMs).then((health) => health.reachable); } export async function readRemoteDaemonHealth( @@ -248,20 +259,28 @@ async function readDaemonHttpHealth( : null; if (!endpoint) return { reachable: false }; const url = new URL(endpoint); - const transport = await loadNodeHttpRequester(url.protocol); const timeoutMs = Math.min( info.baseUrl ? REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS : LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS, probeTimeoutMs ?? Number.POSITIVE_INFINITY, ); if (timeoutMs <= 0) return { reachable: false, timedOut: true }; - // `timedOut` is keyed on the probe's own budget alone: a caller abort must never read as a - // timeout, because a timed-out probe on an RPC-capped budget is answered as the RPC timing out. + const deadline = performance.now() + timeoutMs; const timeoutSignal = AbortSignal.timeout(Math.ceil(timeoutMs)); const signal = callerSignal ? AbortSignal.any([timeoutSignal, callerSignal]) : timeoutSignal; + const transport = await Promise.race([ + loadNodeHttpRequester(url.protocol), + new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(null), { once: true }); + }), + ]); + if (!transport || signal.aborted || performance.now() >= deadline) + return { reachable: false, timedOut: healthProbeExpired(timeoutSignal, deadline) }; return await new Promise((resolve) => { const headers = info.baseUrl ? buildDaemonHttpAuthHeaders(info.token) : {}; const unreachable = (): RemoteDaemonHealth => - timeoutSignal.aborted ? { reachable: false, timedOut: true } : { reachable: false }; + healthProbeExpired(timeoutSignal, deadline) + ? { reachable: false, timedOut: true } + : { reachable: false }; const req = transport.request( { protocol: url.protocol, @@ -281,11 +300,11 @@ async function readDaemonHttpHealth( }); res.on('end', () => { const statusCode = res.statusCode ?? 500; - resolve({ - reachable: statusCode < 500, - statusCode, - ...readHealthPayload(body), - }); + resolve( + healthProbeExpired(timeoutSignal, deadline) + ? { reachable: false, timedOut: true } + : { reachable: statusCode < 500, statusCode, ...readHealthPayload(body) }, + ); }); res.on('error', () => resolve(unreachable())); res.on('aborted', () => resolve(unreachable())); @@ -302,6 +321,10 @@ async function readDaemonHttpHealth( }); } +function healthProbeExpired(signal: AbortSignal, deadline: number): boolean { + return signal.aborted || performance.now() >= deadline; +} + function readHealthPayload(body: string): Omit { try { const parsed = JSON.parse(body) as { upstream?: unknown }; @@ -336,6 +359,29 @@ export async function sendRequest( statePaths: DaemonPaths, timeoutMs: number | undefined, options: SendRequestOptions = {}, +): Promise { + try { + return await sendRequestWithFallback(info, req, preference, statePaths, timeoutMs, options); + } catch (error) { + if (!(error instanceof AppError) || error.details?.reason !== 'daemon_transport_timeout') + throw error; + const expiredBudget = error.details.timeoutMs; + if (typeof expiredBudget !== 'number') throw error; + throw await handleRequestTimeout({ + info, + statePaths, + ...timeoutRequestContext(req, isRemoteDaemon(info), expiredBudget), + }); + } +} + +async function sendRequestWithFallback( + info: DaemonInfo, + req: DaemonRequest, + preference: DaemonTransportPreference, + statePaths: DaemonPaths, + timeoutMs: number | undefined, + options: SendRequestOptions = {}, ): Promise { const transport = chooseTransport(info, preference); const deadline = typeof timeoutMs === 'number' ? performance.now() + timeoutMs : undefined; @@ -373,27 +419,15 @@ async function retryAfterRemoteInstanceMismatch( options: SendRequestOptions, ): Promise { invalidateRemoteDaemonHealth(info); - const probeTimeoutMs = remainingRemoteRequestTimeoutMs( - info, + const probeTimeoutMs = remainingRemoteRequestTimeoutMs(req, timeoutMs, deadline); + const health = await readRemoteDaemonHealth(info, probeTimeoutMs, options.signal); + refuseAbortedRequest(options.signal, req.meta?.requestId); + const remainingMs = remainingRemoteRequestTimeoutMs( req, - statePaths, timeoutMs, deadline, + health.timedOut ? probeTimeoutMs : undefined, ); - const health = await readRemoteDaemonHealth(info, probeTimeoutMs, options.signal); - // An abort that landed during the probe is this caller's cancellation, not the probe's outcome: - // it is answered before the timed-out-probe and unreachable-daemon branches, so a canceled call - // never borrows the timeout's shape or a daemon-unavailable error. - refuseAbortedRequest(options.signal, req.meta?.requestId); - const timedOutRpcBudgetMs = deadlineCappedProbeTimeoutBudgetMs(health, timeoutMs, probeTimeoutMs); - if (timedOutRpcBudgetMs !== undefined) { - throw handleRequestTimeout({ - info, - statePaths, - ...timeoutRequestContext(req, true, timedOutRpcBudgetMs), - }); - } - const remainingMs = remainingRemoteRequestTimeoutMs(info, req, statePaths, timeoutMs, deadline); if (!health.reachable) { throw new AppError('COMMAND_FAILED', 'Remote daemon is unavailable', { daemonBaseUrl: info.baseUrl, @@ -410,37 +444,21 @@ async function retryAfterRemoteInstanceMismatch( } } -/** - * The RPC budget a probe that ran out of time proves, or `undefined` when the probe's timeout was - * its own. The probe's timer starts from the event loop's cached clock, so it can expire while - * `performance.now()` is still short of the deadline; a probe capped by the RPC deadline is - * therefore the deadline speaking, not a slow daemon. - */ -function deadlineCappedProbeTimeoutBudgetMs( - health: RemoteDaemonHealth, - timeoutMs: number | undefined, - probeTimeoutMs: number | undefined, -): number | undefined { - if (!health.timedOut || timeoutMs === undefined) return undefined; - if (probeTimeoutMs === undefined) return undefined; - return probeTimeoutMs <= REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS ? timeoutMs : undefined; -} - function remainingRemoteRequestTimeoutMs( - info: DaemonInfo, req: DaemonRequest, - statePaths: DaemonPaths, timeoutMs: number | undefined, deadline: number | undefined, + timedOutProbeMs?: number, ): number | undefined { if (deadline === undefined || timeoutMs === undefined) return undefined; const remainingMs = deadline - performance.now(); - if (remainingMs > 0) return remainingMs; - throw handleRequestTimeout({ - info, - statePaths, - ...timeoutRequestContext(req, true, timeoutMs), - }); + // A probe capped by the request can expire before the monotonic clock catches up to its timer. + if ( + remainingMs > 0 && + (timedOutProbeMs === undefined || timedOutProbeMs > REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS) + ) + return remainingMs; + throw requestTimeoutError(timeoutMs, req.meta?.requestId); } function isRemoteInstanceMismatch(error: unknown): boolean { @@ -466,7 +484,7 @@ async function sendRequestWithTransport( ): Promise { return transport === 'http' ? await sendHttpRequest(info, req, statePaths, timeoutMs, options) - : await sendSocketRequest(info, req, statePaths, timeoutMs, options); + : await sendSocketRequest(info, req, timeoutMs, options); } function chooseTransport( @@ -561,7 +579,6 @@ function handleTransportError( async function sendSocketRequest( info: DaemonInfo, req: DaemonRequest, - statePaths: DaemonPaths, timeoutMs: number | undefined, options: SendRequestOptions, ): Promise { @@ -585,16 +602,11 @@ async function sendSocketRequest( const timeoutHandle = typeof timeoutMs === 'number' ? setTimeout(() => { + if (settled) return; settled = true; detachCallerAbort(); + reject(requestTimeoutError(timeoutMs, req.meta?.requestId)); socket.destroy(); - reject( - handleRequestTimeout({ - info, - statePaths, - ...timeoutRequestContext(req, false, timeoutMs), - }), - ); }, timeoutMs) : undefined; // Destroying the connection is what makes the daemon mark the request canceled. The timeout @@ -651,6 +663,14 @@ async function sendSocketRequest( }); } +function requestTimeoutError(timeoutMs: number, requestId: string | undefined): AppError { + return new AppError('COMMAND_FAILED', 'Daemon request timed out', { + reason: 'daemon_transport_timeout', + timeoutMs, + requestId, + }); +} + // The fields a timed-out request is described by, read once so a socket and an HTTP timeout cannot // describe the same request differently. type TimeoutRequestFields = Omit[0], 'info' | 'statePaths'>; @@ -802,13 +822,7 @@ async function sendHttpRequest( const timeoutHandle = typeof timeoutMs === 'number' ? setTimeout(() => { - rejectOnce( - handleRequestTimeout({ - info, - statePaths, - ...timeoutRequestContext(req, remote, timeoutMs), - }), - ); + rejectOnce(requestTimeoutError(timeoutMs, req.meta?.requestId)); request.destroy(); }, timeoutMs) : undefined; diff --git a/src/daemon-client/daemon-client.ts b/src/daemon-client/daemon-client.ts index 461c91ac8d..3dc4bfab29 100644 --- a/src/daemon-client/daemon-client.ts +++ b/src/daemon-client/daemon-client.ts @@ -17,17 +17,7 @@ import { prepareRemoteRequestArtifacts, type PreparedRemoteRequest, } from '../remote/daemon-artifacts.ts'; -import { - attachActiveSessionAddressHint, - attachRepairSessionAddressHint, - cleanupDaemonAfterRequest, - ensureDaemon, - isActiveReplaySessionResponse, - isHeldRepairDivergence, - resolveClientSettings, - type DaemonClientSettings, - type EnsuredDaemon, -} from './daemon-client-lifecycle.ts'; +import type { DaemonClientSettings, EnsuredDaemon } from './daemon-client-lifecycle.ts'; import { createRequestGuard, sendRequest } from './daemon-client-transport.ts'; import { isRemoteDaemon, type DaemonInfo } from './daemon-client-metadata.ts'; import { leaseScopeFromRequest } from '@agent-device/contracts/lease-scope'; @@ -42,6 +32,8 @@ export async function sendToDaemon( req: Omit, options: DaemonTransportOptions = {}, ): Promise { + const { resolveClientSettings, ensureDaemon, attachSessionAddressHints } = + await import('./daemon-client-lifecycle.ts'); const requestId = req.meta?.requestId ?? createRequestId(); const debug = Boolean(req.meta?.debug || req.flags?.verbose); // A few internal callers build DaemonRequest directly instead of using the @@ -116,11 +108,7 @@ export async function sendToDaemon( ), { requestId, command: req.command }, ); - return withActiveSessionAddressHint( - withRepairSessionAddressHintIfOwned(response, settings), - requestWithoutAuthFlag, - settings, - ); + return attachSessionAddressHints(response, requestWithoutAuthFlag, settings); }, ); } @@ -244,6 +232,7 @@ async function performDaemonRequestWithCleanup( requestFailed = true; requestError = error; } + const { cleanupDaemonAfterRequest } = await import('./daemon-client-lifecycle.ts'); const finalResponse = await cleanupDaemonAfterRequest(req, daemon, settings, response); if (requestFailed) throw requestError; if (!finalResponse) { @@ -255,48 +244,6 @@ async function performDaemonRequestWithCleanup( return finalResponse; } -/** - * ADR 0012 decision 6 (Fix 1): the owned ephemeral state dir this daemon was - * started at is otherwise unaddressable by a later invocation — hint it here, - * only when the daemon is actually being kept alive for it - * (`settings.ownedStateDir` means `daemon.startedByClient` is also true). - */ -function withRepairSessionAddressHintIfOwned( - response: DaemonResponse, - settings: DaemonClientSettings, -): DaemonResponse { - if (response.ok || !settings.ownedStateDir || !isHeldRepairDivergence(response)) { - return response; - } - return attachRepairSessionAddressHint(response, settings.paths.baseDir); -} - -/** - * ADR 0016 counterpart to `withRepairSessionAddressHintIfOwned` — but unlike - * that one, NOT gated on `settings.ownedStateDir`. An owned ephemeral state - * dir is unaddressable by a later invocation either way, so it's included - * when owned; an explicit `--state-dir`/`AGENT_DEVICE_STATE_DIR` caller - * already knows their own dir, so it's omitted then. But the session's own - * name is cwd-qualified and, per #1394, `session list` cannot rediscover it - * either — so `--session` is still worth hinting even at an explicit state - * dir, which is why this runs for every active-session response regardless - * of `ownedStateDir` (`attachActiveSessionAddressHint` itself decides what, - * if anything, is worth attaching). - */ -function withActiveSessionAddressHint( - response: DaemonResponse, - req: Omit, - settings: DaemonClientSettings, -): DaemonResponse { - if (!response.ok || !isActiveReplaySessionResponse(req, response)) { - return response; - } - return attachActiveSessionAddressHint( - response, - settings.ownedStateDir ? settings.paths.baseDir : undefined, - ); -} - function writeInstallInProgressNotice(command: string | undefined): void { if (!isInstallLikeCommand(command) || process.stderr.isTTY !== true || process.env.CI) return; process.stderr.write( diff --git a/src/daemon-process.ts b/src/daemon-process.ts index fee88d3622..28869a01b5 100644 --- a/src/daemon-process.ts +++ b/src/daemon-process.ts @@ -1,5 +1,6 @@ import { isProcessAlive, + isProcessPid, readHostProcessIdentityObservations, readProcessCommand, readProcessStartTime, @@ -72,6 +73,7 @@ export async function waitForDaemonExit( identity: DaemonProcessIdentity, options: { timeoutMs: number; pollMs?: number }, ): Promise { + if (!isProcessPid(identity.pid)) return { exited: false, elapsedMs: 0 }; const startedAt = Date.now(); const deadline = startedAt + options.timeoutMs; const pollMs = options.pollMs ?? DAEMON_EXIT_POLL_MS; @@ -114,6 +116,7 @@ export async function stopDaemonProcess( killTimeoutMs: number; }, ): Promise { + if (!isProcessPid(observed.pid)) return { status: 'retained', reason: 'identity-unverified' }; if (!observed.startTime?.trim()) { if (!isProcessAlive(observed.pid)) return { status: 'not-running' }; return { status: 'retained', reason: 'missing-start-time' }; diff --git a/src/daemon-registration-owner.ts b/src/daemon-registration-owner.ts index 0092f087af..2a329ffe8e 100644 --- a/src/daemon-registration-owner.ts +++ b/src/daemon-registration-owner.ts @@ -1,13 +1,22 @@ import fs from 'node:fs'; -import { normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; +import os from 'node:os'; +import path from 'node:path'; +import { runCmdDetachedMonitored, type ExecDetachedExit } from '@agent-device/host-kit/command'; +import { AppError, normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; import { stopDaemonProcess, waitForDaemonExit, type DaemonTerminationResult, } from './daemon-process.ts'; -import { readCurrentOwnerIdentity, type OwnerIdentity } from '@agent-device/host-kit/process'; +import { + readCurrentOwnerIdentity, + ownerIdentityMatches, + readProcessStartTime, + type OwnerIdentity, +} from '@agent-device/host-kit/process'; import { publishFileSync, + inspectProcessLock, tryAcquireProcessLock, acquireProcessLockAcquisition, type ProcessLockAttempt, @@ -15,9 +24,15 @@ import { } from '@agent-device/host-kit/file'; import { emitDiagnostic, withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; -import type { DaemonPaths } from './daemon-resolution.ts'; +import { + resolveDaemonPaths, + type DaemonPaths, + type DaemonServerMode, +} from './daemon-resolution.ts'; +import { findUnrecoveredRepairCommitFailure } from './session-repair-tombstone.ts'; import { readRegisteredDaemonOwnership, + readRegisteredDaemonIdentity, type RegisteredDaemonOwnership, } from './daemon-registration.ts'; import { @@ -161,6 +176,113 @@ function truncateDaemonLog(logPath: string): void { } } +declare const privateReplayState: unique symbol; +export type OwnedReplayStateDir = Readonly<{ + paths: Readonly; + [privateReplayState]: true; +}>; +export type DaemonStartupLaunch = Readonly<{ + pid: number; + startTime?: string; + exited: Promise; + readIdentity(): OwnerIdentity; +}>; +type OwnedStartup = { launch: DaemonStartupLaunch; joined: boolean; startTime?: string }; +type PrivateReplayState = { + paths: Readonly; + startups: OwnedStartup[]; + sealed: boolean; + retirement?: Promise; +}; +const privateReplayStates = new WeakMap(); + +/** Creates deletion authority only for a fresh private replay directory. */ +export function createOwnedReplayStateDir(): OwnedReplayStateDir { + const paths = Object.freeze( + resolveDaemonPaths(fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-daemon-'))), + ); + const owned = Object.freeze({ paths }) as OwnedReplayStateDir; + privateReplayStates.set(owned, { paths, startups: [], sealed: false }); + return owned; +} + +/** Launches and monitors the actual child before recording its private-directory authority. */ +export function launchDaemonProcess( + input: Readonly<{ + paths: DaemonPaths; + args: string[]; + serverMode: DaemonServerMode; + ownedStateDir?: OwnedReplayStateDir; + }>, +): DaemonStartupLaunch { + const owned = input.ownedStateDir && requirePrivateReplayState(input.ownedStateDir, input.paths); + if (owned?.sealed) + throw new AppError('COMMAND_FAILED', 'Replay daemon startup admission is closed.', { + reason: 'daemon_startup_admission_closed', + }); + fs.mkdirSync(input.paths.baseDir, { recursive: true }); + const logFd = fs.openSync(input.paths.logPath, 'a'); + try { + const monitored = runCmdDetachedMonitored(process.execPath, input.args, { + env: { + ...process.env, + AGENT_DEVICE_STATE_DIR: input.paths.baseDir, + AGENT_DEVICE_DAEMON_SERVER_MODE: input.serverMode, + }, + stdio: ['ignore', logFd, logFd], + }); + const boundPaths = { ...input.paths }; + const startup: OwnedStartup = { + launch: Object.freeze({ + ...monitored, + get startTime() { + return startup.startTime; + }, + readIdentity() { + recoverStartupBirth(boundPaths, startup); + return { pid: monitored.pid, startTime: startup.startTime ?? null }; + }, + }), + joined: false, + }; + if (owned) owned.startups.push(startup); + void monitored.exited.then(() => { + startup.joined = true; + }); + startup.startTime = readProcessStartTime(monitored.pid) ?? undefined; + return startup.launch; + } finally { + fs.closeSync(logFd); + } +} + +function recoverStartupBirth(paths: DaemonPaths, startup: OwnedStartup): void { + if (startup.startTime || startup.joined) return; + const registration = readRegisteredDaemonIdentity(paths.infoPath); + if (registration?.pid !== startup.launch.pid || !registration.startTime) return; + const lock = inspectProcessLock(paths.lockPath); + if (lock.state !== 'held' || !ownerIdentityMatches(lock.owner, registration)) return; + startup.startTime = registration.startTime; +} + +function requirePrivateReplayState( + capability: OwnedReplayStateDir, + paths: DaemonPaths, +): PrivateReplayState { + const owned = privateReplayStates.get(capability); + if ( + !owned || + Object.entries(owned.paths).some(([key, value]) => paths[key as keyof DaemonPaths] !== value) + ) { + throw new AppError( + 'COMMAND_FAILED', + 'Private replay directory ownership could not be verified.', + { reason: 'daemon_private_state_unowned' }, + ); + } + return owned; +} + export type DaemonRetirementInput = Readonly<{ paths: DaemonPaths; observed: OwnerIdentity | null; @@ -169,16 +291,25 @@ export type DaemonRetirementInput = Readonly<{ lockTimeoutMs?: number; }>; +type RepairCommitFailure = NonNullable>; type ConfirmedDaemonTermination = Extract; export type DaemonRetirementResult = - | Readonly<{ status: 'retired'; termination: ConfirmedDaemonTermination; removedInfo: boolean }> - | Readonly<{ status: 'absent'; removedInfo: false }> + | Readonly<{ + status: 'retired'; + termination: ConfirmedDaemonTermination; + removedInfo: boolean; + removedStateDir?: boolean; + repairCommitFailure?: RepairCommitFailure; + }> + | Readonly<{ status: 'absent'; removedInfo: false; removedStateDir?: boolean }> | Readonly<{ status: 'retained'; termination?: DaemonTerminationResult; removedInfo: boolean; + removedStateDir?: boolean; reason: | 'ownership-unproven' + | 'startup-unconfirmed' | 'exit-unconfirmed' | 'stop-failed' | 'lock-busy' @@ -186,19 +317,74 @@ export type DaemonRetirementResult = | 'metadata-unreadable' | 'retirement-unconfirmed'; error?: NormalizedError; + repairCommitFailure?: RepairCommitFailure; }>; /** Stops only the captured daemon lifetime, then retires its registration under the startup lock. */ export async function stopAndRetireDaemon( - input: DaemonRetirementInput & Readonly<{ mode: 'graceful' | 'force' }>, + input: DaemonRetirementInput & + Readonly<{ + mode: 'graceful' | 'force'; + ownedStateDir?: OwnedReplayStateDir; + startupJoinTimeoutMs?: number; + }>, ): Promise { - return await retireObservedDaemon(input, (identity) => - stopDaemonProcess(identity, { - mode: input.mode, - termTimeoutMs: input.termTimeoutMs ?? 3_000, - killTimeoutMs: input.killTimeoutMs ?? 1_000, - }), + let owned: PrivateReplayState | undefined; + let observed = input.observed; + try { + if (input.ownedStateDir) { + owned = requirePrivateReplayState(input.ownedStateDir, input.paths); + owned.sealed = true; + observed = confirmOwnedStartupIdentity(owned, observed); + if (owned.retirement) return await owned.retirement; + } + } catch (error) { + await joinOwnedStartups(owned, input.startupJoinTimeoutMs ?? 1_000); + return { + status: 'retained', + reason: 'ownership-unproven', + removedInfo: false, + error: normalizeError(error), + }; + } + const retirement = retireObservedDaemon( + { ...input, observed }, + (identity) => + stopDaemonProcess(identity, { + mode: input.mode, + termTimeoutMs: input.termTimeoutMs ?? 3_000, + killTimeoutMs: input.killTimeoutMs ?? 1_000, + }), + owned, + input.startupJoinTimeoutMs ?? 1_000, ); + if (owned) owned.retirement = retirement; + const result = await retirement; + if (owned && (result.status === 'absent' || !result.removedStateDir)) + owned.retirement = undefined; + return result; +} + +function confirmOwnedStartupIdentity( + owned: PrivateReplayState, + observed: OwnerIdentity | null, +): OwnerIdentity | null { + if (!observed) { + if (owned.startups.length === 0) return null; + throw unownedStartupError(); + } + const startup = owned.startups.find(({ launch }) => launch.pid === observed.pid); + if (!startup) throw unownedStartupError(); + const { launch } = startup; + const identity = observed.startTime ? observed : launch.readIdentity(); + if (launch.startTime && launch.startTime === identity.startTime) return identity; + throw unownedStartupError(); +} + +function unownedStartupError(): AppError { + return new AppError('COMMAND_FAILED', 'The observed daemon is not an owned startup lifetime.', { + reason: 'daemon_private_startup_unowned', + }); } /** Recovers a confirmed abandoned registration without signaling a live process. */ @@ -217,36 +403,35 @@ export async function recoverAbandonedDaemonRegistration( async function retireObservedDaemon( input: DaemonRetirementInput, terminate: (identity: OwnerIdentity) => Promise, + owned?: PrivateReplayState, + startupJoinTimeoutMs = 1_000, ): Promise { const paths = { ...input.paths }; const observed = input.observed && { ...input.observed }; - let termination: ConfirmedDaemonTermination | undefined; + let termination: DaemonTerminationResult | undefined; + let stopFailure: NormalizedError | undefined; if (observed) { try { - const result = await terminate(observed); - if (result.status !== 'exited') - return { - status: 'retained', - reason: 'exit-unconfirmed', - termination: result, - removedInfo: false, - }; - termination = result; + termination = await terminate(observed); } catch (error) { - return { - status: 'retained', - reason: 'stop-failed', - removedInfo: false, - error: normalizeError(error), - }; + stopFailure = normalizeError(error); } } - return await retireDaemonRegistration({ ...input, paths }, termination); + const joined = await joinOwnedStartups(owned, startupJoinTimeoutMs); + if (stopFailure) + return { status: 'retained', reason: 'stop-failed', removedInfo: false, error: stopFailure }; + if (termination && termination.status !== 'exited') + return { status: 'retained', reason: 'exit-unconfirmed', termination, removedInfo: false }; + if (!joined) { + return { status: 'retained', reason: 'startup-unconfirmed', termination, removedInfo: false }; + } + return await retireDaemonRegistration({ ...input, paths }, termination, owned); } async function retireDaemonRegistration( input: DaemonRetirementInput, termination: ConfirmedDaemonTermination | undefined, + owned?: PrivateReplayState, ): Promise { const paths = input.paths; let acquisition: ProcessLockAcquisition; @@ -268,7 +453,11 @@ async function retireDaemonRegistration( error: failure, }; } - let result: DaemonRetirementResult; + let result: DaemonRetirementResult = { + status: 'retained', + reason: 'retirement-unconfirmed', + removedInfo: false, + }; try { const removal = removeRegistrationUnderLock( paths.infoPath, @@ -276,12 +465,13 @@ async function retireDaemonRegistration( acquisition, ); result = retirementAfterRemoval(removal, termination); + result = retirePrivateStateIfEligible(owned, acquisition, result); } catch (error) { result = { status: 'retained', reason: 'retirement-unconfirmed', termination, - removedInfo: false, + removedInfo: result.removedInfo, error: normalizeError(error), }; } @@ -339,3 +529,58 @@ async function recordRegistrationWarning( emitDiagnostic({ level: 'warn', phase, data: { error: normalizeError(error) } }); }); } + +async function joinOwnedStartups( + owned: PrivateReplayState | undefined, + timeoutMs: number, +): Promise { + if (!owned || owned.startups.every((startup) => startup.joined)) return true; + let timer: ReturnType | undefined; + try { + return await Promise.race([ + Promise.all(owned.startups.map(({ launch }) => launch.exited)).then(() => true), + new Promise((resolve) => { + timer = setTimeout(() => resolve(false), timeoutMs); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + +function retirePrivateStateIfEligible( + owned: PrivateReplayState | undefined, + acquisition: ProcessLockAcquisition, + result: DaemonRetirementResult, +): DaemonRetirementResult { + if ( + !owned || + (result.status !== 'retired' && !(result.status === 'absent' && owned.startups.length === 0)) + ) + return result; + try { + acquisition.assertHeld(); + const failure = findUnrecoveredRepairCommitFailure(owned.paths.sessionsDir); + if (failure) + return result.status === 'retired' + ? { ...result, removedStateDir: false, repairCommitFailure: failure } + : { + status: 'retained', + reason: 'retirement-unconfirmed', + removedInfo: false, + removedStateDir: false, + repairCommitFailure: failure, + }; + acquisition.assertHeld(); + fs.rmSync(owned.paths.baseDir, { recursive: true, force: true }); + return { ...result, removedStateDir: true }; + } catch (error) { + return { + ...result, + status: 'retained', + reason: 'retirement-unconfirmed', + removedStateDir: false, + error: normalizeError(error), + }; + } +} diff --git a/src/daemon-registration.ts b/src/daemon-registration.ts index ad598afc3c..22f892971b 100644 --- a/src/daemon-registration.ts +++ b/src/daemon-registration.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import { ownerIdentityDiffers, + isProcessPid, ownerIdentityMatches, type OwnerIdentity, } from '@agent-device/host-kit/process'; @@ -53,7 +54,7 @@ function parseRegistration(parsed: { processStartTime?: unknown; }): ParsedRegistration { const pid = parsed.pid; - if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 0) { + if (!isProcessPid(pid)) { return { pid: null, startTime: null }; } return { pid, startTime: readableStartTime(parsed.processStartTime) }; diff --git a/src/daemon/__tests__/daemon-stop.test.ts b/src/daemon/__tests__/daemon-stop.test.ts index 8a294910b6..13ac6851d4 100644 --- a/src/daemon/__tests__/daemon-stop.test.ts +++ b/src/daemon/__tests__/daemon-stop.test.ts @@ -2,15 +2,9 @@ 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(() => ({ - stopDaemonProcess: vi.fn(), - sleep: vi.fn(async () => undefined), -})); - -vi.mock('../../daemon-process.ts', () => ({ stopDaemonProcess: mocks.stopDaemonProcess })); -vi.mock('@agent-device/host-kit/retry', async (importOriginal) => ({ - ...(await importOriginal()), - sleep: mocks.sleep, +const mocks = vi.hoisted(() => ({ stopAndRetireDaemon: vi.fn() })); +vi.mock('../../daemon-registration-owner.ts', () => ({ + stopAndRetireDaemon: mocks.stopAndRetireDaemon, })); import { resolveDaemonPaths } from '../../daemon-resolution.ts'; @@ -45,40 +39,34 @@ test('reports not-running when daemon metadata is absent', async () => { test('retained identity verification is reported as failure without known cleanup', async () => { const paths = createDaemonPaths(); - mocks.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason: 'identity-unverified' }); + mocks.stopAndRetireDaemon.mockResolvedValue(retainedExit('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, - }, - ); + expect(mocks.stopAndRetireDaemon).toHaveBeenCalledWith({ + paths, + observed: { pid: 123, startTime: 'start-time' }, + mode: 'graceful', + termTimeoutMs: 10_000, + killTimeoutMs: 2_000, + }); }); 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.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason: 'missing-start-time' }); + mocks.stopAndRetireDaemon.mockResolvedValue(retainedExit('missing-start-time')); await expect(stopDaemon({ paths })).rejects.toMatchObject({ details: { terminationReason: 'missing-start-time' }, }); - expect(mocks.stopDaemonProcess).toHaveBeenCalledWith( - { pid: 123, startTime: null }, - expect.anything(), + expect(mocks.stopAndRetireDaemon).toHaveBeenCalledWith( + expect.objectContaining({ paths, observed: { pid: 123, startTime: null } }), ); }); 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' }, - }); + mocks.stopAndRetireDaemon.mockResolvedValue(retired('already-exited')); expect(await stopDaemon({ paths: createDaemonPaths() })).toMatchObject({ stopped: false, mode: 'not-running', @@ -86,8 +74,15 @@ test('a previously exited verified lifetime is reported as not-running', async ( }); 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({ + const paths = createDaemonPaths(); + fs.writeFileSync(paths.infoPath, JSON.stringify({ pid: 123 })); + mocks.stopAndRetireDaemon.mockResolvedValue({ + status: 'retained', + reason: 'exit-unconfirmed', + removedInfo: false, + termination: { status: 'not-running' }, + }); + expect(await stopDaemon({ paths })).toMatchObject({ stopped: false, mode: 'not-running', }); @@ -95,32 +90,24 @@ test('an already released pid without start time remains not-running without cle test('confirmed TERM exit preserves graceful report behavior and configured budgets', async () => { const paths = createDaemonPaths(); - mocks.stopDaemonProcess.mockImplementation(async () => { - fs.rmSync(paths.infoPath, { force: true }); - return { status: 'exited', mode: 'graceful', identity: { pid: 123, startTime: 'start-time' } }; - }); + mocks.stopAndRetireDaemon.mockResolvedValue(retired('graceful')); 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', - termTimeoutMs: 11, - killTimeoutMs: 7, - }, - ); + expect(mocks.stopAndRetireDaemon).toHaveBeenCalledWith({ + paths, + observed: { pid: 123, startTime: 'start-time' }, + mode: 'graceful', + termTimeoutMs: 11, + killTimeoutMs: 7, + }); }); test('confirmed KILL exit preserves unknown provider cleanup', async () => { - mocks.stopDaemonProcess.mockResolvedValue({ - status: 'exited', - mode: 'forced', - identity: { pid: 123, startTime: 'start-time' }, - }); + mocks.stopAndRetireDaemon.mockResolvedValue(retired('forced')); expect(await stopDaemon({ paths: createDaemonPaths() })).toMatchObject({ stopped: true, mode: 'forced', @@ -133,10 +120,58 @@ test('confirmed KILL exit preserves unknown provider cleanup', async () => { test.each(['signal-failed', 'exit-timeout'])( '%s cannot become a successful stop', async (reason) => { - mocks.stopDaemonProcess.mockResolvedValue({ status: 'retained', reason }); + mocks.stopAndRetireDaemon.mockResolvedValue(retainedExit(reason)); await expect(stopDaemon({ paths: createDaemonPaths() })).rejects.toMatchObject({ code: 'COMMAND_FAILED', details: { reason: 'daemon_exit_unconfirmed', terminationReason: reason }, }); }, ); + +function retainedExit(reason: string) { + return { + status: 'retained', + reason: 'exit-unconfirmed', + removedInfo: false, + termination: { status: 'retained', reason }, + }; +} + +function retired(mode: string) { + return { + status: 'retired', + removedInfo: true, + termination: { status: 'exited', mode, identity: { pid: 123, startTime: 'start-time' } }, + }; +} + +test.each(['registration-replaced', 'retirement-unconfirmed'])( + '%s after confirmed exit cannot report a completed retirement', + async (reason) => { + const paths = createDaemonPaths(); + mocks.stopAndRetireDaemon.mockResolvedValue({ + ...retired('forced'), + status: 'retained', + reason, + removedInfo: false, + error: { + code: 'UNKNOWN', + message: 'retained', + hint: 'Inspect retained state.', + diagnosticId: 'diag-retire', + logPath: '/retained/daemon.log', + }, + }); + await expect(stopDaemon({ paths })).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + details: { + reason: 'daemon_retirement_unconfirmed', + retirement: { status: 'retained', reason }, + hint: 'Inspect retained state.', + diagnosticId: 'diag-retire', + logPath: '/retained/daemon.log', + }, + }); + expect(fs.existsSync(paths.infoPath)).toBe(true); + }, +); diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index bf5be1697b..c5e53eefcf 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -1,7 +1,6 @@ -import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; -import { stopDaemonProcess } from '../daemon-process.ts'; -import { sleep } from '@agent-device/host-kit/retry'; +import type { DaemonRetirementResult } from '../daemon-registration-owner.ts'; +import type { OwnerIdentity } from '@agent-device/host-kit/process'; import type { DaemonPaths } from '../daemon-resolution.ts'; import { readRegisteredDaemonIdentity } from '../daemon-registration.ts'; @@ -9,7 +8,6 @@ import type { DeviceClaimRecord, ProviderReleaseRecord } from '../daemon-shutdow const DAEMON_STOP_GRACE_TIMEOUT_MS = 10_000; const DAEMON_STOP_KILL_TIMEOUT_MS = 2_000; -const DAEMON_STOP_METADATA_WAIT_MS = 1_000; export type DaemonStopResult = { stopped: boolean; @@ -45,25 +43,19 @@ export async function stopDaemon(params: { }): Promise { const info = readRegisteredDaemonIdentity(params.paths.infoPath); if (!info) return notRunningResult(); - const termination = await stopDaemonProcess(info, { + const { stopAndRetireDaemon } = await import('../daemon-registration-owner.ts'); + const retirement = await stopAndRetireDaemon({ + paths: params.paths, + observed: 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 (termination.status === 'not-running' || termination.mode === 'already-exited') { + if (retirement.status === 'retained' && retirement.termination?.status === 'not-running') return notRunningResult(); - } - if (termination.mode === 'graceful') { - await waitForDaemonMetadataRemoval(params.paths, DAEMON_STOP_METADATA_WAIT_MS); + if (retirement.status !== 'retired') throw daemonRetirementError(info, retirement); + if (retirement.termination.mode === 'already-exited') return notRunningResult(); + if (retirement.termination.mode === 'graceful') { return { stopped: true, mode: 'graceful', @@ -92,6 +84,27 @@ export async function stopDaemon(params: { }; } +function daemonRetirementError( + info: OwnerIdentity, + retirement: Exclude, +): AppError { + const termination = retirement.status === 'retained' ? retirement.termination : undefined; + const failure = termination?.status === 'retained' ? termination : undefined; + const error = retirement.status === 'retained' ? retirement.error : undefined; + const { hint, diagnosticId, logPath } = error ?? {}; + return new AppError('COMMAND_FAILED', 'Daemon retirement could not be confirmed.', { + pid: info.pid, + processStartTime: info.startTime, + reason: failure ? 'daemon_exit_unconfirmed' : 'daemon_retirement_unconfirmed', + terminationReason: failure?.reason, + signal: failure?.signal, + retirement, + hint, + diagnosticId, + logPath, + }); +} + export function readDaemonStopIdentity( infoPath: string, ): { pid: number; processStartTime: string } | null { @@ -100,14 +113,6 @@ export function readDaemonStopIdentity( return { pid: info.pid, processStartTime: info.startTime }; } -async function waitForDaemonMetadataRemoval(paths: DaemonPaths, timeoutMs: number): Promise { - const startedAt = Date.now(); - while (Date.now() - startedAt < timeoutMs) { - if (!fs.existsSync(paths.infoPath) && !fs.existsSync(paths.lockPath)) return; - await sleep(25); - } -} - function notRunningResult(): DaemonStopResult { return { stopped: false, diff --git a/src/session-repair-tombstone.test.ts b/src/session-repair-tombstone.test.ts new file mode 100644 index 0000000000..0cceeb3fa1 --- /dev/null +++ b/src/session-repair-tombstone.test.ts @@ -0,0 +1,75 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { mkdtempForTestSync } from './__tests__/test-utils/tmp-dir.ts'; +import { + findUnrecoveredRepairCommitFailure, + readRepairTombstoneFile, + resolveRepairTombstonePath, +} from './session-repair-tombstone.ts'; + +function writeRepairTombstone(raw: string) { + const sessionsDir = mkdtempForTestSync('agent-device-repair-tombstone-'); + const sessionDir = path.join(sessionsDir, 'session-a'); + fs.mkdirSync(sessionDir); + const tombstonePath = resolveRepairTombstonePath(sessionDir); + fs.writeFileSync(tombstonePath, raw); + return { sessionsDir, tombstonePath }; +} + +function rawRepairTombstone(fields: { reapedAt?: string; sourcePath?: string }): string { + const entries = [ + '"owner":"session-a"', + `"expiresAt":${Date.now() + 60_000}`, + '"commitFailure":{"code":"COMMAND_FAILED","message":"write failed"}', + ]; + if (fields.reapedAt !== undefined) entries.push(`"reapedAt":${fields.reapedAt}`); + if (fields.sourcePath !== undefined) entries.push(`"sourcePath":${fields.sourcePath}`); + return `{${entries.join(',')}}\n`; +} + +test.each([ + ['with sourcePath', { sourcePath: '/flows/login.ad' }], + ['without sourcePath', undefined], +])('cleanup accepts finite reap timestamps %s', (_label, sourcePath) => { + const reapedAt = Date.now(); + const tombstone = { + owner: 'session-a', + reapedAt, + expiresAt: reapedAt + 60_000, + ...(sourcePath ?? {}), + commitFailure: { code: 'COMMAND_FAILED', message: 'write failed' }, + }; + const { sessionsDir, tombstonePath } = writeRepairTombstone(`${JSON.stringify(tombstone)}\n`); + + expect(readRepairTombstoneFile(tombstonePath)).toEqual(tombstone); + expect(findUnrecoveredRepairCommitFailure(sessionsDir)).toEqual({ + sessionName: 'session-a', + tombstone, + }); +}); + +test.each([ + ['missing reapedAt', {}], + ['string reapedAt', { reapedAt: JSON.stringify('not-a-timestamp') }], + ['non-finite reapedAt', { reapedAt: '1e400' }], + ['numeric sourcePath', { reapedAt: String(Date.now()), sourcePath: '7' }], + ['null sourcePath', { reapedAt: String(Date.now()), sourcePath: 'null' }], +])('malformed %s metadata is rejected and remains on disk', (_label, fields) => { + const raw = rawRepairTombstone(fields); + const { sessionsDir, tombstonePath } = writeRepairTombstone(raw); + + expect(readRepairTombstoneFile(tombstonePath)).toBeUndefined(); + assert.throws( + () => findUnrecoveredRepairCommitFailure(sessionsDir), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, 'repair_evidence_invalid'); + assert.equal(error.details?.path, tombstonePath); + return true; + }, + ); + expect(fs.readFileSync(tombstonePath, 'utf8')).toBe(raw); +}); diff --git a/src/session-repair-tombstone.ts b/src/session-repair-tombstone.ts index dedf854736..638942782c 100644 --- a/src/session-repair-tombstone.ts +++ b/src/session-repair-tombstone.ts @@ -66,12 +66,7 @@ function readRepairTombstone(tombstonePath: string): RepairSessionTombstone | un function parseRepairTombstone(raw: string, tombstonePath: string): RepairSessionTombstone { try { const parsed = JSON.parse(raw) as RepairSessionTombstone; - if ( - !Number.isFinite(parsed?.expiresAt) || - typeof parsed?.owner !== 'string' || - !validRepairCommitFailure(parsed.commitFailure) - ) - throw new Error('Invalid repair tombstone fields'); + if (!validRepairTombstone(parsed)) throw new Error('Invalid repair tombstone fields'); return parsed; } catch (error) { throw new AppError( @@ -83,6 +78,17 @@ function parseRepairTombstone(raw: string, tombstonePath: string): RepairSession } } +function validRepairTombstone(value: unknown): value is RepairSessionTombstone { + const tombstone = value as RepairSessionTombstone | null; + return ( + Number.isFinite(tombstone?.reapedAt) && + Number.isFinite(tombstone?.expiresAt) && + typeof tombstone?.owner === 'string' && + (tombstone.sourcePath === undefined || typeof tombstone.sourcePath === 'string') && + validRepairCommitFailure(tombstone.commitFailure) + ); +} + function validRepairCommitFailure(value: unknown): boolean { const failure = value as RepairSessionTombstone['commitFailure'] | null; return ( diff --git a/test/integration/provider-scenarios/managed-runtime-automation.fixtures.ts b/test/integration/provider-scenarios/managed-runtime-automation.fixtures.ts index 1bab9f5722..1efbe07592 100644 --- a/test/integration/provider-scenarios/managed-runtime-automation.fixtures.ts +++ b/test/integration/provider-scenarios/managed-runtime-automation.fixtures.ts @@ -1,5 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; +import { vi } from 'vitest'; import { runCmd } from '@agent-device/host-kit/command'; import { Deadline } from '@agent-device/host-kit/retry'; import { AppError } from '@agent-device/kernel/errors'; @@ -159,6 +160,7 @@ export async function withManagedAdbFixture( ].join('\n'), ); fs.chmodSync(adbPath, 0o755); + const restoreGroupSignals = refuseVanishedFixtureGroups(); const previousPath = process.env.PATH; process.env.PATH = `${root}${path.delimiter}${previousPath ?? ''}`; try { @@ -174,7 +176,18 @@ export async function withManagedAdbFixture( : [], }); } finally { + restoreGroupSignals(); if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; } } + +/** The fake adb has no descendants; an exited leader leaves no group to signal. */ +function refuseVanishedFixtureGroups(): () => void { + const signal = process.kill.bind(process); + const spy = vi.spyOn(process, 'kill').mockImplementation((pid, kind = 'SIGTERM') => { + if (pid < 0) signal(-pid, 0); + return signal(pid, kind); + }); + return () => spy.mockRestore(); +} diff --git a/test/integration/support/daemon-test-cleanup.test.ts b/test/integration/support/daemon-test-cleanup.test.ts new file mode 100644 index 0000000000..35e64b8768 --- /dev/null +++ b/test/integration/support/daemon-test-cleanup.test.ts @@ -0,0 +1,176 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { test, vi } from 'vitest'; +import { + isProcessAlive, + readHostProcessIdentityObservations, +} from '@agent-device/host-kit/process'; +import { cleanupDaemonTestState } from './daemon-test-cleanup.ts'; +import { resolveDaemonPaths } from '../../../src/daemon-resolution.ts'; +import { stopDaemonProcess } from '../../../src/daemon-process.ts'; +import { mkdtempForTestSync } from '../../../src/__tests__/test-utils/tmp-dir.ts'; +import { + spawnRegisteredDaemonFixture, + waitForRegisteredDaemonFixture, + finishRegisteredDaemonFixture, +} from '../../../src/__tests__/test-utils/registered-daemon-fixture.ts'; + +const fields = { + httpPort: 4210, + token: 'fixture', + version: 'test', + codeOrigin: 'checkout' as const, + codeSignature: 'fixture', +}; + +test.each(['string', 'out-of-range'])( + 'malformed %s registration retains the directory and live daemon', + async (kind) => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-invalid-registration-')); + const child = spawnRegisteredDaemonFixture(paths, fields, undefined); + const warnings = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const info = await waitForRegisteredDaemonFixture(paths, child); + const malformed = JSON.stringify({ + ...info, + pid: kind === 'string' ? String(info.pid) : 2_147_483_648, + }); + fs.writeFileSync(paths.infoPath, malformed); + await cleanupDaemonTestState(paths.baseDir, null); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), malformed); + assert.equal(isProcessAlive(child.pid), true); + assert.equal(warnings.mock.calls.length, 1); + } finally { + warnings.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + } + }, +); + +test.each([false, true])( + 'cleanup joins a registered successor when the old observation lacks birth proof: %s', + async (missingBirth) => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-replaced-registration-')); + const original = spawnRegisteredDaemonFixture(paths, fields, undefined); + try { + const observed = await waitForRegisteredDaemonFixture(paths, original); + const stopped = await stopDaemonProcess( + { pid: original.pid, startTime: observed.processStartTime ?? null }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 1_000 }, + ); + assert.equal(stopped.status, 'exited'); + await original.exited; + const replacement = spawnRegisteredDaemonFixture(paths, fields, undefined); + await waitForRegisteredDaemonFixture(paths, replacement); + await cleanupDaemonTestState(paths.baseDir, { + ...observed, + processStartTime: missingBirth ? undefined : observed.processStartTime, + }); + assert.ok( + !isProcessAlive(replacement.pid) || + readHostProcessIdentityObservations([replacement.pid]) + .get(replacement.pid) + ?.state.startsWith('Z'), + 'the registered replacement must be dead before cleanup returns', + ); + await replacement.exited; + assert.equal(isProcessAlive(replacement.pid), false); + assert.equal(fs.existsSync(paths.baseDir), false); + } finally { + await finishRegisteredDaemonFixture(paths.baseDir); + } + }, +); + +test.each(['invalid-json', 'ownerless'])( + 'cleanup joins its observed child and retains %s metadata', + async (kind) => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-corrupt-observed-')); + const child = spawnRegisteredDaemonFixture(paths, fields, undefined); + const warnings = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const observed = await waitForRegisteredDaemonFixture(paths, child); + const corrupt = kind === 'invalid-json' ? '{invalid' : '{"pid": "unknown"}'; + fs.writeFileSync(paths.infoPath, corrupt); + await cleanupDaemonTestState(paths.baseDir, observed); + assert.ok( + !isProcessAlive(child.pid) || + readHostProcessIdentityObservations([child.pid]).get(child.pid)?.state.startsWith('Z'), + 'the observed child must be terminated before cleanup returns', + ); + await child.exited; + assert.equal(isProcessAlive(child.pid), false); + assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), corrupt); + assert.equal(warnings.mock.calls.length, 1); + } finally { + warnings.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + } + }, +); + +test('cleanup retains an unpublished successor holding the registration lock', async () => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-unpublished-successor-')); + const original = spawnRegisteredDaemonFixture(paths, fields, undefined); + const warnings = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const observed = await waitForRegisteredDaemonFixture(paths, original); + assert.equal( + ( + await stopDaemonProcess( + { pid: original.pid, startTime: observed.processStartTime ?? null }, + { mode: 'force', termTimeoutMs: 0, killTimeoutMs: 1_000 }, + ) + ).status, + 'exited', + ); + await original.exited; + fs.rmSync(paths.infoPath); + fs.rmSync(paths.baseDir + '/registration-held'); + fs.writeFileSync(paths.baseDir + '/defer-publication', 'wait'); + const successor = spawnRegisteredDaemonFixture(paths, fields, undefined); + await vi.waitFor( + () => assert.equal(fs.existsSync(paths.baseDir + '/registration-held'), true), + { timeout: 4_000, interval: 10 }, + ); + await cleanupDaemonTestState(paths.baseDir, observed); + assert.equal(fs.existsSync(paths.baseDir), true); + assert.equal(fs.existsSync(paths.infoPath), false); + assert.equal(isProcessAlive(successor.pid), true); + assert.equal(warnings.mock.calls.length, 1); + } finally { + warnings.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + } +}); + +test.for(['same-pid', 'different-pid'] as const)( + 'registered birth proof supersedes only a %s unproven observation', + async (kind) => { + const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-pid-observation-')); + const otherPaths = resolveDaemonPaths(mkdtempForTestSync('daemon-test-other-observation-')); + const child = spawnRegisteredDaemonFixture(paths, fields, undefined); + const other = spawnRegisteredDaemonFixture(otherPaths, fields, undefined); + const warnings = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await waitForRegisteredDaemonFixture(paths, child); + await waitForRegisteredDaemonFixture(otherPaths, other); + await cleanupDaemonTestState(paths.baseDir, { + pid: kind === 'same-pid' ? child.pid : other.pid, + }); + assert.ok( + !isProcessAlive(child.pid) || + readHostProcessIdentityObservations([child.pid]).get(child.pid)?.state.startsWith('Z'), + 'the birth-proven registered child must be terminated before cleanup returns', + ); + await child.exited; + assert.equal(fs.existsSync(paths.baseDir), kind === 'different-pid'); + assert.equal(warnings.mock.calls.length, kind === 'different-pid' ? 1 : 0); + assert.equal(isProcessAlive(other.pid), true); + } finally { + warnings.mockRestore(); + await finishRegisteredDaemonFixture(paths.baseDir); + await finishRegisteredDaemonFixture(otherPaths.baseDir); + } + }, +); diff --git a/test/integration/support/daemon-test-cleanup.ts b/test/integration/support/daemon-test-cleanup.ts index 371159c521..0f1febc50f 100644 --- a/test/integration/support/daemon-test-cleanup.ts +++ b/test/integration/support/daemon-test-cleanup.ts @@ -1,7 +1,17 @@ import fs from 'node:fs'; -import path from 'node:path'; import { normalizeError } from '@agent-device/kernel/errors'; +import { tryAcquireProcessLock } from '@agent-device/host-kit/file'; +import { + readCurrentOwnerIdentity, + ownerIdentityMatches, + type OwnerIdentity, +} from '@agent-device/host-kit/process'; import { stopDaemonProcess } from '../../../src/daemon-process.ts'; +import { resolveDaemonPaths } from '../../../src/daemon-resolution.ts'; +import { + readRegisteredDaemonIdentity, + readRegisteredDaemonOwnership, +} from '../../../src/daemon-registration.ts'; type TestDaemonIdentity = { pid: number; processStartTime?: string }; @@ -11,29 +21,64 @@ export async function cleanupDaemonTestState( 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; + const paths = resolveDaemonPaths(stateDir); + const identities: OwnerIdentity[] = observed + ? [{ pid: observed.pid, startTime: observed.processStartTime ?? null }] + : []; + let registrationFailure: unknown; + try { + const registered = readIdentity(paths.infoPath); + if (registered) identities.push(registered); + } catch (error) { + registrationFailure = error; + } + const confirmed: OwnerIdentity[] = []; + const retained: OwnerIdentity[] = []; + for (const identity of identities) { + const termination = await stopDaemonProcess(identity, { + mode: 'graceful', + termTimeoutMs: 1_500, + killTimeoutMs: 1_500, + }); + if (termination.status === 'exited') confirmed.push(identity); + else if (termination.status === 'retained') retained.push(identity); + } + if (registrationFailure) throw registrationFailure; + if ( + retained.some( + (identity) => + Boolean(identity.startTime?.trim()) || + !confirmed.some((proof) => proof.pid === identity.pid), + ) || + confirmed.length === 0 + ) + throw new Error('Daemon termination could not be confirmed'); + const attempt = tryAcquireProcessLock({ + lockDirPath: paths.lockPath, + owner: { ...readCurrentOwnerIdentity(), acquiredAtMs: Date.now() }, + description: 'daemon test cleanup', + }); + if (attempt.status !== 'acquired') + throw new Error('Daemon registration is held or unproven during test cleanup'); + const { acquisition } = attempt; + try { + acquisition.assertHeld(); + const current = readIdentity(paths.infoPath); + if (current && !confirmed.some((identity) => ownerIdentityMatches(identity, current))) + throw new Error('Daemon registration changed during test cleanup'); + acquisition.assertHeld(); + fs.rmSync(stateDir, { recursive: true, force: true }); + } finally { + await acquisition.release(); } - 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; - } +function readIdentity(infoPath: string): OwnerIdentity | null { + if (readRegisteredDaemonOwnership(infoPath, null).state === 'absent') return null; + const identity = readRegisteredDaemonIdentity(infoPath); + if (!identity) throw new Error('Daemon registration identity is invalid or unreadable'); + return identity; } diff --git a/test/wire-compat/ledger.json b/test/wire-compat/ledger.json index c6cf7cbb49..dd39538cd5 100644 --- a/test/wire-compat/ledger.json +++ b/test/wire-compat/ledger.json @@ -67,7 +67,7 @@ "src/daemon-client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:888246763c48670e7da893054d025744654f8715c3b4906312617a2b5028316b", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:3bac36fa97090b273afe1128103e1bf476d05441fd9de41317f1b78775b88304", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealthLink": "sha256:7702598468b4c82b4ffa93064cd6bdfec938c1c527f7e6007c0584f3884a6eea", - "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:a483fbdd845db39ffea596061ff04e9db4acdafd20b593dfed74e7c82a93e522", + "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:f40cc2f5bfb8add78f99b11db7842bc244735786aa390d10a38ef025e16dc53a", "src/daemon-client/daemon-client-transport.ts#readHealthLink": "sha256:f56404b94d73da57de7248719c9136b760d2be8b4a53cde5315a2311b088425b", "src/daemon-client/daemon-client-transport.ts#readHealthPayload": "sha256:4e85ffc3e35e02379c393e9312344757e003cf1f0ad9eb8d1f77d90c81c861f1", "src/daemon-client/daemon-client-transport.ts#readRemoteDaemonHealth": "sha256:df174d1ccf73851ca58bd75533fb4660424003b98bd9e41fe942cf3ab3698e5f", @@ -431,8 +431,8 @@ }, { "declaration": "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth", - "digest": "sha256:a483fbdd845db39ffea596061ff04e9db4acdafd20b593dfed74e7c82a93e522", - "rationale": "A restart retry must tell a probe that ran out of time from one that failed, so the client can report the RPC deadline instead of 'Remote daemon is unavailable'. `timedOut` is client-local: the prober sets it on its own timeout and abort paths and never reads it from a /health payload. The #3178 caller signal joins the probe's own timeout in-process (AbortSignal.any) and never enters the /health request or the accepted payload fields, so a released daemon or proxy is probed and parsed exactly as before." + "digest": "sha256:f40cc2f5bfb8add78f99b11db7842bc244735786aa390d10a38ef025e16dc53a", + "rationale": "#3116 includes requester loading and response completion in the existing health-probe budget. `timedOut` remains client-local; it is never read from a /health payload. Request routes, authentication, accepted fields and protocol-version admission are unchanged, so released daemons and proxies still send payloads this client parses correctly. This is an implementation-only timing change under ADR 0006." } ] } diff --git a/vitest.config.ts b/vitest.config.ts index 5e42d9810e..1bbf9259af 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -182,6 +182,8 @@ export default defineConfig({ // decisions over fixture state-dir listings, so they need no daemon, // device, or subprocess. 'test/integration/support/daemon-leak-model.test.ts', + // Cleanup uses real detached registration owners and process-exit proof; no device is needed. + 'test/integration/support/daemon-test-cleanup.test.ts', // The Android failed-step evidence reader: it replays adb output through the probe // seam, so the crash/process/activity selectors need no emulator to be pinned. 'test/integration/android-emulator-e2e/device-evidence.test.ts', diff --git a/website/docs/docs/installation.md b/website/docs/docs/installation.md index eca4ff398d..78776b4ea4 100644 --- a/website/docs/docs/installation.md +++ b/website/docs/docs/installation.md @@ -115,10 +115,23 @@ vega device list - A runner startup failure is typed, not prose: `error.details.reason` is one of `signing_no_development_team`, `signing_provisioning_profile_missing`, `bundle_identifier_already_registered`, `signing_unspecified`, `devtools_security_developer_mode_disabled` (the Mac's `DevToolsSecurity` setting, which says nothing about the device's Developer Mode toggle), `device_developer_mode_disabled`, `device_developer_disk_image_unavailable`, or `build_failed_unclassified` when nothing proved a cause. Branch on `details.reason` and follow `hint`; the code stays `COMMAND_FAILED` for every reason. - The two `device_*` reasons come from the iPhone itself, read over `xcrun devicectl device info details` before the runner builds: `developerModeStatus` for the Settings toggle and `ddiServicesAvailable` for the developer disk image. They are reported apart on purpose. A phone with Developer Mode off cannot serve its disk image either, so it gets the toggle reason; a phone with the toggle on and only the image down gets the disk-image reason, which is a device-support install that has not finished rather than a setting anyone turned off. - If device setup is slow, keep the device connected and inspect daemon diagnostics after retrying. -- If daemon startup reports stale metadata, remove stale files and retry: - - `/daemon.json` - - `/daemon.lock` - - default state dir is `~/.agent-device` for packaged installs; source checkouts default to a worktree-scoped dir under `~/.agent-device/dev/` unless `AGENT_DEVICE_STATE_DIR` or `--state-dir` is set - - `agent-device session state-dir` prints the resolved state dir without starting the daemon - - after pulling the worktree-scoped daemon change in a source checkout, stop any legacy default daemon once with `AGENT_DEVICE_STATE_DIR=~/.agent-device pnpm clean:daemon` - - worktree-scoped state dirs outlive deleted worktrees; `pnpm clean:daemon --prune-dev` removes dirs under `~/.agent-device/dev/` with no live daemon and no activity for 14 days (one line printed per removed dir) + +## Daemon startup and upgrades + +If daemon startup fails, retry with `--debug` and inspect the retained state and diagnostics. `agent-device session state-dir` prints the resolved directory without starting a daemon. + +Before upgrading across the daemon lock change, stop every older client and daemon using that directory with their original CLI. Prevent older versions from returning while the upgraded version runs. Use a single deployed version or separate environments for concurrent installations. + +If you used a source checkout before worktree-specific state directories, stop its daemon in `~/.agent-device` from that older checkout: + +```sh +AGENT_DEVICE_STATE_DIR="$HOME/.agent-device" pnpm clean:daemon +``` + +The upgraded checkout defaults to a different directory, so its plain `pnpm clean:daemon` does not target that legacy daemon. + +Startup refuses legacy lock files and unverified ownership. Confirm every user of the state directory stopped before manual recovery; removing `daemon.json` or `daemon.lock` alone is not a safe reset. + +Packaged installs default to `~/.agent-device`; source checkouts use a worktree directory under `~/.agent-device/dev/`. `AGENT_DEVICE_STATE_DIR` or `--state-dir` overrides either default. + +For source checkouts, `pnpm clean:daemon --prune-dev` first stops the invoking checkout's daemon, even when its state directory has recent activity. It then selects development directories with no activity for 14 days, using the newest mtime of the directory and its immediate children. It retires only registration it can confirm abandoned. Directories, session artifacts and logs remain. From 6569be1660e528d1b55eec1a4889938cdc8baf78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 15:43:55 +0200 Subject: [PATCH 2/6] refactor: share daemon registration identity proof --- src/daemon-client/daemon-client-lifecycle.ts | 10 +++++----- src/daemon-client/daemon-client-transport.ts | 4 +++- src/daemon-registration-owner.ts | 4 ++-- src/daemon-registration.ts | 13 +++++++++++++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index 88249fa579..acef7bbf85 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -1,5 +1,6 @@ import { attachActiveSessionAddressHint } from './daemon-client-address-hints.ts'; import fs from 'node:fs'; +import { processLockHoldsDaemonIdentity } from '../daemon-registration.ts'; import net from 'node:net'; import { AppError, normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; import { readReplayDivergenceResume } from '@agent-device/ad-replay/divergence'; @@ -233,11 +234,10 @@ function registrationAllowsDaemonObservation( ): boolean { return ( inspection.state === 'absent' || - (inspection.state === 'held' && - inspection.owner.pid === info.pid && - inspection.owner.startTime !== null && - inspection.owner.startTime.trim().length > 0 && - inspection.owner.startTime === info.processStartTime) + processLockHoldsDaemonIdentity(inspection, { + pid: info.pid, + startTime: info.processStartTime ?? null, + }) ); } diff --git a/src/daemon-client/daemon-client-transport.ts b/src/daemon-client/daemon-client-transport.ts index 3f54b59615..c99860e5a8 100644 --- a/src/daemon-client/daemon-client-transport.ts +++ b/src/daemon-client/daemon-client-transport.ts @@ -274,7 +274,9 @@ async function readDaemonHttpHealth( }), ]); if (!transport || signal.aborted || performance.now() >= deadline) - return { reachable: false, timedOut: healthProbeExpired(timeoutSignal, deadline) }; + return healthProbeExpired(timeoutSignal, deadline) + ? { reachable: false, timedOut: true } + : { reachable: false }; return await new Promise((resolve) => { const headers = info.baseUrl ? buildDaemonHttpAuthHeaders(info.token) : {}; const unreachable = (): RemoteDaemonHealth => diff --git a/src/daemon-registration-owner.ts b/src/daemon-registration-owner.ts index 2a329ffe8e..37155abdaf 100644 --- a/src/daemon-registration-owner.ts +++ b/src/daemon-registration-owner.ts @@ -10,7 +10,6 @@ import { } from './daemon-process.ts'; import { readCurrentOwnerIdentity, - ownerIdentityMatches, readProcessStartTime, type OwnerIdentity, } from '@agent-device/host-kit/process'; @@ -31,6 +30,7 @@ import { } from './daemon-resolution.ts'; import { findUnrecoveredRepairCommitFailure } from './session-repair-tombstone.ts'; import { + processLockHoldsDaemonIdentity, readRegisteredDaemonOwnership, readRegisteredDaemonIdentity, type RegisteredDaemonOwnership, @@ -261,7 +261,7 @@ function recoverStartupBirth(paths: DaemonPaths, startup: OwnedStartup): void { const registration = readRegisteredDaemonIdentity(paths.infoPath); if (registration?.pid !== startup.launch.pid || !registration.startTime) return; const lock = inspectProcessLock(paths.lockPath); - if (lock.state !== 'held' || !ownerIdentityMatches(lock.owner, registration)) return; + if (!processLockHoldsDaemonIdentity(lock, registration)) return; startup.startTime = registration.startTime; } diff --git a/src/daemon-registration.ts b/src/daemon-registration.ts index 22f892971b..0e677e45a4 100644 --- a/src/daemon-registration.ts +++ b/src/daemon-registration.ts @@ -1,4 +1,5 @@ import fs from 'node:fs'; +import type { ProcessLockInspection } from '@agent-device/host-kit/file'; import { ownerIdentityDiffers, isProcessPid, @@ -18,6 +19,18 @@ export function readRegisteredDaemonIdentity(infoPath: string): OwnerIdentity | return record.status === 'registered' ? record.identity : null; } +export function processLockHoldsDaemonIdentity( + inspection: ProcessLockInspection, + identity: OwnerIdentity, +): boolean { + return ( + inspection.state === 'held' && + typeof identity.startTime === 'string' && + identity.startTime.trim().length > 0 && + ownerIdentityMatches(inspection.owner, identity) + ); +} + /** The raw record's identity. A pid of `null` is a file that names no owner, not owner zero. */ type ParsedRegistration = { pid: number | null; startTime: string | null }; From 7da6b098f493ae20c965af16e26578de5f8ea98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 15:43:55 +0200 Subject: [PATCH 3/6] chore(gates): acknowledge composed health probe timing --- test/wire-compat/ledger.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/wire-compat/ledger.json b/test/wire-compat/ledger.json index dd39538cd5..632f6649d0 100644 --- a/test/wire-compat/ledger.json +++ b/test/wire-compat/ledger.json @@ -67,7 +67,7 @@ "src/daemon-client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:888246763c48670e7da893054d025744654f8715c3b4906312617a2b5028316b", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:3bac36fa97090b273afe1128103e1bf476d05441fd9de41317f1b78775b88304", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealthLink": "sha256:7702598468b4c82b4ffa93064cd6bdfec938c1c527f7e6007c0584f3884a6eea", - "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:f40cc2f5bfb8add78f99b11db7842bc244735786aa390d10a38ef025e16dc53a", + "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:3af10adcb49672910d1a0f7a9cdc095c108adf64ff3487c51ddd1be1495b6e43", "src/daemon-client/daemon-client-transport.ts#readHealthLink": "sha256:f56404b94d73da57de7248719c9136b760d2be8b4a53cde5315a2311b088425b", "src/daemon-client/daemon-client-transport.ts#readHealthPayload": "sha256:4e85ffc3e35e02379c393e9312344757e003cf1f0ad9eb8d1f77d90c81c861f1", "src/daemon-client/daemon-client-transport.ts#readRemoteDaemonHealth": "sha256:df174d1ccf73851ca58bd75533fb4660424003b98bd9e41fe942cf3ab3698e5f", @@ -207,7 +207,7 @@ { "declaration": "src/remote/daemon-artifacts.ts#DownloadRemoteArtifactParams", "digest": "sha256:3ec63be2dbdb542e30b19d1500bc16874607823afb7ae392e92b0b535865859e", - "rationale": "#2246 adds the optional isDirectory field. It is pure client-local state — never serialized, never sent to the daemon — that tells the CLIENT'S OWN download logic to extract a tar body instead of writing it verbatim; the GET /artifacts/:id request and response framing are unchanged. Every existing call site (screenshot, recording) omits it and keeps writing a single file exactly as before." + "rationale": "#2246 adds the optional isDirectory field. It is pure client-local state \u2014 never serialized, never sent to the daemon \u2014 that tells the CLIENT'S OWN download logic to extract a tar body instead of writing it verbatim; the GET /artifacts/:id request and response framing are unchanged. Every existing call site (screenshot, recording) omits it and keeps writing a single file exactly as before." }, { "declaration": "src/remote/daemon-artifacts.ts#downloadRemoteArtifact", @@ -222,7 +222,7 @@ { "declaration": "src/daemon-client/daemon-client-rpc.ts#buildHttpRpcPayload", "digest": "sha256:caddf184cd57a41d9910c414bba72ce5a0503e22a2a4ef3a47f95fed67037127", - "rationale": "#2110 added the optional providerApp parameter to lease allocation; #2494 extends the same additive projection to the full provider-allocation flag set the lease-lifecycle provider reads in prepareSession — device selection (platform, device), the app/os, the session-naming fields, and the configured device-feature and AWS knobs — so a fresh remote allocation reaches BrowserStack/AWS with a named, configured session instead of failing on a missing --device/--platform or landing in Untitled Project/Build. Producer and consumer both read one shared projection (readLeaseAllocateProviderFlags). Every field is optional and forwarded only when the caller supplied it: a released protocol-2 daemon ignores unknown JSON-RPC parameters, and a request carrying none of these retains the previous payload byte-for-byte." + "rationale": "#2110 added the optional providerApp parameter to lease allocation; #2494 extends the same additive projection to the full provider-allocation flag set the lease-lifecycle provider reads in prepareSession \u2014 device selection (platform, device), the app/os, the session-naming fields, and the configured device-feature and AWS knobs \u2014 so a fresh remote allocation reaches BrowserStack/AWS with a named, configured session instead of failing on a missing --device/--platform or landing in Untitled Project/Build. Producer and consumer both read one shared projection (readLeaseAllocateProviderFlags). Every field is optional and forwarded only when the caller supplied it: a released protocol-2 daemon ignores unknown JSON-RPC parameters, and a request carrying none of these retains the previous payload byte-for-byte." }, { "declaration": "src/daemon/server/http-server.ts#toLeaseDaemonRequest", @@ -282,7 +282,7 @@ { "declaration": "src/daemon/server/http-server.ts#authorizeAuxiliaryHttpRequest", "digest": "sha256:1c323f97c3cefec8f95633b6abd127342bf75494bfb109d1cc6eadf3ceacaeae", - "rationale": "#2198 context: adds one optional field to the value this DAEMON-INTERNAL gate hands its own route handlers — sessionNamespace, the caller's tenant plus whether the daemon partitioned that caller's session names. Nothing about it is serialized: no request header is read that was not read before, no response field is added, and a refusal keeps the same 401 {ok:false,error,code} REST body sendRestJsonError already sent. What it enables is a refusal DROPPED, never one added — an unattested (client-declared) tenant no longer has the : prefix rule applied to it, because scopeRequestSession never applied that prefix to its sessions either. A released peer that now gets 200 where it got 401 is getting the ndjson record it asked for and already parses." + "rationale": "#2198 context: adds one optional field to the value this DAEMON-INTERNAL gate hands its own route handlers \u2014 sessionNamespace, the caller's tenant plus whether the daemon partitioned that caller's session names. Nothing about it is serialized: no request header is read that was not read before, no response field is added, and a refusal keeps the same 401 {ok:false,error,code} REST body sendRestJsonError already sent. What it enables is a refusal DROPPED, never one added \u2014 an unattested (client-declared) tenant no longer has the : prefix rule applied to it, because scopeRequestSession never applied that prefix to its sessions either. A released peer that now gets 200 where it got 401 is getting the ndjson record it asked for and already parses." }, { "declaration": "src/daemon/request-diagnostics-http.ts#RequestDiagnosticsHttpAuthorizer", @@ -327,7 +327,7 @@ { "declaration": "src/remote/upload-client.ts#tryDirectUploadWithResume", "digest": "sha256:67596a2ff2b8a5402566c3b7b99a6fa1baaf9d5de3d647bf35398185cc45eea6", - "rationale": "#2946 stops a canceled upload from re-preflighting and lifts the retry leg into its own function. The request sequence a daemon sees — preflight, direct PUT, finalize, or the legacy fallback after a retryable failure — is unchanged." + "rationale": "#2946 stops a canceled upload from re-preflighting and lifts the retry leg into its own function. The request sequence a daemon sees \u2014 preflight, direct PUT, finalize, or the legacy fallback after a retryable failure \u2014 is unchanged." }, { "declaration": "src/remote/upload-client.ts#finalizeDirectUpload", @@ -431,8 +431,8 @@ }, { "declaration": "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth", - "digest": "sha256:f40cc2f5bfb8add78f99b11db7842bc244735786aa390d10a38ef025e16dc53a", - "rationale": "#3116 includes requester loading and response completion in the existing health-probe budget. `timedOut` remains client-local; it is never read from a /health payload. Request routes, authentication, accepted fields and protocol-version admission are unchanged, so released daemons and proxies still send payloads this client parses correctly. This is an implementation-only timing change under ADR 0006." + "digest": "sha256:3af10adcb49672910d1a0f7a9cdc095c108adf64ff3487c51ddd1be1495b6e43", + "rationale": "The health-probe budget includes requester loading and response completion. The caller AbortSignal joins the probe timer only in-process; caller cancellation remains distinct from probe expiry. No signal, timedOut flag, request route, auth header or accepted health payload field changes on the wire, so released daemons and proxies parse and respond as before (ADR 0006)." } ] } From 3dc1fea619d2915653516aaac7ba0329d14aaf3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 15:50:14 +0200 Subject: [PATCH 4/6] refactor: keep health probe outcome classification consistent --- src/daemon-client/daemon-client-transport.ts | 30 +++++++++++--------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/daemon-client/daemon-client-transport.ts b/src/daemon-client/daemon-client-transport.ts index c99860e5a8..0bf96b8473 100644 --- a/src/daemon-client/daemon-client-transport.ts +++ b/src/daemon-client/daemon-client-transport.ts @@ -247,16 +247,20 @@ export async function readRemoteDaemonHealth( return health; } +function daemonHealthEndpoint(info: DaemonInfo): string | null { + return info.baseUrl + ? buildDaemonHttpUrl(info.baseUrl, 'health') + : info.httpPort + ? `http://127.0.0.1:${info.httpPort}/health` + : null; +} + async function readDaemonHttpHealth( info: DaemonInfo, probeTimeoutMs?: number, callerSignal?: AbortSignal, ): Promise { - const endpoint = info.baseUrl - ? buildDaemonHttpUrl(info.baseUrl, 'health') - : info.httpPort - ? `http://127.0.0.1:${info.httpPort}/health` - : null; + const endpoint = daemonHealthEndpoint(info); if (!endpoint) return { reachable: false }; const url = new URL(endpoint); const timeoutMs = Math.min( @@ -273,16 +277,10 @@ async function readDaemonHttpHealth( signal.addEventListener('abort', () => resolve(null), { once: true }); }), ]); - if (!transport || signal.aborted || performance.now() >= deadline) - return healthProbeExpired(timeoutSignal, deadline) - ? { reachable: false, timedOut: true } - : { reachable: false }; + const unreachable = (): RemoteDaemonHealth => unreachableDaemonHealth(timeoutSignal, deadline); + if (!transport || healthProbeExpired(signal, deadline)) return unreachable(); return await new Promise((resolve) => { const headers = info.baseUrl ? buildDaemonHttpAuthHeaders(info.token) : {}; - const unreachable = (): RemoteDaemonHealth => - healthProbeExpired(timeoutSignal, deadline) - ? { reachable: false, timedOut: true } - : { reachable: false }; const req = transport.request( { protocol: url.protocol, @@ -323,6 +321,12 @@ async function readDaemonHttpHealth( }); } +function unreachableDaemonHealth(timeoutSignal: AbortSignal, deadline: number): RemoteDaemonHealth { + return healthProbeExpired(timeoutSignal, deadline) + ? { reachable: false, timedOut: true } + : { reachable: false }; +} + function healthProbeExpired(signal: AbortSignal, deadline: number): boolean { return signal.aborted || performance.now() >= deadline; } From 951c22aa40d29143c367bdc18df905ac4f9c01fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 15:50:14 +0200 Subject: [PATCH 5/6] chore(gates): pin composed health probe declaration --- test/wire-compat/ledger.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/wire-compat/ledger.json b/test/wire-compat/ledger.json index 632f6649d0..599923237e 100644 --- a/test/wire-compat/ledger.json +++ b/test/wire-compat/ledger.json @@ -67,7 +67,7 @@ "src/daemon-client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:888246763c48670e7da893054d025744654f8715c3b4906312617a2b5028316b", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:3bac36fa97090b273afe1128103e1bf476d05441fd9de41317f1b78775b88304", "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealthLink": "sha256:7702598468b4c82b4ffa93064cd6bdfec938c1c527f7e6007c0584f3884a6eea", - "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:3af10adcb49672910d1a0f7a9cdc095c108adf64ff3487c51ddd1be1495b6e43", + "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:6ce1f9be1e6a977130d0875003ac22da2de5e90b514a3b22b30b9347e63d81c7", "src/daemon-client/daemon-client-transport.ts#readHealthLink": "sha256:f56404b94d73da57de7248719c9136b760d2be8b4a53cde5315a2311b088425b", "src/daemon-client/daemon-client-transport.ts#readHealthPayload": "sha256:4e85ffc3e35e02379c393e9312344757e003cf1f0ad9eb8d1f77d90c81c861f1", "src/daemon-client/daemon-client-transport.ts#readRemoteDaemonHealth": "sha256:df174d1ccf73851ca58bd75533fb4660424003b98bd9e41fe942cf3ab3698e5f", @@ -207,7 +207,7 @@ { "declaration": "src/remote/daemon-artifacts.ts#DownloadRemoteArtifactParams", "digest": "sha256:3ec63be2dbdb542e30b19d1500bc16874607823afb7ae392e92b0b535865859e", - "rationale": "#2246 adds the optional isDirectory field. It is pure client-local state \u2014 never serialized, never sent to the daemon \u2014 that tells the CLIENT'S OWN download logic to extract a tar body instead of writing it verbatim; the GET /artifacts/:id request and response framing are unchanged. Every existing call site (screenshot, recording) omits it and keeps writing a single file exactly as before." + "rationale": "#2246 adds the optional isDirectory field. It is pure client-local state — never serialized, never sent to the daemon — that tells the CLIENT'S OWN download logic to extract a tar body instead of writing it verbatim; the GET /artifacts/:id request and response framing are unchanged. Every existing call site (screenshot, recording) omits it and keeps writing a single file exactly as before." }, { "declaration": "src/remote/daemon-artifacts.ts#downloadRemoteArtifact", @@ -222,7 +222,7 @@ { "declaration": "src/daemon-client/daemon-client-rpc.ts#buildHttpRpcPayload", "digest": "sha256:caddf184cd57a41d9910c414bba72ce5a0503e22a2a4ef3a47f95fed67037127", - "rationale": "#2110 added the optional providerApp parameter to lease allocation; #2494 extends the same additive projection to the full provider-allocation flag set the lease-lifecycle provider reads in prepareSession \u2014 device selection (platform, device), the app/os, the session-naming fields, and the configured device-feature and AWS knobs \u2014 so a fresh remote allocation reaches BrowserStack/AWS with a named, configured session instead of failing on a missing --device/--platform or landing in Untitled Project/Build. Producer and consumer both read one shared projection (readLeaseAllocateProviderFlags). Every field is optional and forwarded only when the caller supplied it: a released protocol-2 daemon ignores unknown JSON-RPC parameters, and a request carrying none of these retains the previous payload byte-for-byte." + "rationale": "#2110 added the optional providerApp parameter to lease allocation; #2494 extends the same additive projection to the full provider-allocation flag set the lease-lifecycle provider reads in prepareSession — device selection (platform, device), the app/os, the session-naming fields, and the configured device-feature and AWS knobs — so a fresh remote allocation reaches BrowserStack/AWS with a named, configured session instead of failing on a missing --device/--platform or landing in Untitled Project/Build. Producer and consumer both read one shared projection (readLeaseAllocateProviderFlags). Every field is optional and forwarded only when the caller supplied it: a released protocol-2 daemon ignores unknown JSON-RPC parameters, and a request carrying none of these retains the previous payload byte-for-byte." }, { "declaration": "src/daemon/server/http-server.ts#toLeaseDaemonRequest", @@ -282,7 +282,7 @@ { "declaration": "src/daemon/server/http-server.ts#authorizeAuxiliaryHttpRequest", "digest": "sha256:1c323f97c3cefec8f95633b6abd127342bf75494bfb109d1cc6eadf3ceacaeae", - "rationale": "#2198 context: adds one optional field to the value this DAEMON-INTERNAL gate hands its own route handlers \u2014 sessionNamespace, the caller's tenant plus whether the daemon partitioned that caller's session names. Nothing about it is serialized: no request header is read that was not read before, no response field is added, and a refusal keeps the same 401 {ok:false,error,code} REST body sendRestJsonError already sent. What it enables is a refusal DROPPED, never one added \u2014 an unattested (client-declared) tenant no longer has the : prefix rule applied to it, because scopeRequestSession never applied that prefix to its sessions either. A released peer that now gets 200 where it got 401 is getting the ndjson record it asked for and already parses." + "rationale": "#2198 context: adds one optional field to the value this DAEMON-INTERNAL gate hands its own route handlers — sessionNamespace, the caller's tenant plus whether the daemon partitioned that caller's session names. Nothing about it is serialized: no request header is read that was not read before, no response field is added, and a refusal keeps the same 401 {ok:false,error,code} REST body sendRestJsonError already sent. What it enables is a refusal DROPPED, never one added — an unattested (client-declared) tenant no longer has the : prefix rule applied to it, because scopeRequestSession never applied that prefix to its sessions either. A released peer that now gets 200 where it got 401 is getting the ndjson record it asked for and already parses." }, { "declaration": "src/daemon/request-diagnostics-http.ts#RequestDiagnosticsHttpAuthorizer", @@ -327,7 +327,7 @@ { "declaration": "src/remote/upload-client.ts#tryDirectUploadWithResume", "digest": "sha256:67596a2ff2b8a5402566c3b7b99a6fa1baaf9d5de3d647bf35398185cc45eea6", - "rationale": "#2946 stops a canceled upload from re-preflighting and lifts the retry leg into its own function. The request sequence a daemon sees \u2014 preflight, direct PUT, finalize, or the legacy fallback after a retryable failure \u2014 is unchanged." + "rationale": "#2946 stops a canceled upload from re-preflighting and lifts the retry leg into its own function. The request sequence a daemon sees — preflight, direct PUT, finalize, or the legacy fallback after a retryable failure — is unchanged." }, { "declaration": "src/remote/upload-client.ts#finalizeDirectUpload", @@ -431,7 +431,7 @@ }, { "declaration": "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth", - "digest": "sha256:3af10adcb49672910d1a0f7a9cdc095c108adf64ff3487c51ddd1be1495b6e43", + "digest": "sha256:6ce1f9be1e6a977130d0875003ac22da2de5e90b514a3b22b30b9347e63d81c7", "rationale": "The health-probe budget includes requester loading and response completion. The caller AbortSignal joins the probe timer only in-process; caller cancellation remains distinct from probe expiry. No signal, timedOut flag, request route, auth header or accepted health payload field changes on the wire, so released daemons and proxies parse and respond as before (ADR 0006)." } ] From 5b12609ef5b6dd140d5a8b60f09738281e73778b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 19:43:20 +0200 Subject: [PATCH 6/6] test: preserve scoped repair evidence and restart probe cancellation --- .../daemon-client-abort-timeout.test.ts | 54 +++++++++++++++++++ src/session-repair-tombstone.test.ts | 4 +- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts b/src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts index d88325338c..294eff629b 100644 --- a/src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts +++ b/src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts @@ -11,6 +11,8 @@ * abort path's `clearTimeout` and only the abort case fails. */ import assert from 'node:assert/strict'; +import http from 'node:http'; +import { DAEMON_HTTP_INSTANCE_MISMATCH_HEADER } from '@agent-device/contracts/daemon-http'; import { test, vi } from 'vitest'; import { AppError, isRequestCanceledError } from '@agent-device/kernel/errors'; import { getRequestSignal } from '@agent-device/host-kit/request'; @@ -20,6 +22,7 @@ import { sendRequest } from '../daemon-client-transport.ts'; import { resolveDaemonPaths } from '../../daemon-resolution.ts'; import { closeLoopbackServer, + listenOnLoopback, skipWhenLoopbackUnavailable, trackLoopbackSockets, type SkippableTestContext, @@ -111,3 +114,54 @@ test('an abort with a timeout armed clears the timer instead of running the time test('the same budget without a signal reaches the timeout seam', async (t) => { await runTimerDiscipline(t, 'timed-out'); }); + +test('an abort during a restart health probe never retires the daemon or redispatches', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + handleRequestTimeoutCalls.length = 0; + const controller = new AbortController(); + let rpcCount = 0; + let healthCount = 0; + let reachedHealth!: () => void; + const healthStarted = new Promise((resolve) => { + reachedHealth = resolve; + }); + const server = http.createServer((req, res) => { + if (req.url === '/health') { + healthCount += 1; + reachedHealth(); + return; + } + rpcCount += 1; + res.statusCode = 409; + res.setHeader(DAEMON_HTTP_INSTANCE_MISMATCH_HEADER, 'true'); + res.end(); + }); + const destroySockets = trackLoopbackSockets(server); + try { + const port = await listenOnLoopback(server); + const request = sendRequest( + { + baseUrl: `http://127.0.0.1:${port}`, + token: 'secret', + pid: 1, + remoteInstanceId: 'previous-instance', + }, + { token: 'secret', command: 'devices', session: 'default', positionals: [], flags: {} }, + 'auto', + STATE_PATHS, + 1_000, + { signal: controller.signal }, + ); + const rejection = assert.rejects(request, (error: unknown) => isRequestCanceledError(error)); + await healthStarted; + controller.abort(); + await rejection; + assert.equal(rpcCount, 1); + assert.equal(healthCount, 1); + assert.deepEqual(handleRequestTimeoutCalls, []); + } finally { + controller.abort(); + destroySockets(); + await closeLoopbackServer(server); + } +}); diff --git a/src/session-repair-tombstone.test.ts b/src/session-repair-tombstone.test.ts index 0cceeb3fa1..444faf2e0f 100644 --- a/src/session-repair-tombstone.test.ts +++ b/src/session-repair-tombstone.test.ts @@ -44,7 +44,7 @@ test.each([ }; const { sessionsDir, tombstonePath } = writeRepairTombstone(`${JSON.stringify(tombstone)}\n`); - expect(readRepairTombstoneFile(tombstonePath)).toEqual(tombstone); + expect(readRepairTombstoneFile(tombstonePath, 'session-a')).toEqual(tombstone); expect(findUnrecoveredRepairCommitFailure(sessionsDir)).toEqual({ sessionName: 'session-a', tombstone, @@ -61,7 +61,7 @@ test.each([ const raw = rawRepairTombstone(fields); const { sessionsDir, tombstonePath } = writeRepairTombstone(raw); - expect(readRepairTombstoneFile(tombstonePath)).toBeUndefined(); + expect(readRepairTombstoneFile(tombstonePath, 'session-a')).toBeUndefined(); assert.throws( () => findUnrecoveredRepairCommitFailure(sessionsDir), (error: unknown) => {