diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a9c8b4468..4f2e4491fc 100644 --- a/CHANGELOG.md +++ b/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 `. 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. - `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 ` 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. diff --git a/packages/command-registry/src/flag-definitions-action.ts b/packages/command-registry/src/flag-definitions-action.ts index 9df695c6ec..0a760234ee 100644 --- a/packages/command-registry/src/flag-definitions-action.ts +++ b/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, }, ]; diff --git a/packages/contracts/src/session-action.ts b/packages/contracts/src/session-action.ts index 666bce9492..4a4b2a79da 100644 --- a/packages/contracts/src/session-action.ts +++ b/packages/contracts/src/session-action.ts @@ -28,6 +28,8 @@ export type SessionAction = { noRecord?: boolean; }; result?: Record; + /** 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. diff --git a/packages/daemon/src/core/__tests__/request-runtime-binding-router.test.ts b/packages/daemon/src/core/__tests__/request-runtime-binding-router.test.ts index 15cd4e57fc..85474e876a 100644 --- a/packages/daemon/src/core/__tests__/request-runtime-binding-router.test.ts +++ b/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((resolve) => { + markCleanupStarted = resolve; + }); + const cleanupFinished = new Promise((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 | undefined }) => void = () => {}; + const queued = new Promise<{ previous: Promise | 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) { 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) { }); 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({ diff --git a/packages/daemon/src/core/__tests__/session-action-recorder.test.ts b/packages/daemon/src/core/__tests__/session-action-recorder.test.ts index 97fb0b2b6d..94d2c33467 100644 --- a/packages/daemon/src/core/__tests__/session-action-recorder.test.ts +++ b/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']); }); diff --git a/packages/daemon/src/core/__tests__/session-script-writer.test.ts b/packages/daemon/src/core/__tests__/session-script-writer.test.ts index 6e88b8cd3a..fe63725e46 100644 --- a/packages/daemon/src/core/__tests__/session-script-writer.test.ts +++ b/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'], + ); + }, +); diff --git a/packages/daemon/src/core/session-action-recorder.ts b/packages/daemon/src/core/session-action-recorder.ts index 3617e1c251..7f832ad7c3 100644 --- a/packages/daemon/src/core/session-action-recorder.ts +++ b/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` diff --git a/packages/daemon/src/core/session-script-writer.ts b/packages/daemon/src/core/session-script-writer.ts index 3c410ca358..2f76acd75c 100644 --- a/packages/daemon/src/core/session-script-writer.ts +++ b/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); diff --git a/packages/daemon/src/server/request-execution-scope.ts b/packages/daemon/src/server/request-execution-scope.ts index 21eef92c7a..26f2cb74ee 100644 --- a/packages/daemon/src/server/request-execution-scope.ts +++ b/packages/daemon/src/server/request-execution-scope.ts @@ -266,6 +266,27 @@ export async function createRequestExecutionScope(params: { claimRecoveryGateway: params.claimRecoveryGateway, }); + let bindingCleanup: Promise | undefined; + const disposeBindings = (): Promise => { + bindingCleanup ??= (async () => { + try { + await runtimeBindings?.[Symbol.asyncDispose](); + } finally { + await claimAdmission?.[Symbol.asyncDispose](); + } + })(); + return bindingCleanup; + }; + const runAdmittedAndDispose = async (task: () => Promise): Promise => { + try { + return await scope.runAdmitted(task); + } finally { + // The router consumes the same cleanup outcome when it finalizes the response, + // preserving a primary command failure over a secondary disposal failure. + await Promise.allSettled([disposeBindings()]); + } + }; + const scope: RequestExecutionScope = { req: scopedReq, command, @@ -330,22 +351,16 @@ export async function createRequestExecutionScope(params: { runLocked: async (task) => { throwIfRequestCanceled(scopedReq.meta?.requestId); if (!openWait) { - return await requestExecutionLocks.run(async () => await scope.runAdmitted(task)); + return await requestExecutionLocks.run(async () => await runAdmittedAndDispose(task)); } return await openWait.runWhenDeviceIsUnheld({ acquireLocks: requestExecutionLocks.run, - task: async () => await scope.runAdmitted(task), + task: async () => await runAdmittedAndDispose(task), }); }, // Claims outlive the bindings they guard: release only once no device // operation from this request can still run. - [Symbol.asyncDispose]: async () => { - try { - await runtimeBindings?.[Symbol.asyncDispose](); - } finally { - await claimAdmission?.[Symbol.asyncDispose](); - } - }, + [Symbol.asyncDispose]: disposeBindings, }; requestScopeFinalizers.set(scope, (response) => { if (shouldRecordRequestEvents) { diff --git a/packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts b/packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts index 9787199b53..8c44f5e52e 100644 --- a/packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts +++ b/packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts @@ -281,85 +281,88 @@ test('diagnostic get/is reads mid-repair are excluded from the healed script by expect(freshRun.ok).toBe(true); }); -test("a --record'ed diagnostic read lands in the healed script (the diverged-step-was-a-get case)", async () => { - const ctx = setup('agent-device-repair-record-exclusion-record-'); - const filePath = writeReplayFile(ctx.root, [ - 'open "Demo" --relaunch', - SAVE_ANNOTATION, - 'click id="save"', - 'click id="confirm"', - 'close', - ]); - const { invoke, divergence } = await armAndDiverge(ctx, filePath); - const session = ctx.sessionStore.get(ctx.sessionName)!; +test.each(['get', 'snapshot'])( + "a --record'ed %s correction satisfies the resume guard and persists in the healed script", + async (command) => { + const ctx = setup('agent-device-repair-record-exclusion-record-'); + const filePath = writeReplayFile(ctx.root, [ + 'open "Demo" --relaunch', + SAVE_ANNOTATION, + 'click id="save"', + 'click id="confirm"', + 'close', + ]); + const { invoke, divergence } = await armAndDiverge(ctx, filePath); + const session = ctx.sessionStore.get(ctx.sessionName)!; - // --- The agent's correction for the diverged step IS itself a read (the - // wave-3 E3 shape): `get attrs` on the renamed control, explicitly forced - // into the heal with `--record` since it would otherwise be excluded. --- - ctx.sessionStore.recordAction(storeSessionForTest(ctx.sessionStore, session), { - command: 'get', - positionals: ['attrs', '@e7'], - flags: { record: true }, - result: { selectorChain: ['id="save-v2"'] }, - interactiveObservation: true, - }); - expect(session.actions.map((a) => a.command)).toEqual(['open', 'get']); + // --- The agent's correction for the diverged step IS itself a read (the + // wave-3 E3 shape): `get attrs` on the renamed control, explicitly forced + // into the heal with `--record` since it would otherwise be excluded. --- + ctx.sessionStore.recordAction(storeSessionForTest(ctx.sessionStore, session), { + command, + positionals: command === 'get' ? ['attrs', '@e7'] : [], + flags: { record: true }, + result: { selectorChain: ['id="save-v2"'] }, + interactiveObservation: true, + }); + expect(session.actions.map((a) => a.command)).toEqual(['open', command]); - const leg2 = await runReplayForTest({ - req: baseReq({ - positionals: [filePath], - flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest }, - }), - sessionName: ctx.sessionName, - logPath: ctx.logPath, - sessionStore: ctx.sessionStore, - invoke, - }); - expect(leg2.ok).toBe(true); - expect(session.actions.map((a) => a.command)).toEqual(['open', 'get', 'click']); + const leg2 = await runReplayForTest({ + req: baseReq({ + positionals: [filePath], + flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest }, + }), + sessionName: ctx.sessionName, + logPath: ctx.logPath, + sessionStore: ctx.sessionStore, + invoke, + }); + expect(leg2.ok).toBe(true); + expect(session.actions.map((a) => a.command)).toEqual(['open', command, 'click']); - const closeResponse = await handleCloseCommand({ - platformServices: daemonPlatformServicesFixture(), - req: { - token: 't', - session: ctx.sessionName, - command: 'close', - positionals: [], - flags: { saveScript: true }, - }, - sessionName: ctx.sessionName, - logPath: ctx.logPath, - sessionStore: ctx.sessionStore, - leaseRegistry: ctx.leaseRegistry, - }); - expect(closeResponse.ok).toBe(true); + const closeResponse = await handleCloseCommand({ + platformServices: daemonPlatformServicesFixture(), + req: { + token: 't', + session: ctx.sessionName, + command: 'close', + positionals: [], + flags: { saveScript: true }, + }, + sessionName: ctx.sessionName, + logPath: ctx.logPath, + sessionStore: ctx.sessionStore, + leaseRegistry: ctx.leaseRegistry, + }); + expect(closeResponse.ok).toBe(true); - const healedPath = path.join(ctx.root, 'flow.healed.ad'); - const healedScript = fs.readFileSync(healedPath, 'utf8'); - const parsed = parseReplayScriptDetailed(healedScript); - // The --record'ed corrective read DOES land in the heal, resolved to a - // selector (never a bare @ref). - expect(parsed.actions.map((a) => a.command)).toEqual(['open', 'get', 'click', 'close']); - const getAction = parsed.actions.find((a) => a.command === 'get'); - expect(getAction?.positionals).toEqual(['attrs', 'id="save-v2"']); - const bareRefs = parsed.actions.flatMap((a) => a.positionals.filter((p) => p.startsWith('@'))); - expect(bareRefs).toEqual([]); + const healedPath = path.join(ctx.root, 'flow.healed.ad'); + const healedScript = fs.readFileSync(healedPath, 'utf8'); + const parsed = parseReplayScriptDetailed(healedScript); + // The --record'ed corrective read DOES land in the heal, resolved to a + // selector (never a bare @ref). + expect(parsed.actions.map((a) => a.command)).toEqual(['open', command, 'click', 'close']); + const correction = parsed.actions.find((a) => a.command === command); + expect(correction?.positionals).toEqual(command === 'get' ? ['attrs', 'id="save-v2"'] : []); + const bareRefs = parsed.actions.flatMap((a) => a.positionals.filter((p) => p.startsWith('@'))); + expect(bareRefs).toEqual([]); - // --- Replays cleanly in a fresh session too. --- - const fresh = setup('agent-device-repair-record-exclusion-record-fresh-'); - const freshInvoke = makeRecordingReplayInvoke({ - sessionStore: fresh.sessionStore, - sessionName: fresh.sessionName, - }); - const freshRun = await runReplayForTest({ - req: baseReq({ positionals: [healedPath] }), - sessionName: fresh.sessionName, - logPath: fresh.logPath, - sessionStore: fresh.sessionStore, - invoke: freshInvoke, - }); - expect(freshRun.ok).toBe(true); -}); + // --- Replays cleanly in a fresh session too. --- + const fresh = setup('agent-device-repair-record-exclusion-record-fresh-'); + const freshInvoke = makeRecordingReplayInvoke({ + sessionStore: fresh.sessionStore, + sessionName: fresh.sessionName, + }); + const freshRun = await runReplayForTest({ + req: baseReq({ positionals: [healedPath] }), + sessionName: fresh.sessionName, + logPath: fresh.logPath, + sessionStore: fresh.sessionStore, + invoke: freshInvoke, + }); + expect(freshRun.ok).toBe(true); + }, +); test('empty-segment guard: a --from resume refuses with an actionable --record hint when only excluded diagnostic reads happened since the divergence', async () => { const ctx = setup('agent-device-repair-record-exclusion-empty-segment-'); @@ -390,6 +393,12 @@ test('empty-segment guard: a --from resume refuses with an actionable --record h result: { found: true }, interactiveObservation: true, }); + ctx.sessionStore.recordAction(storeSessionForTest(ctx.sessionStore, session), { + command: 'snapshot', + positionals: [], + flags: {}, + interactiveObservation: true, + }); expect(session.actions.map((a) => a.command)).toEqual(['open']); const blindResume = await runReplayForTest({ @@ -430,3 +439,54 @@ test('non-repair authoring recording is unchanged: a read in a fresh `open --sav // Recorded normally — no `--record` needed, no repair segment in play. expect(session.actions.map((a) => a.command)).toEqual(['get']); }); + +test('an authored snapshot plan step survives replay and close into the healed file', async () => { + const ctx = setup('agent-device-repair-authored-snapshot-'); + const filePath = writeReplayFile(ctx.root, ['open "Demo" --relaunch', 'snapshot -i', 'close']); + const invoke = makeRecordingReplayInvoke({ + sessionStore: ctx.sessionStore, + sessionName: ctx.sessionName, + }); + const replay = await runReplayForTest({ + req: baseReq({ positionals: [filePath], flags: { saveScript: true } }), + sessionName: ctx.sessionName, + logPath: ctx.logPath, + sessionStore: ctx.sessionStore, + invoke, + }); + expect(replay.ok).toBe(true); + + const closed = await handleCloseCommand({ + platformServices: daemonPlatformServicesFixture(), + req: { + token: 't', + session: ctx.sessionName, + command: 'close', + positionals: [], + flags: { saveScript: true }, + }, + sessionName: ctx.sessionName, + logPath: ctx.logPath, + sessionStore: ctx.sessionStore, + leaseRegistry: ctx.leaseRegistry, + }); + expect(closed.ok).toBe(true); + + const healedPath = path.join(ctx.root, 'flow.healed.ad'); + const parsed = parseReplayScriptDetailed(fs.readFileSync(healedPath, 'utf8')); + expect(parsed.actions.map((action) => action.command)).toEqual(['open', 'snapshot', 'close']); + expect(parsed.actions[1]?.flags.snapshotInteractiveOnly).toBe(true); + + const fresh = setup('agent-device-repair-authored-snapshot-fresh-'); + const freshRun = await runReplayForTest({ + req: baseReq({ positionals: [healedPath] }), + sessionName: fresh.sessionName, + logPath: fresh.logPath, + sessionStore: fresh.sessionStore, + invoke: makeRecordingReplayInvoke({ + sessionStore: fresh.sessionStore, + sessionName: fresh.sessionName, + }), + }); + expect(freshRun.ok).toBe(true); +}); diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-open-foreground.test.ts b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-open-foreground.test.ts index 68f32315f7..1879c8c52b 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-open-foreground.test.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-open-foreground.test.ts @@ -1,6 +1,8 @@ import type { DaemonResponse } from '@agent-device/kernel/contracts'; import { daemonPlatformServicesFixture } from '../../../../__tests__/support/platform-services-fixture.ts'; import { beforeEach, expect, test, vi } from 'vitest'; +import { snapshotRuntimeFixture } from '../../../../__tests__/support/snapshot-runtime-fixture.ts'; +import { repairPublication } from '../../../../__tests__/support/session-factories.ts'; import { fakeCreateCommandSurface } from '../../../../__tests__/support/command-surface-fake.ts'; const dispatchSnapshotViaRuntime = vi.hoisted(() => vi.fn()); @@ -209,7 +211,7 @@ test('attaches the initial INTERACTIVE snapshot by delegating to the existing sn ...req, command: 'snapshot', positionals: [], - flags: { ...req.flags, snapshotInteractiveOnly: true }, + flags: { ...req.flags, snapshotInteractiveOnly: true, noRecord: true }, }, sessionName: 'default', sessionRef: ref, @@ -312,3 +314,31 @@ test('a THROWN snapshot-capture failure never masks the successful open either', ]); } }); + +test.each([undefined, true])( + 'a replayed foreground open does not record its composite snapshot (record=%s)', + async (record) => { + const realSnapshotRuntime = await vi.importActual< + typeof import('../../../snapshot-runtime.ts') + >('../../../snapshot-runtime.ts'); + dispatchSnapshotViaRuntime.mockImplementation(realSnapshotRuntime.dispatchSnapshotViaRuntime); + sessionStore.update(ref, { scriptPublication: repairPublication('complete') }); + + const result = await composeOpenWithInitialSnapshot({ + ...snapshotRuntimeFixture(), + createCommandSurface: fakeCreateCommandSurface, + req: baseRequest({ + flags: { foreground: true, record }, + positionals: ['Demo'], + internal: { replayPlanStep: true }, + }), + ref, + logPath: '/tmp/daemon.log', + sessionStore, + openResponse: okOpenResponse, + }); + + expect(result).toMatchObject({ ok: true, data: { snapshot: { nodes: [] } } }); + expect(sessionStore.requireCurrent(ref).actions).toEqual([]); + }, +); diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/session-open-foreground.ts b/packages/daemon/src/sessions/session-lifecycle/internal/session-open-foreground.ts index ec96f79bd1..bc5e329202 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/session-open-foreground.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/session-open-foreground.ts @@ -128,7 +128,7 @@ export async function composeOpenWithInitialSnapshot(params: { // The promised composition IS `snapshot -i`: the CLI maps `-i` to // `snapshotInteractiveOnly` (flag-definitions-workflow.ts), which is the // key the snapshot runtime reads as `interactiveOnly`. - flags: { ...req.flags, snapshotInteractiveOnly: true }, + flags: { ...req.flags, snapshotInteractiveOnly: true, noRecord: true }, }, sessionName: ref.address, sessionRef: ref, diff --git a/src/__tests__/cli-config-retired-record.test.ts b/src/__tests__/cli-config-retired-record.test.ts new file mode 100644 index 0000000000..3c8f0d4657 --- /dev/null +++ b/src/__tests__/cli-config-retired-record.test.ts @@ -0,0 +1,101 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { runCliCapture } from './cli-capture.ts'; +import { makeTempWorkspace } from './cli-config-fixtures.ts'; + +/** + * The retired `record` config surface (`src/commands/schema/cli-config.ts`): + * the config key `record` and AGENT_DEVICE_RECORD were released as command + * defaults, but the repair-segment recording opt-in they feed needs + * per-invocation provenance a config default cannot carry. They fail closed + * with migration guidance instead of silently doing nothing. + */ + +test('retired record config key is refused with migration guidance on every config surface', async () => { + const { root, home, project } = makeTempWorkspace(); + fs.mkdirSync(path.join(home, '.agent-device'), { recursive: true }); + fs.writeFileSync(path.join(project, 'agent-device.json'), JSON.stringify({ record: true })); + fs.writeFileSync(path.join(home, '.agent-device', 'config.json'), JSON.stringify({})); + + const result = await runCliCapture(['snapshot'], { + cwd: project, + env: { HOME: home }, + defaultResponse: { ok: true, data: {} }, + }); + assert.equal(result.code, 1); + assert.match(result.stderr, /"record" in project config file .* was removed/); + assert.match(result.stderr, /--record is per-invocation only/); + + fs.rmSync(path.join(project, 'agent-device.json')); + fs.writeFileSync( + path.join(home, '.agent-device', 'config.json'), + JSON.stringify({ record: true }), + ); + const fromUserConfig = await runCliCapture(['snapshot'], { + cwd: project, + env: { HOME: home }, + defaultResponse: { ok: true, data: {} }, + }); + assert.equal(fromUserConfig.code, 1); + assert.match(fromUserConfig.stderr, /"record" in config file .* was removed/); + + fs.rmSync(root, { recursive: true, force: true }); +}); + +test('retired record config key is refused in an explicit --config file', async () => { + const { root, home, project } = makeTempWorkspace(); + const configPath = path.join(root, 'chosen.json'); + fs.writeFileSync(configPath, JSON.stringify({ record: true })); + + const result = await runCliCapture(['snapshot', '--config', configPath], { + cwd: project, + env: { HOME: home }, + defaultResponse: { ok: true, data: {} }, + }); + assert.equal(result.code, 1); + assert.match(result.stderr, /"record" in config file .*chosen\.json was removed/); + assert.match(result.stderr, /--record is per-invocation only/); + + fs.rmSync(root, { recursive: true, force: true }); +}); + +test('retired record config key is refused in an AGENT_DEVICE_CONFIG file', async () => { + const { root, home, project } = makeTempWorkspace(); + const configPath = path.join(root, 'from-env.json'); + fs.writeFileSync(configPath, JSON.stringify({ record: true })); + + const result = await runCliCapture(['snapshot'], { + cwd: project, + env: { HOME: home, AGENT_DEVICE_CONFIG: configPath }, + defaultResponse: { ok: true, data: {} }, + }); + assert.equal(result.code, 1); + assert.match(result.stderr, /"record" in config file .*from-env\.json was removed/); + + fs.rmSync(root, { recursive: true, force: true }); +}); + +test('retired AGENT_DEVICE_RECORD fails closed only for commands that accept --record', async () => { + const { root, home, project } = makeTempWorkspace(); + const env = { HOME: home, AGENT_DEVICE_RECORD: 'true' }; + + const snapshot = await runCliCapture(['snapshot'], { + cwd: project, + env, + defaultResponse: { ok: true, data: {} }, + }); + assert.equal(snapshot.code, 1); + assert.match(snapshot.stderr, /AGENT_DEVICE_RECORD was removed/); + assert.match(snapshot.stderr, /--record is per-invocation only/); + + const unaffected = await runCliCapture(['devices', '--json'], { + cwd: project, + env, + defaultResponse: { ok: true, data: { devices: [] } }, + }); + assert.equal(unaffected.code, null); + + fs.rmSync(root, { recursive: true, force: true }); +}); diff --git a/src/commands/schema/cli-config.ts b/src/commands/schema/cli-config.ts index 2db6ad4876..1e3aadb96e 100644 --- a/src/commands/schema/cli-config.ts +++ b/src/commands/schema/cli-config.ts @@ -106,6 +106,18 @@ function loadSingleConfigFile(entry: ConfigPath): Partial { }); } +// The `record` config key was released (v0.21.10) as a project/user-config default, but +// the repair-segment exclusion it feeds needs per-invocation provenance a config default +// cannot carry. The key is retired: `--record` stays a per-invocation flag, and a config +// file or environment still naming the key fails with this migration guidance rather +// than silently doing nothing. +const RETIRED_RECORD_CONFIG_KEY = { + flagKey: 'record', + envVar: 'AGENT_DEVICE_RECORD', + migration: + '--record is per-invocation only: pass --record on the command, or record: true in the SDK/MCP call, to keep a corrective read in a healed script', +} as const; + function parseConfigObject( source: Record, origin: { source: ConfigFileSource; label: string }, @@ -115,6 +127,12 @@ function parseConfigObject( ([key]) => key !== 'plugins' || origin.source !== 'user', ); for (const [rawKey, rawValue] of entries) { + if (rawKey === RETIRED_RECORD_CONFIG_KEY.flagKey) { + throw new AppError( + 'INVALID_ARGS', + `Config key "${rawKey}" in ${origin.label} was removed; ${RETIRED_RECORD_CONFIG_KEY.migration}.`, + ); + } const key = rawKey as FlagKey; const spec = getOptionSpec(key); if (!spec) { @@ -158,6 +176,17 @@ function parseConfigObject( const RETIRED_MAX_SIZE_ENV_COMMANDS = new Set(['screenshot', 'record']); function readEnvFlagDefaults(env: EnvMap, command: string | null): Partial { + if ( + command !== null && + getOptionSpec(RETIRED_RECORD_CONFIG_KEY.flagKey)?.supportsCommand(command) === true && + typeof env[RETIRED_RECORD_CONFIG_KEY.envVar] === 'string' && + env[RETIRED_RECORD_CONFIG_KEY.envVar]!.trim().length > 0 + ) { + throw new AppError( + 'INVALID_ARGS', + `${RETIRED_RECORD_CONFIG_KEY.envVar} was removed; ${RETIRED_RECORD_CONFIG_KEY.migration}.`, + ); + } const retiredEnvValue = env[RETIRED_SCREENSHOT_MAX_SIZE.envVar]; if ( command !== null && diff --git a/test/integration/provider-scenarios/cli-record-publication.test.ts b/test/integration/provider-scenarios/cli-record-publication.test.ts new file mode 100644 index 0000000000..cc61adaae6 --- /dev/null +++ b/test/integration/provider-scenarios/cli-record-publication.test.ts @@ -0,0 +1,231 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test, onTestFinished, vi } from 'vitest'; +import * as codeSignature from '@agent-device/host-kit/code-signature'; +import { resolveLocalDaemonCodeIdentity } from '../../../src/daemon-client/daemon-launch-spec.ts'; +import { parseReplayScriptDetailed } from '@agent-device/ad-script'; +import { createProviderScenarioHarness } from './harness.ts'; +import { PROVIDER_SCENARIO_ANDROID } from './fixtures.ts'; +import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from '../../../src/__tests__/test-utils/android-snapshot-helper.ts'; +import { + createAndroidProviderShellState, + updateAndroidProviderImeShellState, +} from './android-ime-lifecycle-world.ts'; +import { androidRepairAdbResult } from './replay-repair-world.ts'; +import { runCliCapture } from '../../../src/__tests__/cli-capture.ts'; +import { makeTempWorkspace } from '../../../src/__tests__/cli-config-fixtures.ts'; +import { createAgentDeviceClient } from '../../../src/agent-device-client.ts'; +import { createCommandToolExecutor } from '../../../src/mcp/command-tools.ts'; +import { installIsolatedCliTestEnv } from '../../../src/__tests__/cli-test-env.ts'; + +async function repairPublication(root: string) { + // The runtime runs inside Vitest, so publish the daemon entry's identity instead of Vitest's. + const identity = await resolveLocalDaemonCodeIdentity(); + if (identity.origin !== 'checkout') throw new Error('expected checkout identity'); + const signature = vi + .spyOn(codeSignature, 'resolveDaemonCodeSignature') + .mockReturnValue(identity.codeSignature); + onTestFinished(() => signature.mockRestore()); + const shellState = createAndroidProviderShellState(); + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => ({ + snapshotHelperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + exec: async (args) => { + updateAndroidProviderImeShellState(args, shellState); + return androidRepairAdbResult(args, shellState); + }, + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + const selection = { platform: 'android' as const, serial: PROVIDER_SCENARIO_ANDROID.id }; + const client = daemon.client(); + try { + await client.apps.open({ app: 'settings', ...selection }); + const repairPath = path.join(root, 'repair.ad'); + fs.writeFileSync( + repairPath, + 'snapshot -i\npress label="NoSuchControl"\nis visible id=com.android.settings:id/search\n', + ); + const divergence = await client.replay + .run({ path: repairPath, saveScript: true, ...selection }) + .then( + () => { + throw new Error('expected repair divergence'); + }, + (error: { + details: { + divergence: { + resume: { from: number; planDigest: string; repairSessionHeld: boolean }; + }; + }; + }) => error.details.divergence.resume, + ); + expect(divergence.repairSessionHeld).toBe(true); + return { + config: { stateDir: daemon.stateDir, daemonTransport: 'http' as const, ...selection }, + close: () => daemon.close(), + commands: async () => { + await client.replay.run({ + path: repairPath, + resumeFrom: divergence.from + 1, + resumePlanDigest: divergence.planDigest, + ...selection, + }); + await client.apps.close({ saveScript: true }); + return parseReplayScriptDetailed( + fs.readFileSync(path.join(root, 'repair.healed.ad'), 'utf8'), + ).actions.map((action) => action.command); + }, + }; + } catch (error) { + await daemon.close(); + throw error; + } +} + +test.each([false, true])( + 'CLI keeps a corrective snapshot only with an explicit --record, not with a retired record config key=%s', + async (explicit) => { + const { root, home, project } = makeTempWorkspace(); + const publication = await repairPublication(root); + fs.writeFileSync(path.join(project, 'agent-device.json'), JSON.stringify({ record: true })); + onTestFinished(() => publication.close()); + const result = await runCliCapture( + [ + 'snapshot', + '--json', + '--session', + 'default', + '--state-dir', + publication.config.stateDir, + '--daemon-transport', + 'http', + '--platform', + 'android', + '--serial', + publication.config.serial, + ...(explicit ? ['--record'] : []), + ], + { + cwd: project, + env: { HOME: home, USERPROFILE: home }, + useRealDaemonClient: true, + }, + ); + // The retired project-config key fails closed with migration guidance, so the + // explicit `--record` call never reaches the repair session either. + expect(result.code).toBe(1); + const failureOutput = `${result.stdout}${result.stderr}`; + expect(failureOutput).toMatch(/\\"record\\" in project config file .* was removed/); + expect(failureOutput).toMatch(/--record is per-invocation only/); + }, +); + +test('CLI keeps an explicitly recorded corrective snapshot with no config key present', async () => { + const { root, home, project } = makeTempWorkspace(); + const publication = await repairPublication(root); + onTestFinished(() => publication.close()); + const result = await runCliCapture( + [ + 'snapshot', + '--json', + '--session', + 'default', + '--state-dir', + publication.config.stateDir, + '--daemon-transport', + 'http', + '--platform', + 'android', + '--serial', + publication.config.serial, + '--record', + ], + { + cwd: project, + env: { HOME: home, USERPROFILE: home }, + useRealDaemonClient: true, + }, + ); + expect(result.code, result.stderr || result.stdout).not.toBe(1); + expect(await publication.commands(), JSON.stringify(result)).toEqual([ + 'snapshot', + 'snapshot', + 'is', + 'close', + ]); +}); + +test.each([false, true])( + 'MCP keeps a corrective snapshot only with an explicit record input, not with a retired record config key=%s', + async (explicit) => { + const { root, home, project } = makeTempWorkspace(); + const publication = await repairPublication(root); + fs.writeFileSync(path.join(project, 'agent-device.json'), JSON.stringify({ record: true })); + onTestFinished(() => publication.close()); + const restoreEnv = installIsolatedCliTestEnv({ + HOME: home, + USERPROFILE: home, + AGENT_DEVICE_STATE_DIR: publication.config.stateDir, + AGENT_DEVICE_DAEMON_TRANSPORT: 'http', + }); + const previousCwd = process.cwd(); + process.chdir(project); + try { + const executor = createCommandToolExecutor(); + // Config resolution runs before the command and rejects the call with the + // migration error, whether or not the tool input carries `record`. + await expect( + executor.execute('snapshot', { + platform: 'android', + serial: publication.config.serial, + ...(explicit ? { record: true } : {}), + }), + ).rejects.toThrow(/"record" in project config file .* was removed/); + } finally { + process.chdir(previousCwd); + restoreEnv(); + } + }, +); + +test('MCP keeps an explicitly recorded corrective snapshot with no config key present', async () => { + const { root, home, project } = makeTempWorkspace(); + const publication = await repairPublication(root); + onTestFinished(() => publication.close()); + const restoreEnv = installIsolatedCliTestEnv({ + HOME: home, + USERPROFILE: home, + AGENT_DEVICE_STATE_DIR: publication.config.stateDir, + AGENT_DEVICE_DAEMON_TRANSPORT: 'http', + }); + const previousCwd = process.cwd(); + process.chdir(project); + try { + const executor = createCommandToolExecutor(); + const result = await executor.execute('snapshot', { + platform: 'android', + serial: publication.config.serial, + record: true, + }); + expect(result.isError, JSON.stringify(result)).toBe(false); + expect(await publication.commands(), JSON.stringify(result)).toEqual([ + 'snapshot', + 'snapshot', + 'is', + 'close', + ]); + } finally { + process.chdir(previousCwd); + restoreEnv(); + } +}); + +test('SDK per-call record explicitly retains a corrective snapshot', async () => { + const { root } = makeTempWorkspace(); + const publication = await repairPublication(root); + onTestFinished(() => publication.close()); + const client = createAgentDeviceClient(publication.config); + await client.capture.snapshot({ record: true }); + expect(await publication.commands()).toEqual(['snapshot', 'snapshot', 'is', 'close']); +}); diff --git a/test/integration/provider-scenarios/harness.ts b/test/integration/provider-scenarios/harness.ts index fd26d9a11a..13f7036f7d 100644 --- a/test/integration/provider-scenarios/harness.ts +++ b/test/integration/provider-scenarios/harness.ts @@ -70,6 +70,7 @@ export type ProviderScenarioHarness = { ) => Promise; client: () => AgentDeviceClient; hasSession: (session?: string) => Promise; + stateDir: string; baseUrl: string; httpPort: number; token: string; @@ -268,6 +269,7 @@ export async function createProviderScenarioHarness( client: () => client, hasSession: async (name = 'default') => (await client.sessions.list()).some((session) => session.name === name), + stateDir, baseUrl, httpPort: runtime.httpPort, token: runtime.token, diff --git a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts index 6386244ec0..bc04751795 100644 --- a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts +++ b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts @@ -17,9 +17,8 @@ import { test } from 'vitest'; import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from '../../../src/__tests__/test-utils/android-snapshot-helper.ts'; import type { AndroidAdbProvider } from '@agent-device/platform-android/mechanics'; import { assertRpcError, assertRpcOk } from './assertions.ts'; -import { androidSettingsXml, androidSnapshotHelperOutput } from './android-world.ts'; +import { androidRepairAdbResult } from './replay-repair-world.ts'; import { - androidImeLifecycleAdbResult, createAndroidProviderShellState, updateAndroidProviderImeShellState, } from './android-ime-lifecycle-world.ts'; @@ -136,11 +135,12 @@ async function assertRepairSegmentReadExclusion(params: { // (b) The corrective read (the wave-3 E3 shape: the diverged step is itself // a read), forced into the heal with `--record`. - const correctiveRead = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { + await daemon.client().interactions.get({ + format: 'text', + selector: SEARCH_SELECTOR, ...selection, record: true, }); - assertRpcOk(correctiveRead); // (c) `--record` and `--no-record` are opposite intents for one action. const conflictingFlags = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { @@ -183,33 +183,3 @@ function healedLines(script: string, command: string): string[] { .map((line) => line.trim()) .filter((line) => line === command || line.startsWith(`${command} `)); } - -function androidRepairAdbResult( - args: readonly string[], - shellState: ReturnType, -): { - stdout: string; - stderr: string; - exitCode: number; -} { - const ime = androidImeLifecycleAdbResult(args.join(' '), args, shellState); - if (ime) return ime; - if (args.join(' ') === 'shell getprop sys.boot_completed') { - return { stdout: '1\n', stderr: '', exitCode: 0 }; - } - if (args.join(' ') === 'shell dumpsys window windows') { - return { - stdout: 'mCurrentFocus=Window{42 u0 com.android.settings/.Settings}\n', - stderr: '', - exitCode: 0, - }; - } - if (args.join(' ').startsWith('shell am instrument ')) { - return { - stdout: androidSnapshotHelperOutput(androidSettingsXml('Display')), - stderr: '', - exitCode: 0, - }; - } - return { stdout: '', stderr: '', exitCode: 0 }; -} diff --git a/test/integration/provider-scenarios/replay-repair-world.ts b/test/integration/provider-scenarios/replay-repair-world.ts new file mode 100644 index 0000000000..9bbd129919 --- /dev/null +++ b/test/integration/provider-scenarios/replay-repair-world.ts @@ -0,0 +1,35 @@ +import { androidSettingsXml, androidSnapshotHelperOutput } from './android-world.ts'; +import { + androidImeLifecycleAdbResult, + createAndroidProviderShellState, +} from './android-ime-lifecycle-world.ts'; + +export function androidRepairAdbResult( + args: readonly string[], + shellState: ReturnType, +): { + stdout: string; + stderr: string; + exitCode: number; +} { + const ime = androidImeLifecycleAdbResult(args.join(' '), args, shellState); + if (ime) return ime; + if (args.join(' ') === 'shell getprop sys.boot_completed') { + return { stdout: '1\n', stderr: '', exitCode: 0 }; + } + if (args.join(' ') === 'shell dumpsys window windows') { + return { + stdout: 'mCurrentFocus=Window{42 u0 com.android.settings/.Settings}\n', + stderr: '', + exitCode: 0, + }; + } + if (args.join(' ').startsWith('shell am instrument ')) { + return { + stdout: androidSnapshotHelperOutput(androidSettingsXml('Display')), + stderr: '', + exitCode: 0, + }; + } + return { stdout: '', stderr: '', exitCode: 0 }; +} diff --git a/website/docs/docs/configuration.md b/website/docs/docs/configuration.md index f72b6af2ae..eb2896d7b0 100644 --- a/website/docs/docs/configuration.md +++ b/website/docs/docs/configuration.md @@ -96,6 +96,7 @@ explicit config: - local code and write destinations (`reporter`, `reportJunit`, `saveScript`, `launchConsole`) Project config can also set `snapshotDepth`, `snapshotScope`, `screenshotScale`, `activity`, `relaunch`, `shutdown`, `fps`, and `quality`. Local paths and executable modules, such as `stepsFile` and `reporter`, are allowed only in user or explicit config. +The `record` config key and `AGENT_DEVICE_RECORD` were removed. `--record` is per-invocation only: pass it on the command, or set `record: true` in an SDK/MCP call, to keep a corrective read in a healed script. `install-from-source` can read a GitHub Actions artifact source from user or explicit config when the remote daemon supports resolving CI artifacts. Repository config rejects this key: diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 497177f3dd..120cf72652 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -351,6 +351,8 @@ Text output prints a compact summary of the same fields; `--json`/MCP carry the ## Resume a failed replay +During a repair armed with `replay --save-script`, diagnostic reads stay out of the healed script. To keep a corrective read, pass `--record` on that invocation, or `record: true` in that SDK/MCP call. `--record` is per-invocation only: the config key `record` and `AGENT_DEVICE_RECORD` were removed, and a config file or environment still naming them fails with migration guidance. Authored replay observations are retained automatically. + `replay --from --plan-digest ` resumes **at** plan step `n`, not after it, skipping `1..n-1` without executing them. Both flags come from a divergence report's `resume` field — `from` is the failed step, `planDigest` is the digest of the exact unchanged plan that produced it. Choose one recovery workflow: