fix(daemon): retain cleanup locks and preserve healed snapshots

CHANGELOG.md

@@ -12,6 +12,8 @@
 - 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 <path>`. The takeover notice moves from `Replacing daemon … : <reason>` 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 <path>`. 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.
 - `lease_allocate` (and `client.leases.allocate()`) with a `leaseProvider` that no registered runtime claims now refuses before recording a lease, with `UNSUPPORTED_OPERATION`, `error.details.reason: provider-not-registered`, `details.provider`, `details.registeredProviders`, and `details.failedPlugins` naming a plugin that failed to register. Previously a plugin provider that the daemon never loaded passed admission and recorded a lease with no device behind it. Leases with no provider (a `leaseBackend` alone included), `proxy` and `cloud` need no runtime and still allocate. Restart the daemon with the provider configured, then retry.
+- Healed scripts retain authored snapshot steps and snapshots explicitly included with `--record` during repair. Diagnostic snapshots remain excluded. Behavior change: the config key `record` and `AGENT_DEVICE_RECORD` are retired — `--record` is per-invocation only, and a config file or environment naming the key fails with migration guidance naming `--record` and the SDK/MCP `record: true` call option.
+- Requests that share a session or device now queue until the preceding request finishes disposing its runtime binding and releasing its device claim, including after failure or cancellation.
 - Add `uninstall <app>` 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.
 - On iOS, a `fill` that runs under a degraded XCTest snapshot channel now aims at a point, including ref, selector and replay fills. When the runner identifies no text input under that point, it refuses with `NO_TEXT_INPUT_AT_POINT` (`error.details.reason: no_text_input_at_point`, `dispatched: no`) before dispatching its focus tap, and no field changes; when the input lookup itself cannot answer, it refuses with `TEXT_INPUT_PROBE_UNAVAILABLE` (`error.details.reason: text_input_probe_unavailable`). Previously such a fill cleared and typed through the app-level first responder, so it could rewrite a field the caller never named, or report a clear failure after emptying it. A ref or selector fill whose rect centre misses the real field (an outer container, a non-input node) now needs a point inside the input's own rect.

packages/command-registry/src/flag-definitions-action.ts

@@ -432,8 +432,8 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [
     // the synopsis stays the invocation they are choosing between (#1271 stage 2).
     usageHidden: true,
     usageDescription:
-      'Force-record this action even though its command is observation-only and would otherwise be excluded from a repair-armed heal by default (mutually exclusive with --no-record)',
-    projectConfig: true,
+      'Force-record this action into a repair-armed heal; per-invocation only, the retired config key "record" no longer opts in (mutually exclusive with --no-record)',
+    projectConfig: false,
     recorded: true,
   },
 ];

packages/contracts/src/session-action.ts

@@ -28,6 +28,8 @@ export type SessionAction = {
     noRecord?: boolean;
   };
   result?: Record<string, unknown>;
+  /** Retain authored plan observations or explicitly recorded repair observations. */
+  publicationIntent?: 'authored' | 'explicit-record';
   /**
    * ADR 0012 decision 3: parsed or record-time-computed `target-v1`
    * evidence, written as a comment immediately before this action's line.

packages/daemon/src/core/__tests__/request-runtime-binding-router.test.ts

@@ -1,5 +1,11 @@
 import fs from 'node:fs';
 import { expect, test, vi } from 'vitest';
+import {
+  clearRequestCanceled,
+  markRequestCanceled,
+  throwIfRequestCanceled,
+} from '@agent-device/host-kit/request';
+import { AppError } from '@agent-device/kernel/errors';
 import { applicationLifecycleOperationFacts } from '@agent-device/contracts/application-lifecycle-runtime';
 import {
   type DeviceBinding,
@@ -17,6 +23,7 @@ import { makeSessionStore } from '../../__tests__/support/store-factory.ts';
 import { LeaseRegistry } from '../../resources/lease-registry.ts';
 import { createRequestHandler } from '../../__tests__/support/test-device-runtime-gateway.ts';
 import { fakeCreateCommandSurface } from '../../__tests__/support/command-surface-fake.ts';
+import { getLeaseRegistryExecutionLocks } from '../../server/request-execution-scope.ts';
 
 test('request binding disposes once after response while adopted app-log handle survives', async () => {
   const runtime = makeGateway();
@@ -51,8 +58,81 @@ test('primary request failure survives a rejecting binding disposal', async () =
   expect(phases).toContain('request_binding_cleanup_failed');
 });
 
+for (const outcome of ['success', 'throw', 'cancellation'] as const) {
+  for (const key of ['session', 'device'] as const) {
+    test(`binding cleanup holds the ${key} execution lock after ${outcome}`, async () => {
+      const runtime = makeGateway();
+      const { handler, sessionStore, leaseRegistry } = makeHandler(runtime.gateway);
+      const current = sessionStore.get('session')!;
+      sessionStore.publish('other-session', { ...current, name: 'other-session', actions: [] });
+      const requestId = `cleanup-${key}-${outcome}`;
+      let markCleanupStarted: () => void = () => {};
+      let finishCleanup: () => void = () => {};
+      const cleanupStarted = new Promise<void>((resolve) => {
+        markCleanupStarted = resolve;
+      });
+      const cleanupFinished = new Promise<void>((resolve) => {
+        finishCleanup = resolve;
+      });
+      runtime.bindingDispose.mockImplementationOnce(async () => {
+        markCleanupStarted();
+        await cleanupFinished;
+      });
+      let doctorCalls = 0;
+      vi.spyOn(runtime.operations, 'appLogDoctor').mockImplementation(async () => {
+        if (doctorCalls++ > 0) return { backend: 'android', checks: {}, notes: [] };
+        if (outcome === 'throw') throw new AppError('COMMAND_FAILED', 'doctor failed');
+        if (outcome === 'cancellation') {
+          markRequestCanceled(requestId);
+          throwIfRequestCanceled(requestId);
+        }
+        return { backend: 'android', checks: {}, notes: [] };
+      });
+      const first = handler({ ...request(['doctor']), meta: { requestId } });
+      await cleanupStarted;
+      const locks = getLeaseRegistryExecutionLocks(leaseRegistry);
+      const sharedKey = key === 'session' ? 'session:session' : 'device:emulator-5554';
+      let markQueued: (entry: { previous: Promise<unknown> | undefined }) => void = () => {};
+      const queued = new Promise<{ previous: Promise<unknown> | undefined }>((resolve) => {
+        markQueued = resolve;
+      });
+      const setLock = locks.set.bind(locks);
+      const queueSpy = vi.spyOn(locks, 'set').mockImplementation((lockKey, completion) => {
+        if (lockKey === sharedKey) markQueued({ previous: locks.get(lockKey) });
+        return setLock(lockKey, completion);
+      });
+      const second = handler({
+        ...request(['doctor']),
+        session: key === 'session' ? 'session' : 'other-session',
+        meta: { requestId: `${requestId}-queued` },
+      });
+      try {
+        expect((await queued).previous).toBeDefined();
+        expect(runtime.bind).toHaveBeenCalledTimes(1);
+      } finally {
+        queueSpy.mockRestore();
+        finishCleanup();
+        clearRequestCanceled(requestId);
+        await Promise.all([first, second]);
+      }
+      const [firstResponse, secondResponse] = await Promise.all([first, second]);
+      expect(firstResponse.ok).toBe(outcome === 'success');
+      if (outcome === 'cancellation') {
+        expect(firstResponse).toMatchObject({
+          ok: false,
+          error: { code: 'COMMAND_FAILED', details: { reason: 'request_canceled' } },
+        });
+      }
+      expect(secondResponse.ok).toBe(true);
+      expect(runtime.bind).toHaveBeenCalledTimes(2);
+      expect(runtime.bindingDispose).toHaveBeenCalledTimes(2);
+    });
+  }
+}
+
 function makeHandler(gateway: DeviceRuntimeGateway<PlatformRuntimeOperations>) {
   const sessionStore = makeSessionStore('request-runtime-binding-router-');
+  const leaseRegistry = new LeaseRegistry();
   sessionStore.publish('session', {
     name: 'session',
     device: { platform: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator' },
@@ -62,12 +142,13 @@ function makeHandler(gateway: DeviceRuntimeGateway<PlatformRuntimeOperations>) {
   });
   return {
     sessionStore,
+    leaseRegistry,
     handler: createRequestHandler({
       createCommandSurface: fakeCreateCommandSurface,
       logPath: '/tmp/daemon.log',
       token: 'token',
       sessionStore,
-      leaseRegistry: new LeaseRegistry(),
+      leaseRegistry,
       deviceInventoryGateways: createTestDeviceInventoryGateways(),
       deviceRuntimeGateway: gateway,
       trackDownloadableArtifact: () => 'artifact',
@@ -181,7 +262,7 @@ function makeGateway(disposeError?: Error) {
     bind,
     shutdown: async () => {},
   };
-  return { gateway, bind, bindingDispose, forceCleanup, handle };
+  return { gateway, bind, bindingDispose, forceCleanup, handle, operations };
 }
 
 const unavailableRecording = Object.freeze({

packages/daemon/src/core/__tests__/session-action-recorder.test.ts

@@ -7,10 +7,42 @@ import { isSessionRecording } from '../session-script-publication-capability.ts'
  * (`selector-recording.test.ts` covers which commands set `interactiveObservation`).
  */
 import { test, expect } from 'vitest';
-import { recordActionEntry, type RecordActionEntry } from '../session-action-recorder.ts';
+import {
+  isInteractiveObservation,
+  recordActionEntry,
+  type RecordActionEntry,
+} from '../session-action-recorder.ts';
 import { authoringPublication, makeIosSession } from '../../__tests__/support/session-factories.ts';
 
-test('an observation-only action is excluded while repair-armed and no --record is given', () => {
+test.each([
+  { command: 'wait', replayPlanStep: false, intent: undefined },
+  { command: 'diff', replayPlanStep: false, intent: undefined },
+  { command: 'snapshot', replayPlanStep: false, intent: undefined },
+  { command: 'snapshot', replayPlanStep: true, intent: 'authored' },
+])(
+  'publication intent for $command with replayPlanStep=$replayPlanStep',
+  ({ command, replayPlanStep, intent }) => {
+    const session = makeIosSession('default');
+    const req = {
+      token: 't',
+      session: 'default',
+      command,
+      positionals: [],
+      internal: { replayPlanStep },
+    };
+    const action = recordActionEntry(session, {
+      command,
+      positionals: [],
+      flags: {},
+      interactiveObservation: isInteractiveObservation(req),
+    });
+
+    expect(action).toBeDefined();
+    expect(action?.publicationIntent).toBe(intent);
+  },
+);
+
+test('an observation without --record is excluded while repair-armed', () => {
   const session = makeIosSession('default', {
     scriptPublication: {
       kind: 'repair',
@@ -47,6 +79,8 @@ test('--record forces an observation-only action through while repair-armed', ()
     interactiveObservation: true,
   });
   expect(action).toBeDefined();
+  expect(action?.flags.record).toBe(true);
+  expect(action?.publicationIntent).toBe('explicit-record');
   expect(session.actions.map((a) => a.command)).toEqual(['get']);
 });
 

packages/daemon/src/core/__tests__/session-script-writer.test.ts

@@ -3,7 +3,7 @@ import { test, expect, vi } from 'vitest';
 import fs from 'node:fs';
 import path from 'node:path';
 import { HEAL_COMPLETE_SENTINEL, SessionScriptWriter } from '../session-script-writer.ts';
-import { recordActionEntry } from '../session-action-recorder.ts';
+import { isInteractiveObservation, recordActionEntry } from '../session-action-recorder.ts';
 import {
   makeAuthoringSession,
   makeRepairArmedSession,
@@ -119,6 +119,38 @@ test('write() carries a recorded wait --raw through to the script and back', ()
   expect(parsed.actions[0]?.flags.snapshotRaw).toBe(true);
 });
 
+test.each(['authored', 'explicit-record'] as const)(
+  'healed output preserves a snapshot with %s publication intent',
+  (intent) => {
+    const root = mkdtempForTestSync('agent-device-script-writer-retained-snapshot-');
+    const writer = new SessionScriptWriter(path.join(root, 'sessions'));
+    const session = makeRepairCompleteSession('default');
+    const req = {
+      token: 't',
+      session: 'default',
+      command: 'snapshot',
+      positionals: [],
+      flags:
+        intent === 'explicit-record'
+          ? {
+              record: true,
+              snapshotInteractiveOnly: true,
+            }
+          : { snapshotInteractiveOnly: true },
+      ...(intent === 'authored' ? { internal: { replayPlanStep: true } } : {}),
+    };
+    recordActionEntry(session, {
+      command: req.command,
+      positionals: req.positionals,
+      flags: req.flags,
+      interactiveObservation: isInteractiveObservation(req),
+    });
+    const { parsed } = writeAndParse(writer, session);
+    expect(parsed.actions.map((entry) => entry.command)).toEqual(['snapshot']);
+    expect(parsed.actions[0]?.flags.snapshotInteractiveOnly).toBe(true);
+  },
+);
+
 test('a boundary-sliced script still strips diagnostic snapshot actions', () => {
   const root = mkdtempForTestSync('agent-device-script-writer-snapshot-strip-');
   const writer = new SessionScriptWriter(path.join(root, 'sessions'));
@@ -127,6 +159,7 @@ test('a boundary-sliced script still strips diagnostic snapshot actions', () =>
     actions: [
       action({ command: 'open', positionals: ['Demo'] }),
       action({ command: 'snapshot', positionals: [] }),
+      action({ command: 'snapshot', positionals: [], flags: { record: true } }),
       action({ command: 'click', positionals: ['label="Save"'] }),
     ],
   });
@@ -834,3 +867,32 @@ test('#1533: an ARMED authoring recording is untouched by the abort gate', () =>
   const { parsed } = writeAndParse(writer, session);
   expect(parsed.actions.map((a) => a.command)).toEqual(['click']);
 });
+
+test.each([
+  { record: undefined, replayPlanStep: false },
+  { record: true, replayPlanStep: false },
+  { record: true, replayPlanStep: true },
+])(
+  'ordinary authoring retains only authored snapshots with record=$record and replayPlanStep=$replayPlanStep',
+  ({ record, replayPlanStep }) => {
+    const root = mkdtempForTestSync('agent-device-script-writer-authoring-snapshot-');
+    const writer = new SessionScriptWriter(path.join(root, 'sessions'));
+    const session = makeAuthoringSession('default', {
+      actions: [action({ command: 'open', positionals: ['Demo'] })],
+    });
+    recordActionEntry(session, {
+      command: 'snapshot',
+      positionals: [],
+      flags: {
+        record,
+        snapshotInteractiveOnly: true,
+      },
+      interactiveObservation: !replayPlanStep,
+    });
+
+    const { parsed } = writeAndParse(writer, session);
+    expect(parsed.actions.map((entry) => entry.command)).toEqual(
+      replayPlanStep ? ['open', 'snapshot'] : ['open'],
+    );
+  },
+);

packages/daemon/src/core/session-action-recorder.ts

@@ -86,6 +86,13 @@ export function recordActionEntry(
     runtime: recordedEntry.runtime,
     flags: sanitizeFlags(recordedEntry.flags),
     result: recordedEntry.result,
+    ...(OBSERVATION_ONLY_COMMANDS.has(recordedEntry.command)
+      ? recordedEntry.interactiveObservation === false
+        ? { publicationIntent: 'authored' as const }
+        : recordedEntry.flags.record === true
+          ? { publicationIntent: 'explicit-record' as const }
+          : {}
+      : {}),
     ...(recordedEntry.targetEvidence ? { targetEvidence: recordedEntry.targetEvidence } : {}),
     ...(recordedEntry.targetEvidences ? { targetEvidences: recordedEntry.targetEvidences } : {}),
   };
@@ -360,7 +367,7 @@ function isExcludedRepairSegmentObservation(
 ): boolean {
   if (!entry.interactiveObservation) return false;
   if (repairSessionBoundary(session) === undefined) return false;
-  return entry.flags?.record !== true;
+  return entry.flags.record !== true;
 }
 
 // The keys a recorded action carries derive from each flag declaration's `recorded`

packages/daemon/src/core/session-script-writer.ts

@@ -312,7 +312,13 @@ function buildOptimizedActions(
   const relevantActions = session.actions.slice(boundary ?? 0);
   const optimized: SessionAction[] = [];
   for (const action of relevantActions) {
-    if (action.command === 'snapshot') continue;
+    if (
+      action.command === 'snapshot' &&
+      action.publicationIntent !== 'authored' &&
+      !(repairArmed && action.publicationIntent === 'explicit-record')
+    ) {
+      continue;
+    }
     const optimizedAction = optimizeSelectorChainAction(action);
     if (optimizedAction) {
       optimized.push(optimizedAction);