From ddd1817dcc3c12ffecdb166ec8c30f852a3cb488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 15 Oct 2026 22:04:12 +0200 Subject: [PATCH 01/15] fix(daemon): answer a named missing session on the session, not the device (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A command that names a session this daemon does not hold used to fall through to resolveTargetDevice({}) — the session-attached command carries no device selectors, so the device was picked from the host inventory. With two devices booted the caller met AMBIGUOUS_MATCH telling it to select a device; with one it met an app-session error. Either way the answer depended on how many devices happened to be up and sent the caller after a device selector when what it lost was its session. Refuse at the seam instead: requireNamedSessionHoldsDevice throws SESSION_NOT_FOUND with typed reason named_session_not_found and dispatched=no, wired into every resolution that would otherwise choose a device (snapshot, selector runtime, recording, sessionless close). A request that never named a session stays sessionless by design, and one that names a device selector as well keeps its stated target. Provenance is traced, not inferred: namesSessionExplicitly reads meta.sessionExplicit or a non-blank flags.session, replacing the two copies of hasExplicitSessionFlag. It lives in @agent-device/kernel/ contracts, beside the meta field it reads, so every consumer already evaluates it; the guard module reaches the hot seams through a function-scoped import, keeping the eager-closure gate at parity (ADR-0019 loading shape). The refusal is pinned by a router regression test, unit tests, and a new daemon.route.named-session-refusal dispatch-disclosure row. --- contracts/fixtures/dispatch-disclosure.json | 6 + .../core/explicit-session-resolution.test.ts | 119 ++++++++++++++++ .../src/core/explicit-session-resolution.ts | 60 ++++++++ .../src/core/snapshot-runtime-binding.ts | 2 +- packages/daemon/src/core/snapshot-session.ts | 6 +- .../request-dispatch-disclosure.test.ts | 16 +++ .../src/execution/selector-runtime-backend.ts | 6 + packages/daemon/src/execution/wait-runtime.ts | 2 +- .../resources/session-device-resolution.ts | 4 - .../request-router-named-session.test.ts | 134 ++++++++++++++++++ .../handlers/record-runtime-request.ts | 4 +- .../src/sessions/handlers/record-runtime.ts | 7 + .../src/sessions/handlers/session-appstate.ts | 5 +- .../daemon/src/sessions/handlers/snapshot.ts | 12 +- .../internal/session-close.ts | 7 +- .../daemon/src/sessions/session-routing.ts | 11 +- packages/kernel/src/contracts.test.ts | 19 +++ packages/kernel/src/contracts.ts | 16 +++ packages/kernel/src/errors.test.ts | 1 + packages/kernel/src/errors.ts | 9 ++ website/docs/docs/commands.md | 1 + 21 files changed, 415 insertions(+), 32 deletions(-) create mode 100644 packages/daemon/src/core/explicit-session-resolution.test.ts create mode 100644 packages/daemon/src/core/explicit-session-resolution.ts create mode 100644 packages/daemon/src/server/__tests__/request-router-named-session.test.ts create mode 100644 packages/kernel/src/contracts.test.ts 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/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..87a23b96de --- /dev/null +++ b/packages/daemon/src/core/explicit-session-resolution.ts @@ -0,0 +1,60 @@ +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, + PRE_DISPATCH_REFUSAL_REASONS, + 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.`, + { + reason: PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound, + dispatched: 'no', + session: 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<(typeof PRE_DISPATCH_REFUSAL_REASONS)['namedSessionNotFound']>, + ); +} + +/** + * 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..56206bdbd3 100644 --- a/packages/daemon/src/core/snapshot-session.ts +++ b/packages/daemon/src/core/snapshot-session.ts @@ -9,12 +9,14 @@ 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 ?? {})); + 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..418e64086a 100644 --- a/packages/daemon/src/execution/request-dispatch-disclosure.test.ts +++ b/packages/daemon/src/execution/request-dispatch-disclosure.test.ts @@ -273,6 +273,21 @@ 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. + */ +async function namedSessionRefusedBeforeDeviceSelection(): Promise { + const failure = await route(makeSession(SESSION), { + command: 'snapshot', + positionals: [], + session: 'session-never-opened', + meta: { requestId: 'named-session-refusal', sessionExplicit: true }, + }); + assert.fail(`expected the named-session refusal, got ${String(failure)}`); +} + function captureNodes(nodes: readonly unknown[]): void { gestureRuntimeSpies.captureSnapshot.mockResolvedValue({ backend: 'xctest', @@ -354,6 +369,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..b8a6d193eb 100644 --- a/packages/daemon/src/execution/selector-runtime-backend.ts +++ b/packages/daemon/src/execution/selector-runtime-backend.ts @@ -116,6 +116,12 @@ async function resolveSelectorRuntimeDevice( const ref = params.sessionStore.lookup(params.sessionName); const session = ref?.session; if (!session && requireSession) return { ok: false, response: noActiveSessionError() }; + const { requireNamedSessionHoldsDevice } = await import('../core/explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ + req: params.req, + sessionAddress: params.sessionName, + session, + }); 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..6b4db458e2 --- /dev/null +++ b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts @@ -0,0 +1,134 @@ +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 { + createRequestHandler, + lifecycleDeviceRuntimeGateway, +} from '../../__tests__/support/test-device-runtime-gateway.ts'; +import type { DaemonRequest } from '../../core/daemon-request.ts'; +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 }) { + 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: lifecycleDeviceRuntimeGateway, + 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('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. + const listings = { count: 0 }; + const { handler } = makeHandler(listings); + + const response = await handler(request({ flags: { platform: 'ios', udid: 'SIM-A' } })); + + 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 () => { + const { handler } = makeHandler({ count: 0 }); + + 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}"`); +}); 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..705a4209ae 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -140,6 +140,13 @@ async function resolveRecordingSession( params: RecordRuntimeHandlerParams, ref: SessionRef | undefined, ): Promise> { + const { requireNamedSessionHoldsDevice } = + await import('../../core/explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ + req: params.req, + sessionAddress: params.sessionName, + session: ref?.session, + }); 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 }; diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index d9aff811f5..f1f7cd3b15 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -1,8 +1,7 @@ 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 { errorResponse, namesSessionExplicitly } from '@agent-device/kernel/contracts'; import { - hasExplicitSessionFlag, requireSessionOrExplicitSelector, resolveCommandDevice, selectorTargetsSessionDevice, @@ -55,7 +54,7 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge const flags = req.flags ?? {}; const normalizedPlatform = flags.platform; - if (!session && hasExplicitSessionFlag(flags)) { + if (!session && namesSessionExplicitly(req)) { return { refusal: appStateSessionNotFoundResponse(sessionName, normalizedPlatform) }; } 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/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/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..8547bd5955 100644 --- a/packages/kernel/src/contracts.ts +++ b/packages/kernel/src/contracts.ts @@ -256,6 +256,22 @@ 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`. + */ +export function namesSessionExplicitly(request: { + flags?: { session?: unknown } | 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..3b5ff066b1 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 diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index e0014faa7f..da4a2a51ca 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -502,6 +502,7 @@ 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`. +- `named_session_not_found`: the request named a session this daemon does not hold, with no device selector to fall back on. The daemon lost its sessions (replaced or restarted), or the name is wrong; `agent-device session list` shows what it holds. - `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. From 31005a210ff89a78969d88887c574ba215106934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 05:45:59 +0200 Subject: [PATCH 02/15] fix(batch): stamp the step session flag only when the caller named one (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The batch runner stamped flags.session on every step so nested invokes route to the parent's resolved address. The wire `session` field already does that; the flag is a provenance carrier, and the new named-session refusal reads it. An unconditional stamp therefore made a sessionless batch look caller-addressed and met SESSION_NOT_FOUND at the daemon — an error no caller of the batch surface asked for, with advice ("omit --session") the surface does not even offer. The stamp now follows the same traced provenance as every other carrier: meta.sessionExplicit or a caller-set step flag. Also from the same review: the named-session disclosure row's driver now pins the producer's own code and typed reason, so a reverted guard cannot satisfy it with the device-selection error it replaced; commands.md states which pre-dispatch refusals answer session_or_device_selector_required even for a named-but-unheld session. --- .../src/__tests__/batch.test.ts | 41 +++++++++++++++++++ packages/command-registry/src/batch.ts | 7 +++- .../request-dispatch-disclosure.test.ts | 12 +++++- .../session-devices-batch-runtime.test.ts | 4 +- src/sdk/batch-public.test.ts | 4 +- website/docs/docs/commands.md | 2 +- 6 files changed, 64 insertions(+), 6 deletions(-) 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/execution/request-dispatch-disclosure.test.ts b/packages/daemon/src/execution/request-dispatch-disclosure.test.ts index 418e64086a..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'; @@ -276,7 +276,10 @@ async function sessionOrSelectorRefusedBeforeRouting(): Promise { /** * 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. + * 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), { @@ -284,6 +287,11 @@ async function namedSessionRefusedBeforeDeviceSelection(): Promise { 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)}`); } 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/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 da4a2a51ca..3cb3ce07cb 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -501,7 +501,7 @@ 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. The daemon lost its sessions (replaced or restarted), or the name is wrong; `agent-device session list` shows what it holds. - `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. From 23782d71c1f0614cef1c41b404509ff439759b58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 07:19:27 +0200 Subject: [PATCH 03/15] fix(daemon): answer the named-session refusal with its own reason at every seam Round-2 review found #3674's refusal reaching some callers as an untyped event. `get`/`is` answered a missing session with the generic "no active session" error before the provenance guard could run; `record start` let its app-scope INVALID_ARGS refusal win over a session that was gone altogether; `appstate` carried no typed details and refused requests that had also stated a concrete device. The refusal details now have one producer, `namedSessionNotFoundDetails` beside its sibling builders, that the guard's AppError and `appstate`'s platform-aware response both build from; each seam orders the guard before its own missing-session refusal; `appstate` preserves a request naming an identity selector, matching the resolution guard. The guard module's function-scoped import is confined to the missing-session branch it can fire on: that keeps the eager- closure gate at parity and removes a first-time transform cost from the held-session wait path, which under coverage instrumentation alone overran `wait-runtime.test.ts`'s fake-timer budget at the unconditional form (3/3 deterministic failures restored, 3/3 passes at this form). `namesSessionExplicitly` now accepts the generic flag-record shape the batch runner carries, fixing the TS2345 the batch call site raised. --- .../src/core/explicit-session-resolution.ts | 11 ++-- packages/daemon/src/core/snapshot-session.ts | 12 +++- .../src/execution/selector-runtime-backend.ts | 20 ++++-- .../request-router-named-session.test.ts | 66 +++++++++++++++++-- .../__tests__/session-appstate.test.ts | 58 +++++++++++++++- .../src/sessions/handlers/record-runtime.ts | 20 ++++-- .../src/sessions/handlers/session-appstate.ts | 14 +++- .../internal/session-close.ts | 2 + packages/kernel/src/contracts.ts | 6 +- packages/kernel/src/errors.ts | 16 +++++ 10 files changed, 196 insertions(+), 29 deletions(-) diff --git a/packages/daemon/src/core/explicit-session-resolution.ts b/packages/daemon/src/core/explicit-session-resolution.ts index 87a23b96de..8820b3daf9 100644 --- a/packages/daemon/src/core/explicit-session-resolution.ts +++ b/packages/daemon/src/core/explicit-session-resolution.ts @@ -3,7 +3,7 @@ import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; import { AppError, - PRE_DISPATCH_REFUSAL_REASONS, + namedSessionNotFoundDetails, type RefusalDetails, } from '@agent-device/kernel/errors'; import { hasDeviceSelectionInput } from './device/device-selector-intent.ts'; @@ -18,14 +18,15 @@ export function namedSessionNotFoundError(sessionAddress: string): AppError { 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.`, + // One producer for the refusal shape: the reason, dispatched marker, and session address come + // from the kernel builder every named-session seam shares (#3674); only this site's message + // adds the open/omit guidance and the session-list hint. { - reason: PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound, - dispatched: 'no', - session: sessionAddress, + ...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<(typeof PRE_DISPATCH_REFUSAL_REASONS)['namedSessionNotFound']>, + } satisfies RefusalDetails<'named_session_not_found'>, ); } diff --git a/packages/daemon/src/core/snapshot-session.ts b/packages/daemon/src/core/snapshot-session.ts index 56206bdbd3..615bbc0f53 100644 --- a/packages/daemon/src/core/snapshot-session.ts +++ b/packages/daemon/src/core/snapshot-session.ts @@ -14,8 +14,16 @@ export async function resolveSessionDevice( ) { const ref = boundRef ?? sessionStore.lookup(sessionName); const session = ref ? sessionStore.requireCurrent(ref) : undefined; - const { requireNamedSessionHoldsDevice } = await import('./explicit-session-resolution.ts'); - requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session }); + // The guard asks whether a *missing* session was named, so a held session answers it for free + // and the refused path is the only one that loads the module — keeping the guard off the eager + // closure of every request that resolves through here (ADR-0019 loading shape, #3674). It also + // keeps a first-time module load out of the held-session hot path, whose transform cost under + // coverage instrumentation alone overran `wait-runtime.test.ts`'s fake-timer budget when the + // import ran unconditionally. + 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/selector-runtime-backend.ts b/packages/daemon/src/execution/selector-runtime-backend.ts index b8a6d193eb..4d241e902e 100644 --- a/packages/daemon/src/execution/selector-runtime-backend.ts +++ b/packages/daemon/src/execution/selector-runtime-backend.ts @@ -115,13 +115,21 @@ async function resolveSelectorRuntimeDevice( params.captureProof ??= {}; const ref = params.sessionStore.lookup(params.sessionName); const session = ref?.session; + // Ordered before the required-session return on purpose: a request that *named* this address + // deserves the typed named-session refusal, not the generic one; one that named nothing still + // lands on noActiveSessionError below, which is its own pre-existing refusal. The guard can only + // fire for a missing session, so only that path 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 { requireNamedSessionHoldsDevice } = await import('../core/explicit-session-resolution.ts'); - requireNamedSessionHoldsDevice({ - req: params.req, - sessionAddress: params.sessionName, - session, - }); const device = session?.device ?? (await resolveTargetDevice(params.req.flags ?? {})); return { ok: true, ref, session, device }; } 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 index 6b4db458e2..f7783a46b8 100644 --- a/packages/daemon/src/server/__tests__/request-router-named-session.test.ts +++ b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts @@ -3,10 +3,13 @@ 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'; @@ -35,7 +38,10 @@ const BOOTED_SIMULATORS: DeviceInfo[] = [ bootedSimulator('SIM-B', 'Perf iPhone 17'), ]; -function makeHandler(listings: { count: number }) { +function makeHandler( + listings: { count: number }, + deviceRuntimeGateway: DeviceRuntimeGateway = lifecycleDeviceRuntimeGateway, +) { const sessionStore = makeSessionStore('agent-device-router-named-session-'); const handler = createRequestHandler({ createCommandSurface: fakeCreateCommandSurface, @@ -43,7 +49,7 @@ function makeHandler(listings: { count: number }) { token: 'test-token', sessionStore, leaseRegistry: new LeaseRegistry(), - deviceRuntimeGateway: lifecycleDeviceRuntimeGateway, + deviceRuntimeGateway, deviceInventoryGateways: createTestDeviceInventoryGateways({ local: async () => { listings.count += 1; @@ -90,6 +96,39 @@ test('the refusal answers without listing the host devices at all', async () => 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); +}); + 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. @@ -108,12 +147,21 @@ test('the same command with no session named still resolves a device sessionless 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. + // 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 { handler } = makeHandler(listings); + 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, @@ -123,7 +171,11 @@ test('a named session beside a device selector keeps honouring the device it nam }); test('a sessionless close naming a missing session answers on the session', async () => { - const { handler } = makeHandler({ count: 0 }); + // 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'] })); @@ -131,4 +183,8 @@ test('a sessionless close naming a missing session answers on the session', asyn 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__/session-appstate.test.ts b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts index f60bc51a7b..5918c709a1 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,53 @@ test('appstate --platform apple missing-session error never suggests omitting -- } }); +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.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 705a4209ae..56f3b8088b 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -99,6 +99,19 @@ async function handleRecordCommandUnsafe( const { req, sessionName, sessionStore } = params; const existingRef = sessionStore.lookup(sessionName); const existingSession = existingRef?.session; + // The named-session refusal runs before any record-specific refusal: a request whose session + // this daemon lost is answered on the session first, whatever it wanted to record (#3674). The + // guard can only fire for a missing session, so only that path pays the function-scoped import + // (ADR-0019 loading shape). + if (!existingSession) { + const { requireNamedSessionHoldsDevice } = + await import('../../core/explicit-session-resolution.ts'); + requireNamedSessionHoldsDevice({ + req, + sessionAddress: sessionName, + session: existingSession, + }); + } const { plan, scope } = resolveRecordPlan(req, existingSession); if (plan.kind === 'start' && !isWholeScreenRecordingScope(scope) && !existingSession) { return missingAppSessionResponse(req); @@ -140,13 +153,6 @@ async function resolveRecordingSession( params: RecordRuntimeHandlerParams, ref: SessionRef | undefined, ): Promise> { - const { requireNamedSessionHoldsDevice } = - await import('../../core/explicit-session-resolution.ts'); - requireNamedSessionHoldsDevice({ - req: params.req, - sessionAddress: params.sessionName, - session: ref?.session, - }); 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 }; diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index f1f7cd3b15..cf064d4a41 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -1,6 +1,8 @@ import type { DaemonResponse } from '@agent-device/kernel/contracts'; import { isApplePlatform, type PlatformSelector } from '@agent-device/kernel/device'; import { errorResponse, namesSessionExplicitly } from '@agent-device/kernel/contracts'; +import { namedSessionNotFoundDetails } from '@agent-device/kernel/errors'; +import { hasExplicitDeviceSelector } from '../../core/device/device-selector-intent.ts'; import { requireSessionOrExplicitSelector, resolveCommandDevice, @@ -21,15 +23,19 @@ function appStateSessionNotFoundResponse( // 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. + // The details are the shared named-session refusal's, so one branch on + // `named_session_not_found` sees the same shape wherever a lost session answers (#3674). 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), ); } @@ -54,7 +60,13 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge const flags = req.flags ?? {}; const normalizedPlatform = flags.platform; - if (!session && namesSessionExplicitly(req)) { + // Same priority as the resolution guard (#3674), read for what this command can actually serve: + // an identity selector (`--device`/`--udid`/`--serial`) states a concrete target the daemon can + // answer sessionlessly — an Android device, or an iOS one the caller will run `open` against — + // so the refusal stays out of the way there. A bare `--platform` filter states no device this + // command can answer without a session (Apple appstate is session-required one step later), so + // for those the typed named-session refusal remains the truest answer. + if (!session && namesSessionExplicitly(req) && !hasExplicitDeviceSelector(flags)) { return { refusal: appStateSessionNotFoundResponse(sessionName, normalizedPlatform) }; } 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 c44d1e71c5..1d99ea3957 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts @@ -433,6 +433,8 @@ async function closeWithoutSession(params: { bindDevice?: BindDeviceRuntime; }): Promise { const { req, sessionName, logPath, inspectFacts, bindDevice, platformServices } = params; + // There is no session here by definition, so this is exactly the refused shape; the guard's own + // provenance check still decides between the typed refusal and sessionless close. const { requireNamedSessionHoldsDevice } = await import('../../../core/explicit-session-resolution.ts'); requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session: undefined }); diff --git a/packages/kernel/src/contracts.ts b/packages/kernel/src/contracts.ts index 8547bd5955..a46848f172 100644 --- a/packages/kernel/src/contracts.ts +++ b/packages/kernel/src/contracts.ts @@ -261,10 +261,12 @@ export const NO_ACTIVE_SESSION_MESSAGE = 'No active session. Run open first.'; * 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`. + * `--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 } | undefined; + flags?: { session?: unknown } | Record | undefined; meta?: { sessionExplicit?: boolean } | undefined; }): boolean { if (request.meta?.sessionExplicit === true) return true; diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 3b5ff066b1..81221e42e1 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -310,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'] From a5ef4d8db83f50e846bc316333f635125bf84273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 08:44:56 +0200 Subject: [PATCH 04/15] fix(daemon): answer Apple appstate on the named session whatever the selector states appstate's identity-selector exception over-preserved: a named missing session with an explicit Apple device fell through to the session-required refusal, a route with no sessionless answer. Keep the typed named-session refusal wherever the command cannot serve the stated target without a session, and let only non-Apple identity selectors keep their target. Also pins the appstate refusal's typed reason/session/dispatched details, adds the record seam's no-inventory assertion, and trims seam comments to their invariants. --- .../src/core/explicit-session-resolution.ts | 3 --- packages/daemon/src/core/snapshot-session.ts | 8 ++------ .../src/execution/selector-runtime-backend.ts | 7 ++----- .../request-router-named-session.test.ts | 1 + .../__tests__/session-appstate-input.test.ts | 5 ++++- .../src/sessions/handlers/record-runtime.ts | 6 ++---- .../src/sessions/handlers/session-appstate.ts | 18 +++++++++--------- .../internal/session-close.ts | 2 -- website/docs/docs/commands.md | 2 +- 9 files changed, 21 insertions(+), 31 deletions(-) diff --git a/packages/daemon/src/core/explicit-session-resolution.ts b/packages/daemon/src/core/explicit-session-resolution.ts index 8820b3daf9..b33ee79512 100644 --- a/packages/daemon/src/core/explicit-session-resolution.ts +++ b/packages/daemon/src/core/explicit-session-resolution.ts @@ -18,9 +18,6 @@ export function namedSessionNotFoundError(sessionAddress: string): AppError { 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.`, - // One producer for the refusal shape: the reason, dispatched marker, and session address come - // from the kernel builder every named-session seam shares (#3674); only this site's message - // adds the open/omit guidance and the session-list hint. { ...namedSessionNotFoundDetails(sessionAddress), hint: diff --git a/packages/daemon/src/core/snapshot-session.ts b/packages/daemon/src/core/snapshot-session.ts index 615bbc0f53..e65439e2bd 100644 --- a/packages/daemon/src/core/snapshot-session.ts +++ b/packages/daemon/src/core/snapshot-session.ts @@ -14,12 +14,8 @@ export async function resolveSessionDevice( ) { const ref = boundRef ?? sessionStore.lookup(sessionName); const session = ref ? sessionStore.requireCurrent(ref) : undefined; - // The guard asks whether a *missing* session was named, so a held session answers it for free - // and the refused path is the only one that loads the module — keeping the guard off the eager - // closure of every request that resolves through here (ADR-0019 loading shape, #3674). It also - // keeps a first-time module load out of the held-session hot path, whose transform cost under - // coverage instrumentation alone overran `wait-runtime.test.ts`'s fake-timer budget when the - // import ran unconditionally. + // 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 }); diff --git a/packages/daemon/src/execution/selector-runtime-backend.ts b/packages/daemon/src/execution/selector-runtime-backend.ts index 4d241e902e..aa329d7900 100644 --- a/packages/daemon/src/execution/selector-runtime-backend.ts +++ b/packages/daemon/src/execution/selector-runtime-backend.ts @@ -115,11 +115,8 @@ async function resolveSelectorRuntimeDevice( params.captureProof ??= {}; const ref = params.sessionStore.lookup(params.sessionName); const session = ref?.session; - // Ordered before the required-session return on purpose: a request that *named* this address - // deserves the typed named-session refusal, not the generic one; one that named nothing still - // lands on noActiveSessionError below, which is its own pre-existing refusal. The guard can only - // fire for a missing session, so only that path pays the function-scoped import (ADR-0019 - // loading shape). + // 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'); 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 index f7783a46b8..976e6627bd 100644 --- a/packages/daemon/src/server/__tests__/request-router-named-session.test.ts +++ b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts @@ -127,6 +127,7 @@ test('a record start naming a missing session answers on the session, not on app 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 () => { 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..6737f5ac4a 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,9 @@ 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'); } }); diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 56f3b8088b..00a6f500a7 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -99,10 +99,8 @@ async function handleRecordCommandUnsafe( const { req, sessionName, sessionStore } = params; const existingRef = sessionStore.lookup(sessionName); const existingSession = existingRef?.session; - // The named-session refusal runs before any record-specific refusal: a request whose session - // this daemon lost is answered on the session first, whatever it wanted to record (#3674). The - // guard can only fire for a missing session, so only that path pays the function-scoped import - // (ADR-0019 loading shape). + // Ordered before every record-specific refusal: a named missing session answers on the + // session. Only that branch pays the function-scoped import (ADR-0019 loading shape). if (!existingSession) { const { requireNamedSessionHoldsDevice } = await import('../../core/explicit-session-resolution.ts'); diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index cf064d4a41..c10647c104 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -23,8 +23,6 @@ function appStateSessionNotFoundResponse( // 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. - // The details are the shared named-session refusal's, so one branch on - // `named_session_not_found` sees the same shape wherever a lost session answers (#3674). if (isApplePlatform(platform)) { return errorResponse( 'SESSION_NOT_FOUND', @@ -60,13 +58,15 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge const flags = req.flags ?? {}; const normalizedPlatform = flags.platform; - // Same priority as the resolution guard (#3674), read for what this command can actually serve: - // an identity selector (`--device`/`--udid`/`--serial`) states a concrete target the daemon can - // answer sessionlessly — an Android device, or an iOS one the caller will run `open` against — - // so the refusal stays out of the way there. A bare `--platform` filter states no device this - // command can answer without a session (Apple appstate is session-required one step later), so - // for those the typed named-session refusal remains the truest answer. - if (!session && namesSessionExplicitly(req) && !hasExplicitDeviceSelector(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) }; } 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 1d99ea3957..c44d1e71c5 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts @@ -433,8 +433,6 @@ async function closeWithoutSession(params: { bindDevice?: BindDeviceRuntime; }): Promise { const { req, sessionName, logPath, inspectFacts, bindDevice, platformServices } = params; - // There is no session here by definition, so this is exactly the refused shape; the guard's own - // provenance check still decides between the typed refusal and sessionless close. const { requireNamedSessionHoldsDevice } = await import('../../../core/explicit-session-resolution.ts'); requireNamedSessionHoldsDevice({ req, sessionAddress: sessionName, session: undefined }); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 3cb3ce07cb..86c2294fab 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -502,7 +502,7 @@ 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`. 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. The daemon lost its sessions (replaced or restarted), or the name is wrong; `agent-device session list` shows what it holds. +- `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. - `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. From f9470754b58e5540a0f3b62a34a17ff58e126e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 13:49:10 +0200 Subject: [PATCH 05/15] test(daemon): pin that the appstate refusal resolves no device (#3674) The appstate refusal reported dispatched: 'no', a value it writes itself, so the test would still pass if the handler resolved a device before answering it. The new assertion pins the behavior instead: resolving the target device never runs on the named-session path. Reordering the guard to resolve-then-refuse with the same code, message, reason and dispatched fields passes every other pin and fails only this one. --- .../sessions/handlers/__tests__/session-appstate-input.test.ts | 1 + 1 file changed, 1 insertion(+) 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 6737f5ac4a..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 @@ -242,6 +242,7 @@ test('appstate with explicit missing session returns SESSION_NOT_FOUND', async ( 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(); } }); From 2ce7a127495002f1008044b5c49641868a03c21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 15:30:38 +0200 Subject: [PATCH 06/15] fix(daemon): let record stop reach the durable manifest a named session addresses (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The named-session guard covered every record action, so after a daemon restart — the exact state #3674 serves — `record stop --session qa` was refused before the on-disk `screen-recording.resource.json` was read, killing the manifest recovery that `help record` and commands.md promise. That recovery is only reachable through a named session: the manifest path derives from the session address, and the lane proves the manifest's session and device before serving. Start is different: it can only be answered on a chosen device, so it keeps the pre-selection refusal. Order the guard on the resolved plan (`plan.kind === 'start'`, still before the app-scope refusal) and pin both directions: the replayed completed export survives store loss, and a start after the same loss still refuses on the session. Reverting the guard fails both start pins; keeping it stop-wide fails the replay pin. Also complete the refusal taxonomy in commands.md: the session-required family answers the same event as a reasonless SESSION_NOT_FOUND, so document that branch. --- .../record-runtime-stop-recovery.test.ts | 61 ++++++++++++++++++- .../src/sessions/handlers/record-runtime.ts | 12 ++-- website/docs/docs/commands.md | 2 +- 3 files changed, 69 insertions(+), 6 deletions(-) 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..b0dc52794b 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,6 +1,6 @@ +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 { screenRecordingResourceStore } from '@agent-device/capture-kit/screen-recording-resource-store'; import { @@ -9,6 +9,65 @@ import { recordingResourcePath, } from './record-runtime.fixtures.ts'; +// The stop lane that runs WITHOUT a held session resolves its device through the selection +// dispatch. The daemon-restart tests below pin that lane, so the dispatch answers with the +// harness device here; the live-handle tests never reach it because their session is held. +const RESTART_SELECTED_DEVICE = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', +} as const; +vi.mock('@agent-device/device-selection/dispatch-resolve', () => ({ + resolveTargetDevice: vi.fn(async () => ({ ...RESTART_SELECTED_DEVICE })), +})); + +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. + 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); + + const recovered = await harness.run(['stop'], { sessionExplicit: true }); + + 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); + + 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(); +}); + 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/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 00a6f500a7..7e0c65a1f3 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -99,9 +99,14 @@ async function handleRecordCommandUnsafe( const { req, sessionName, sessionStore } = params; const existingRef = sessionStore.lookup(sessionName); const existingSession = existingRef?.session; - // Ordered before every record-specific refusal: a named missing session answers on the - // session. Only that branch pays the function-scoped import (ADR-0019 loading shape). - if (!existingSession) { + const { plan, scope } = resolveRecordPlan(req, existingSession); + // Only the start half can be answered on a device, so only it answers on the session: a named + // missing session refuses before any device is chosen, ahead of the record-specific refusals. + // The stop half must reach the durable manifest first — after a daemon restart the memory-only + // session is gone for exactly the callers whose manifest the named session addresses, and the + // recovery lane proves the manifest's session and device before serving anything. + // Only that branch pays the function-scoped import (ADR-0019 loading shape). + if (!existingSession && plan.kind === 'start') { const { requireNamedSessionHoldsDevice } = await import('../../core/explicit-session-resolution.ts'); requireNamedSessionHoldsDevice({ @@ -110,7 +115,6 @@ async function handleRecordCommandUnsafe( session: existingSession, }); } - const { plan, scope } = resolveRecordPlan(req, existingSession); if (plan.kind === 'start' && !isWholeScreenRecordingScope(scope) && !existingSession) { return missingAppSessionResponse(req); } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 86c2294fab..74b162d379 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -502,7 +502,7 @@ 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`. 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. +- `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) answer the same event as `SESSION_NOT_FOUND` with no reason; treat a reasonless `SESSION_NOT_FOUND` on a named session as this one. - `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. From 7be63bcc8867f93df66e5f9f56d5ccec692d7378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 17:04:04 +0200 Subject: [PATCH 07/15] docs: scope the reasonless-session branch to the cause, not the dispatch (#3674) The named-session row sits under the retry legend's dispatched: no umbrella, but the session-required family answers with no details at all. Reading the cause equivalence under that umbrella would promise a non-dispatch those responses never make, inviting a blind retry. The row now says the branch is on the missing-session cause only, and the missing dispatched keeps its usual no-guarantee reading. --- website/docs/docs/commands.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 74b162d379..c0466494fc 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -502,7 +502,7 @@ 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`. 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) answer the same event as `SESSION_NOT_FOUND` with no reason; treat a reasonless `SESSION_NOT_FOUND` on a named session as this one. +- `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 `SESSION_NOT_FOUND` with no `details` at all — so the same branch on the cause applies, but the `dispatched: no` guarantee above does not: read the missing `dispatched` as no guarantee, as usual. - `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. From c13c054c4e545558a6f46daa48626af03cc0e1dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 18:39:35 +0200 Subject: [PATCH 08/15] fix(daemon): take record stop recovery from the manifest, never from a host sweep (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A named session the daemon no longer holds was still answered on an unconstrained resolveTargetDevice for record stop, so a multi-boot host replied AMBIGUOUS_MATCH before the durable manifest — the one thing that can answer that stop — was read. The named-session rule now lives at the handler's only device selection: a request addressed to a session takes its device from a decodable manifest envelope, and a start, or a stop with nothing decodable to recover from, refuses with the typed named-session error. Selector and sessionless lanes keep their targets, and the manifest's own session and device proofs still fail closed. Co-Authored-Worked-On-By: Claude --- .../screen-recording-stop-recovery.ts | 22 +++- .../request-router-named-session.test.ts | 18 +++ .../record-runtime-stop-recovery.test.ts | 109 ++++++++++++++++- .../__tests__/record-runtime.fixtures.ts | 4 +- .../src/sessions/handlers/record-runtime.ts | 111 ++++++++++++++---- 5 files changed, 235 insertions(+), 29 deletions(-) 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..d3930832f1 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 device is not re-picked from the host; it is read from the manifest the named session + * addresses, 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/daemon/src/server/__tests__/request-router-named-session.test.ts b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts index 976e6627bd..22ebf30a7a 100644 --- a/packages/daemon/src/server/__tests__/request-router-named-session.test.ts +++ b/packages/daemon/src/server/__tests__/request-router-named-session.test.ts @@ -130,6 +130,24 @@ test('a record start naming a missing session answers on the session, not on app 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. 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 b0dc52794b..d693ee46da 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 @@ -2,6 +2,7 @@ import { expect, test, vi } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; 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, @@ -10,22 +11,33 @@ import { } from './record-runtime.fixtures.ts'; // The stop lane that runs WITHOUT a held session resolves its device through the selection -// dispatch. The daemon-restart tests below pin that lane, so the dispatch answers with the -// harness device here; the live-handle tests never reach it because their session is held. -const RESTART_SELECTED_DEVICE = { +// dispatch. On a host that answers anything, an unconstrained request is exactly the #3674 +// symptom, so the dispatch double here behaves like a multi-device host: it only answers a +// request that states a device identity, and an unconstrained one gets the ambiguity a +// multi-boot host really returns. The restart tests below pass only while the recovery lane +// resolves from the manifest instead of sweeping the host. +const RESTART_RECORDED_DEVICE = { platform: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator', } as const; +const resolveTargetDeviceMock = vi.fn(async (flags: { udid?: string; serial?: string } = {}) => { + if (flags.serial === RESTART_RECORDED_DEVICE.id || flags.udid === RESTART_RECORDED_DEVICE.id) { + return { ...RESTART_RECORDED_DEVICE }; + } + throw new AppError('AMBIGUOUS_MATCH', 'Multiple attached devices detected'); +}); vi.mock('@agent-device/device-selection/dispatch-resolve', () => ({ - resolveTargetDevice: vi.fn(async () => ({ ...RESTART_SELECTED_DEVICE })), + 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. + // 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-'), @@ -37,9 +49,93 @@ test('record stop after a daemon restart replays the completed export for the na 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 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' }, @@ -58,6 +154,7 @@ test('record start after a daemon restart still refuses the named missing sessio 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 }); @@ -66,6 +163,8 @@ test('record start after a daemon restart still refuses the named missing sessio 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 () => { 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..4ccdf73608 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/record-runtime.fixtures.ts @@ -75,7 +75,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 +84,7 @@ export function makeRecordRuntimeHarness( session: sessionName, command: 'record', positionals, - flags: {}, + flags: flags ?? {}, ...(meta ? { meta } : {}), }, }), diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 7e0c65a1f3..2a7ad52f5d 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,25 +111,19 @@ async function handleRecordCommandUnsafe( const existingRef = sessionStore.lookup(sessionName); const existingSession = existingRef?.session; const { plan, scope } = resolveRecordPlan(req, existingSession); - // Only the start half can be answered on a device, so only it answers on the session: a named - // missing session refuses before any device is chosen, ahead of the record-specific refusals. - // The stop half must reach the durable manifest first — after a daemon restart the memory-only - // session is gone for exactly the callers whose manifest the named session addresses, and the - // recovery lane proves the manifest's session and device before serving anything. - // Only that branch pays the function-scoped import (ADR-0019 loading shape). - if (!existingSession && plan.kind === 'start') { - const { requireNamedSessionHoldsDevice } = - await import('../../core/explicit-session-resolution.ts'); - requireNamedSessionHoldsDevice({ - req, - sessionAddress: sessionName, - session: 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( @@ -154,13 +159,77 @@ 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 an unconstrained sweep of the host, + * which with several booted devices would answer `AMBIGUOUS_MATCH` (or, worse, silently pick a + * different device) before the manifest is read. A decodable manifest carries the exact device the + * recording was started on, so the stop recovery resolves that device; a start, or a stop with no + * decodable manifest to recover from, refuses with the named-session error. The manifest's own + * session and device are still proven against the request by `assertManifestBelongsToRequest` + * before anything is served, so a manifest belonging to another session or device fails closed. + * + * 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, with the manifest assertion failing the request closed when the stated device is not + * the one the recording was started on. + */ +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 exact device a durable manifest names, by the identity flag that family answers + * to. Families without an identity flag keep the request's own selection — an address the manifest + * recorded but the request cannot restate stays resolved as it always has been, and the + * device-match assertion below still refuses a different device. + */ +async function resolveManifestDevice( + params: RecordRuntimeHandlerParams, + recorded: DeviceIdentity, +): Promise { + const selector: ResolveDeviceFlags = { platform: recorded.family }; + const identityFlag = deviceIdentityFlag(recorded.family); + if (!identityFlag) { + // A family whose manifest identity no flag can restate keeps the request's own selection; the + // manifest assertion still refuses anything but the device the recording was started on. + return await resolveTargetDevice(params.req.flags ?? {}); + } + selector[identityFlag] = recorded.id; + return await resolveTargetDevice(selector); +} + async function startRecording( params: RecordRuntimeHandlerParams, session: SessionState, From 54a38c6ad5402a7fb4b3c375e574023be12cb152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 18:39:45 +0200 Subject: [PATCH 09/15] fix(daemon): keep the typed named-session reason on the appstate device route (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `appstate --session lost --udid ` without `--platform` passed the admission guard, listed the host, resolved onto an Apple device, and answered the reasonless session-required refusal — contradicting the commands.md promise and paying for a listing the typed answer makes unnecessary. The device-runtime lane now answers a named-missing session with the same typed refusal as every other seam; a request that named no session keeps the plain session-required advice, and the non-Apple identity-selector lane is untouched. The refusal builder moved next to its sibling Apple refusals so both routes share one wire shape. Co-Authored-Worked-On-By: Claude --- .../__tests__/session-appstate.test.ts | 33 +++++++++++++++++++ .../session-appstate-apple-session.ts | 32 +++++++++++++++++- .../session-appstate-device-runtime.ts | 20 ++++++++--- .../src/sessions/handlers/session-appstate.ts | 27 ++------------- 4 files changed, 83 insertions(+), 29 deletions(-) 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 5918c709a1..7d833bfaf5 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -69,6 +69,39 @@ test('appstate --platform apple missing-session error never suggests omitting -- } }); +test('appstate --udid with no --platform still answers the typed reason for a named missing session', async () => { + // `--udid` is the Apple identity flag, so a bare `--udid ` reaches the device-runtime lane + // and lands on an Apple device. The commands.md row promises the typed reason "even with an + // explicit device", so this lane — the only appstate route that resolves a device — answers it + // too, not the reasonless session-required refusal. + 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).mockResolvedValue(iosSim); + try { + const response = await handleAppStateCommand({ + platformServices: daemonPlatformServicesFixture(), + req: appStateRequest('lost', { session: 'lost', udid: 'SIM-UDID-A' }), + sessionName: 'lost', + sessionStore: makeSessionStore(), + }); + + 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'); + } 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 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..ae958b8d47 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,15 @@ import { publicPlatformString, type DeviceInfo, } from '@agent-device/kernel/device'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; +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); @@ -26,8 +30,16 @@ export async function queryDeviceRuntimeAppState( params: RuntimeCommandHandlerParams, device: DeviceInfo, ): Promise { - if (isIosFamily(device)) return appleSessionRequiredRefusal('ios'); - if (isMacOs(device)) return appleSessionRequiredRefusal('macos'); + if (isIosFamily(device) || isMacOs(device)) { + // A request that names a session this daemon does not hold and resolves onto an Apple device + // is missing its session, not its device (#3674): the same typed refusal the session-required + // selectors answer, whatever flag routed it here. A request that named no session keeps the + // plain session-required refusal, which is the right advice for it. + if (!params.sessionStore.get(params.sessionName) && namesSessionExplicitly(params.req)) { + 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 c10647c104..42e732270d 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -1,7 +1,6 @@ import type { DaemonResponse } from '@agent-device/kernel/contracts'; -import { isApplePlatform, type PlatformSelector } from '@agent-device/kernel/device'; -import { errorResponse, namesSessionExplicitly } from '@agent-device/kernel/contracts'; -import { namedSessionNotFoundDetails } from '@agent-device/kernel/errors'; +import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; +import { isApplePlatform } from '@agent-device/kernel/device'; import { hasExplicitDeviceSelector } from '../../core/device/device-selector-intent.ts'; import { requireSessionOrExplicitSelector, @@ -10,33 +9,13 @@ import { } 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.`, - 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), - ); -} - type AppStateSessionStoreEntry = ReturnType; /** The answer of target admission: a refusal, or the authority that answers with its session. */ From 4be28951ca91fd8f74c4307602ea68891436d488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 18:39:45 +0200 Subject: [PATCH 10/15] docs: correct the reasonless-session family to the details.reason level (#3674) The r4 sentence claimed the family answers with no details at all and voided the dispatched guarantee; the disclosure layer stamps click/fill/screenshot-style reads dispatched: no and type/swipe-style mutations dispatched: unknown, so the claim was false for every member it named and wrong in both reading directions. The row now states the fact at the details.reason level, keeps the usual dispatched reading, and treats only absence (logs/network/audio) as no guarantee. Co-Authored-Worked-On-By: Claude --- website/docs/docs/commands.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index c0466494fc..dc74c981de 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -502,7 +502,7 @@ 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`. 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 `SESSION_NOT_FOUND` with no `details` at all — so the same branch on the cause applies, but the `dispatched: no` guarantee above does not: read the missing `dispatched` as no guarantee, as usual. +- `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. From 67711281e0bef3b64085c758584f95c7e479100c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 18:53:07 +0200 Subject: [PATCH 11/15] refactor(daemon): decide the appstate named-session refusal once, at admission (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The device-runtime lane re-read the session store and re-ran the naming check after awaiting device resolution, so a store change mid-await could move the request into a different error family than the one it was admitted into, and the classifier was spelled out twice. Admission now carries one typed namedSessionMissing fact, decided from the same store read as the admission session itself. A bare --udid still resolves its stated target first and answers the typed reason from the resolved device — pinned by asserting the resolution call, not just the payload — and the store-changes-during-resolution case pins the snapshot semantics. The manifest-first docstrings say what actually happens: the manifest's identity is re-resolved against the host for the live device fields, with the session/device assertions failing closed. Co-Authored-Worked-On-By: Claude --- .../screen-recording-stop-recovery.ts | 6 +- .../__tests__/session-appstate.test.ts | 74 ++++++++++++++++++- .../src/sessions/handlers/record-runtime.ts | 11 +-- .../session-appstate-device-runtime.ts | 12 +-- .../src/sessions/handlers/session-appstate.ts | 22 +++++- 5 files changed, 104 insertions(+), 21 deletions(-) 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 d3930832f1..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 @@ -79,9 +79,9 @@ export function screenRecordingManifestIsTerminal(params: ScreenRecordingManifes * 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 device is not re-picked from the host; it is read from the manifest the named session - * addresses, and `assertManifestBelongsToRequest` still fails the request closed when the - * resolved device or the session address does not match what the manifest recorded. + * 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, 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 7d833bfaf5..751cc0ea80 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -69,11 +69,13 @@ test('appstate --platform apple missing-session error never suggests omitting -- } }); -test('appstate --udid with no --platform still answers the typed reason for a named missing session', async () => { +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. The commands.md row promises the typed reason "even with an - // explicit device", so this lane — the only appstate route that resolves a device — answers it - // too, not the reasonless session-required refusal. + // explicit device", so this lane answers it too — and it answers it from the resolved device, + // NOT from a pre-resolution guard: the request states a target, and the route must first prove + // that target by resolving it. A refusal answered before resolution is a different policy, so + // the resolution call itself is the pin here. const iosSim: DeviceInfo = { platform: 'apple', appleOs: 'ios', @@ -83,13 +85,25 @@ test('appstate --udid with no --platform still answers the typed reason for a na target: 'mobile', booted: true, }; - const resolved = vi.mocked(resolveTargetDevice).mockResolvedValue(iosSim); + const resolved = vi.mocked(resolveTargetDevice); + resolved.mockReset(); + resolved.mockResolvedValue(iosSim); + // The refusal precedes runtime admission, so a fact read here would mean the lane queried a + // device it had just refused; the pin is that it never gets that far. + 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('ios'), { + available: false, + reason: 'owner-capability-missing', + }); + }, }); expect(response.ok).toBe(false); @@ -97,6 +111,58 @@ test('appstate --udid with no --platform still answers the typed reason for a na 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 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(); } diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 2a7ad52f5d..20150dc008 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -174,11 +174,12 @@ async function resolveRecordingSession( * 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 an unconstrained sweep of the host, * which with several booted devices would answer `AMBIGUOUS_MATCH` (or, worse, silently pick a - * different device) before the manifest is read. A decodable manifest carries the exact device the - * recording was started on, so the stop recovery resolves that device; a start, or a stop with no - * decodable manifest to recover from, refuses with the named-session error. The manifest's own - * session and device are still proven against the request by `assertManifestBelongsToRequest` - * before anything is served, so a manifest belonging to another session or device fails closed. + * different device) before the manifest is read. A decodable manifest names the exact device the + * recording was started on; the stop recovery re-resolves that identity, so the host is asked + * only for the device the manifest already specified. A start, or a stop with no decodable + * manifest to recover from, refuses with the named-session error. + * `assertManifestBelongsToRequest` still proves the resolved device and the session address + * against the manifest before anything is served, so a mismatch fails closed. * * 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 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 ae958b8d47..d186d99023 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts @@ -5,7 +5,6 @@ import { publicPlatformString, type DeviceInfo, } from '@agent-device/kernel/device'; -import { namesSessionExplicitly } from '@agent-device/kernel/contracts'; import type { DaemonResponse } from '@agent-device/kernel/contracts'; import { resolveAndroidSerialAllowlist } from '@agent-device/kernel/device-isolation'; import { admitRuntimeUse } from '../../core/runtime-admission.ts'; @@ -29,13 +28,14 @@ function sortedAndroidSerialAllowlist(value: string | undefined): string[] | und export async function queryDeviceRuntimeAppState( params: RuntimeCommandHandlerParams, device: DeviceInfo, + namedSessionMissing: boolean, ): Promise { if (isIosFamily(device) || isMacOs(device)) { - // A request that names a session this daemon does not hold and resolves onto an Apple device - // is missing its session, not its device (#3674): the same typed refusal the session-required - // selectors answer, whatever flag routed it here. A request that named no session keeps the - // plain session-required refusal, which is the right advice for it. - if (!params.sessionStore.get(params.sessionName) && namesSessionExplicitly(params.req)) { + // A request admitted without the session it names (#3674) keeps the typed refusal whatever + // flag routed it here — the fact comes from admission, so resolving the device cannot change + // the error family. A request that named no session keeps the plain session-required refusal, + // which is the right advice for it. + if (namedSessionMissing) { return appStateSessionNotFoundResponse(params.sessionName, device.platform); } return appleSessionRequiredRefusal(isMacOs(device) ? 'macos' : 'ios'); diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index 42e732270d..713f66d606 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -22,7 +22,17 @@ type AppStateSessionStoreEntry = ReturnType Date: Fri, 16 Oct 2026 19:04:13 +0200 Subject: [PATCH 12/15] fix(daemon): own the appstate test owner's platform family and scope two doc claims (#3674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare-udid pin built its runtime owner with `ios`, which is a public selector, not a Platform — typecheck TS2345 at the lint-package gate. The fixture device's own family, `apple`, is the value the owner takes. Alongside it, two docstrings said more than the code does: the manifest lane asks the host only for the manifest's device where the family has an identity flag (families without one keep the request's own selection, proven by the manifest assertion), and the admission-carried fact keeps the refusal stable once a device resolves — resolution itself can still fail first with the device's own error. Co-Authored-Worked-On-By: Claude --- .../sessions/handlers/__tests__/session-appstate.test.ts | 2 +- packages/daemon/src/sessions/handlers/record-runtime.ts | 6 +++--- .../sessions/handlers/session-appstate-device-runtime.ts | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-) 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 751cc0ea80..b12c4cc9f8 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -99,7 +99,7 @@ test('appstate --udid with no --platform resolves the stated device and THEN ans sessionStore: makeSessionStore(), inspectFacts: async (inspectedDevice) => { inspected.push(inspectedDevice.id); - return createUnavailableRuntimeFactsForTest(inspectedDevice, localRuntimeOwner('ios'), { + return createUnavailableRuntimeFactsForTest(inspectedDevice, localRuntimeOwner('apple'), { available: false, reason: 'owner-capability-missing', }); diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 20150dc008..13e459177e 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -175,9 +175,9 @@ async function resolveRecordingSession( * the durable manifest the session address finds — never from an unconstrained sweep of the host, * which with several booted devices would answer `AMBIGUOUS_MATCH` (or, worse, silently pick a * different device) before the manifest is read. A decodable manifest names the exact device the - * recording was started on; the stop recovery re-resolves that identity, so the host is asked - * only for the device the manifest already specified. A start, or a stop with no decodable - * manifest to recover from, refuses with the named-session error. + * recording was started on; the stop recovery re-resolves that identity, so where its family has + * an identity flag the host is asked only for the device the manifest already specified. A start, + * or a stop with no decodable manifest to recover from, refuses with the named-session error. * `assertManifestBelongsToRequest` still proves the resolved device and the session address * against the manifest before anything is served, so a mismatch fails closed. * 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 d186d99023..3be34bca2b 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts @@ -32,9 +32,10 @@ export async function queryDeviceRuntimeAppState( ): Promise { if (isIosFamily(device) || isMacOs(device)) { // A request admitted without the session it names (#3674) keeps the typed refusal whatever - // flag routed it here — the fact comes from admission, so resolving the device cannot change - // the error family. A request that named no session keeps the plain session-required refusal, - // which is the right advice for it. + // flag routed it here — the fact comes from admission, so once a device resolves, a store + // change mid-await cannot move this refusal into another family. (Resolution itself can + // still fail first, and that error is the device's.) A request that named no session keeps + // the plain session-required refusal, which is the right advice for it. if (namedSessionMissing) { return appStateSessionNotFoundResponse(params.sessionName, device.platform); } From 31f4d58eaf4978ee5fdb5604ab379f3e0db8619a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 20:47:32 +0200 Subject: [PATCH 13/15] fix(record): keep host scoping in manifest recovery and pin every recovery lane Manifest re-resolution dropped iosSimulatorDeviceSet and androidDeviceAllowlist, regressing recovery scoped to a simulator set or allowlist, and the flagless- family lane still swept the host from the request's flags. The selector now carries the manifest's family always, the identity flag where the family has one, and the request's host-scoping selectors; no named recovery lane sweeps. Pins: recovery preserves the allowlist (drop-carry mutant caught), flagless recovery resolves under the manifest family (req.flags-sweep mutant caught), sessionless appstate on an Apple device keeps the plain session-required advice (always-typed mutant caught). Trims the restated fail-closed invariant down to its owning site. --- .../record-runtime-stop-recovery.test.ts | 83 +++++++++++++++++-- .../__tests__/session-appstate.test.ts | 49 +++++++++-- .../src/sessions/handlers/record-runtime.ts | 40 +++++---- .../session-appstate-device-runtime.ts | 8 +- .../src/sessions/handlers/session-appstate.ts | 11 +-- 5 files changed, 143 insertions(+), 48 deletions(-) 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 d693ee46da..a9d1d95d8f 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 @@ -10,22 +10,28 @@ import { recordingResourcePath, } from './record-runtime.fixtures.ts'; -// The stop lane that runs WITHOUT a held session resolves its device through the selection -// dispatch. On a host that answers anything, an unconstrained request is exactly the #3674 -// symptom, so the dispatch double here behaves like a multi-device host: it only answers a -// request that states a device identity, and an unconstrained one gets the ambiguity a -// multi-boot host really returns. The restart tests below pass only while the recovery lane -// resolves from the manifest instead of sweeping the host. +// 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 resolveTargetDeviceMock = vi.fn(async (flags: { udid?: string; serial?: string } = {}) => { +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.platform === 'linux') return { ...FLAGLESS_RECORDED_DEVICE }; throw new AppError('AMBIGUOUS_MATCH', 'Multiple attached devices detected'); }); vi.mock('@agent-device/device-selection/dispatch-resolve', () => ({ @@ -63,6 +69,69 @@ test('record stop after a daemon restart replays the completed export for the na ); }); +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'); + await harness.run(['start', outPath], undefined, { androidDeviceAllowlist: 'emulator-5554' }); + 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 }, + { + 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 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'); + 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 } }); + 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. 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 b12c4cc9f8..2b327b5a71 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -71,11 +71,9 @@ 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. The commands.md row promises the typed reason "even with an - // explicit device", so this lane answers it too — and it answers it from the resolved device, - // NOT from a pre-resolution guard: the request states a target, and the route must first prove - // that target by resolving it. A refusal answered before resolution is a different policy, so - // the resolution call itself is the pin here. + // 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', @@ -88,8 +86,6 @@ test('appstate --udid with no --platform resolves the stated device and THEN ans const resolved = vi.mocked(resolveTargetDevice); resolved.mockReset(); resolved.mockResolvedValue(iosSim); - // The refusal precedes runtime admission, so a fact read here would mean the lane queried a - // device it had just refused; the pin is that it never gets that far. const inspected: string[] = []; try { const response = await handleAppStateCommand({ @@ -124,6 +120,45 @@ test('appstate --udid with no --platform resolves the stated device and THEN ans } }); +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(); + } 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 diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 13e459177e..b424d0d768 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -172,19 +172,15 @@ async function resolveRecordingSession( * 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 an unconstrained sweep of the host, - * which with several booted devices would answer `AMBIGUOUS_MATCH` (or, worse, silently pick a - * different device) before the manifest is read. A decodable manifest names the exact device the - * recording was started on; the stop recovery re-resolves that identity, so where its family has - * an identity flag the host is asked only for the device the manifest already specified. A start, - * or a stop with no decodable manifest to recover from, refuses with the named-session error. - * `assertManifestBelongsToRequest` still proves the resolved device and the session address - * against the manifest before anything is served, so a mismatch fails closed. + * the durable manifest the session address finds, never from a sweep of every device on the host: + * a decodable manifest is re-resolved under the manifest's family (plus the request's host + * scoping), 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, with the manifest assertion failing the request closed when the stated device is not - * the one the recording was started on. + * from it. */ async function resolveUnheldRecordingDevice( params: RecordRuntimeHandlerParams, @@ -211,23 +207,25 @@ function namesSessionWithoutTarget(req: DaemonRequest): boolean { } /** - * Re-resolves the exact device a durable manifest names, by the identity flag that family answers - * to. Families without an identity flag keep the request's own selection — an address the manifest - * recorded but the request cannot restate stays resolved as it always has been, and the - * device-match assertion below still refuses a different device. + * Re-resolves the device a durable manifest names: the manifest's family always, its identity flag + * where the family has one, plus the request's host-scoping selectors — `iosSimulatorDeviceSet` + * and `androidDeviceAllowlist` say WHERE to look, not which device, so carrying them preserves a + * recovery scoped to a simulator set or allowlist instead of regressing it to DEVICE_NOT_FOUND. + * No request device/identity selector reaches here: `assertManifestBelongsToRequest` proves the + * resolved device against the manifest. */ async function resolveManifestDevice( params: RecordRuntimeHandlerParams, recorded: DeviceIdentity, ): Promise { - const selector: ResolveDeviceFlags = { platform: recorded.family }; + const { iosSimulatorDeviceSet, androidDeviceAllowlist } = params.req.flags ?? {}; + const selector: ResolveDeviceFlags = { + platform: recorded.family, + ...(iosSimulatorDeviceSet === undefined ? {} : { iosSimulatorDeviceSet }), + ...(androidDeviceAllowlist === undefined ? {} : { androidDeviceAllowlist }), + }; const identityFlag = deviceIdentityFlag(recorded.family); - if (!identityFlag) { - // A family whose manifest identity no flag can restate keeps the request's own selection; the - // manifest assertion still refuses anything but the device the recording was started on. - return await resolveTargetDevice(params.req.flags ?? {}); - } - selector[identityFlag] = recorded.id; + if (identityFlag) selector[identityFlag] = recorded.id; return await resolveTargetDevice(selector); } 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 3be34bca2b..0e120a9898 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts @@ -31,11 +31,9 @@ export async function queryDeviceRuntimeAppState( namedSessionMissing: boolean, ): Promise { if (isIosFamily(device) || isMacOs(device)) { - // A request admitted without the session it names (#3674) keeps the typed refusal whatever - // flag routed it here — the fact comes from admission, so once a device resolves, a store - // change mid-await cannot move this refusal into another family. (Resolution itself can - // still fail first, and that error is the device's.) A request that named no session keeps - // the plain session-required refusal, which is the right advice for it. + // 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); } diff --git a/packages/daemon/src/sessions/handlers/session-appstate.ts b/packages/daemon/src/sessions/handlers/session-appstate.ts index 713f66d606..272a301395 100644 --- a/packages/daemon/src/sessions/handlers/session-appstate.ts +++ b/packages/daemon/src/sessions/handlers/session-appstate.ts @@ -25,12 +25,9 @@ type AppStateTargetDecision = | { answeredBy: 'device-runtime'; session: AppStateSessionStoreEntry; - /** - * Decided from the same store read as `session`: the request names a session this daemon - * does not hold. The device-runtime lane answers an Apple device from this admission-time - * fact, not from a re-read after the await — resolution cannot change which error family - * the request was admitted into. - */ + /** 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; }; @@ -74,8 +71,6 @@ function admitAppStateTarget(params: RuntimeCommandHandlerParams): AppStateTarge if (useSessionStateForApple && session) { return { answeredBy: 'apple-session' as const, session }; } - // Only an identity selector reaches the device-runtime lane with a named-but-missing session - // (the guard above refused the rest); this is the one read that decides it. return { answeredBy: 'device-runtime' as const, session, From d1b6071209732471e0225703cf1dae1efa5933b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 21:26:07 +0200 Subject: [PATCH 14/15] fix(record): carry lease discovery context and pin every scoping half MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manifest re-resolution forwarded the host-scoping selectors but dropped the request's lease/provider-discovery fields (leaseId, leaseProvider, deviceKey, clientId), which flow into provider discovery — a leased named record stop resolved outside its lease, unlike base, which passed the whole request flags. The non-identity discovery context now has one owning read in device-selector-intent next to the selector classification, and resolveManifestDevice forwards it. Pins: apple-manifest recovery carries iosSimulatorDeviceSet (the surviving drop-spread mutant; deleting either spread line now fails a test), recovery carries the lease context (drop-context mutant caught), and the sessionless plain-advice test asserts the resolver call on the stated udid so a refusal that skips resolving an invalid UDID fails it. The record fixture builds its durable envelope with the same kernel deviceIdentity() the transports use, so an apple manifest is decodable and every pin asserts a healthy start/stop and a decoded manifest before exercising recovery. --- .../src/core/device/device-selector-intent.ts | 33 +++++++ .../record-runtime-stop-recovery.test.ts | 91 ++++++++++++++++++- .../__tests__/record-runtime.fixtures.ts | 23 +++-- .../__tests__/session-appstate.test.ts | 7 ++ .../src/sessions/handlers/record-runtime.ts | 17 ++-- 5 files changed, 153 insertions(+), 18 deletions(-) diff --git a/packages/daemon/src/core/device/device-selector-intent.ts b/packages/daemon/src/core/device/device-selector-intent.ts index 88413ff80f..317d6e861e 100644 --- a/packages/daemon/src/core/device/device-selector-intent.ts +++ b/packages/daemon/src/core/device/device-selector-intent.ts @@ -8,6 +8,39 @@ const LOCKABLE_DEVICE_SCOPE_KEYS: ReadonlyArray = [ 'androidDeviceAllowlist', ]; +/** + * Request-scoped lease and provider-discovery context. These say WHO may see which devices + * (a lease or a provider client narrows the inventory the resolver can reach), never WHICH + * device, so a lane that rebuilds a selector must forward them or resolution silently + * widens outside the lease. `device-selector-intent` classifies selectors; this is the one + * read that keeps that classification's non-identity discovery keys together. + */ +export type LeaseDiscoveryContext = { + leaseId?: string; + leaseProvider?: string; + deviceKey?: string; + clientId?: string; +}; + +const LEASE_DISCOVERY_KEYS = [ + 'leaseId', + 'leaseProvider', + 'deviceKey', + 'clientId', +] as const satisfies ReadonlyArray; + +export function leaseDiscoveryContextFromFlags( + flags: CommandFlags | undefined, +): LeaseDiscoveryContext { + if (!flags) return {}; + const context: Record = {}; + for (const key of LEASE_DISCOVERY_KEYS) { + const value = flags[key]; + if (typeof value === 'string' && value.trim().length > 0) context[key] = value; + } + return context; +} + export function hasExplicitDeviceSelector(flags: CommandFlags | undefined): boolean { return hasExplicitDeviceIdentitySelector({ deviceName: flags?.device, 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 a9d1d95d8f..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 @@ -20,6 +20,14 @@ const RESTART_RECORDED_DEVICE = { 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', @@ -31,6 +39,7 @@ const resolveTargetDeviceMock = vi.fn(async (flags: Record { + // 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 @@ -117,8 +199,9 @@ test('record stop recovery for a family without identity flags resolves under th 'capture.mp4', ); fs.writeFileSync(outPath, 'mp4'); - await harness.run(['start', outPath]); - await harness.run(['stop']); + 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); 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 4ccdf73608..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, @@ -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.test.ts b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts index 2b327b5a71..1151835b25 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-appstate.test.ts @@ -154,6 +154,13 @@ test('appstate --udid on an Apple device without naming a session keeps the plai 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(); } diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index b424d0d768..684a71705d 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -29,7 +29,10 @@ import { resolveTargetDevice, type ResolveDeviceFlags, } from '@agent-device/device-selection/dispatch-resolve'; -import { hasDeviceSelectionInput } from '../../core/device/device-selector-intent.ts'; +import { + hasDeviceSelectionInput, + leaseDiscoveryContextFromFlags, +} 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'; @@ -208,11 +211,12 @@ function namesSessionWithoutTarget(req: DaemonRequest): boolean { /** * Re-resolves the device a durable manifest names: the manifest's family always, its identity flag - * where the family has one, plus the request's host-scoping selectors — `iosSimulatorDeviceSet` - * and `androidDeviceAllowlist` say WHERE to look, not which device, so carrying them preserves a - * recovery scoped to a simulator set or allowlist instead of regressing it to DEVICE_NOT_FOUND. - * No request device/identity selector reaches here: `assertManifestBelongsToRequest` proves the - * resolved device against the manifest. + * where the family has one, plus the request's host-scoping and lease/discovery selectors — + * `iosSimulatorDeviceSet` and `androidDeviceAllowlist` say WHERE to look, the lease fields say WHO + * may see the inventory, and none of them states WHICH device. Dropping any of them would resolve + * a scoped or leased recording outside the scope or lease it started under. No request + * device/identity selector reaches here: `assertManifestBelongsToRequest` proves the resolved + * device against the manifest. */ async function resolveManifestDevice( params: RecordRuntimeHandlerParams, @@ -221,6 +225,7 @@ async function resolveManifestDevice( const { iosSimulatorDeviceSet, androidDeviceAllowlist } = params.req.flags ?? {}; const selector: ResolveDeviceFlags = { platform: recorded.family, + ...leaseDiscoveryContextFromFlags(params.req.flags), ...(iosSimulatorDeviceSet === undefined ? {} : { iosSimulatorDeviceSet }), ...(androidDeviceAllowlist === undefined ? {} : { androidDeviceAllowlist }), }; From 3c99486b390eaeb2e67f8475106b13eea544f439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 01:46:45 +0200 Subject: [PATCH 15/15] refactor(record): spread the request scope into manifest re-resolution and show it in the request log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manifest lane hand-picked the scope and lease keys to forward; a key added to ResolveDeviceFlags later would be dropped silently on this lane. Admission (namesSessionWithoutTarget) already proves the request states no device, udid, serial, platform, or target, so spreading its flags carries exactly the scope and lease keys — now and later — while the manifest still decides family and identity. This deletes LeaseDiscoveryContext, its key list, and the helper. The resolve_target_device phase now echoes the requested udid and simulator set when the request states them, so a manifest-first recovery is provable from the request log alone (the maintainer's scoped-set evidence requirement). Pinned: dropping the spread fails all three recovery scope pins; dropping either echoed field fails the new phase test. --- .../src/core/device/device-selector-intent.ts | 33 --------------- .../src/sessions/handlers/record-runtime.ts | 30 ++++++------- packages/device-selection/package.json | 3 ++ .../src/__tests__/dispatch-resolve.test.ts | 42 +++++++++++++++++++ .../device-selection/src/dispatch-resolve.ts | 5 +++ packages/device-selection/tsconfig.json | 15 ++++++- pnpm-lock.yaml | 4 ++ 7 files changed, 80 insertions(+), 52 deletions(-) diff --git a/packages/daemon/src/core/device/device-selector-intent.ts b/packages/daemon/src/core/device/device-selector-intent.ts index 317d6e861e..88413ff80f 100644 --- a/packages/daemon/src/core/device/device-selector-intent.ts +++ b/packages/daemon/src/core/device/device-selector-intent.ts @@ -8,39 +8,6 @@ const LOCKABLE_DEVICE_SCOPE_KEYS: ReadonlyArray = [ 'androidDeviceAllowlist', ]; -/** - * Request-scoped lease and provider-discovery context. These say WHO may see which devices - * (a lease or a provider client narrows the inventory the resolver can reach), never WHICH - * device, so a lane that rebuilds a selector must forward them or resolution silently - * widens outside the lease. `device-selector-intent` classifies selectors; this is the one - * read that keeps that classification's non-identity discovery keys together. - */ -export type LeaseDiscoveryContext = { - leaseId?: string; - leaseProvider?: string; - deviceKey?: string; - clientId?: string; -}; - -const LEASE_DISCOVERY_KEYS = [ - 'leaseId', - 'leaseProvider', - 'deviceKey', - 'clientId', -] as const satisfies ReadonlyArray; - -export function leaseDiscoveryContextFromFlags( - flags: CommandFlags | undefined, -): LeaseDiscoveryContext { - if (!flags) return {}; - const context: Record = {}; - for (const key of LEASE_DISCOVERY_KEYS) { - const value = flags[key]; - if (typeof value === 'string' && value.trim().length > 0) context[key] = value; - } - return context; -} - export function hasExplicitDeviceSelector(flags: CommandFlags | undefined): boolean { return hasExplicitDeviceIdentitySelector({ deviceName: flags?.device, diff --git a/packages/daemon/src/sessions/handlers/record-runtime.ts b/packages/daemon/src/sessions/handlers/record-runtime.ts index 684a71705d..2a7aa23d87 100644 --- a/packages/daemon/src/sessions/handlers/record-runtime.ts +++ b/packages/daemon/src/sessions/handlers/record-runtime.ts @@ -29,10 +29,7 @@ import { resolveTargetDevice, type ResolveDeviceFlags, } from '@agent-device/device-selection/dispatch-resolve'; -import { - hasDeviceSelectionInput, - leaseDiscoveryContextFromFlags, -} from '../../core/device/device-selector-intent.ts'; +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'; @@ -176,9 +173,10 @@ async function resolveRecordingSession( * * 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 under the manifest's family (plus the request's host - * scoping), 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 + * 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 @@ -210,24 +208,20 @@ function namesSessionWithoutTarget(req: DaemonRequest): boolean { } /** - * Re-resolves the device a durable manifest names: the manifest's family always, its identity flag - * where the family has one, plus the request's host-scoping and lease/discovery selectors — - * `iosSimulatorDeviceSet` and `androidDeviceAllowlist` say WHERE to look, the lease fields say WHO - * may see the inventory, and none of them states WHICH device. Dropping any of them would resolve - * a scoped or leased recording outside the scope or lease it started under. No request - * device/identity selector reaches here: `assertManifestBelongsToRequest` proves the resolved - * device against the manifest. + * 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 { iosSimulatorDeviceSet, androidDeviceAllowlist } = params.req.flags ?? {}; const selector: ResolveDeviceFlags = { + ...(params.req.flags ?? {}), platform: recorded.family, - ...leaseDiscoveryContextFromFlags(params.req.flags), - ...(iosSimulatorDeviceSet === undefined ? {} : { iosSimulatorDeviceSet }), - ...(androidDeviceAllowlist === undefined ? {} : { androidDeviceAllowlist }), }; const identityFlag = deviceIdentityFlag(recorded.family); if (identityFlag) selector[identityFlag] = recorded.id; 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/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: