diff --git a/src/__tests__/test-utils/registered-daemon-fixture.ts b/src/__tests__/test-utils/registered-daemon-fixture.ts index 63ee16a7bc..12af382c10 100644 --- a/src/__tests__/test-utils/registered-daemon-fixture.ts +++ b/src/__tests__/test-utils/registered-daemon-fixture.ts @@ -13,7 +13,7 @@ const actualCommand = await vi.importActual; startTime: string | null } + Array<{ launch: ReturnType; startTime: string | null }> >(); /** A real registration owner, advertising the caller's HTTP fixture and joining before deletion. */ @@ -29,10 +29,10 @@ export function registeredDaemonFixtureArgs( entry, `import fs from 'node:fs'; import path from 'node:path'; -import { tryAcquireDaemonRegistration } from ${JSON.stringify(registrationUrl)}; +import { DAEMON_STARTUP_EXIT_CODES, tryAcquireDaemonRegistration } from ${JSON.stringify(registrationUrl)}; const paths = ${JSON.stringify(paths)}; const acquired = await tryAcquireDaemonRegistration(paths); -if (acquired.status !== 'acquired') process.exit(75); +if (acquired.status !== 'acquired') process.exit(DAEMON_STARTUP_EXIT_CODES[acquired.status]); process.on('SIGTERM', async () => { const deferred = path.join(paths.baseDir, 'repair-on-shutdown.json'); if (fs.existsSync(deferred)) { @@ -43,6 +43,8 @@ process.on('SIGTERM', async () => { 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); `, @@ -60,13 +62,14 @@ export function spawnRegisteredDaemonFixture( registeredDaemonFixtureArgs(paths, fields), options, ); - children.set(paths.baseDir, { launch: child, startTime: readProcessStartTime(child.pid) }); + const owned = children.get(paths.baseDir) ?? []; + owned.push({ launch: child, startTime: readProcessStartTime(child.pid) }); + children.set(paths.baseDir, owned); return child; } export async function finishRegisteredDaemonFixture(stateDir: string): Promise { - const owned = children.get(stateDir); - if (owned) { + for (const owned of children.get(stateDir) ?? []) { const child = owned.launch; const termination = await stopDaemonProcess( { pid: child.pid, startTime: owned.startTime }, @@ -74,8 +77,8 @@ export async function finishRegisteredDaemonFixture(stateDir: string): 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, + ); }); } @@ -357,7 +364,7 @@ test('sendToDaemon retains unknown metadata after a spawn failure', async () => assert.equal(results[0]?.removedInfo, false); assert.equal(attempts, 1); } finally { - fs.rmSync(stateDir, { recursive: true, force: true }); + await finishRegisteredDaemonFixture(stateDir); } }); @@ -402,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; @@ -416,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 { @@ -431,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); } }); @@ -510,7 +516,7 @@ 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(); } } @@ -553,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); } }); @@ -594,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); } }); @@ -697,7 +703,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); } }); @@ -741,7 +747,7 @@ 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); } }); @@ -1275,7 +1281,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); } }); 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 fa87f3f65b..7991d75c32 100644 --- a/src/daemon-client/__tests__/daemon-client-startup-race.test.ts +++ b/src/daemon-client/__tests__/daemon-client-startup-race.test.ts @@ -1,7 +1,24 @@ 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 } 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 { DAEMON_STARTUP_EXIT_CODES } from '../../daemon-registration-owner.ts'; vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ ...(await importOriginal()), @@ -9,205 +26,337 @@ vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ })); 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 WINNER_PID = winner.pid; -const LOSER_PID = 43_301; - -const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored); -const mockSleep = vi.mocked(sleep); - -afterEach(() => { - winner.alive = true; - mockRunCmdDetached.mockReset(); - mockSleep.mockReset(); - mockSleep.mockImplementation(async () => {}); - vi.unstubAllEnvs(); +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(); }); -/** 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; -}); +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) { + const deadline = Date.now() + 2_000; + while (!fs.existsSync(file)) { + assert.ok(Date.now() < deadline, `fixture did not publish ${file}`); + await actualRetry.sleep(10); + } +} -/** 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); + } }); +} - try { - const response = await sendToDaemon({ - session: 'default', - command: 'test', - positionals: [], - flags: { stateDir }, - meta: { requestId: 'req-start-race-test' }, +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); + 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, 1); - 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); } }); diff --git a/src/daemon-client/__tests__/daemon-client.test.ts b/src/daemon-client/__tests__/daemon-client.test.ts index accebf0782..cca31e8a26 100644 --- a/src/daemon-client/__tests__/daemon-client.test.ts +++ b/src/daemon-client/__tests__/daemon-client.test.ts @@ -140,44 +140,20 @@ 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'/, - ); + 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/); + } }); test('canConnectSocket times out stalled local daemon probes', async () => { diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index 7e75749e61..7b25dd630c 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -8,10 +8,11 @@ import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { isProcessAlive } from '@agent-device/host-kit/process'; import { sleep } from '@agent-device/host-kit/retry'; -import { inspectProcessLock } from '@agent-device/host-kit/file'; +import { inspectProcessLock, type ProcessLockInspection } from '@agent-device/host-kit/file'; import type { findUnrecoveredRepairCommitFailure } from '../session-repair-tombstone.ts'; import { + DAEMON_STARTUP_EXIT_CODES, createOwnedReplayStateDir, recoverAbandonedDaemonRegistration, type DaemonRetirementResult, @@ -36,14 +37,10 @@ import { import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import { - cleanupStaleDaemonLockIfSafe, getDaemonMetadataState, - isDaemonLockHeldByAnotherDaemon, isRemoteDaemon, readDaemonInfo, - removeDaemonInfo, resolveDaemonStartupHint, - stopDaemonProcessForTakeover, type DaemonInfo, } from './daemon-client-metadata.ts'; import { @@ -69,9 +66,11 @@ export type EnsuredDaemon = { 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; @@ -166,7 +165,6 @@ async function ensureLocalDaemon(settings: DaemonClientSettings): Promise { +async function readReusableLocalDaemon( + settings: DaemonClientSettings, + deadline?: number, +): Promise { + 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; 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; } +function registrationAllowsDaemonObservation( + inspection: ProcessLockInspection, + info: DaemonInfo, +): boolean { + return ( + inspection.state === 'absent' || + (inspection.state === 'held' && + inspection.owner.pid === info.pid && + inspection.owner.startTime === (info.processStartTime ?? null)) + ); +} + +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 @@ -220,12 +265,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', @@ -261,9 +308,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; @@ -298,8 +346,9 @@ function emitDaemonTakeoverNotice(info: DaemonInfo, reason: string, stateDir: st } type FailedDaemonStartup = { - cleanup: DaemonRetirementResult; + cleanup?: DaemonRetirementResult; startError?: string; + startupError?: NormalizedError; daemonProcess?: ExecDetachedExit | { pid: number }; retry: boolean; }; @@ -314,10 +363,11 @@ async function startLocalDaemon(settings: DaemonClientSettings): Promise = failure ?? {}; const state = getDaemonMetadataState(settings.paths); const daemonLogTail = readRecentLogTail(settings.paths.logPath); throw new AppError('COMMAND_FAILED', 'Failed to start daemon', { @@ -329,8 +379,9 @@ async function startLocalDaemon(settings: DaemonClientSettings): Promise { const cleanup = await stopAndRetireDaemon({ paths: settings.paths, observed: { pid: launch.pid, startTime: launch.startTime ?? null }, mode: 'graceful', + ownedStateDir, + termTimeoutMs: Math.min(3_000, remainingStartupBudget(deadline)), + killTimeoutMs: 1_000, lockTimeoutMs: 0, }); - const inspection = inspectProcessLock(settings.paths.lockPath); - const available = + 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')); - return { - cleanup, - retry: startup.kind === 'early_exit' && available, - startError: startup.kind === 'early_exit' ? describeDaemonEarlyExit(startup.exit) : undefined, - daemonProcess: startup.kind === 'early_exit' ? startup.exit : { pid: launch.pid }, - }; + inspection.liveness === 'owner-process-reused')) + ); } /** @@ -383,7 +480,7 @@ async function attemptLocalDaemonStartup( * (`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 @@ -629,36 +726,102 @@ export function attachActiveSessionAddressHint( } 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 } }; - } - // 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 }; + 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 } }; } - 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); + } 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, + settings.ownedStateDir, + ); + 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 { return ( diff --git a/src/daemon-client/daemon-client-metadata.ts b/src/daemon-client/daemon-client-metadata.ts index bf886cef5c..5e9d060daa 100644 --- a/src/daemon-client/daemon-client-metadata.ts +++ b/src/daemon-client/daemon-client-metadata.ts @@ -1,11 +1,6 @@ import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; -import { shellQuote } from '@agent-device/kernel/device-shell'; -import { - isAgentDeviceDaemonProcess, - stopDaemonProcess, - type DaemonTerminationResult, -} from '../daemon-process.ts'; +import { stopDaemonProcess, type DaemonTerminationResult } from '../daemon-process.ts'; import type { DaemonCodeOrigin } from '@agent-device/host-kit/code-signature'; @@ -32,12 +27,6 @@ export type DaemonInfo = { remoteUpstreamInstanceId?: string; }; -type DaemonLockInfo = { - pid: number; - processStartTime?: string; - startedAt?: number; -}; - export type DaemonMetadataState = { hasInfo: boolean; hasLock: boolean; @@ -96,35 +85,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); } @@ -133,20 +93,6 @@ 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 function getDaemonMetadataState(paths: DaemonPaths): DaemonMetadataState { return { hasInfo: fs.existsSync(paths.infoPath), @@ -187,21 +133,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 { diff --git a/src/daemon-client/daemon-client-transport.ts b/src/daemon-client/daemon-client-transport.ts index d09d5af386..aa0f93820d 100644 --- a/src/daemon-client/daemon-client-transport.ts +++ b/src/daemon-client/daemon-client-transport.ts @@ -72,23 +72,31 @@ 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; + if (await canConnectWithTransport(info, transport, deadline - Date.now())) return true; const fallback = chooseAutoFallbackTransport(info, preference, transport); - return fallback ? await canConnectWithTransport(info, fallback) : false; + return fallback ? await canConnectWithTransport(info, fallback, deadline - Date.now()) : 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 }, () => { @@ -100,7 +108,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); }); @@ -110,8 +118,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( diff --git a/website/docs/docs/installation.md b/website/docs/docs/installation.md index eca4ff398d..cef44e2760 100644 --- a/website/docs/docs/installation.md +++ b/website/docs/docs/installation.md @@ -115,10 +115,15 @@ 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. + +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` 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.