diff --git a/contracts/fixtures/dispatch-disclosure.json b/contracts/fixtures/dispatch-disclosure.json index 7bf37cbdb3..d11a1d70a9 100644 --- a/contracts/fixtures/dispatch-disclosure.json +++ b/contracts/fixtures/dispatch-disclosure.json @@ -107,6 +107,12 @@ "trigger": "a device-bound command with no active session and no device selector: requireSessionOrExplicitSelector refuses before any target resolves, carrying details.reason session_or_device_selector_required", "dispatched": "no" }, + { + "id": "daemon.route.named-session-refusal", + "producer": "daemon", + "trigger": "a command naming a session this daemon does not hold and no device selector: requireNamedSessionHoldsDevice refuses with SESSION_NOT_FOUND and details.reason named_session_not_found before any device is listed or resolved", + "dispatched": "no" + }, { "id": "daemon.route.find-type-refused-after-focus", "producer": "daemon", diff --git a/packages/capture-kit/src/capture-admission/screen-recording-stop-recovery.ts b/packages/capture-kit/src/capture-admission/screen-recording-stop-recovery.ts index 585a292243..74fec4dc93 100644 --- a/packages/capture-kit/src/capture-admission/screen-recording-stop-recovery.ts +++ b/packages/capture-kit/src/capture-admission/screen-recording-stop-recovery.ts @@ -8,7 +8,12 @@ import type { ScreenRecordingCompletion, } from '@agent-device/contracts/screen-recording-runtime'; import type { DurableResourceEnvelope } from '@agent-device/contracts/durable-resource-envelope'; -import { deviceIdentity, sameDeviceIdentity, type DeviceInfo } from '@agent-device/kernel/device'; +import { + deviceIdentity, + sameDeviceIdentity, + type DeviceIdentity, + type DeviceInfo, +} from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { isRecord } from '@agent-device/kernel/record'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; @@ -70,6 +75,21 @@ export function screenRecordingManifestIsTerminal(params: ScreenRecordingManifes return record.status === 'decoded' && record.envelope.lifecycle === 'completed'; } +/** + * The device a stop without a held session must be answered on: the one the session's durable + * manifest names, when a decodable manifest exists. `undefined` means there is nothing to recover + * from, and a caller that addressed a session answers on the session instead of choosing a device. + * The caller still resolves this identity against the host to recover the live device fields, and + * `assertManifestBelongsToRequest` still fails the request closed when the resolved device or the + * session address does not match what the manifest recorded. + */ +export function screenRecordingManifestDevice( + params: ScreenRecordingManifestParams, +): DeviceIdentity | undefined { + const { record } = readSessionManifest(params); + return record.status === 'decoded' ? record.envelope.device : undefined; +} + function readSessionManifest(params: ScreenRecordingManifestParams): Readonly<{ resourcePath: string; record: DurableCaptureResourceRecord<'screen-recording'>; diff --git a/packages/command-registry/src/__tests__/batch.test.ts b/packages/command-registry/src/__tests__/batch.test.ts index d62d8411b0..a3018a81e3 100644 --- a/packages/command-registry/src/__tests__/batch.test.ts +++ b/packages/command-registry/src/__tests__/batch.test.ts @@ -58,6 +58,47 @@ function batchRequest(commands: string[], responseLevel?: ResponseLevel): BatchR }; } +// #3674: the daemon refuses a request that names a session it does not hold. A batch step is +// addressed through `session:`, so only a caller who named a session may see one in `flags`: +// stamping every step would hand that refusal to sessionless batches, whose steps are meant to +// resolve their device from the session or the selectors. +test('sessionless batch steps carry no session flag, so they stay sessionless at the daemon', async () => { + const seen: Array | undefined> = []; + const request: BatchRequest = { + token: 't', + command: 'batch', + positionals: [], + flags: { batchSteps: [{ command: 'snapshot' }] }, + }; + const response = await runBatch(request, 'cwd:1a2b3c4d5e6f7a8b:default', async (req) => { + seen.push(req.flags as Record | undefined); + return { ok: true, data: {} }; + }); + + assert.equal(response.ok, true); + assert.equal(seen[0]?.session, undefined); +}); + +test('a batch whose caller named a session stamps that address onto every step', async () => { + const seen: Array | undefined> = []; + const request: BatchRequest = { + token: 't', + command: 'batch', + positionals: [], + session: 'e2e-ios-0', + meta: { sessionExplicit: true }, + flags: { batchSteps: [{ command: 'snapshot' }, { command: 'get' }] }, + }; + const response = await runBatch(request, 'e2e-ios-0', async (req) => { + seen.push(req.flags as Record | undefined); + return { ok: true, data: {} }; + }); + + assert.equal(response.ok, true); + assert.equal(seen[0]?.session, 'e2e-ios-0'); + assert.equal(seen[1]?.session, 'e2e-ios-0'); +}); + // #2997: `testIme` joined INHERITED_PARENT_FLAG_KEYS for the flow command's session // opens; batch shares that list, so its steps inherit the parent's opt-in the same way. // Pinning it here keeps the shared-list addition deliberate for both consumers. diff --git a/packages/command-registry/src/batch.ts b/packages/command-registry/src/batch.ts index f0fad3e615..88144559f2 100644 --- a/packages/command-registry/src/batch.ts +++ b/packages/command-registry/src/batch.ts @@ -6,6 +6,7 @@ import { type DaemonResponse, type ResponseLevel, isNonDefaultResponseLevel, + namesSessionExplicitly, } from '@agent-device/kernel/contracts'; import { AppError, asAppError, type DaemonError } from '@agent-device/kernel/errors'; import { isRecord } from '@agent-device/kernel/record'; @@ -320,7 +321,11 @@ async function runBatchStep( const isFinalStep = stepNumber === totalSteps; const stepStartedAt = Date.now(); const stepFlags = buildBatchStepFlags(req.flags, step.flags); - if (stepFlags.session === undefined) { + // The step is already addressed through `session:` below. Stamp the flag only when the caller + // genuinely named a session: an unconditional stamp would make a sessionless batch look + // caller-addressed to the daemon's named-session refusal, which no caller of this surface asked + // for (#3674). A step's own recorded flag always wins, explicit parent or not. + if (stepFlags.session === undefined && namesSessionExplicitly(req)) { stepFlags.session = sessionName; } const response = await invoke( diff --git a/packages/daemon/src/core/explicit-session-resolution.test.ts b/packages/daemon/src/core/explicit-session-resolution.test.ts new file mode 100644 index 0000000000..fe01fbe014 --- /dev/null +++ b/packages/daemon/src/core/explicit-session-resolution.test.ts @@ -0,0 +1,119 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { DaemonWireRequest } from '@agent-device/contracts/command'; +import { PRE_DISPATCH_REFUSAL_REASONS, normalizeError } from '@agent-device/kernel/errors'; +import type { SessionState } from './session-state.ts'; +import { + namedSessionNotFoundError, + requireNamedSessionHoldsDevice, +} from './explicit-session-resolution.ts'; + +function request(overrides: Partial = {}): DaemonWireRequest { + return { + token: 't', + session: 'e2e-ios-0', + command: 'snapshot', + positionals: [], + ...overrides, + }; +} + +function heldSession(address = 'e2e-ios-0'): SessionState { + return { + name: address, + device: { platform: 'android', id: 'emulator-5554', name: 'Pixel 9', kind: 'emulator' }, + createdAt: 0, + actions: [], + }; +} + +function normalizeRefusal(error: unknown) { + assert.ok(error instanceof Error); + return normalizeError(error); +} + +test('a named session the daemon does not hold is refused on the session, not the device', () => { + assert.throws( + () => + requireNamedSessionHoldsDevice({ + req: request({ meta: { sessionExplicit: true } }), + sessionAddress: 'e2e-ios-0', + session: undefined, + }), + (error: unknown) => { + const refusal = normalizeRefusal(error); + assert.equal(refusal.code, 'SESSION_NOT_FOUND'); + assert.equal( + refusal.message, + 'No active session "e2e-ios-0". Run agent-device open --session e2e-ios-0 first, ' + + 'or omit --session to run against a device selector.', + ); + assert.equal(refusal.details?.reason, PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + assert.equal(refusal.details?.dispatched, 'no'); + assert.equal(refusal.details?.session, 'e2e-ios-0'); + assert.match(String(refusal.hint), /agent-device session list/); + return true; + }, + ); +}); + +test('the session flag alone names a session as firmly as the client meta does', () => { + // The predicate itself is pinned in @agent-device/kernel/contracts; what the guard owes is + // that BOTH carriers reach the refusal. + const fromMeta = request({ meta: { sessionExplicit: true } }); + const fromFlag = request({ flags: { session: 'e2e-ios-0' } }); + for (const req of [fromMeta, fromFlag]) { + assert.throws( + () => + requireNamedSessionHoldsDevice({ req, sessionAddress: 'e2e-ios-0', session: undefined }), + { code: 'SESSION_NOT_FOUND' }, + ); + } +}); + +test('a request that never named a session keeps its sessionless device resolution', () => { + requireNamedSessionHoldsDevice({ + req: request({ session: 'cwd:1a2b3c4d5e6f7a8b:ios' }), + sessionAddress: 'cwd:1a2b3c4d5e6f7a8b:ios', + session: undefined, + }); + requireNamedSessionHoldsDevice({ + req: request({ session: 'default', flags: {} }), + sessionAddress: 'default', + session: undefined, + }); +}); + +test('a session the daemon holds never reaches the refusal', () => { + requireNamedSessionHoldsDevice({ + req: request({ meta: { sessionExplicit: true } }), + sessionAddress: 'e2e-ios-0', + session: heldSession(), + }); +}); + +test('a named session beside a device selector keeps the target the caller stated', () => { + for (const flags of [ + { udid: 'SIM-1' }, + { serial: 'emulator-5554' }, + { device: 'iPhone 17' }, + { platform: 'ios' as const }, + { target: 'tv' as const }, + ]) { + requireNamedSessionHoldsDevice({ + req: request({ flags, meta: { sessionExplicit: true } }), + sessionAddress: 'e2e-ios-0', + session: undefined, + }); + } +}); + +test('the recovery command echoes the address back, quoted only where the shell needs it', () => { + // A scoped store key is already a safe shell word, so the hint hands back the exact address + // `session list` printed; a name with a space has to arrive quoted to run as one argument. + const scoped = normalizeRefusal(namedSessionNotFoundError('cwd:1a2b3c4d5e6f7a8b:ios')); + assert.match(String(scoped.message), /--session cwd:1a2b3c4d5e6f7a8b:ios first/); + assert.equal(scoped.details?.session, 'cwd:1a2b3c4d5e6f7a8b:ios'); + const spaced = normalizeRefusal(namedSessionNotFoundError('my session')); + assert.match(String(spaced.message), /--session 'my session' first/); +}); diff --git a/packages/daemon/src/core/explicit-session-resolution.ts b/packages/daemon/src/core/explicit-session-resolution.ts new file mode 100644 index 0000000000..b33ee79512 --- /dev/null +++ b/packages/daemon/src/core/explicit-session-resolution.ts @@ -0,0 +1,58 @@ +import type { DaemonWireRequest } from '@agent-device/contracts/command'; +import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; +import { + AppError, + namedSessionNotFoundDetails, + type RefusalDetails, +} from '@agent-device/kernel/errors'; +import { hasDeviceSelectionInput } from './device/device-selector-intent.ts'; +import type { SessionState } from './session-state.ts'; + +/** + * The answer to a request that names a session this daemon does not hold: the session, not the + * device, is what is missing, and the command stopped before anything could reach a device. + */ +export function namedSessionNotFoundError(sessionAddress: string): AppError { + const sessionArg = shellQuoteIfNeeded(sessionAddress); + return new AppError( + 'SESSION_NOT_FOUND', + `No active session "${sessionAddress}". Run agent-device open --session ${sessionArg} first, or omit --session to run against a device selector.`, + { + ...namedSessionNotFoundDetails(sessionAddress), + hint: + 'Run agent-device session list to see the sessions this daemon holds. ' + + "A daemon that was replaced or restarted starts without its predecessor's sessions.", + } satisfies RefusalDetails<'named_session_not_found'>, + ); +} + +/** + * Refuses a request whose session the daemon does not hold, at the seam where that request would + * otherwise have had its device chosen from its own selectors. + * + * The fall-through this closes is what an unheld session leaves behind. A session-attached command + * names no device, so resolving one from its flags meant picking from every device on the host: + * with one booted the caller met an app-session error, with two an `AMBIGUOUS_MATCH` telling it to + * select a device — an answer that depends on how many devices happen to be up, and that sends the + * caller after a device selector when what it lost was its session (#3674). Refusing here also skips + * the device listing the fall-through paid for. + * + * Two shapes keep the path they chose: a request that never named a session, which is using the + * daemon sessionlessly by design, and one that names a device selector as well, which states its + * target outright and needs no session to supply one. The predicate deciding the first is + * `namesSessionExplicitly` (`@agent-device/kernel/contracts`), traced from what the request carries + * rather than from the session string. + */ +export function requireNamedSessionHoldsDevice( + params: Readonly<{ + req: DaemonWireRequest; + sessionAddress: string; + session: SessionState | undefined; + }>, +): void { + if (params.session) return; + if (!namesSessionExplicitly(params.req)) return; + if (hasDeviceSelectionInput(params.req.flags)) return; + throw namedSessionNotFoundError(params.sessionAddress); +} diff --git a/packages/daemon/src/core/snapshot-runtime-binding.ts b/packages/daemon/src/core/snapshot-runtime-binding.ts index 67dff13948..12313c63be 100644 --- a/packages/daemon/src/core/snapshot-runtime-binding.ts +++ b/packages/daemon/src/core/snapshot-runtime-binding.ts @@ -154,7 +154,7 @@ export async function resolveBoundSnapshotCaptureRuntime( const { ref, session, device } = await resolveSessionDevice( sessionStore, sessionName, - req.flags, + req, params.sessionRef, ); if ( diff --git a/packages/daemon/src/core/snapshot-session.ts b/packages/daemon/src/core/snapshot-session.ts index b5dba3370a..e65439e2bd 100644 --- a/packages/daemon/src/core/snapshot-session.ts +++ b/packages/daemon/src/core/snapshot-session.ts @@ -9,12 +9,18 @@ import { SessionStore } from './session-store.ts'; export async function resolveSessionDevice( sessionStore: SessionStore, sessionName: string, - flags: DaemonRequest['flags'], + req: DaemonRequest, boundRef?: SessionRef, ) { const ref = boundRef ?? sessionStore.lookup(sessionName); const session = ref ? sessionStore.requireCurrent(ref) : undefined; - const device = session?.device ?? (await resolveTargetDevice(flags ?? {})); + // Only a missing session can be refused for being named, so only that branch pays the + // function-scoped import (ADR-0019 loading shape). + if (!session) { + const { requireNamedSessionHoldsDevice } = await import('./explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session }); + } + const device = session?.device ?? (await resolveTargetDevice(req.flags ?? {})); return { ref, session, device }; } diff --git a/packages/daemon/src/execution/request-dispatch-disclosure.test.ts b/packages/daemon/src/execution/request-dispatch-disclosure.test.ts index a618235dae..44eddbe412 100644 --- a/packages/daemon/src/execution/request-dispatch-disclosure.test.ts +++ b/packages/daemon/src/execution/request-dispatch-disclosure.test.ts @@ -34,7 +34,7 @@ vi.mock('../core/device/device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}), })); -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, PRE_DISPATCH_REFUSAL_REASONS } from '@agent-device/kernel/errors'; import { resolveCommandRecordingEffect } from '@agent-device/command-registry/registry'; import type { AndroidObservationAdapter } from '@agent-device/contracts/android-observation'; import type { DeviceRuntimeGateway, RuntimeFacts } from '@agent-device/contracts/platform-runtime'; @@ -273,6 +273,29 @@ async function sessionOrSelectorRefusedBeforeRouting(): Promise { assert.fail(`expected the routing refusal, got ${String(failure)}`); } +/** + * The store holds `SESSION` and the request names another address, which is what a run against a + * replaced daemon looks like. The row is about the refusal ordering: `snapshot` would otherwise + * have chosen a device from the host inventory and answered with that. `snapshot` declares an + * `observes-app` effect, so the seam stamps `dispatched: no` on any refusal — the driver has to + * pin the producer's own code and reason, or a reverted guard would satisfy the row with the + * device-selection error it replaced. + */ +async function namedSessionRefusedBeforeDeviceSelection(): Promise { + const failure = await route(makeSession(SESSION), { + command: 'snapshot', + positionals: [], + session: 'session-never-opened', + meta: { requestId: 'named-session-refusal', sessionExplicit: true }, + }).catch((error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'SESSION_NOT_FOUND'); + assert.equal(error.details?.reason, PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + throw error; + }); + assert.fail(`expected the named-session refusal, got ${String(failure)}`); +} + function captureNodes(nodes: readonly unknown[]): void { gestureRuntimeSpies.captureSnapshot.mockResolvedValue({ backend: 'xctest', @@ -354,6 +377,7 @@ const DRIVERS: Record Promise> = { 'daemon.route.scroll-then-dialog-read-refused': androidScrollThenDialogReadRefused, 'daemon.route.read-only-get': readOnlyGet, 'daemon.route.session-or-selector-refusal': sessionOrSelectorRefusedBeforeRouting, + 'daemon.route.named-session-refusal': namedSessionRefusedBeforeDeviceSelection, }; const ROWS = dispatchDisclosureRowsOwnedBy( diff --git a/packages/daemon/src/execution/selector-runtime-backend.ts b/packages/daemon/src/execution/selector-runtime-backend.ts index c158c83ee5..aa329d7900 100644 --- a/packages/daemon/src/execution/selector-runtime-backend.ts +++ b/packages/daemon/src/execution/selector-runtime-backend.ts @@ -115,6 +115,17 @@ async function resolveSelectorRuntimeDevice( params.captureProof ??= {}; const ref = params.sessionStore.lookup(params.sessionName); const session = ref?.session; + // Ordered before the required-session return: a named missing session answers with its own + // typed reason. Only that branch pays the function-scoped import (ADR-0019 loading shape). + if (!session) { + const { requireNamedSessionHoldsDevice } = + await import('../core/explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ + req: params.req, + sessionAddress: params.sessionName, + session, + }); + } if (!session && requireSession) return { ok: false, response: noActiveSessionError() }; const device = session?.device ?? (await resolveTargetDevice(params.req.flags ?? {})); return { ok: true, ref, session, device }; diff --git a/packages/daemon/src/execution/wait-runtime.ts b/packages/daemon/src/execution/wait-runtime.ts index c08f6711cf..c012b6318e 100644 --- a/packages/daemon/src/execution/wait-runtime.ts +++ b/packages/daemon/src/execution/wait-runtime.ts @@ -47,7 +47,7 @@ export async function dispatchWaitViaRuntime(params: DispatchWaitParams): Promis const parsedOrResponse = parseWaitRequest(req); if ('ok' in parsedOrResponse) return parsedOrResponse; const parsed = parsedOrResponse; - const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req.flags); + const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req); // ADR 0019: facts are the only support authority, through the selector family's one // admit-then-bind entry. A duration wait observes nothing, so it never asks for a binding — // exactly the cell legacy admission skipped by testing `parsed.kind !== 'sleep'`. diff --git a/packages/daemon/src/resources/session-device-resolution.ts b/packages/daemon/src/resources/session-device-resolution.ts index ad5380d51c..5aad910c4b 100644 --- a/packages/daemon/src/resources/session-device-resolution.ts +++ b/packages/daemon/src/resources/session-device-resolution.ts @@ -28,10 +28,6 @@ export function requireSessionOrExplicitSelector( ); } -export function hasExplicitSessionFlag(flags: DaemonRequest['flags'] | undefined): boolean { - return typeof flags?.session === 'string' && flags.session.trim().length > 0; -} - export async function resolveCommandDevice(params: { session: SessionState | undefined; flags: DaemonRequest['flags'] | undefined; diff --git a/packages/daemon/src/server/__tests__/request-router-named-session.test.ts b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts new file mode 100644 index 0000000000..22ebf30a7a --- /dev/null +++ b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts @@ -0,0 +1,209 @@ +import { createTestDeviceInventoryGateways } from '@agent-device/device-selection/inventory-gateways-fixtures'; +import { test, expect } from 'vitest'; +import path from 'node:path'; +import { fakeCreateCommandSurface } from '../../__tests__/support/command-surface-fake.ts'; +import { + createLifecycleDeviceRuntimeGatewaySpies, + createRequestHandler, + lifecycleDeviceRuntimeGateway, +} from '../../__tests__/support/test-device-runtime-gateway.ts'; +import type { DaemonRequest } from '../../core/daemon-request.ts'; +import type { DeviceRuntimeGateway } from '@agent-device/contracts/platform-runtime'; +import type { PlatformRuntimeOperations } from '@agent-device/contracts/platform-runtime-operations'; +import { LeaseRegistry } from '../../resources/lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/support/store-factory.ts'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { PRE_DISPATCH_REFUSAL_REASONS } from '@agent-device/kernel/errors'; +import { mkdtempForTestSync } from '@agent-device/test-kit'; + +// #3674: a run whose daemon was replaced mid-test keeps sending commands to the session it opened. +// The answer has to name the session that went missing, and must not depend on how many devices +// happen to be booted on the host. + +const LOST_SESSION = 'e2e-ios-0'; + +function bootedSimulator(id: string, name: string): DeviceInfo { + return { + platform: 'apple', + target: 'mobile', + id, + name, + kind: 'simulator', + booted: true, + }; +} + +const BOOTED_SIMULATORS: DeviceInfo[] = [ + bootedSimulator('SIM-A', 'TinyConcepts E2E'), + bootedSimulator('SIM-B', 'Perf iPhone 17'), +]; + +function makeHandler( + listings: { count: number }, + deviceRuntimeGateway: DeviceRuntimeGateway = lifecycleDeviceRuntimeGateway, +) { + const sessionStore = makeSessionStore('agent-device-router-named-session-'); + const handler = createRequestHandler({ + createCommandSurface: fakeCreateCommandSurface, + logPath: path.join(mkdtempForTestSync('daemon'), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceRuntimeGateway, + deviceInventoryGateways: createTestDeviceInventoryGateways({ + local: async () => { + listings.count += 1; + return BOOTED_SIMULATORS; + }, + }), + trackDownloadableArtifact: () => 'artifact-id', + }); + return { handler, sessionStore }; +} + +function request(overrides: Partial = {}): DaemonRequest { + return { + token: 'test-token', + session: LOST_SESSION, + command: 'snapshot', + positionals: [], + flags: {}, + meta: { requestId: 'named-session-request', sessionExplicit: true }, + ...overrides, + }; +} + +test('a command naming a session this daemon does not hold answers on the session, not the device', async () => { + const { handler } = makeHandler({ count: 0 }); + + const response = await handler(request()); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.message).toContain(`No active session "${LOST_SESSION}"`); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.dispatched).toBe('no'); + expect(response.error.details?.session).toBe(LOST_SESSION); +}); + +test('the refusal answers without listing the host devices at all', async () => { + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + await handler(request()); + + expect(listings.count).toBe(0); +}); + +test('a required-session command naming a missing session gets the typed refusal, not the generic one', async () => { + // `get` requires a session, so its seam used to answer any missing session with the generic + // "no active session" error before the provenance check could run. A caller that *named* the + // session still deserves to learn which address is gone — the same event, the same typed reason. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler(request({ command: 'get', positionals: ['text', '@ref-1'] })); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe(LOST_SESSION); + expect(listings.count).toBe(0); +}); + +test('a record start naming a missing session answers on the session, not on app scope', async () => { + // `record start` defaults to app scope and used to answer a missing session with the + // app-session INVALID_ARGS refusal. When the session itself is gone, the session is the first + // cause, and the typed named-session refusal wins over the record-specific one. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler(request({ command: 'record', positionals: ['start'] })); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe(LOST_SESSION); + expect(listings.count).toBe(0); +}); + +test('a record stop naming a missing session answers on the session, not on a host sweep', async () => { + // The daemon-restart recovery (#3674): the durable manifest is the only thing that can answer + // this stop, and the manifest is addressed by the session — so with two booted devices the + // answer is still the typed session refusal or the manifest's own verdict, never the + // AMBIGUOUS_MATCH an unconstrained sweep of this inventory would answer. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler(request({ command: 'record', positionals: ['stop'] })); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe(LOST_SESSION); + expect(listings.count).toBe(0); +}); + +test('the same command with no session named still resolves a device sessionlessly', async () => { + // The closest negative: same daemon, same two booted simulators, same command. What changes is + // only whether the caller named a session, and that is what the answer must turn on. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler( + request({ session: 'default', meta: { requestId: 'sessionless-request' } }), + ); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('AMBIGUOUS_MATCH'); + expect(listings.count).toBeGreaterThan(0); +}); + +test('a named session beside a device selector keeps honouring the device it names', async () => { + // The other closest negative: the same missing session, but the request states its target + // outright. Then the session is not what is missing and the refusal must stay out of the way — + // proven by the runtime gateway actually binding the named simulator, not by the absence of a + // particular error code (AMBIGUOUS_MATCH would satisfy a weaker assertion). + const listings = { count: 0 }; + const { gateway, inspectFacts } = createLifecycleDeviceRuntimeGatewaySpies(); + const { handler } = makeHandler(listings, gateway); + + const response = await handler(request({ flags: { platform: 'ios', udid: 'SIM-A' } })); + + // Positive proof, not an absence: the runtime lane inspected SIM-A (and never SIM-B), so the + // request's stated target won. Any refusal after that point is the device's own — here the + // iOS app-session rule — and never the named-session one the missing session would answer. + const inspectedDeviceIds = inspectFacts.mock.calls.map(([call]) => call.id); + expect(inspectedDeviceIds).toContain('SIM-A'); + expect(inspectedDeviceIds).not.toContain('SIM-B'); + if (!response.ok) { + expect(response.error.details?.reason).not.toBe( + PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound, + ); + } + expect(listings.count).toBeGreaterThan(0); +}); + +test('a sessionless close naming a missing session answers on the session', async () => { + // The close fall-through is the sibling of the snapshot one: it names the missing session with + // the same typed refusal and resolves no device at all. The generic SESSION_NOT_FOUND the + // sessionless close used to answer must not satisfy this. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler(request({ command: 'close', positionals: ['com.example.app'] })); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.message).toContain(`No active session "${LOST_SESSION}"`); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.dispatched).toBe('no'); + expect(response.error.details?.session).toBe(LOST_SESSION); + expect(listings.count).toBe(0); +}); diff --git a/packages/daemon/src/sessions/handlers/__tests__/record-runtime-stop-recovery.test.ts b/packages/daemon/src/sessions/handlers/__tests__/record-runtime-stop-recovery.test.ts index 4414dc4618..190353c9ba 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/record-runtime-stop-recovery.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/record-runtime-stop-recovery.test.ts @@ -1,7 +1,8 @@ +import { expect, test, vi } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; -import { expect, test } from 'vitest'; import { mkdtempForTestSync } from '@agent-device/test-kit'; +import { AppError } from '@agent-device/kernel/errors'; import { screenRecordingResourceStore } from '@agent-device/capture-kit/screen-recording-resource-store'; import { expectDecodedCompletedRecording, @@ -9,6 +10,315 @@ import { recordingResourcePath, } from './record-runtime.fixtures.ts'; +// The stop lane WITHOUT a held session resolves through the selection dispatch. The double acts +// like a multi-device host: it answers only requests that state the recorded identity (or the +// linux family), so a lane that swept the host's flags instead of resolving from the manifest +// gets the ambiguity such a host really returns. +const RESTART_RECORDED_DEVICE = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', +} as const; +const APPLE_RECORDED_DEVICE = { + platform: 'apple', + appleOs: 'ios', + id: 'IOS-UDID-1', + name: 'iPhone 17 Pro', + kind: 'simulator', + target: 'mobile', +} as const; +const FLAGLESS_RECORDED_DEVICE = { + platform: 'linux', + id: 'linux', + name: 'Linux', + kind: 'device', + target: 'desktop', +} as const; +const resolveTargetDeviceMock = vi.fn(async (flags: Record = {}) => { + if (flags.serial === RESTART_RECORDED_DEVICE.id || flags.udid === RESTART_RECORDED_DEVICE.id) { + return { ...RESTART_RECORDED_DEVICE }; + } + if (flags.udid === APPLE_RECORDED_DEVICE.id) return { ...APPLE_RECORDED_DEVICE }; + if (flags.platform === 'linux') return { ...FLAGLESS_RECORDED_DEVICE }; + throw new AppError('AMBIGUOUS_MATCH', 'Multiple attached devices detected'); +}); +vi.mock('@agent-device/device-selection/dispatch-resolve', () => ({ + resolveTargetDevice: (...args: unknown[]) => + resolveTargetDeviceMock(...(args as [Record | undefined])), +})); + +test('record stop after a daemon restart replays the completed export for the named session', async () => { + // The #3674 state itself: sessions are memory-only, so a restart drops `recording`, while the + // on-disk manifest survives. The caller still names the session (the only address that finds + // the manifest at all), and `help record` promises the completed recording on that stop — even + // on a host whose unconstrained resolution would answer AMBIGUOUS_MATCH. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-'); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const recovered = await harness.run(['stop'], { sessionExplicit: true }); + + expect(recovered).toMatchObject({ + ok: true, + data: { recording: 'stopped', outPath, recordingBackend: 'adb screenrecord' }, + }); + // The recovery re-resolves the manifest's exact device, never an unconstrained sweep. + expect(resolveTargetDeviceMock).toHaveBeenCalledWith( + expect.objectContaining({ serial: 'emulator-5554' }), + ); +}); + +test('record stop recovery after a daemon restart keeps the request host-scoping selectors', async () => { + // --android-device-allowlist says WHERE to look, not which device. A recovery that dropped it + // would resolve (or fail) outside the allowlist the recording was started under. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-scoping-'); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-scoping-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + expect( + await harness.run(['start', outPath], undefined, { androidDeviceAllowlist: 'emulator-5554' }), + ).toMatchObject({ ok: true }); + expect(await harness.run(['stop'])).toMatchObject({ ok: true }); + expectDecodedCompletedRecording(harness.sessionStore, harness.sessionName); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const recovered = await harness.run( + ['stop'], + { sessionExplicit: true }, + { + androidDeviceAllowlist: 'emulator-5554', + }, + ); + + expect(recovered).toMatchObject({ + ok: true, + data: { recording: 'stopped', outPath }, + }); + expect(resolveTargetDeviceMock).toHaveBeenCalledWith( + expect.objectContaining({ + serial: 'emulator-5554', + androidDeviceAllowlist: 'emulator-5554', + }), + ); +}); + +test('record stop recovery on an Apple manifest keeps the request simulator-device-set', async () => { + // The apple-family twin of the allowlist pin: --ios-simulator-device-set scopes where an Apple + // recovery may look, and the identity it re-states is the udid. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-ios-set-', { + platform: 'apple', + }); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-ios-set-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + expect( + await harness.run(['start', outPath], undefined, { iosSimulatorDeviceSet: '/sets/qa.set' }), + ).toMatchObject({ ok: true }); + expect(await harness.run(['stop'])).toMatchObject({ ok: true }); + expectDecodedCompletedRecording(harness.sessionStore, harness.sessionName); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const recovered = await harness.run( + ['stop'], + { sessionExplicit: true }, + { + iosSimulatorDeviceSet: '/sets/qa.set', + }, + ); + + expect(recovered).toMatchObject({ ok: true, data: { recording: 'stopped', outPath } }); + expect(resolveTargetDeviceMock).toHaveBeenCalledWith( + expect.objectContaining({ + platform: 'apple', + udid: 'IOS-UDID-1', + iosSimulatorDeviceSet: '/sets/qa.set', + }), + ); +}); + +test('record stop recovery carries the request lease and discovery context', async () => { + // Lease fields say WHO may see the inventory, not WHICH device; a recovery that resolved + // without them would look outside the lease the recording was started under. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-lease-'); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-lease-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + const leaseFlags = { + leaseId: 'lease-1', + leaseProvider: 'proxy', + deviceKey: 'android:mobile:emulator-5554', + clientId: 'client-1', + }; + expect(await harness.run(['start', outPath], undefined, { ...leaseFlags })).toMatchObject({ + ok: true, + }); + expect(await harness.run(['stop'])).toMatchObject({ ok: true }); + expectDecodedCompletedRecording(harness.sessionStore, harness.sessionName); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const recovered = await harness.run(['stop'], { sessionExplicit: true }, { ...leaseFlags }); + + expect(recovered).toMatchObject({ ok: true, data: { recording: 'stopped', outPath } }); + expect(resolveTargetDeviceMock).toHaveBeenCalledWith(expect.objectContaining(leaseFlags)); +}); + +test('record stop recovery for a family without identity flags resolves under the manifest family', async () => { + // A linux manifest names no identity flag the request could restate. The recovery still must + // not sweep the host from the request's (empty) flags: it asks for the manifest's family, and + // the double answers only such family-scoped requests. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-flagless-', { + platform: 'linux', + }); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-flagless-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + expect(await harness.run(['start', outPath])).toMatchObject({ ok: true }); + expect(await harness.run(['stop'])).toMatchObject({ ok: true }); + expectDecodedCompletedRecording(harness.sessionStore, harness.sessionName); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const recovered = await harness.run(['stop'], { sessionExplicit: true }); + + expect(recovered).toMatchObject({ ok: true, data: { recording: 'stopped', outPath } }); + expect(resolveTargetDeviceMock).toHaveBeenCalledWith( + expect.objectContaining({ platform: 'linux' }), + ); +}); + +test('record stop for a named session with no decodable manifest refuses on the session', async () => { + // No manifest means there is nothing to recover from and nothing to take a device from: the + // answer is the named-session refusal, and the host is never swept for a device. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-no-manifest-'); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the published session'); + harness.sessionStore.retire(ref); + + const response = await harness.run(['stop'], { sessionExplicit: true }); + + expect(response).toMatchObject({ + ok: false, + error: { code: 'SESSION_NOT_FOUND', details: { reason: 'named_session_not_found' } }, + }); + expect(resolveTargetDeviceMock).not.toHaveBeenCalled(); +}); + +test('a named stop recovery keeps failing closed when the manifest names another session', async () => { + // The manifest-first lane reads the envelope before resolving, so the session proof must stay + // on the served path: a manifest whose sessionId belongs to another address never answers. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-cross-session-', { + sessionName: 'recording-a', + }); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-cross-session-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + const resourcePath = recordingResourcePath(harness.sessionStore, harness.sessionName); + const record = screenRecordingResourceStore.read(resourcePath); + if (record.status !== 'decoded') throw new Error('Expected decoded recording manifest'); + screenRecordingResourceStore.write(resourcePath, { + ...record.envelope, + sessionId: 'recording-b', + }); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + + const response = await harness.run(['stop'], { sessionExplicit: true }); + + expect(response).toMatchObject({ + ok: false, + error: { code: 'COMMAND_FAILED', details: { reason: 'runtime-contract-invalid' } }, + }); +}); + +test('record stop after a daemon restart answers an explicitly targeted request from its device', async () => { + // The closest positive: the same unheld named session, but the request states the very device + // the manifest names, so the selector lane serves the recovery — the stated target still wins. + const harness = makeRecordRuntimeHarness('record-runtime-restart-stop-targeted-'); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-stop-targeted-output-'), + 'capture.mp4', + ); + fs.writeFileSync(outPath, 'mp4'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the stopped recording session'); + harness.sessionStore.retire(ref); + + const recovered = await harness.run( + ['stop'], + { sessionExplicit: true }, + { + platform: 'android', + serial: 'emulator-5554', + }, + ); + + expect(recovered).toMatchObject({ + ok: true, + data: { recording: 'stopped', outPath, recordingBackend: 'adb screenrecord' }, + }); +}); + +test('record start after a daemon restart still refuses the named missing session', async () => { + // The sibling direction of the restart pair: start has no manifest to recover, so the + // named-session refusal keeps its priority for it. + const harness = makeRecordRuntimeHarness('record-runtime-restart-start-'); + const outPath = path.join( + mkdtempForTestSync('record-runtime-restart-start-output-'), + 'capture.mp4', + ); + await harness.run(['start', outPath]); + const ref = harness.sessionStore.lookup(harness.sessionName); + if (!ref) throw new Error('Expected the live recording session'); + harness.sessionStore.retire(ref); + resolveTargetDeviceMock.mockClear(); + + const response = await harness.run(['start', outPath], { sessionExplicit: true }); + + expect(response).toMatchObject({ + ok: false, + error: { code: 'SESSION_NOT_FOUND', details: { reason: 'named_session_not_found' } }, + }); + expect(harness.runtime.start).toHaveBeenCalledOnce(); + // A start has no manifest to recover from, so the refusal answers before any device is sought. + expect(resolveTargetDeviceMock).not.toHaveBeenCalled(); +}); + test('record stop with a live handle binds no runtime and terminalizes the durable record', async () => { const harness = makeRecordRuntimeHarness('record-runtime-live-stop-'); const outPath = path.join(mkdtempForTestSync('record-runtime-live-stop-output-'), 'capture.mp4'); diff --git a/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts b/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts index 18397e0dbd..d053c89efb 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts @@ -12,6 +12,7 @@ import { import type { PlatformRuntimeOperations } from '@agent-device/contracts/platform-runtime-operations'; import type { ScreenRecordingLiveHandle } from '@agent-device/contracts/screen-recording-runtime'; import type { DaemonRequest } from '../../../core/daemon-request.ts'; +import { deviceIdentity } from '@agent-device/kernel/device'; import { unavailableDeploymentSnapshotAndShutdownOperationFacts } from '@agent-device/test-kit'; import { createDurableResourceEnvelope } from '@agent-device/capture-kit'; import { makeSessionStore } from '../../../__tests__/support/store-factory.ts'; @@ -38,7 +39,7 @@ export function makeRecordRuntimeHarness( sessionName?: string; appBundleId?: string; recordOnlySession?: boolean; - platform?: 'android' | 'linux' | 'vega'; + platform?: 'android' | 'apple' | 'linux' | 'vega'; runtime?: RuntimeOptions; } = {}, ) { @@ -51,7 +52,16 @@ export function makeRecordRuntimeHarness( ? { platform: 'linux', id: 'linux', name: 'Linux', kind: 'device', target: 'desktop' } : options.platform === 'vega' ? { platform: 'vega', id: 'vega', name: 'Vega', kind: 'device', target: 'tv' } - : { platform: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator' }, + : options.platform === 'apple' + ? { + platform: 'apple', + appleOs: 'ios', + id: 'IOS-UDID-1', + name: 'iPhone 17 Pro', + kind: 'simulator', + target: 'mobile', + } + : { platform: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator' }, ...(options.appBundleId ? { appBundleId: options.appBundleId } : {}), ...(options.recordOnlySession ? { recordOnlySession: true } : {}), createdAt: 1, @@ -75,7 +85,7 @@ export function makeRecordRuntimeHarness( sessionName, sessionStore, runtime, - run: (positionals: string[], meta?: DaemonRequest['meta']) => + run: (positionals: string[], meta?: DaemonRequest['meta'], flags?: DaemonRequest['flags']) => handleRecordCommand({ platformServices: daemonPlatformServicesFixture(), ...common, @@ -84,7 +94,7 @@ export function makeRecordRuntimeHarness( session: sessionName, command: 'record', positionals, - flags: {}, + flags: flags ?? {}, ...(meta ? { meta } : {}), }, }), @@ -154,12 +164,9 @@ function makeRuntime(session: SessionState, options: RuntimeOptions = {}) { envelope: createDurableResourceEnvelope({ resourceKind: 'screen-recording', sessionId: input.sessionId, - device: { - id: session.device.id, - family: session.device.platform, - kind: session.device.kind, - ...(session.device.target === undefined ? {} : { target: session.device.target }), - }, + // Same authoritative identity projection the recording transports build their + // envelopes with, so a fixture manifest carries exactly what a real one would. + device: deviceIdentity(session.device), owner, fence: input.fence, lifecycle: 'open', diff --git a/packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts b/packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts index 41578df148..bd503710c7 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts @@ -16,7 +16,7 @@ import { mockInspectDeviceRuntimeFacts, } from '../../../__tests__/support/handlers/session-command-harness.ts'; import { mkdtempForTestSync } from '@agent-device/test-kit'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, PRE_DISPATCH_REFUSAL_REASONS } from '@agent-device/kernel/errors'; import type { AppStateRuntimeInput, AppStateRuntimeResult, @@ -239,6 +239,10 @@ test('appstate with explicit missing session returns SESSION_NOT_FOUND', async ( expect(response.error.code).toBe('SESSION_NOT_FOUND'); expect(response.error.message).toMatch(/no active session "sim"/i); expect(response.error.message).not.toMatch(/omit --session/i); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe('sim'); + expect(response.error.details?.dispatched).toBe('no'); + expect(mockResolveTargetDevice).not.toHaveBeenCalled(); } }); diff --git a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts index f60bc51a7b..1151835b25 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -1,7 +1,12 @@ import { daemonPlatformServicesFixture } from '../../../__tests__/support/platform-services-fixture.ts'; -import { test, expect } from 'vitest'; +import { test, expect, vi } from 'vitest'; import { makeSessionStore } from '../../../__tests__/support/handlers/session-test-harness.ts'; import { handleAppStateCommand } from '../session-appstate.ts'; +import { PRE_DISPATCH_REFUSAL_REASONS } from '@agent-device/kernel/errors'; +import { resolveTargetDevice } from '@agent-device/device-selection/dispatch-resolve'; +import { createUnavailableRuntimeFactsForTest } from '@agent-device/test-kit'; +import { localRuntimeOwner } from '@agent-device/contracts/platform-runtime'; +import type { DeviceInfo } from '@agent-device/kernel/device'; import type { DaemonRequest } from '../../../core/daemon-request.ts'; function appStateRequest(session: string, flags: DaemonRequest['flags']): DaemonRequest { @@ -21,6 +26,10 @@ test('appstate returns missing-session error for explicit session flag', async ( if (!response.ok) { expect(response.error.code).toBe('SESSION_NOT_FOUND'); expect(response.error.message).toMatch(/Run open with --session named first/i); + // The same typed refusal every named-session seam answers with (#3674). + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.dispatched).toBe('no'); + expect(response.error.details?.session).toBe('named'); } }); @@ -60,6 +69,194 @@ test('appstate --platform apple missing-session error never suggests omitting -- } }); +test('appstate --udid with no --platform resolves the stated device and THEN answers the typed reason', async () => { + // `--udid` is the Apple identity flag, so a bare `--udid ` reaches the device-runtime lane + // and lands on an Apple device, which answers the typed reason the commands.md row promises + // "even with an explicit device". The pin is resolution-first: the route must prove the stated + // target before refusing it, so a resolution call on that exact device is required here. + const iosSim: DeviceInfo = { + platform: 'apple', + appleOs: 'ios', + id: 'SIM-UDID-A', + name: 'iPhone 17 Pro', + kind: 'simulator', + target: 'mobile', + booted: true, + }; + const resolved = vi.mocked(resolveTargetDevice); + resolved.mockReset(); + resolved.mockResolvedValue(iosSim); + const inspected: string[] = []; + try { + const response = await handleAppStateCommand({ + platformServices: daemonPlatformServicesFixture(), + req: appStateRequest('lost', { session: 'lost', udid: 'SIM-UDID-A' }), + sessionName: 'lost', + sessionStore: makeSessionStore(), + inspectFacts: async (inspectedDevice) => { + inspected.push(inspectedDevice.id); + return createUnavailableRuntimeFactsForTest(inspectedDevice, localRuntimeOwner('apple'), { + available: false, + reason: 'owner-capability-missing', + }); + }, + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe('lost'); + // Resolution ran, once, on the identity the request stated — the refusal is the device + // runtime lane's, after the target was proven, not a guard that skipped the stated device. + expect(resolved).toHaveBeenCalledTimes(1); + expect(resolved).toHaveBeenCalledWith( + expect.objectContaining({ udid: 'SIM-UDID-A' }), + expect.anything(), + ); + expect(inspected).toEqual([]); + } finally { + resolved.mockReset(); + } +}); + +test('appstate --udid on an Apple device without naming a session keeps the plain session-required advice', async () => { + // The typed named-session reason belongs to requests that named a session this daemon does not + // hold. A request that named none (only the implicit default) gets the plain advice to run + // open — the device-runtime arm must not type a reason it has no session for. + const iosSim: DeviceInfo = { + platform: 'apple', + appleOs: 'ios', + id: 'SIM-UDID-B', + name: 'iPhone 16', + kind: 'simulator', + target: 'mobile', + booted: true, + }; + const resolved = vi.mocked(resolveTargetDevice); + resolved.mockReset(); + resolved.mockResolvedValue(iosSim); + try { + const response = await handleAppStateCommand({ + platformServices: daemonPlatformServicesFixture(), + req: appStateRequest('default', { udid: 'SIM-UDID-B' }), + sessionName: 'default', + sessionStore: makeSessionStore(), + inspectFacts: async (inspectedDevice) => + createUnavailableRuntimeFactsForTest(inspectedDevice, localRuntimeOwner('apple'), { + available: false, + reason: 'owner-capability-missing', + }), + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('SESSION_NOT_FOUND'); + expect(response.error.message).toMatch(/iOS appstate requires an active session/i); + expect(response.error.details?.reason).toBeUndefined(); + // Resolution-first holds for the plain advice too: the stated UDID is proven before the + // refusal, so an implementation that skipped resolving an invalid UDID fails here. + expect(resolved).toHaveBeenCalledTimes(1); + expect(resolved).toHaveBeenCalledWith( + expect.objectContaining({ udid: 'SIM-UDID-B' }), + expect.anything(), + ); + } finally { + resolved.mockReset(); + } +}); + +test('appstate keeps the admission-time named-session verdict when the store changes during resolution', async () => { + // The error family is decided once, at admission. If another caller recreates the session + // while this request is resolving its device, the Apple refusal this request was admitted into + // stays the typed named-session one — a mid-await store change must not silently switch the + // request into a different family (nor answer from a session it never admitted). + const iosSim: DeviceInfo = { + platform: 'apple', + appleOs: 'ios', + id: 'SIM-UDID-C', + name: 'iPhone Air', + kind: 'simulator', + target: 'mobile', + booted: true, + }; + const sessionStore = makeSessionStore(); + const resolved = vi.mocked(resolveTargetDevice); + resolved.mockReset(); + resolved.mockImplementation(async () => { + // The concurrent store change, planted mid-await exactly where a re-read would disagree. + sessionStore.publish('lost', { + name: 'lost', + device: iosSim, + createdAt: 1, + actions: [], + }); + return iosSim; + }); + try { + const response = await handleAppStateCommand({ + platformServices: daemonPlatformServicesFixture(), + req: appStateRequest('lost', { session: 'lost', udid: 'SIM-UDID-C' }), + sessionName: 'lost', + sessionStore, + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); + expect(response.error.details?.session).toBe('lost'); + } finally { + resolved.mockReset(); + } +}); + +test('appstate with a named session and an identity device selector keeps the stated target', async () => { + // Same priority as the resolution guard (#3674): the request names a session this daemon lost + // but states a concrete device, so it is not answered on the missing session. It resolves the + // stated device and reaches that device's own runtime admission — proven by the resolution and + // fact inspection naming emulator-5554 — and any refusal is the device's, never the named one. + const device: DeviceInfo = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel 9', + kind: 'emulator', + target: 'mobile', + booted: true, + }; + // This file's harness stubs device resolution; this test drives that stub and asserts the + // request's stated identity reaches it. + const resolved = vi.mocked(resolveTargetDevice).mockResolvedValue(device); + const inspected: string[] = []; + try { + const response = await handleAppStateCommand({ + platformServices: daemonPlatformServicesFixture(), + req: appStateRequest('named', { session: 'named', platform: 'android', device: 'Pixel 9' }), + sessionName: 'named', + sessionStore: makeSessionStore(), + inspectFacts: async (inspectedDevice) => { + inspected.push(inspectedDevice.id); + return createUnavailableRuntimeFactsForTest(inspectedDevice, localRuntimeOwner('android'), { + available: false, + reason: 'owner-capability-missing', + }); + }, + }); + + expect(resolved).toHaveBeenCalledWith( + expect.objectContaining({ platform: 'android', device: 'Pixel 9' }), + expect.anything(), + ); + expect(inspected).toEqual(['emulator-5554']); + if (!response.ok) { + expect(response.error.details?.reason).not.toBe( + PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound, + ); + } + } finally { + resolved.mockReset(); + } +}); + test('appstate refuses --platform ios without a session at the message level', async () => { const response = await handleAppStateCommand({ platformServices: daemonPlatformServicesFixture(), diff --git a/packages/daemon/src/sessions/handlers/record-runtime-request.ts b/packages/daemon/src/sessions/handlers/record-runtime-request.ts index d357e26dce..a3fc5920d2 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime-request.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime-request.ts @@ -7,7 +7,7 @@ import { import { retiredScreenshotMaxSizeFlagError } from '@agent-device/contracts/capture'; import { AppError } from '@agent-device/kernel/errors'; import type { DaemonRequest } from '../../core/daemon-request.ts'; -import { hasExplicitSessionFlag } from '../session-routing.ts'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; const IOS_DEVICE_RECORD_MIN_FPS = 1; const IOS_DEVICE_RECORD_MAX_FPS = 120; @@ -60,7 +60,7 @@ export function missingAppSessionResponse(req: DaemonRequest) { ok: false as const, error: { code: 'INVALID_ARGS', - message: hasExplicitSessionFlag(req) + message: namesSessionExplicitly(req) ? 'record start with app scope and an explicit session requires an active app session; run open first, or use --scope device to record the full screen' : 'record start defaults to app scope and requires an active app session; run open first, or use --scope device to record the full screen', }, diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 855dfacd01..2a7aa23d87 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -18,8 +18,18 @@ import { } from '@agent-device/contracts/screen-recording-runtime-plan'; import { isWholeScreenRecordingScope } from '@agent-device/contracts/recording'; import { AppError, normalizeError } from '@agent-device/kernel/errors'; -import type { Platform } from '@agent-device/kernel/device'; -import { resolveTargetDevice } from '@agent-device/device-selection/dispatch-resolve'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; +import { + deviceIdentityFlag, + type DeviceIdentity, + type DeviceInfo, + type Platform, +} from '@agent-device/kernel/device'; +import { + resolveTargetDevice, + type ResolveDeviceFlags, +} from '@agent-device/device-selection/dispatch-resolve'; +import { hasDeviceSelectionInput } from '../../core/device/device-selector-intent.ts'; import { ensureBoundDeviceReady } from '../../core/request-runtime-binding.ts'; import { type ScreenRecordingAdmissionLedger } from '@agent-device/capture-kit/screen-recording-admission-ledger'; import { createScreenRecordingRecoveryControl } from '@agent-device/capture-kit/screen-recording-recovery'; @@ -31,6 +41,7 @@ import { } from '@agent-device/capture-kit/screen-recording-session-resource'; import { resolveScreenRecordingStopRecovery, + screenRecordingManifestDevice, screenRecordingManifestIsTerminal, } from '@agent-device/capture-kit/screen-recording-stop-recovery'; import { resolveSessionScope } from '../session-routing.ts'; @@ -100,10 +111,19 @@ async function handleRecordCommandUnsafe( const existingRef = sessionStore.lookup(sessionName); const existingSession = existingRef?.session; const { plan, scope } = resolveRecordPlan(req, existingSession); - if (plan.kind === 'start' && !isWholeScreenRecordingScope(scope) && !existingSession) { + // The app-scope refusal states what a start needs beyond a device, so it answers only for a + // request that can still reach one: a start naming a session this daemon does not hold is + // answered on the session first, at resolveRecordingSession below, where every record lane + // meets the one named-session rule at the only device selection the handler makes. + if ( + plan.kind === 'start' && + !isWholeScreenRecordingScope(scope) && + !existingSession && + !namesSessionWithoutTarget(req) + ) { return missingAppSessionResponse(req); } - const resolvedSession = await resolveRecordingSession(params, existingRef); + const resolvedSession = await resolveRecordingSession(params, existingRef, plan); const { session } = resolvedSession; if (plan.kind === 'start') { return await startRecording( @@ -139,13 +159,75 @@ function resolveRecordPlan(req: DaemonRequest, session: SessionState | undefined async function resolveRecordingSession( params: RecordRuntimeHandlerParams, ref: SessionRef | undefined, + plan: ReturnType['plan'], ): Promise> { - const device = ref?.session.device ?? (await resolveTargetDevice(params.req.flags ?? {})); - await params.retainDeviceExecutionLock(device.id); if (ref) return { session: params.sessionStore.requireCurrent(ref), ref, needsReadiness: false }; + const device = await resolveUnheldRecordingDevice(params, plan); + await params.retainDeviceExecutionLock(device.id); return { session: createRecordOnlySession(params, device), needsReadiness: true }; } +/** + * The device an unheld record request is answered on, and the single place the record handler + * applies the named-session rule (#3674) at its only device selection. + * + * A request that names a session this daemon does not hold and states no device is answered from + * the durable manifest the session address finds, never from a sweep of every device on the host: + * a decodable manifest is re-resolved with the request's scope and lease keys under the manifest's + * family (admission proved the request states no device), and a start or a stop with no decodable + * manifest to recover from refuses with the named-session error. + * `assertManifestBelongsToRequest` owns the proof that the resolved device + * and session address match the manifest. + * + * The two shapes that keep their chosen target are unchanged: a request that never named a + * session resolves sessionlessly from its flags, and one that states a device selector resolves + * from it. + */ +async function resolveUnheldRecordingDevice( + params: RecordRuntimeHandlerParams, + plan: ReturnType['plan'], +): Promise { + const { req, sessionName } = params; + if (!namesSessionWithoutTarget(req)) return await resolveTargetDevice(req.flags ?? {}); + if (plan.kind === 'stop-recovery') { + const recorded = screenRecordingManifestDevice({ + sessionName, + sessionStore: params.sessionStore, + }); + if (recorded) { + return await resolveManifestDevice(params, recorded); + } + } + const { namedSessionNotFoundError } = await import('../../core/explicit-session-resolution.ts'); + throw namedSessionNotFoundError(sessionName); +} + +/** Whether this request is addressed to a session and leaves its device to the session. */ +function namesSessionWithoutTarget(req: DaemonRequest): boolean { + return namesSessionExplicitly(req) && !hasDeviceSelectionInput(req.flags); +} + +/** + * Re-resolves the device a durable manifest names. Admission (`namesSessionWithoutTarget`) has + * already proven this request states no device, udid, serial, platform, or target, so spreading + * its flags carries exactly the scope and lease keys `ResolveDeviceFlags` declares — now and + * later — without carrying any device choice. The manifest then decides which device: family + * always, identity flag where the family has one. `assertManifestBelongsToRequest` proves the + * resolved device against the manifest. + */ +async function resolveManifestDevice( + params: RecordRuntimeHandlerParams, + recorded: DeviceIdentity, +): Promise { + const selector: ResolveDeviceFlags = { + ...(params.req.flags ?? {}), + platform: recorded.family, + }; + const identityFlag = deviceIdentityFlag(recorded.family); + if (identityFlag) selector[identityFlag] = recorded.id; + return await resolveTargetDevice(selector); +} + async function startRecording( params: RecordRuntimeHandlerParams, session: SessionState, diff --git a/packages/daemon/src/sessions/handlers/session-appstate-apple-session.ts b/packages/daemon/src/sessions/handlers/session-appstate-apple-session.ts index ce22e11c7f..4bb31cc472 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate-apple-session.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate-apple-session.ts @@ -1,14 +1,20 @@ import type { DaemonResponse } from '@agent-device/kernel/contracts'; import type { AppleApplicationState } from '@agent-device/kernel/snapshot'; import { + isApplePlatform, isMacOs, isIosFamily, publicPlatformString, type DeviceInfo, + type PlatformSelector, } from '@agent-device/kernel/device'; import type { SessionSurface } from '@agent-device/contracts/session'; import { errorResponse } from '@agent-device/kernel/contracts'; -import { asAppError, isRequestCanceledError } from '@agent-device/kernel/errors'; +import { + asAppError, + isRequestCanceledError, + namedSessionNotFoundDetails, +} from '@agent-device/kernel/errors'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { appStateUse } from '@agent-device/contracts/platform-runtime-operations'; import { admitRuntimeUse } from '../../core/runtime-admission.ts'; @@ -30,6 +36,30 @@ const IOS_APPSTATE_SESSION_REQUIRED_MESSAGE = const MACOS_APPSTATE_SESSION_REQUIRED_MESSAGE = 'macOS appstate requires an active session on the target device. Run open first (for example: open --session macos --platform macos "System Settings").'; +/** + * The refusal for an appstate whose session the daemon does not hold. Every Apple selector — + * `apple`, `ios`, or `macos` — answers appstate only from an open session and its runner, so the + * "omit --session" route they would otherwise be pointed at just reaches the session-required + * refusal. Send them straight to opening the session. + */ +export function appStateSessionNotFoundResponse( + sessionName: string, + platform: PlatformSelector | undefined, +): DaemonResponse { + if (isApplePlatform(platform)) { + return errorResponse( + 'SESSION_NOT_FOUND', + `No active session "${sessionName}". Run open with --session ${sessionName} first.`, + namedSessionNotFoundDetails(sessionName), + ); + } + return errorResponse( + 'SESSION_NOT_FOUND', + `No active session "${sessionName}". Run open with --session ${sessionName} first, or omit --session to query by device selector.`, + namedSessionNotFoundDetails(sessionName), + ); +} + /** * Apple platforms answer appstate from a session and its runner, never from a bare device query: * this is the refusal for a request whose Apple target has no session to read. diff --git a/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts b/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts index 0e28cb005a..0e120a9898 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts @@ -1,4 +1,3 @@ -import type { DaemonResponse } from '@agent-device/kernel/contracts'; import { appStateUse } from '@agent-device/contracts/platform-runtime-operations'; import { isIosFamily, @@ -6,10 +5,14 @@ import { publicPlatformString, type DeviceInfo, } from '@agent-device/kernel/device'; +import type { DaemonResponse } from '@agent-device/kernel/contracts'; import { resolveAndroidSerialAllowlist } from '@agent-device/kernel/device-isolation'; import { admitRuntimeUse } from '../../core/runtime-admission.ts'; import type { RuntimeCommandHandlerParams } from '../../core/session-runtime-admission.ts'; -import { appleSessionRequiredRefusal } from './session-appstate-apple-session.ts'; +import { + appStateSessionNotFoundResponse, + appleSessionRequiredRefusal, +} from './session-appstate-apple-session.ts'; function sortedAndroidSerialAllowlist(value: string | undefined): string[] | undefined { const allowlist = resolveAndroidSerialAllowlist(value); @@ -25,9 +28,17 @@ function sortedAndroidSerialAllowlist(value: string | undefined): string[] | und export async function queryDeviceRuntimeAppState( params: RuntimeCommandHandlerParams, device: DeviceInfo, + namedSessionMissing: boolean, ): Promise { - if (isIosFamily(device)) return appleSessionRequiredRefusal('ios'); - if (isMacOs(device)) return appleSessionRequiredRefusal('macos'); + if (isIosFamily(device) || isMacOs(device)) { + // The admission-time fact (see AppStateTargetDecision): resolution already succeeded, so the + // only question left is whether this request was admitted without the session it names. A + // request that named no session keeps the plain session-required refusal. + if (namedSessionMissing) { + return appStateSessionNotFoundResponse(params.sessionName, device.platform); + } + return appleSessionRequiredRefusal(isMacOs(device) ? 'macos' : 'ios'); + } const flags = params.req.flags ?? {}; const admitted = await admitRuntimeUse({ command: 'appstate', diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index d9aff811f5..272a301395 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -1,46 +1,35 @@ import type { DaemonResponse } from '@agent-device/kernel/contracts'; -import { isApplePlatform, type PlatformSelector } from '@agent-device/kernel/device'; -import { errorResponse } from '@agent-device/kernel/contracts'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; +import { isApplePlatform } from '@agent-device/kernel/device'; +import { hasExplicitDeviceSelector } from '../../core/device/device-selector-intent.ts'; import { - hasExplicitSessionFlag, requireSessionOrExplicitSelector, resolveCommandDevice, selectorTargetsSessionDevice, } from '../../resources/session-device-resolution.ts'; import type { RuntimeCommandHandlerParams } from '../../core/session-runtime-admission.ts'; import { + appStateSessionNotFoundResponse, appleSessionAppStateResponse, appleSessionRequiredRefusal, type AppStateSessionRecord, } from './session-appstate-apple-session.ts'; import { queryDeviceRuntimeAppState } from './session-appstate-device-runtime.ts'; -function appStateSessionNotFoundResponse( - sessionName: string, - platform: PlatformSelector | undefined, -): DaemonResponse { - // Every Apple selector — `apple`, `ios`, or `macos` — answers appstate only from an open - // session and its runner, so the "omit --session" route they would otherwise be pointed at - // just reaches the session-required refusal. Send them straight to opening the session. - if (isApplePlatform(platform)) { - return errorResponse( - 'SESSION_NOT_FOUND', - `No active session "${sessionName}". Run open with --session ${sessionName} first.`, - ); - } - return errorResponse( - 'SESSION_NOT_FOUND', - `No active session "${sessionName}". Run open with --session ${sessionName} first, or omit --session to query by device selector.`, - ); -} - type AppStateSessionStoreEntry = ReturnType; /** The answer of target admission: a refusal, or the authority that answers with its session. */ type AppStateTargetDecision = | { refusal: DaemonResponse } | { answeredBy: 'apple-session'; session: AppStateSessionRecord } - | { answeredBy: 'device-runtime'; session: AppStateSessionStoreEntry }; + | { + answeredBy: 'device-runtime'; + session: AppStateSessionStoreEntry; + /** Decided from the same store read as `session`, so a mid-await store change (e.g. the + * session being recreated during device resolution) cannot move the refusal later answered + * into another family. */ + namedSessionMissing: boolean; + }; /** * Has this appstate request an answerable target, and which authority answers it: the Apple @@ -55,7 +44,15 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge const flags = req.flags ?? {}; const normalizedPlatform = flags.platform; - if (!session && hasExplicitSessionFlag(flags)) { + // Same priority as the resolution guard: a non-Apple identity selector states a target this + // command answers sessionlessly, so the refusal stays out of the way there. Apple-family + // appstate requires an open session whatever the selector states, so the typed refusal remains + // the only actionable answer for it. + if ( + !session && + namesSessionExplicitly(req) && + (!hasExplicitDeviceSelector(flags) || isApplePlatform(normalizedPlatform)) + ) { return { refusal: appStateSessionNotFoundResponse(sessionName, normalizedPlatform) }; } @@ -74,7 +71,11 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge if (useSessionStateForApple && session) { return { answeredBy: 'apple-session' as const, session }; } - return { answeredBy: 'device-runtime' as const, session }; + return { + answeredBy: 'device-runtime' as const, + session, + namedSessionMissing: !session && namesSessionExplicitly(req), + }; } /** @@ -95,5 +96,5 @@ export async function handleAppStateCommand( flags: params.req.flags ?? {}, platformServices: params.platformServices, }); - return await queryDeviceRuntimeAppState(params, device); + return await queryDeviceRuntimeAppState(params, device, target.namedSessionMissing); } diff --git a/packages/daemon/src/sessions/handlers/snapshot.ts b/packages/daemon/src/sessions/handlers/snapshot.ts index 2377b23363..82f2e6ed8b 100644 --- a/packages/daemon/src/sessions/handlers/snapshot.ts +++ b/packages/daemon/src/sessions/handlers/snapshot.ts @@ -114,11 +114,7 @@ const SNAPSHOT_COMMAND_HANDLER_IMPLS = { platformServices, platformResourceCleanup, }) => { - const { ref, session, device } = await resolveSessionDevice( - sessionStore, - sessionName, - req.flags, - ); + const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req); return await withSessionlessRunnerCleanup( session, device, @@ -149,11 +145,7 @@ const SNAPSHOT_COMMAND_HANDLER_IMPLS = { }) => { const parsedSettings = parseSettingsArgs(req); if (!parsedSettings.ok) return parsedSettings; - const { ref, session, device } = await resolveSessionDevice( - sessionStore, - sessionName, - req.flags, - ); + const { ref, session, device } = await resolveSessionDevice(sessionStore, sessionName, req); return await withSessionlessRunnerCleanup( session, device, diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-devices-batch-runtime.test.ts b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-devices-batch-runtime.test.ts index 56c57b34f4..992119acef 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-devices-batch-runtime.test.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-devices-batch-runtime.test.ts @@ -356,7 +356,9 @@ test('batch step pins nested requests to the resolved session', async () => { ); expect(response.ok).toBe(true); - expect(seenSessions).toEqual([{ session: 'resolved-session', flagSession: 'resolved-session' }]); + // The wire address stays pinned to the resolved session. The step flag carries it only when the + // caller named a session (#3674): an unconditional stamp would read as caller-provened. + expect(seenSessions).toEqual([{ session: 'resolved-session', flagSession: undefined }]); }); test('close clears retained materialized install paths bound to the session', async () => { diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts index 3286112e38..c44d1e71c5 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts @@ -206,6 +206,7 @@ export async function handleSessionCloseCommands( if (!ref) { return await closeWithoutSession({ req, + sessionName, logPath, inspectFacts: params.inspectFacts, platformServices: params.platformServices, @@ -425,12 +426,16 @@ async function closeAppWithoutEndingSession(params: { async function closeWithoutSession(params: { req: DaemonRequest; + sessionName: string; logPath: string; inspectFacts?: InspectDeviceRuntimeFacts; platformServices: DaemonPlatformServices; bindDevice?: BindDeviceRuntime; }): Promise { - const { req, logPath, inspectFacts, bindDevice, platformServices } = params; + const { req, sessionName, logPath, inspectFacts, bindDevice, platformServices } = params; + const { requireNamedSessionHoldsDevice } = + await import('../../../core/explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session: undefined }); if (!req.positionals || req.positionals.length === 0) return errorResponse('SESSION_NOT_FOUND', 'No active session'); const device = await resolveCommandDevice({ diff --git a/packages/daemon/src/sessions/session-routing.ts b/packages/daemon/src/sessions/session-routing.ts index d929f4040a..8f8aff815d 100644 --- a/packages/daemon/src/sessions/session-routing.ts +++ b/packages/daemon/src/sessions/session-routing.ts @@ -12,6 +12,7 @@ import type { DaemonRequest } from '../core/daemon-request.ts'; import type { SessionRef, SessionState } from '../core/session-state.ts'; import type { SessionScope } from '@agent-device/contracts/session'; import { SessionStore } from '../core/session-store.ts'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; import { listSessionSelectorConflicts, type SessionSelectorConflict, @@ -43,7 +44,7 @@ export function resolveEffectiveSessionName( options: ImplicitSessionRoutingOptions = {}, ): string { const requested = req.session || DEFAULT_SESSION_NAME; - if (hasExplicitSessionFlag(req)) return requested; + if (namesSessionExplicitly(req)) return requested; const scope = resolveSessionScope(req); if (scope.kind === 'cwd') { return resolveImplicitWorkspaceSession(req, sessionStore, scope.id, options); @@ -74,7 +75,7 @@ export function resolveSessionScope(req: DaemonWireRequest): SessionScope { return { kind: 'tenant', id: tenantId }; } if ( - hasExplicitSessionFlag(req) || + namesSessionExplicitly(req) || (req.session || DEFAULT_SESSION_NAME) !== DEFAULT_SESSION_NAME ) { return { kind: 'named-local' }; @@ -106,12 +107,6 @@ export function isImplicitSessionScopeConflict(req: DaemonRequest, session: Sess return session.sessionScope.id !== scope.id; } -export function hasExplicitSessionFlag(req: DaemonWireRequest): boolean { - if (req.meta?.sessionExplicit === true) return true; - const value = (req.flags as CommandFlags | undefined)?.session; - return typeof value === 'string' && value.trim().length > 0; -} - function formatScopedSessionName(scopeId: string, sessionName: string): string { return `${IMPLICIT_SESSION_KEY_PREFIX}:${scopeId}:${sessionName}`; } diff --git a/packages/device-selection/package.json b/packages/device-selection/package.json index 4c7c335d56..57a1b98fc3 100644 --- a/packages/device-selection/package.json +++ b/packages/device-selection/package.json @@ -35,5 +35,8 @@ "types": "./src/inventory-gateways.fixtures.ts", "default": "./src/inventory-gateways.fixtures.ts" } + }, + "devDependencies": { + "@agent-device/test-kit": "workspace:*" } } diff --git a/packages/device-selection/src/__tests__/dispatch-resolve.test.ts b/packages/device-selection/src/__tests__/dispatch-resolve.test.ts index a370eba74e..7be4fce22b 100644 --- a/packages/device-selection/src/__tests__/dispatch-resolve.test.ts +++ b/packages/device-selection/src/__tests__/dispatch-resolve.test.ts @@ -1,5 +1,11 @@ import { afterEach, beforeEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { mkdtempForTestSync } from '@agent-device/test-kit'; +import { + flushDiagnosticsToSessionFile, + withDiagnosticsScope, +} from '@agent-device/host-kit/diagnostics'; const mockFindIosSimulatorInstalledApp = vi.fn(); const mockListAppleDevices = vi.fn(); @@ -599,6 +605,42 @@ test('resolveTargetDevice keeps simulator-set scope separate from the macOS host assert.equal(requests[1]?.iosSimulatorSetPath, undefined); }); +test('the resolve_target_device phase shows the requested udid and simulator set', async () => { + // Recovery lanes answer FROM the request log: a manifest-first stop must be provable from the + // phase line alone, so the requested identity and the set it was scoped to are echoed there. + const logDir = mkdtempForTestSync('dispatch-resolve-diag-'); + const logPath = `${logDir}/requests.ndjson`; + await withTestDeviceInventory( + { + local: async () => [simulator], + findInstalledApp: mockFindIosSimulatorInstalledApp, + }, + async () => { + await withDiagnosticsScope( + { session: 'qa', command: 'record', debug: true, logPath }, + async () => { + await resolveTargetDeviceInContext({ + platform: 'apple', + udid: simulator.id, + iosSimulatorDeviceSet: '/tmp/recovery-set', + }); + flushDiagnosticsToSessionFile({ force: true }); + }, + ); + }, + ); + + const phases = fs + .readFileSync(logPath, 'utf8') + .split('\n') + .filter(Boolean) + .map((line) => JSON.parse(line) as { phase: string; data?: Record }) + .filter((entry) => entry.phase === 'resolve_target_device'); + assert.equal(phases.length, 1); + assert.equal(phases[0]?.data?.udid, simulator.id); + assert.equal(phases[0]?.data?.iosSimulatorSetPath, '/tmp/recovery-set'); +}); + test('resolveTargetDevice discovers simulators in the daemon operator set when the request names none', async () => { vi.stubEnv('AGENT_DEVICE_IOS_SIMULATOR_DEVICE_SET', '/tmp/operator-set'); const requests: DeviceInventoryRequest[] = []; diff --git a/packages/device-selection/src/dispatch-resolve.ts b/packages/device-selection/src/dispatch-resolve.ts index bd0382c93e..3378153e0e 100644 --- a/packages/device-selection/src/dispatch-resolve.ts +++ b/packages/device-selection/src/dispatch-resolve.ts @@ -104,6 +104,11 @@ export async function resolveTargetDeviceSelection( const diagnosticData = { platform: inventoryRequest.platform, target: flags.target, + // The selector the resolution was asked for, echoed for recovery lanes whose whole job is + // re-resolving one stated device: which udid was requested and which simulator set was + // scoped must be visible in the request log without debug-only dumps. + ...(inventoryRequest.udid ? { udid: inventoryRequest.udid } : {}), + ...(iosSimulatorSetPath ? { iosSimulatorSetPath } : {}), cacheHit: false, }; return await withDiagnosticTimer( diff --git a/packages/device-selection/tsconfig.json b/packages/device-selection/tsconfig.json index 247a915b90..4ddd6ceaf5 100644 --- a/packages/device-selection/tsconfig.json +++ b/packages/device-selection/tsconfig.json @@ -10,5 +10,18 @@ "tsBuildInfoFile": "./dist-types/index.tsbuildinfo" }, "include": ["src"], - "references": [{ "path": "../contracts" }, { "path": "../host-kit" }, { "path": "../kernel" }] + "references": [ + { + "path": "../contracts" + }, + { + "path": "../host-kit" + }, + { + "path": "../kernel" + }, + { + "path": "../test-kit" + } + ] } diff --git a/packages/kernel/src/contracts.test.ts b/packages/kernel/src/contracts.test.ts new file mode 100644 index 0000000000..45976dd723 --- /dev/null +++ b/packages/kernel/src/contracts.test.ts @@ -0,0 +1,19 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { namesSessionExplicitly } from './contracts.ts'; + +test('a client that set the explicit-session meta names its session', () => { + assert.equal(namesSessionExplicitly({ meta: { sessionExplicit: true } }), true); +}); + +test('a non-blank session flag names a session as firmly as the client meta does', () => { + assert.equal(namesSessionExplicitly({ flags: { session: 'e2e-ios-0' } }), true); +}); + +test('a blank session flag and an unset meta name no session', () => { + for (const session of ['', ' ']) { + assert.equal(namesSessionExplicitly({ flags: { session } }), false); + } + assert.equal(namesSessionExplicitly({}), false); + assert.equal(namesSessionExplicitly({ meta: { sessionExplicit: false } }), false); +}); diff --git a/packages/kernel/src/contracts.ts b/packages/kernel/src/contracts.ts index 8e034226ba..a46848f172 100644 --- a/packages/kernel/src/contracts.ts +++ b/packages/kernel/src/contracts.ts @@ -256,6 +256,24 @@ export function errorResponse( export const NO_ACTIVE_SESSION_MESSAGE = 'No active session. Run open first.'; +/** + * Whether the caller named the session it wants, traced from what the request carries rather than + * from the session string. Every request has a session *name*, because an unnamed one inherits the + * workspace's own session address, so comparing names cannot answer this. Both carriers count: + * every client sets `meta.sessionExplicit`, and the commands whose own input schema declares + * `--session` also deliver the value in `flags.session`. The flag bag reaches this predicate in + * both shapes the wire carries it in: a command's declared flag type, or a generic record from a + * runner that merges flags (batch steps); the string check below is what decides either way. + */ +export function namesSessionExplicitly(request: { + flags?: { session?: unknown } | Record | undefined; + meta?: { sessionExplicit?: boolean } | undefined; +}): boolean { + if (request.meta?.sessionExplicit === true) return true; + const value = request.flags?.session; + return typeof value === 'string' && value.trim().length > 0; +} + /** * Shared "No active session. Run open first." failure used by handlers that require * an open session before dispatching. diff --git a/packages/kernel/src/errors.test.ts b/packages/kernel/src/errors.test.ts index ce59b2950c..795a9ad691 100644 --- a/packages/kernel/src/errors.test.ts +++ b/packages/kernel/src/errors.test.ts @@ -20,6 +20,7 @@ test('the pre-dispatch refusal reasons are the values consumers branch on', () = assert.deepEqual(PRE_DISPATCH_REFUSAL_REASONS, { sessionAppRequired: 'session_app_required', sessionOrDeviceSelectorRequired: 'session_or_device_selector_required', + namedSessionNotFound: 'named_session_not_found', appNotInstalled: 'app_not_installed', }); }); diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 3715a92e74..81221e42e1 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -274,6 +274,15 @@ export const PRE_DISPATCH_REFUSAL_REASONS = { * find a device for it; the command it asks for stays in the message. */ sessionOrDeviceSelectorRequired: 'session_or_device_selector_required', + /** + * The caller named a session by hand and this daemon holds no session under that address, so the + * command stopped before its device was chosen. Recovery: `open` that session again, or drop + * `--session` and name a device selector to keep using the daemon sessionlessly. Unlike + * `sessionOrDeviceSelectorRequired`, the caller *did* supply a target — a session address — and it + * is that address which is gone, which is what a caller retries on after its daemon was replaced + * or restarted mid-run (#3674). + */ + namedSessionNotFound: 'named_session_not_found', /** * An app-scoped setting named an app that resolves to no installed app on the target, so nothing * was granted, revoked, or cleared under that name. Recovery: run `apps` for the exact bundle id @@ -301,6 +310,22 @@ export function sessionAppRequiredDetails(): RefusalDetails< }; } +/** + * The details of a request whose hand-named session this daemon does not hold. Every seam that + * answers this event (the resolution guard in the daemon, `appstate`'s platform-aware refusal) + * builds its details here, so one branch on `named_session_not_found` sees the same shape + * wherever a lost session surfaces (#3674). + */ +export function namedSessionNotFoundDetails( + session: string, +): RefusalDetails<(typeof PRE_DISPATCH_REFUSAL_REASONS)['namedSessionNotFound']> { + return { + reason: PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound, + dispatched: 'no', + session, + }; +} + /** The details of an app-scoped setting refused because its named app resolves to no installed app. */ export function appNotInstalledDetails(): RefusalDetails< (typeof PRE_DISPATCH_REFUSAL_REASONS)['appNotInstalled'] diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d306e95b9..f5aa7ad0a3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -402,6 +402,10 @@ importers: '@agent-device/kernel': specifier: workspace:* version: link:../kernel + devDependencies: + '@agent-device/test-kit': + specifier: workspace:* + version: link:../test-kit packages/host-kit: dependencies: diff --git a/src/sdk/batch-public.test.ts b/src/sdk/batch-public.test.ts index a73c0cea2c..6bb917c1f4 100644 --- a/src/sdk/batch-public.test.ts +++ b/src/sdk/batch-public.test.ts @@ -20,8 +20,10 @@ test('public batch entrypoint exports daemon-compatible orchestration helpers', const response = await runBatch(req, 'resolved-session', async (stepReq) => { seenCommands.push(stepReq.command); + // The wire field pins the resolved address; the step flag names a session only when the + // caller did (#3674). assert.equal(stepReq.session, 'resolved-session'); - assert.equal(stepReq.flags?.session, 'resolved-session'); + assert.equal(stepReq.flags?.session, undefined); if (stepReq.command === 'open') { assert.equal(stepReq.flags?.platform, 'ios'); assert.equal(stepReq.flags?.udid, 'sim-1'); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index e0014faa7f..dc74c981de 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -501,7 +501,8 @@ Read-only commands such as `get`, `snapshot`, and `wait` report `no`, and so do These refusals carry `dispatched: no` and a typed `error.details.reason`, so branch on the reason, not the message: -- `session_or_device_selector_required`: the request had neither an active session nor a device selector. Open a session or pass `--platform`. +- `session_or_device_selector_required`: the request had neither an active session nor a device selector. Open a session or pass `--platform`. Commands that require a session or a device selector before they read the request's target (`boot`, `clipboard`, `push`, `apps`, and siblings) answer this even when the request named a session the daemon does not hold. +- `named_session_not_found`: the request named a session this daemon does not hold, with no device selector to fall back on (or no selector it can answer without the session — `appstate` on an Apple target refuses this even with an explicit device). The daemon lost its sessions (replaced or restarted), or the name is wrong; `agent-device session list` shows what it holds. Commands that require a session before they read anything else (interactions, `find` clicks, RN tooling, `screenshot`-style generic commands) answer the same missing-session cause as a message-level `SESSION_NOT_FOUND` carrying no `details.reason` — some carry a `dispatched` stamp from the disclosure layer, some carry no `details` at all; read the cause from the message, and read a present `dispatched` with its usual meaning. - `session_app_required`: a per-app setting was requested but the session binds no app and none was named. Run `open `, or name the app on the request for a setting that takes one (`settings clear-app-state com.example.app`). - `app_not_installed`: the named app resolves to no installed app on the target. Run `apps --all` for the exact bundle id or package, or install the app, then retry. - `webdriver_route_unsupported`: on a WebDriver-backed device cloud, the driver does not implement the route; the limit is the cloud's driver, not the device. A command with a second route (`orientation` has two) tries it before failing. After an ambiguous failure on that transport, only read-only requests are resent.