From e1f0bca89a7f0b8b96777899fdd0dce25f5b0e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 22:11:53 +0200 Subject: [PATCH] feat(cli): ensure the daemon explicitly before attaching in the CLI and MCP The CLI and MCP now call ensureDaemon() once their config is resolved and attach to what it leaves. A command replaces a daemon from another version or build only if it holds no work, instead of destroying its sessions; a busy one fails the command with daemon-busy. Remote connections and a one-shot replay on its private daemon pass through unchanged. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + src/cli.ts | 20 +- src/commands/schema/cli-help.ts | 1 + .../__tests__/daemon-client-ensuring.test.ts | 313 ++++++++++++++++++ src/daemon-client/daemon-client-ensuring.ts | 98 ++++++ src/daemon-client/daemon-client-lifecycle.ts | 22 ++ src/daemon-client/daemon-client.ts | 21 ++ .../command-tools-daemon-lifecycle.test.ts | 52 +++ src/mcp/command-tools.ts | 10 +- .../daemon-replace-exit-flush.test.ts | 4 +- website/docs/docs/commands.md | 1 + 11 files changed, 538 insertions(+), 5 deletions(-) create mode 100644 src/daemon-client/__tests__/daemon-client-ensuring.test.ts create mode 100644 src/daemon-client/daemon-client-ensuring.ts create mode 100644 src/mcp/__tests__/command-tools-daemon-lifecycle.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 725a0793e9..283148d1ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Behavior change: the CLI and MCP now call `ensureDaemon()` explicitly once their config is resolved, then attach to the daemon it leaves. A command that finds a daemon from another version or build replaces it only if the daemon reports no request, session, repair session or live lease. A resumable upload still streaming and a host human-control hold that names no lease are not counted, so a replacement can end them. A daemon that answers nothing on any transport is retired after repeated failed liveness probes, which cannot show it idle. A busy one fails the command with `details.reason: 'daemon-busy'` instead of being replaced with its sessions, and a daemon from a release that predates quiescing fails it with `'daemon-incompatible'`; stop either with `agent-device daemon stop --state-dir `. The takeover notice moves from `Replacing daemon … : ` to `Replaced idle daemon …` (an incompatible daemon) or `Replaced unreachable daemon …` (one that answered nothing). A request that times out no longer retires an unresponsive daemon from the CLI or MCP; the error names `agent-device daemon stop --state-dir `. A one-shot `replay` or `test` with an explicit `--state-dir` leaves the daemon it started running until its idle reap instead of stopping it. Remote `--daemon-base-url` connections and replay on its private ephemeral daemon are unchanged. Node.js clients keep `daemonLifecycle: 'managed'` until 0.22. - Add `ensureDaemon({ stateDir })` and the client config `daemonLifecycle: 'attach' | 'managed'`. An `attach` client connects to the daemon registered in the state directory and never starts, replaces or kills one, including after a request times out. It fails with `details.reason: 'daemon-not-running'` (the hint points at `ensureDaemon()`) or `'daemon-incompatible'` (both identities in `details`, the hint names `agent-device daemon stop`). `ensureDaemon()` starts a missing daemon and replaces an incompatible one that answers only through an authenticated quiesce request the daemon answers itself (a resumable upload still streaming and a human-control hold that names no lease are not counted, so a replacement can end them): it refuses with `daemon-busy` while any request is in flight or any session, repair session or live lease exists, and a daemon from a release before this one is never replaced. A registered daemon that answers nothing is retired, as is one that times out on the quiesce request and then fails repeated liveness probes; neither can be asked. A remote `daemonBaseUrl` connection never starts or retires a local daemon in either mode. `managed` stays the default and behaves as before until 0.22. - Add `uninstall ` and `client.apps.uninstall({ app })` to remove an installed app on adb-backed Android targets, iOS simulators, and CoreDevice iOS devices. Both report `wasInstalled`. A package name or bundle id that is not installed succeeds with `wasInstalled: false`. Other targets, Appium and Limrun sessions included, refuse it before anything runs. - Add `settings mobile-data on|off` for adb-backed Android targets. It switches mobile data through `svc data` and reports the `mobileData` state stored in the global `mobile_data` row; multi-SIM read-back is not covered yet. Every other target refuses it before anything runs, and `--app` is refused because the switch is device-wide. diff --git a/src/cli.ts b/src/cli.ts index 8c0e8bd4c3..9a50fbf6fe 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -11,7 +11,7 @@ import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolu import { exitAfterFlush } from './cli/process-exit.ts'; import { readVersion } from '@agent-device/host-kit/version'; import { pathToFileURL } from 'node:url'; -import { sendToDaemon } from './daemon-client/daemon-client.ts'; +import { createEnsuredDaemonTransport, sendToDaemon } from './daemon-client/daemon-client.ts'; import fs from 'node:fs'; import type { BatchStep } from '@agent-device/contracts/client'; import type { ReplayTestReporterRuntime } from './cli/replay-test/reporting.ts'; @@ -70,9 +70,25 @@ const printJson = ( import('./commands/output/json.ts').then(({ printJson: write }) => write(result)); const DEFAULT_CLI_DEPS: CliDeps = { - sendToDaemon, + sendToDaemon: createEnsuredDaemonTransport({ onReplaced: writeDaemonReplacedNotice }), }; +function writeDaemonReplacedNotice(replaced: { + stateDir: string; + previous: { pid: number; version?: string }; + reason: 'incompatible' | 'unreachable'; +}): void { + const { stateDir, previous, reason } = replaced; + const identity = previous.version + ? `pid ${previous.pid}, v${previous.version}` + : `pid ${previous.pid}`; + process.stderr.write( + reason === 'unreachable' + ? `Replaced unreachable daemon (${identity}) in ${stateDir}.\n` + : `Replaced idle daemon (${identity}) in ${stateDir}: it did not match this client.\n`, + ); +} + const METRO_RUNTIME_OVERRIDE_FLAG_KEYS = new Set([ 'launchUrl', 'kind', diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 62fec6a4a6..bb9d464e40 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -508,6 +508,7 @@ Runner and daemon lifecycle (applies to simulators too): No runner read launches a session app that is not running: snapshot, wait, is, get, a reading find, and an interaction's leading reads (a gesture's viewport read, the capture that resolves a selector click/fill) answer the retriable APP_NOT_RUNNING instead of bare-launching over a launch SpringBoard still holds behind its deep-link confirmation. Only open, activate, and a command that mutates without a leading read bring a stopped app up. close keeps a healthy iOS simulator XCTest runner warm by default so the next open on that simulator (same udid in the same simulator set) skips the runner build, unless --shutdown was requested, the session was recording, or the session held a device lease. A retained runner auto-stops after an idle window (default 5 minutes); set AGENT_DEVICE_IOS_RUNNER_IDLE_STOP_MS to override, or 0 to disable idle stop and retain until daemon exit. Only a ready runner with an observed listener is retained; close retires unfinished prewarm recovery. Losing the retained listener stops that generation, and daemon shutdown stops idle retained runners rather than handing them off without supervision. Each AGENT_DEVICE_STATE_DIR runs its own daemon. It self-exits after an idle window (default 5 minutes, matching the runner idle-stop default) once it has no open sessions, no in-flight requests, and no active recording; set AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS to override, or 0 to disable idle reap. + A command that needs the shared local daemon starts it for its state dir when none runs (device status and device release --stale never contact one). When the running daemon comes from another version or build, the command replaces it only if the daemon reports no work: no in-flight request, session, repair session, or live lease (a streaming resumable upload and a host human-control hold that names no lease are not counted); otherwise it fails with details.reason daemon-busy and leaves the daemon alone. A daemon from a release that predates this check is never replaced and fails with daemon-incompatible. End a busy or older daemon deliberately with agent-device daemon stop --state-dir . Node clients pick daemonLifecycle 'attach' (never starts or replaces a daemon; fails with daemon-not-running or daemon-incompatible) or 'managed' (the default until 0.22), and ensureDaemon() starts or replaces one explicitly. On a machine shared by several agents, set AGENT_DEVICE_SESSION_IDLE_TIMEOUT_MS to also expire an individual session that has taken a host-global device claim and then received no commands for that long; the claim is released and the next command on that session answers SESSION_NOT_FOUND with details.reason SESSION_IDLE_EXPIRED naming the window and the device. Off by default, because on a shared host "idle" and "thinking" are indistinguishable: only set it where an abandoned claim blocking every other agent is the worse failure. Sessions holding a remote lease or any active capture (recording, logs, audio, performance, or trace) are never expired this way. A stale iOS runner lease — its owner process dead, or its AGENT_DEVICE_STATE_DIR deleted — is reclaimed automatically instead of failing with "is already owned by another agent-device daemon". A live owner's runner is also reclaimed when the requesting daemon holds the host-global device claim for that device: claims are exclusive, so holding one proves the runner's owner released the device and merely kept the runner warm. The error remains only for owners outside claim arbitration (a pre-claims build, or daemons pointed at different claim stores). diff --git a/src/daemon-client/__tests__/daemon-client-ensuring.test.ts b/src/daemon-client/__tests__/daemon-client-ensuring.test.ts new file mode 100644 index 0000000000..772324f402 --- /dev/null +++ b/src/daemon-client/__tests__/daemon-client-ensuring.test.ts @@ -0,0 +1,313 @@ +import assert from 'node:assert/strict'; +import { afterEach, test, vi } from 'vitest'; +import { closeLoopbackServer, supportsLoopbackBind } from '@agent-device/test-kit'; +import { AppError } from '@agent-device/kernel/errors'; +import { isProcessAlive } from '@agent-device/host-kit/process'; +import { finishRegisteredDaemonFixtures } from '../../__tests__/test-utils/registered-daemon-fixture.ts'; +import { + busy, + draining, + launchesFreshDaemonAt, + okResult, + registerDaemon, + startScriptedDaemon, + stateDirAndPaths, +} from './explicit-lifecycle-fixture.ts'; + +vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ + ...(await importOriginal()), + runCmdDetachedMonitored: vi.fn(), +})); +import { runCmdDetachedMonitored } from '@agent-device/host-kit/command'; +import { withEnsuredDaemon } from '../daemon-client-ensuring.ts'; +import { daemonNotRunningError } from '../daemon-lifecycle-errors.ts'; +import { createEnsuredDaemonTransport, type sendToDaemon } from '../daemon-client.ts'; +import * as ensureModule from '../daemon-client-ensure.ts'; + +// The CLI and MCP reach the daemon through `withEnsuredDaemon`. The transport underneath is a +// recorder, so what these tests read is the lifecycle each request was handed and what ensuring +// did to the state directory before it. + +const spawn = vi.mocked(runCmdDetachedMonitored); + +afterEach(async () => { + vi.unstubAllEnvs(); + spawn.mockReset(); + await finishRegisteredDaemonFixtures(); +}); + +function recordingTransport() { + const sent: Array<{ command: string; lifecycle: string | undefined }> = []; + const transport: typeof sendToDaemon = async (req, context) => { + sent.push({ command: req.command, lifecycle: context?.daemonLifecycle }); + return { ok: true, data: {} }; + }; + return { sent, transport }; +} + +function request(command: string, flags: Record) { + return { session: 'default', command, positionals: [], flags } as Parameters< + typeof sendToDaemon + >[0]; +} + +test('the first request ensures a missing daemon once and every request attaches to it', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const fresh = await startScriptedDaemon(okResult({})); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + const flags = { stateDir, daemonTransport: 'http' }; + + try { + assert.equal(spawn.mock.calls.length, 0, 'building the transport starts nothing'); + await ensuring(request('devices', flags)); + await ensuring(request('snapshot', flags)); + + assert.equal(spawn.mock.calls.length, 1); + assert.deepEqual(sent, [ + { command: 'devices', lifecycle: 'attach' }, + { command: 'snapshot', lifecycle: 'attach' }, + ]); + } finally { + await closeLoopbackServer(fresh.server); + } +}); + +test('an idle incompatible daemon is replaced, and the replacement is reported', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const stale = await startScriptedDaemon(draining); + const fresh = await startScriptedDaemon(okResult({})); + const { child } = await registerDaemon(paths, { + httpPort: stale.port, + version: '0.0.0-stale', + capabilities: ['quiesce'], + }); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const { sent, transport } = recordingTransport(); + const replaced: Array<{ stateDir: string; pid: number; reason: string }> = []; + const ensuring = withEnsuredDaemon(transport, { + onReplaced: (event) => + replaced.push({ stateDir: event.stateDir, pid: event.previous.pid, reason: event.reason }), + }); + + try { + await ensuring(request('devices', { stateDir, daemonTransport: 'http' })); + + assert.deepEqual(replaced, [ + { stateDir: paths.baseDir, pid: child.pid, reason: 'incompatible' }, + ]); + assert.equal(sent[0]?.lifecycle, 'attach'); + } finally { + await closeLoopbackServer(stale.server); + await closeLoopbackServer(fresh.server); + } +}); + +test('a busy incompatible daemon fails the request before it is sent and keeps running', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const stale = await startScriptedDaemon( + busy({ inFlightRequests: 0, sessions: 1, repairSessions: 0, leases: 0 }), + ); + const { child } = await registerDaemon(paths, { + httpPort: stale.port, + version: '0.0.0-stale', + capabilities: ['quiesce'], + }); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + + try { + const failure = await ensuring(request('devices', { stateDir, daemonTransport: 'http' })).then( + () => assert.fail('expected the request to be refused'), + (error: unknown) => error, + ); + + assert.ok(failure instanceof AppError); + assert.equal(failure.details?.reason, 'daemon-busy'); + assert.deepEqual(sent, []); + assert.equal(isProcessAlive(child.pid), true); + assert.equal(spawn.mock.calls.length, 0); + } finally { + await closeLoopbackServer(stale.server); + } +}); + +test('a remote connection passes through without ensuring or choosing a lifecycle', async () => { + const { stateDir } = stateDirAndPaths(); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + + await ensuring(request('devices', { stateDir, daemonBaseUrl: 'http://127.0.0.1:1' })); + vi.stubEnv('AGENT_DEVICE_DAEMON_BASE_URL', 'http://127.0.0.1:1'); + await ensuring(request('snapshot', { stateDir })); + + assert.deepEqual(sent, [ + { command: 'devices', lifecycle: undefined }, + { command: 'snapshot', lifecycle: undefined }, + ]); + assert.equal(spawn.mock.calls.length, 0); +}); + +test('a one-shot replay on its own private daemon passes through, an explicit state dir does not', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const fresh = await startScriptedDaemon(okResult({})); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + + try { + await ensuring(request('replay', { daemonTransport: 'http' })); + assert.deepEqual(sent, [{ command: 'replay', lifecycle: undefined }]); + assert.equal(spawn.mock.calls.length, 0); + + await ensuring(request('replay', { stateDir, daemonTransport: 'http' })); + assert.equal(sent[1]?.lifecycle, 'attach'); + assert.equal(spawn.mock.calls.length, 1); + } finally { + await closeLoopbackServer(fresh.server); + } +}); + +test('a daemon that disappears between requests is ensured again before the next one is sent', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const fresh = await startScriptedDaemon(okResult({})); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const sent: string[] = []; + let daemonGone = false; + const transport: typeof sendToDaemon = async (req) => { + if (daemonGone) { + daemonGone = false; + throw daemonNotRunningError(stateDir); + } + sent.push(req.command); + return { ok: true, data: {} }; + }; + const ensuring = withEnsuredDaemon(transport); + const flags = { stateDir, daemonTransport: 'http' }; + + try { + await ensuring(request('devices', flags)); + assert.equal(spawn.mock.calls.length, 1); + + daemonGone = true; + await finishRegisteredDaemonFixtures(); + launchesFreshDaemonAt(spawn, paths, fresh.port); + await ensuring(request('snapshot', flags)); + + assert.deepEqual(sent, ['devices', 'snapshot']); + assert.equal(spawn.mock.calls.length, 2, 'the second ensure started a daemon again'); + } finally { + await closeLoopbackServer(fresh.server); + } +}); + +test('a registered daemon that answers nothing is replaced and reported as unreachable', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + // Both are bound before the dead one is closed, so the fresh daemon cannot be handed its port. + const closed = await startScriptedDaemon(okResult({})); + const fresh = await startScriptedDaemon(okResult({})); + await closeLoopbackServer(closed.server); + const { child } = await registerDaemon(paths, { httpPort: closed.port }); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const { transport } = recordingTransport(); + const replaced: string[] = []; + const ensuring = withEnsuredDaemon(transport, { + onReplaced: (event) => replaced.push(`${event.reason}:${event.previous.pid}`), + }); + + try { + await ensuring(request('devices', { stateDir, daemonTransport: 'http' })); + + assert.deepEqual(replaced, [`unreachable:${child.pid}`]); + } finally { + await closeLoopbackServer(fresh.server); + } +}); + +test('a failed ensure is retried by the next request instead of being remembered', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const stale = await startScriptedDaemon( + busy({ inFlightRequests: 1, sessions: 0, repairSessions: 0, leases: 0 }), + ); + const fresh = await startScriptedDaemon(okResult({})); + const { child } = await registerDaemon(paths, { + httpPort: stale.port, + version: '0.0.0-stale', + capabilities: ['quiesce'], + }); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + const flags = { stateDir, daemonTransport: 'http' }; + + try { + await assert.rejects( + ensuring(request('devices', flags)), + (error: unknown) => error instanceof AppError && error.details?.reason === 'daemon-busy', + ); + assert.deepEqual(sent, []); + + // The busy daemon goes away; the same transport ensures again rather than replaying the failure. + await finishRegisteredDaemonFixtures(); + assert.equal(isProcessAlive(child.pid), false); + launchesFreshDaemonAt(spawn, paths, fresh.port); + await ensuring(request('devices', flags)); + + assert.deepEqual(sent, [{ command: 'devices', lifecycle: 'attach' }]); + } finally { + await closeLoopbackServer(stale.server); + await closeLoopbackServer(fresh.server); + } +}); + +test('concurrent first requests share one ensure through the CLI and MCP transport', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const fresh = await startScriptedDaemon(okResult({ via: 'fresh' })); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const ensuring = createEnsuredDaemonTransport(); + const flags = { stateDir, daemonTransport: 'http' }; + const ensure = vi.spyOn(ensureModule, 'ensureDaemon'); + + try { + const responses = await Promise.all([ + ensuring(request('devices', flags)), + ensuring(request('snapshot', flags)), + ]); + + assert.deepEqual(responses, [ + { ok: true, data: { via: 'fresh' } }, + { ok: true, data: { via: 'fresh' } }, + ]); + assert.equal(ensure.mock.calls.length, 1, 'one ensure for both requests'); + assert.equal(spawn.mock.calls.length, 1, 'one daemon start for both requests'); + } finally { + await closeLoopbackServer(fresh.server); + } +}); + +test('a request canceled before it starts does not start a daemon', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const fresh = await startScriptedDaemon(okResult({})); + launchesFreshDaemonAt(spawn, paths, fresh.port); + const { sent, transport } = recordingTransport(); + const ensuring = withEnsuredDaemon(transport); + const canceled = AbortSignal.abort(); + + try { + await ensuring(request('devices', { stateDir, daemonTransport: 'http' }), { signal: canceled }); + + assert.equal(spawn.mock.calls.length, 0); + assert.deepEqual(sent, [{ command: 'devices', lifecycle: 'attach' }]); + } finally { + await closeLoopbackServer(fresh.server); + } +}); diff --git a/src/daemon-client/daemon-client-ensuring.ts b/src/daemon-client/daemon-client-ensuring.ts new file mode 100644 index 0000000000..70553e5c6a --- /dev/null +++ b/src/daemon-client/daemon-client-ensuring.ts @@ -0,0 +1,98 @@ +import { AppError } from '@agent-device/kernel/errors'; +import type { DaemonIdentity, EnsureDaemonOptions } from '@agent-device/contracts/client'; +import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution'; +import type { sendToDaemon } from './daemon-client.ts'; + +type DaemonTransport = typeof sendToDaemon; + +export type EnsuringTransportOptions = { + /** + * Called when ensuring replaced a daemon: an `incompatible` one that held no work, or an + * `unreachable` one that answered nothing. + */ + onReplaced?: (replaced: { + stateDir: string; + previous: DaemonIdentity; + reason: 'incompatible' | 'unreachable'; + }) => void; +}; + +/** + * Makes a transport explicit about the daemon: the first request that needs a local daemon calls + * `ensureDaemon`, and every request then attaches to what it left. The CLI and MCP use this, so + * neither starts a daemon implicitly nor replaces one that holds another client's work. + * + * Ensuring waits for the first request rather than the first call, so a command that never reaches + * the daemon starts none. A remote connection, and a one-shot replay on its own private daemon, pass + * through untouched: neither has a shared local daemon to ensure. + * + * A request that attaches to a daemon that has since gone ensures it again and runs once more; it + * never reached a daemon, so nothing can run twice. + */ +export function withEnsuredDaemon( + transport: DaemonTransport, + options: EnsuringTransportOptions = {}, +): DaemonTransport { + const ensured = new Map>(); + return async (req, context) => { + // Loaded on the first request: the lifecycle module is not part of what every CLI start pays for. + const { resolveEnsurableDaemon } = await import('./daemon-client-lifecycle.ts'); + const target = resolveEnsurableDaemon(req); + if (!target) return await transport(req, context); + const stateDir = resolveDaemonPaths(target.stateDir).baseDir; + // A caller that has already given up starts nothing: attaching refuses before any connection. + if (context?.signal?.aborted) { + return await transport(req, { ...context, daemonLifecycle: 'attach' }); + } + // A finished ensure is kept for the rest of this transport's life. A rejected one is dropped so + // the next request retries, and one whose daemon has since gone is dropped by the attach below. + const ensure = async (): Promise<{ relied: Promise }> => { + let pending = ensured.get(stateDir); + if (!pending) { + const started = ensureLocalDaemon(target, stateDir, options); + pending = started; + ensured.set(stateDir, started); + started.catch(() => { + if (ensured.get(stateDir) === started) ensured.delete(stateDir); + }); + } + await pending; + return { relied: pending }; + }; + const { relied } = await ensure(); + const attach = async () => await transport(req, { ...context, daemonLifecycle: 'attach' }); + try { + return await attach(); + } catch (error) { + // The daemon this invocation ensured is gone (idle reap, a crash, another client's stop). + // An attach that finds none sent nothing, so ensuring again and sending once is safe. + if (!(error instanceof AppError) || error.details?.reason !== 'daemon-not-running') + throw error; + // Only the promise this attach relied on is dropped: a concurrent request may already have + // replaced it with a fresh one. + if (ensured.get(stateDir) === relied) ensured.delete(stateDir); + await ensure(); + return await attach(); + } + }; +} + +async function ensureLocalDaemon( + target: EnsureDaemonOptions, + stateDir: string, + options: EnsuringTransportOptions, +): Promise { + const { ensureDaemon } = await import('./daemon-client-ensure.ts'); + await ensureDaemon({ + ...target, + onStart: (reason, previous) => { + if (previous) { + options.onReplaced?.({ + stateDir, + previous, + reason: reason === 'incompatible' ? 'incompatible' : 'unreachable', + }); + } + }, + }); +} diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index 28d5803e89..536cb052e4 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -117,6 +117,28 @@ export function resolveClientSettings( }; } +/** + * The local daemon a request would use, in the terms `ensureDaemon` takes, or `undefined` when the + * request has no shared local daemon to ensure: a remote connection has none, and a one-shot replay + * or test with no explicit state directory runs on a private daemon of its own. + */ +export function resolveEnsurableDaemon( + req: Omit, +): EnsureDaemonOptions | undefined { + const stateDir = resolveExplicitStateDir(req); + const rawBaseUrl = req.flags?.daemonBaseUrl ?? process.env.AGENT_DEVICE_DAEMON_BASE_URL; + if (rawBaseUrl || shouldUseOwnedReplayStateDir(req, stateDir, rawBaseUrl)) return undefined; + return { + ...(stateDir !== undefined ? { stateDir } : {}), + ...(req.flags?.daemonTransport !== undefined + ? { daemonTransport: req.flags.daemonTransport } + : {}), + ...(req.flags?.daemonServerMode !== undefined + ? { daemonServerMode: req.flags.daemonServerMode } + : {}), + }; +} + /** * The settings of a local daemon with no request behind them, for the caller that ensures one. It * reads no remote base URL: a remote connection has no local daemon to ensure. diff --git a/src/daemon-client/daemon-client.ts b/src/daemon-client/daemon-client.ts index d72826b882..6243e39c8d 100644 --- a/src/daemon-client/daemon-client.ts +++ b/src/daemon-client/daemon-client.ts @@ -16,6 +16,7 @@ import { type PreparedRemoteRequest, } from '@agent-device/remote/daemon-artifacts'; import type { DaemonClientSettings, EnsuredDaemon } from './daemon-client-lifecycle.ts'; +import type { EnsuringTransportOptions } from './daemon-client-ensuring.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'; @@ -123,6 +124,26 @@ export async function sendToDaemon( ); } +/** + * The transport the CLI and MCP use: `sendToDaemon` behind an explicit `ensureDaemon`. The wrapper + * loads on the first request, so a CLI start that never reaches the daemon does not pay for it. + */ +export function createEnsuredDaemonTransport( + options: EnsuringTransportOptions = {}, +): typeof sendToDaemon { + // The load is memoized as a promise, so concurrent first requests share one wrapper and its + // ensure-once state. + let ensured: Promise | undefined; + return async (req, context) => { + ensured ??= import('./daemon-client-ensuring.ts').then(({ withEnsuredDaemon }) => + withEnsuredDaemon(sendToDaemon, options), + ); + return await ( + await ensured + )(req, context); + }; +} + function buildTransportRequest( request: Omit, preparedRemoteRequest: Awaited>, diff --git a/src/mcp/__tests__/command-tools-daemon-lifecycle.test.ts b/src/mcp/__tests__/command-tools-daemon-lifecycle.test.ts new file mode 100644 index 0000000000..cf5de9917d --- /dev/null +++ b/src/mcp/__tests__/command-tools-daemon-lifecycle.test.ts @@ -0,0 +1,52 @@ +import assert from 'node:assert/strict'; +import { afterEach, test, vi } from 'vitest'; +import { closeLoopbackServer, supportsLoopbackBind } from '@agent-device/test-kit'; +import { finishRegisteredDaemonFixtures } from '../../__tests__/test-utils/registered-daemon-fixture.ts'; +import { + launchesFreshDaemonAt, + okResult, + startScriptedDaemon, + stateDirAndPaths, +} from '../../daemon-client/__tests__/explicit-lifecycle-fixture.ts'; + +vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ + ...(await importOriginal()), + runCmdDetachedMonitored: vi.fn(), +})); +import { runCmdDetachedMonitored } from '@agent-device/host-kit/command'; +import * as ensureModule from '../../daemon-client/daemon-client-ensure.ts'; +import { createCommandToolExecutor } from '../command-tools.ts'; + +// An MCP tool call builds its own client, so each call has to ensure the daemon it is about to use +// and then attach to it. This drives the default client factory, not an injected one. + +const spawn = vi.mocked(runCmdDetachedMonitored); + +afterEach(async () => { + vi.unstubAllEnvs(); + spawn.mockReset(); + await finishRegisteredDaemonFixtures(); +}); + +test('each MCP tool call ensures the daemon, then attaches to it', async (t) => { + if (!(await supportsLoopbackBind())) return t.skip('loopback listeners are not permitted'); + const { stateDir, paths } = stateDirAndPaths(); + const daemon = await startScriptedDaemon(okResult({ devices: [] })); + launchesFreshDaemonAt(spawn, paths, daemon.port); + vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); + const ensure = vi.spyOn(ensureModule, 'ensureDaemon'); + const executor = createCommandToolExecutor(); + + try { + const first = await executor.execute('devices', {}); + const second = await executor.execute('devices', {}); + + assert.equal(first.isError, false, JSON.stringify(first.structuredContent)); + assert.equal(second.isError, false, JSON.stringify(second.structuredContent)); + assert.equal(ensure.mock.calls.length, 2, 'one ensure per tool call'); + assert.equal(spawn.mock.calls.length, 1, 'the second call reuses the daemon the first started'); + assert.deepEqual(daemon.rpcCommands, ['devices', 'devices']); + } finally { + await closeLoopbackServer(daemon.server); + } +}); diff --git a/src/mcp/command-tools.ts b/src/mcp/command-tools.ts index b28df15b94..cce69527d6 100644 --- a/src/mcp/command-tools.ts +++ b/src/mcp/command-tools.ts @@ -248,7 +248,15 @@ async function createClient( ): Promise { if (deps.createClient) return await deps.createClient(config); const { createAgentDeviceClient } = await import('../agent-device-client.ts'); - return createAgentDeviceClient(config); + const { createEnsuredDaemonTransport } = await import('../daemon-client/daemon-client.ts'); + // One client per tool call, so each call ensures the daemon it is about to use. + const ensuring = createEnsuredDaemonTransport(); + return createAgentDeviceClient(config, { + // The client builds the same request the daemon transport takes; its type only differs in the + // session being optional, which `sendToDaemon` defaults. + transport: async (req, context) => + await ensuring(req as Parameters[0], context), + }); } async function runCommand( diff --git a/test/integration/daemon-replace-exit-flush.test.ts b/test/integration/daemon-replace-exit-flush.test.ts index d8cc357e6a..34b70a8f46 100644 --- a/test/integration/daemon-replace-exit-flush.test.ts +++ b/test/integration/daemon-replace-exit-flush.test.ts @@ -11,7 +11,7 @@ import { assertNoDaemonLeaks } from './support/daemon-leak-oracle.ts'; import { runCliJson } from './test-helpers.ts'; // #1596: a CLI command that finds its recorded daemon unreachable replaces it -// (`Replacing daemon (pid N, vX) in : unreachable`) and retries +// (`Replaced unreachable daemon (pid N, vX) in .`) and retries // against a fresh one. Three field runs died with zero further agent actions // immediately after that replace plus a SESSION_NOT_FOUND (the fresh daemon // has no sessions yet, which is expected). This file locks down that a @@ -52,7 +52,7 @@ test('daemon replace mid-command returns a structured, parseable error and exits assert.equal(result.status, 1, formatUnexpected('exit code', result)); assert.ok( - result.stderr.includes('Replacing daemon') && result.stderr.includes('unreachable'), + result.stderr.includes('Replaced unreachable daemon'), formatUnexpected('takeover notice on stderr', result), ); assert.ok(result.json, formatUnexpected('parseable JSON stdout', result)); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index e0014faa7f..25fd2d8f42 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -83,6 +83,7 @@ Device claims and the daemon: - `device status` lists host-local device claims without starting or contacting a daemon; `--stale` shows the stale claims it hides by default (owner exited, PID reused, state directory deleted, or daemon replaced), some of whose owners may still be running. Scope it with `--platform` plus `--udid` (Apple) or `--serial` (Android). A claim held by another live or uncertain owner blocks `open`, unless the device rebooted after the claim was taken: then `open` settles that owner's durable resources, takes the claim, and reports the release in `warnings`. - `device release --stale` settles the durable resources of an owner that provably can't release its claim (its process exited or a newer daemon replaced it), then clears the claim. Live, uncertain, PID-reused, corrupt, and deleted-state-directory claims are refused with the reason. See [Sessions](/docs/sessions#release-a-device-another-workspace-holds). +- A command that needs the shared local daemon starts it for its state directory when none runs; `device status` and `device release --stale` never contact one. When the running daemon comes from another version or build, the command replaces it only if it reports no work: no request in flight, no session, no repair session and no live lease. A resumable upload still streaming and a host human-control hold that names no lease are not counted, so a replacement can end them. Otherwise the command fails with `daemon-busy` and leaves the daemon alone. A daemon from a release that predates this check is never replaced, and fails with `daemon-incompatible`. End a busy or older daemon deliberately with `daemon stop`. Node.js clients choose this per client; see [Daemon lifecycle](/docs/client-api#daemon-lifecycle). - `daemon stop --state-dir ` stops a local daemon after verifying its identity. `--clean` also removes the Apple runner processes and leases that daemon retained. Open apps and links: