diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 7817c3b01..1795b55f0 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -36,8 +36,8 @@ // composition file; premature implementation loading and forbidden cross-boundary edges fail (R13). // - Over COMMAND-ATOMIC RUNTIME CUTOVERS: one parametrized gate reads the migrated-command // table (appstate R22, shutdown R23, boot R20, apps R21, install/deploy R24-R27, -// lifecycle R28-R31, devices R17, logs R14, network R15, record R16, snapshot R32, diff R33) -// and proves each command keeps +// lifecycle R28-R31, devices R17, logs R14, network R15, record R16, snapshot R32, diff R33, +// viewport R34, get R36, is R37 — R35 reserved for find) and proves each command keeps // exactly one platform-execution path — retired routes, admission, modules, and widened // runtime access cannot coexist with its operation-fact-derived descriptor and handler. // - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer diff --git a/scripts/layering/runtime-command-cutover-model.ts b/scripts/layering/runtime-command-cutover-model.ts index 55519b615..9674860ad 100644 --- a/scripts/layering/runtime-command-cutover-model.ts +++ b/scripts/layering/runtime-command-cutover-model.ts @@ -28,6 +28,17 @@ export type LegacyRetirementClaim = Readonly<{ daemonOnlyProviderMethods?: readonly string[]; /** `PlatformPlugin` facet keys retired with the legacy adapter. */ pluginFacetKeys?: readonly string[]; + /** + * Static platform command sets this command's admission DATA was removed from — the whole + * retirement of a command whose legacy admission was a capability bucket plus set membership, + * with no adapter module, route, or dispatch projection to name. + * + * Every other form above names something that must NOT exist, which a row can satisfy by + * inventing a name that never existed. This one is two-sided and cannot: each named set must + * still EXIST in production source, and must no longer list the command. A fictional set fails + * the first half, a skipped deletion the second. + */ + staticCommandSets?: readonly string[]; }>; /** @@ -157,6 +168,7 @@ const RETIREMENT_FORMS = [ 'daemonOnlyRouteNames', 'daemonOnlyProviderMethods', 'pluginFacetKeys', + 'staticCommandSets', ] as const satisfies readonly (keyof LegacyRetirementClaim)[]; /** diff --git a/scripts/layering/runtime-command-cutover-policy.test.ts b/scripts/layering/runtime-command-cutover-policy.test.ts index 6f5d26a94..f21f6d178 100644 --- a/scripts/layering/runtime-command-cutover-policy.test.ts +++ b/scripts/layering/runtime-command-cutover-policy.test.ts @@ -285,3 +285,57 @@ test('every shipped row states its claims', () => { [], ); }); + +// A data-only admission retirement — a capability bucket plus static-set membership, with no +// module, route, or dispatch projection to name as gone. Both halves are planted, because the +// half that matters is the one an identifier-shaped claim cannot state: a set that never existed. +const DATA_ONLY_ROW: MigratedCommandCutover = { + ...PLANTED_ROW, + legacyRetirement: { staticCommandSets: ['WEB_QUERY_COMMANDS'] }, +}; + +test('a data-only retirement is a stated claim, so a row needs no invented identifier', () => { + assert.deepEqual(cutoverRowDefects(DATA_ONLY_ROW), []); +}); + +test('planted red: a row claiming a static command set that does not exist is rejected', () => { + assert.deepEqual( + summariesFor( + PLANTED_RULE, + [['src/core/capabilities.ts', `const WEB_QUERY_COMMANDS = ['find'];`]], + [ + { + ...DATA_ONLY_ROW, + legacyRetirement: { staticCommandSets: ['WEB_QUERY_COMMANDS_WITH_PLANTED'] }, + }, + ], + ).filter((summary) => summary.includes('static command set')), + [ + "(planted cutover row): claims retired static command set 'WEB_QUERY_COMMANDS_WITH_PLANTED', which no production source declares", + ], + ); +}); + +test('planted red: a claimed static command set that still lists the command is rejected', () => { + assert.deepEqual( + summariesFor( + PLANTED_RULE, + [['src/core/capabilities.ts', `const WEB_QUERY_COMMANDS = ['find', 'planted'];`]], + [DATA_ONLY_ROW], + ).filter((summary) => summary.includes('still admits')), + ['src/core/capabilities.ts: static command set WEB_QUERY_COMMANDS still admits planted'], + ); +}); + +test('a claimed static command set that exists and dropped the command passes', () => { + // Scoped to this column: the planted row's singular-execution claims are unrelated here and + // have their own cases above. + assert.deepEqual( + summariesFor( + PLANTED_RULE, + [['src/core/capabilities.ts', `const WEB_QUERY_COMMANDS = ['find'];`]], + [DATA_ONLY_ROW], + ).filter((summary) => summary.includes('static command set')), + [], + ); +}); diff --git a/scripts/layering/runtime-command-cutover-policy.ts b/scripts/layering/runtime-command-cutover-policy.ts index fe4335413..a48f058ac 100644 --- a/scripts/layering/runtime-command-cutover-policy.ts +++ b/scripts/layering/runtime-command-cutover-policy.ts @@ -79,6 +79,7 @@ function rowViolations( violations.push(...narrowingViolations(row, file, program)); } violations.push(...exactCallViolations(row, files, programs)); + violations.push(...staticCommandSetViolations(row, files, programs)); const sources = new Map(files.map(({ path, source }) => [path, source])); for (const check of rowChecks(row)) violations.push(...check(sources)); return violations; @@ -356,6 +357,59 @@ function isAdmissionMember( ); } +/** + * A data-only admission retirement, proven from both sides. + * + * A command whose legacy admission was a capability bucket plus membership in a static platform + * command set retires no module, route, or dispatch projection — there is no identifier to name + * as gone. Naming an invented one satisfies the non-empty shape check while proving nothing, so + * the row names the sets themselves: each must still be DECLARED in production source, and must + * no longer carry this command. + * + * The existence half is what an identifier-shaped claim cannot express. The membership half + * overlaps the automatic static-set column for `WEB`/`HARMONY`-named sets, deliberately: stating + * it here keeps the declared claim self-sufficient rather than dependent on that regex. + */ +function staticCommandSetViolations( + row: MigratedCommandCutover, + files: readonly ProductionSource[], + programs: ReadonlyMap, +): UnruledViolation[] { + const declared = row.legacyRetirement.staticCommandSets ?? []; + if (declared.length === 0) return []; + const violations: UnruledViolation[] = []; + const seen = new Set(); + for (const file of files) { + const program = programs.get(file.path); + if (!program) continue; + visitAst(program, (node) => { + const name = staticCommandSetName(node, declared); + if (name === undefined) return; + seen.add(name); + if (containsStringLiteral(node['init'], row.command)) { + violations.push(at(file, node, `static command set ${name} still admits ${row.command}`)); + } + }); + } + for (const name of declared) { + if (seen.has(name)) continue; + violations.push({ + file: `(${row.command} cutover row)`, + line: 1, + message: `claims retired static command set '${name}', which no production source declares`, + }); + } + return violations; +} + +function staticCommandSetName(node: AstNode, declared: readonly string[]): string | undefined { + if (node['type'] !== 'VariableDeclarator') return undefined; + const id = node['id'] as AstNode | undefined; + if (id?.['type'] !== 'Identifier') return undefined; + const name = String(id['name']); + return declared.includes(name) ? name : undefined; +} + function containsStringLiteral(node: unknown, expected: string): boolean { let found = false; visitAst(node, (candidate) => { diff --git a/scripts/layering/runtime-command-cutover-table.ts b/scripts/layering/runtime-command-cutover-table.ts index ecdf21ca3..1e5783e3f 100644 --- a/scripts/layering/runtime-command-cutover-table.ts +++ b/scripts/layering/runtime-command-cutover-table.ts @@ -25,7 +25,8 @@ import { retiredDispatchProjectionViolations } from './runtime-command-cutover-d * A row id is a report heading, so it must be unique across every stack that adds rows here. * `cutoverTableDefects` rejects a duplicate; lifecycle starts at R28 after the accepted * shutdown, install/deploy, and application-lifecycle allocations. Snapshot starts at R32; - * diff follows at R33, viewport at R34, and get at R36 (R35 is reserved for find). + * diff follows at R33, viewport at R34, get at R36, and is at R37. R35 stays reserved for + * find, whose cutover is deferred behind the Wave 5 `focus`/`type` surfaces. */ export const MIGRATED_COMMAND_CUTOVERS: readonly MigratedCommandCutover[] = [ { @@ -548,6 +549,43 @@ export const MIGRATED_COMMAND_CUTOVERS: readonly MigratedCommandCutover[] = [ }, }, }, + { + rule: 'R37 is-runtime-cutover', + command: 'is', + subject: 'element predicate', + tier: 'request-scoped', + execution: 'device-runtime', + // `is` retired no module, route, or dispatch projection — it had none. Its whole legacy + // admission was the capability bucket (rejected by this row's automatic descriptor column) + // plus membership in these two static sets, which is a DATA deletion. Naming the sets proves + // it from both sides: each must still be declared in production source and must no longer + // list `is`, so neither an invented name nor a skipped deletion can satisfy it. + legacyRetirement: { + staticCommandSets: ['HARMONYOS_SUPPORTED_COMMANDS', 'WEB_QUERY_COMMANDS'], + }, + runtimeTypeNames: ['SnapshotRuntimeOperations'], + operations: { names: ['captureSnapshot', 'captureSnapshotWithoutActiveApp'] }, + singularExecution: { + routes: ['dispatchIsViaRuntime'], + operations: ['captureSnapshot', 'captureSnapshotWithoutActiveApp'], + // `is` executes through the shared selector seam, so its capture owners are the SAME + // selectors `snapshot`/`diff`/`get` count. It declares no operation of its own: every + // predicate answers from the resolved tree, so `readTextAtPoint` stays R36's alone. + // + // Scope, stated so this is not read as absolute: the claim covers how a predicate is + // EXECUTED. Since the direct-iOS selector shortcut retired, the bound capture is the only + // thing that answers one. It does NOT claim the route makes no other device call — the + // Android foreground-blocker diagnostic still reaches adb through + // `platforms/android/app-lifecycle.ts`, on the FAILURE path only, where it can enrich an + // already-failed response's message but can never produce or change a verdict. That edge + // is pre-existing, co-owned with `wait`, and recorded as Wave 6 denominator work; R22's + // `appState` is its declared replacement. + operationOwners: { + captureSnapshot: ['selectActiveAppSnapshot'], + captureSnapshotWithoutActiveApp: ['selectSnapshotWithoutActiveApp'], + }, + }, + }, { rule: 'R34 viewport-runtime-cutover', command: 'viewport', diff --git a/src/__tests__/cli-exit-paths.test.ts b/src/__tests__/cli-exit-paths.test.ts index 4c97cf77e..73ceb8e52 100644 --- a/src/__tests__/cli-exit-paths.test.ts +++ b/src/__tests__/cli-exit-paths.test.ts @@ -222,3 +222,39 @@ test('a --debug failure caps the daemon-log-tail dump instead of printing it unb 'expected the byte cap to drop the oldest lines, not just the 200-line cap', ); }); + +// The end-to-end half of `is`'s documented contract: "is evaluates UI predicates against a +// selector expression and exits non-zero on failure" (website/docs/docs/commands.md). +// +// This deliberately does NOT know how the daemon decided. It was written when the direct-iOS +// shortcut answered some predicates itself and returned `{ok: true, pass: false}`, which the CLI +// rendered as `Passed: is text` with exit 0 (#1739). The shortcut is retired and every predicate +// now answers from the bound capture, so the guarantee is structural rather than guard-based — +// and this case survives that change untouched, because a failed assertion must exit non-zero +// whatever produced the failure. +test('a failed `is` predicate exits non-zero, whatever answered it', async () => { + const restoreEnv = installIsolatedCliTestEnv(); + const exitSpy = installExitSpy(); + const stderr = captureStderr(); + const sendToDaemon = async (): Promise => ({ + ok: false, + error: { + code: 'COMMAND_FAILED', + message: 'is text failed for selector id=greeting: expected="Welcome" actual="Goodbye"', + details: { command: 'is', reason: 'predicate_failed', predicate: 'text' }, + }, + }); + + try { + await runCli(['is', 'text', 'id=greeting', 'Welcome'], { sendToDaemon }); + } finally { + stderr.restore(); + exitSpy.restore(); + restoreEnv(); + } + + assert.deepEqual(exitSpy.calls, [1]); + const output = stderr.read(); + assert.ok(output.includes('COMMAND_FAILED'), 'expected the typed failure on stderr'); + assert.ok(!output.includes('Passed'), 'a failed assertion must never render as passed'); +}); diff --git a/src/core/__tests__/capabilities.test.ts b/src/core/__tests__/capabilities.test.ts index cf7cb2878..0b4eb5f5c 100644 --- a/src/core/__tests__/capabilities.test.ts +++ b/src/core/__tests__/capabilities.test.ts @@ -232,7 +232,6 @@ test('macOS supports the Apple runner interaction core but excludes mobile-only 'find', 'focus', 'get', - 'is', 'longpress', 'logs', 'perf', @@ -306,7 +305,6 @@ test('Linux supports desktop interaction commands and blocks mobile/unsupported 'focus', 'get', 'home', - 'is', 'longpress', 'press', 'screenshot', @@ -334,7 +332,6 @@ test('web supports only the initial browser interaction slice', () => { 'find', 'get', 'hover', - 'is', 'press', 'record', 'screenshot', diff --git a/src/core/__tests__/capability-plugin-routing-parity.test.ts b/src/core/__tests__/capability-plugin-routing-parity.test.ts index e2f8375ec..3abb51df8 100644 --- a/src/core/__tests__/capability-plugin-routing-parity.test.ts +++ b/src/core/__tests__/capability-plugin-routing-parity.test.ts @@ -180,11 +180,9 @@ const HARMONYOS_SUPPORTED_COMMANDS_REF = new Set([ 'fill', 'find', 'focus', - 'get', 'home', 'gesture', 'keyboard', - 'is', 'longpress', 'press', 'screenshot', @@ -271,7 +269,6 @@ test('HarmonyOS static capabilities omit runtime-backed command admissions', () 'focus', 'gesture', 'home', - 'is', 'keyboard', 'longpress', 'perf', diff --git a/src/core/capabilities.ts b/src/core/capabilities.ts index e63cc1f59..2a5e4c2aa 100644 --- a/src/core/capabilities.ts +++ b/src/core/capabilities.ts @@ -47,7 +47,6 @@ const HARMONYOS_SUPPORTED_COMMANDS = new Set([ 'home', 'gesture', 'keyboard', - 'is', 'longpress', 'press', 'scroll', @@ -56,7 +55,7 @@ const HARMONYOS_SUPPORTED_COMMANDS = new Set([ 'type', 'wait', ]); -const WEB_QUERY_COMMANDS = ['audio', 'find', 'is', 'wait'] as const; +const WEB_QUERY_COMMANDS = ['audio', 'find', 'wait'] as const; const WEB_INTERACTION_COMMANDS = [ 'click', 'fill', diff --git a/src/core/command-descriptor/__tests__/parity.test.ts b/src/core/command-descriptor/__tests__/parity.test.ts index 69e6d6d25..1f5c32863 100644 --- a/src/core/command-descriptor/__tests__/parity.test.ts +++ b/src/core/command-descriptor/__tests__/parity.test.ts @@ -60,6 +60,7 @@ const NO_CAPABILITY_PUBLIC_COMMANDS = new Set([ PUBLIC_COMMANDS.get, PUBLIC_COMMANDS.install, PUBLIC_COMMANDS.installFromSource, + PUBLIC_COMMANDS.is, PUBLIC_COMMANDS.logs, PUBLIC_COMMANDS.network, PUBLIC_COMMANDS.open, diff --git a/src/core/command-descriptor/registry.ts b/src/core/command-descriptor/registry.ts index 74dbdc7d6..cf79556c7 100644 --- a/src/core/command-descriptor/registry.ts +++ b/src/core/command-descriptor/registry.ts @@ -1203,10 +1203,9 @@ export const RAW_COMMAND_DESCRIPTORS = [ recordsSessionAction: true, recordingEffect: 'observes-app', daemon: { route: 'interaction', refFrameEffect: 'preserve' }, - capability: ALL_DEVICE_COMMAND_CAPABILITY, timeoutPolicy: postActionObservationTimeoutPolicy('is', PRESERVE_DAEMON_TIMEOUT_POLICY), batchable: true, - platformExecution: LEGACY_PLATFORM_EXECUTION, + platformExecution: { kind: 'device-runtime', uses: selectorCaptureRuntimePlanUses }, }, // -- generic (route: generic) -- diff --git a/src/daemon/__tests__/is-runtime.test.ts b/src/daemon/__tests__/is-runtime.test.ts new file mode 100644 index 000000000..add368a0e --- /dev/null +++ b/src/daemon/__tests__/is-runtime.test.ts @@ -0,0 +1,271 @@ +import { beforeEach, expect, test, vi } from 'vitest'; +import type { SnapshotResult } from '@agent-device/contracts/platform'; +import { ANDROID_EMULATOR, IOS_SIMULATOR } from '../../__tests__/test-utils/device-fixtures.ts'; +import { + makeAndroidSession, + makeIosAppSession, + makeIosSession, +} from '../../__tests__/test-utils/session-factories.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { withTestDeviceInventory } from '../../__tests__/test-utils/device-inventory-gateways.ts'; +import type { DaemonRequest } from '../types.ts'; +import { selectorCaptureFixture } from './selector-capture-fixture.ts'; + +const { mockRunAppleRunnerCommand } = vi.hoisted(() => ({ mockRunAppleRunnerCommand: vi.fn() })); + +vi.mock('../../platforms/apple/core/runner/runner-client.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, runAppleRunnerCommand: mockRunAppleRunnerCommand }; +}); + +import { dispatchIsViaRuntime } from '../selector-runtime.ts'; + +beforeEach(() => { + mockRunAppleRunnerCommand.mockReset(); + mockRunAppleRunnerCommand.mockResolvedValue({}); +}); + +// `is` answers every one of its seven predicates from the resolved capture — `isCommand` never +// reaches `backend.readText`. So its whole platform execution is the request-bound capture, and +// these cases bind at `inspectFacts` / `bindDevice`, never at `core/dispatch.ts`. + +const unavailableCapture = { available: false, reason: 'unsupported-device-kind' } as const; +const activeAppRequired = { available: false, reason: 'owner-capability-missing' } as const; + +/** One resolvable button, so a predicate has something real to answer about. */ +function buttonSnapshot(): SnapshotResult { + return { + nodes: [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Continue', + identifier: 'auth_continue', + rect: { x: 10, y: 20, width: 120, height: 44 }, + enabled: true, + hittable: true, + }, + ], + backend: 'android', + }; +} + +function isRequest(session: string, positionals: readonly string[]): DaemonRequest { + return { token: 't', session, command: 'is', positionals: [...positionals], flags: {} }; +} + +test('an admitted is inspects once, binds once, and answers through the bound capture', async () => { + const fixture = selectorCaptureFixture({ snapshot: () => buttonSnapshot() }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-bound', makeAndroidSession('is-bound', { appBundleId: 'com.example.app' })); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-bound', ['visible', 'id=auth_continue']), + sessionName: 'is-bound', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(true); + expect(fixture.inspections).toEqual([ANDROID_EMULATOR]); + expect(fixture.binds).toEqual([ANDROID_EMULATOR]); + expect(fixture.captures.length).toBeGreaterThan(0); +}); + +test('an unavailable capture fact refuses before any bind', async () => { + // The watchOS sentinel shape: capability-supported today, no snapshot backend at the owner. + const fixture = selectorCaptureFixture({ capture: unavailableCapture }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-refused', makeAndroidSession('is-refused', { appBundleId: 'com.a' })); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-refused', ['visible', 'id=auth_continue']), + sessionName: 'is-refused', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(false); + // The inspection is what makes this a typed admission refusal rather than a runtime failure: + // exact owner facts were read once, side-effect-free, and nothing bound or captured after. + expect(fixture.inspections).toEqual([ANDROID_EMULATOR]); + expect(fixture.binds).toEqual([]); + expect(fixture.captures).toEqual([]); +}); + +// The correctness fix this unit declares. On iOS `appBundleId` is the XCUITest attach target: +// with no tracked app the runner's own process comes to the foreground, DISPLACES the app under +// test, and the capture then answers confidently about the runner's own blank screen. Refusing +// beats displacing-and-lying. Android captures the real launcher in the same state, and the +// platform facts already encode that asymmetry — so `is` asks the facts rather than branching. +test('an iOS session with no tracked app is refused with the open hint, not answered from a displaced capture', async () => { + const fixture = selectorCaptureFixture({ + withoutActiveApp: activeAppRequired, + snapshot: () => buttonSnapshot(), + }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-no-app', makeIosSession('is-no-app')); + + const response = await withTestDeviceInventory( + {}, + async () => + await dispatchIsViaRuntime({ + req: isRequest('is-no-app', ['visible', 'id=auth_continue']), + sessionName: 'is-no-app', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }), + ); + + expect(response?.ok).toBe(false); + if (response?.ok === false) { + expect(response.error?.code).toBe('SESSION_NOT_FOUND'); + expect(response.error?.message).toMatch(/requires an active app session/); + } + expect(fixture.binds).toEqual([]); + expect(fixture.captures).toEqual([]); +}); + +test('an iOS session WITH a tracked app still answers, so the refusal is the plan split and not an iOS ban', async () => { + const fixture = selectorCaptureFixture({ + withoutActiveApp: activeAppRequired, + snapshot: () => buttonSnapshot(), + }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-with-app', makeIosAppSession('is-with-app')); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-with-app', ['visible', 'label=Continue']), + sessionName: 'is-with-app', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(true); + expect(fixture.binds).toEqual([IOS_SIMULATOR]); +}); + +test('an Android session with no tracked app proceeds, because the owner advertises the without-active-app capture', async () => { + const fixture = selectorCaptureFixture({ snapshot: () => buttonSnapshot() }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-android-no-app', makeAndroidSession('is-android-no-app')); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-android-no-app', ['visible', 'id=auth_continue']), + sessionName: 'is-android-no-app', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(true); + expect(fixture.binds).toEqual([ANDROID_EMULATOR]); +}); + +// ADR 0019: a refused request reaches the device not at all. This used to guard the direct-iOS +// selector query, which could answer a simple `id=` target without a capture; that shortcut is +// retired, so the runner assertion below now proves the stronger property — on an unavailable +// fact, `is` makes no device call by any route. +test('a refused request reaches the device by no route at all', async () => { + const fixture = selectorCaptureFixture({ capture: unavailableCapture }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-direct-refused', makeIosAppSession('is-direct-refused')); + mockRunAppleRunnerCommand.mockResolvedValue({ + found: true, + nodes: [ + { + index: 0, + type: 'Button', + label: 'Pickup', + identifier: 'shipping-pickup', + selected: true, + rect: { x: 126, y: 555, width: 75, height: 38 }, + enabled: true, + hittable: true, + }, + ], + }); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-direct-refused', ['selected', 'id="shipping-pickup"']), + sessionName: 'is-direct-refused', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(false); + expect(mockRunAppleRunnerCommand).not.toHaveBeenCalled(); + expect(fixture.binds).toEqual([]); +}); + +// `is` is an assertion: it "exits non-zero on failure" (website/docs/docs/commands.md). A +// direct-iOS shortcut used to answer some predicates itself and reported a failed one as a +// completed command — `is text id=… "Wrong Expected Text"` printed `Passed: is text` and exited 0 +// on device (#1739). That shortcut is retired, so the guarantee is now structural rather than +// guard-based: the bound capture is the only thing that answers a predicate, and `isCommand` +// raises COMMAND_FAILED when one fails. +// +// The CLI half — that such a response actually exits non-zero — lives in +// `src/__tests__/cli-exit-paths.test.ts`, at a layer that does not know how the daemon decided. +test('a failing predicate answers COMMAND_FAILED from the bound capture', async () => { + const fixture = selectorCaptureFixture({ + snapshot: () => ({ + nodes: [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Apple Account', + identifier: 'account_row', + rect: { x: 10, y: 20, width: 120, height: 44 }, + enabled: true, + hittable: true, + }, + ], + backend: 'xctest', + }), + }); + const sessionStore = makeSessionStore(); + sessionStore.set('is-direct-false', makeIosAppSession('is-direct-false')); + mockRunAppleRunnerCommand.mockResolvedValue({ + found: true, + text: 'Apple Account', + nodes: [ + { + index: 0, + type: 'Button', + label: 'Apple Account', + identifier: 'account_row', + rect: { x: 10, y: 20, width: 120, height: 44 }, + enabled: true, + hittable: true, + }, + ], + }); + + const response = await dispatchIsViaRuntime({ + req: isRequest('is-direct-false', ['text', 'id=account_row', 'Wrong Expected Text']), + sessionName: 'is-direct-false', + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + // A failed assertion is a failed command on every other path and in the docs; it is one here. + expect(response?.ok).toBe(false); + if (response?.ok === false) { + expect(response.error?.code).toBe('COMMAND_FAILED'); + expect(response.error?.details?.reason).toBe('predicate_failed'); + } + // The bound capture is what answered it. + expect(fixture.captures.length).toBeGreaterThan(0); +}); diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index b34583f8d..4b30900b9 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -513,69 +513,14 @@ test('is visible recaptures web snapshots when cached nodes may lack rects', asy }); }); -test('is selected simple iOS id selector uses runner query without snapshot', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'is-selected-ios-direct-selector'; - sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' })); - mockRunAppleRunnerCommand.mockResolvedValue({ - found: true, - text: 'Pickup', - nodes: [ - { - index: 0, - depth: 0, - type: 'Button', - label: 'Pickup', - identifier: 'shipping-pickup', - selected: true, - rect: { x: 126, y: 555, width: 75, height: 38 }, - enabled: true, - hittable: true, - }, - ], - }); - - const response = await handleInteractionCommands({ - req: { - token: 't', - session: sessionName, - command: 'is', - positionals: ['selected', 'id="shipping-pickup"'], - flags: {}, - }, - sessionName, - sessionStore, - contextFromFlags, - ...getRuntimeBindings(), - }); - - expect(response?.ok).toBe(true); - expect(mockRunAppleRunnerCommand).toHaveBeenCalledWith( - expect.anything(), - { - command: 'querySelector', - selectorKey: 'id', - selectorValue: 'shipping-pickup', - appBundleId: 'com.example.app', - }, - expect.anything(), - ); - expect(mockDispatch).not.toHaveBeenCalledWith( - expect.anything(), - 'snapshot', - expect.anything(), - expect.anything(), - expect.anything(), - ); - if (response?.ok) { - expect(response.data?.predicate).toBe('selected'); - expect(response.data?.pass).toBe(true); - } - const recorded = sessionStore.get(sessionName)?.actions.at(-1); - expect(recorded?.result?.selectorChain).toEqual(['id="shipping-pickup"']); -}); - -test('is simple iOS selector returns false directly when runner predicate fails', async () => { +// PIN CHANGED TWICE (#1739, R37). #557 asserted `ok: true` with `pass: false` and zero snapshots +// here, from the direct-iOS shortcut. That broke `is`'s documented contract — it "exits non-zero +// on failure" (website/docs/docs/commands.md) — and on device printed `Passed: is text` with exit +// 0 for a failed assertion. The reversal made the shortcut answer only when the predicate held; +// the shortcut is now retired outright, so the bound capture answers every predicate and this is +// simply what `is` does. The assertion below is unchanged across both edits because it was always +// about the OUTCOME, not about which path produced it. +test('a failing is predicate is COMMAND_FAILED, never a zero-exit pass', async () => { const sessionStore = makeSessionStore(); const sessionName = 'is-selected-ios-direct-selector-false'; sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' })); @@ -608,67 +553,15 @@ test('is simple iOS selector returns false directly when runner predicate fails' ...getRuntimeBindings(), }); - expect(response?.ok).toBe(true); - expect(mockDispatch.mock.calls.filter((call) => call[1] === 'snapshot')).toHaveLength(0); - if (response?.ok) { - expect(response.data?.predicate).toBe('selected'); - expect(response.data?.pass).toBe(false); + // The session snapshot has no `id=submit`, so the bound capture reports the typed selector + // failure. Nothing can report a failed assertion as a completed command. + expect(response?.ok).toBe(false); + expect(mockDispatch.mock.calls.filter((call) => call[1] === 'snapshot')).toHaveLength(1); + if (response?.ok === false) { + expect(response.error?.code).toBe('COMMAND_FAILED'); } }); -test('is simple iOS selector falls back to snapshot while gesture stabilization is pending', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'is-selected-ios-stabilizing'; - const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); - session.postGestureStabilization = { action: 'swipe', positionals: [], markedAt: Date.now() }; - sessionStore.set(sessionName, session); - - mockDispatch.mockImplementation(async (_device, command) => { - if (command !== 'snapshot') throw new Error(`unexpected command: ${command}`); - return { - nodes: [ - { - index: 0, - depth: 0, - type: 'Window', - rect: { x: 0, y: 0, width: 390, height: 844 }, - }, - { - index: 1, - depth: 1, - parentIndex: 0, - type: 'Button', - label: 'Pickup', - identifier: 'shipping-pickup', - selected: true, - rect: { x: 126, y: 555, width: 75, height: 38 }, - enabled: true, - hittable: true, - }, - ], - backend: 'xctest', - }; - }); - - const response = await handleInteractionCommands({ - req: { - token: 't', - session: sessionName, - command: 'is', - positionals: ['selected', 'id="shipping-pickup"'], - flags: {}, - }, - sessionName, - sessionStore, - contextFromFlags, - ...getRuntimeBindings(), - }); - - expect(response?.ok).toBe(true); - expect(mockRunAppleRunnerCommand).not.toHaveBeenCalled(); - expect(mockDispatch.mock.calls.some((call) => call[1] === 'snapshot')).toBe(true); -}); - test('is visible passes for list text that inherits viewport visibility from an ancestor', async () => { const sessionStore = makeSessionStore(); const sessionName = 'visible-list-item'; diff --git a/src/daemon/selector-runtime-backend.ts b/src/daemon/selector-runtime-backend.ts index 6b88b659d..7c0158477 100644 --- a/src/daemon/selector-runtime-backend.ts +++ b/src/daemon/selector-runtime-backend.ts @@ -2,7 +2,7 @@ import type { AgentDeviceBackend, BackendSnapshotResult } from '../backend.ts'; import { resolveTargetDevice } from '../core/dispatch.ts'; import { createAgentDevice } from '../runtime.ts'; import { isMacOs, isApplePlatform, publicPlatformString } from '@agent-device/kernel/device'; -import { noActiveSessionError, requireCommandSupported } from './handlers/response.ts'; +import { noActiveSessionError } from './handlers/response.ts'; import type { SnapshotState, SnapshotNode } from '@agent-device/kernel/snapshot'; import { findNodeByLabel } from '../core/snapshot-node-lookup.ts'; import { runAppleRunnerCommand } from '../platforms/apple/core/runner/runner-client.ts'; @@ -95,10 +95,17 @@ async function resolveSelectorRuntimeDevice( } /** - * A migrated selector command's runtime: facts-first admission, exactly one binding, and a - * backend whose every capture goes through the bound operation. A sibling unit migrates by - * naming its command here instead of passing a `capability` to {@link createSelectorRuntime}; - * nothing else in this module or `selector-capture-runtime.ts` needs to change. + * THE selector runtime: facts-first admission, exactly one binding, and a backend whose every + * capture goes through the bound operation. Since `is` (R37) there is no other one — the legacy + * capability-admitted `createSelectorRuntime` and its `requireCommandSupported` call were its + * last consumer and retired with it, so a selector command cannot reach the device on a + * capability bucket even by mistake. + * + * ADR 0019 §6: a `device-runtime` command reaches the device only after resolve -> admit -> + * bind, so THIS CALL COMES FIRST in its route — ahead of every shortcut, including the + * direct-iOS selector query that answers some targets without a capture. That query is a fast + * path *within* an admitted request, never a way around exact-owner facts or the one-binding + * invariant. `get` (R36) and `is` (R37) both order it this way. */ export async function createBoundSelectorRuntime( params: SelectorRuntimeParams, @@ -125,29 +132,6 @@ export async function createBoundSelectorRuntime( }; } -/** - * The legacy capability-admitted selector runtime, for the selector commands whose ADR 0019 - * unit has not landed. The union narrows as each one migrates, and the last selector unit - * deletes this function together with its `requireCommandSupported` call. - */ -export async function createSelectorRuntime( - params: SelectorRuntimeParams, - options: { requireSession: boolean; capability: 'is' }, -): Promise { - const resolved = await resolveSelectorRuntimeDevice(params, options.requireSession); - if (!resolved.ok) return resolved; - const unsupported = requireCommandSupported(options.capability, resolved.device); - if (unsupported) return { ok: false, response: unsupported }; - return { - ok: true, - runtime: createSelectorRuntimeForDevice({ - ...params, - session: resolved.session, - device: resolved.device, - }), - }; -} - function createSelectorBackend(params: SelectorRuntimeDeviceParams): AgentDeviceBackend { // The bound operation is the ONLY element read. Both consumers of the shared backend read — // `get text` and read-only `find … get text` — construct a bound backend, so there is no second diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index ea53978f2..1f255ecb2 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -17,9 +17,7 @@ import { checkIsArgs, checkWaitText, checkFindArgs, - evaluateIsPredicate, isReadOnlyFindAction, - type IsPredicate, } from '@agent-device/selectors'; import { refSnapshotFlagGuardResponse } from './handlers/interaction-flags.ts'; import { parseVersionedRefPositional } from './handlers/interaction-touch-targets.ts'; @@ -49,7 +47,6 @@ import { import { isSessionRecording } from './session-script-publication-capability.ts'; import { createBoundSelectorRuntime, - createSelectorRuntime, createSelectorRuntimeForDevice, type SelectorRuntimeParams, } from './selector-runtime-backend.ts'; @@ -67,15 +64,6 @@ type DirectIosSelectorFallbackResult = | DirectIosSelectorErrorResult | null; -type ResolvedDirectIosSelectorQuery = - | { - session: SessionState; - selector: DirectIosSelectorTarget; - result: DirectIosSelectorQueryResult; - } - | DirectIosSelectorErrorResult - | null; - export async function dispatchFindReadOnlyViaRuntime( params: SelectorRuntimeParams, ): Promise { @@ -234,26 +222,19 @@ export async function dispatchIsViaRuntime( checked.hint ? { hint: checked.hint } : undefined, ); } - const { predicate, expectedText } = checked; - const split = { selectorExpression: checked.selectorExpression }; - // ADR 0012 decision 3 / #1349: recording and a guarded replay dispatch both - // require the snapshot path — evidence and the post-resolution identity - // guard are computed from the resolution tree. + const { predicate, selectorExpression, expectedText } = checked; + // ADR 0012 decision 3 / #1349: a guarded replay dispatch resolves through the snapshot path so + // the post-resolution identity guard runs against the resolution tree. const replayTargetGuard = req.internal?.replayTargetGuard; - const recordingSession = isSessionRecording(params.sessionStore.get(params.sessionName)); - if (!replayTargetGuard && !recordingSession) { - const directResponse = await dispatchDirectIosSelectorIs( - params, - predicate as IsPredicate, - split.selectorExpression, - expectedText, - ); - if (directResponse) return directResponse; - } - const resolvedRuntime = await createSelectorRuntime(params, { + // ADR 0019: `is` declares `device-runtime`, so its request path reaches the device ONLY through + // the operations R37 declares. Every predicate — including the simple iOS `id=` selector a + // direct runner query used to answer without a capture — resolves through the bound capture. + // Admission before a bypass is not the same as executing through the seam, so the bypass is + // gone rather than merely ordered after admission. + const resolvedRuntime = await createBoundSelectorRuntime(params, { requireSession: true, - capability: 'is', + command: 'is', }); if (!resolvedRuntime.ok) return resolvedRuntime.response; @@ -261,8 +242,8 @@ export async function dispatchIsViaRuntime( const result = await resolvedRuntime.runtime.selectors.is({ session: params.sessionName, requestId: req.meta?.requestId, - predicate: predicate as IsPredicate, - selector: split.selectorExpression, + predicate, + selector: selectorExpression, expectedText, expectedResolvedTarget: replayTargetGuard, }); @@ -322,7 +303,7 @@ export async function dispatchWaitViaRuntime( mintedGeneration: versionedRef.generation, }); } - // Wait builds its runtime directly (no createSelectorRuntime), so the consumed-snapshot slot + // Wait builds its runtime directly (no createBoundSelectorRuntime), so the consumed-snapshot slot // must be initialized here too or sessionless waits have nowhere to report the capture from. params.consumedSnapshot ??= {}; const execute = async () => { @@ -376,38 +357,6 @@ function readRecordedResolutionTarget( return { node: node as SnapshotNode, preActionNodes: preActionNodes as SnapshotNode[] }; } -async function dispatchDirectIosSelectorIs( - params: SelectorRuntimeParams, - predicate: IsPredicate, - selectorExpression: string, - expectedText: string, -): Promise { - if (predicate === 'hidden') return null; - const directQuery = await resolveDirectIosSelectorQuery(params, selectorExpression); - if (isDirectIosSelectorErrorResult(directQuery)) return directQuery.response; - if (!directQuery?.result.found || !directQuery.result.node) return null; - - const payload = - predicate === 'exists' - ? { - predicate, - pass: true, - selector: directQuery.selector.raw, - matches: 1, - selectorChain: [directQuery.selector.raw], - } - : buildDirectIosIsResult( - predicate, - expectedText, - directQuery.selector.raw, - directQuery.session, - directQuery.result.node, - ); - if (!payload) return null; - recordIfSession(params.sessionStore, params.sessionName, params.req, payload); - return { ok: true, data: stripSelectorChain(payload) }; -} - async function dispatchDirectIosSelectorWait( params: SelectorRuntimeParams & { session: SessionState | undefined; @@ -439,19 +388,6 @@ async function dispatchDirectIosSelectorWait( ); } -async function resolveDirectIosSelectorQuery( - params: SelectorRuntimeParams, - selectorExpression: string, -): Promise { - const session = params.sessionStore.get(params.sessionName); - const selector = readSimpleIosSelectorTarget({ session, selectorExpression }); - if (!session || !selector) return null; - const result = await queryDirectIosSelectorOrFallback(params, session, selector); - if (isDirectIosSelectorErrorResult(result)) return result; - if (!result) return null; - return { session, selector, result }; -} - /** * The single querySelector client for the local XCTest runner: a live, * tree-independent read (and its found/text/node shape) for exactly one @@ -506,34 +442,11 @@ async function queryDirectIosSelectorOrFallback( } function isDirectIosSelectorErrorResult( - result: DirectIosSelectorFallbackResult | ResolvedDirectIosSelectorQuery, + result: DirectIosSelectorFallbackResult, ): result is DirectIosSelectorErrorResult { return result !== null && 'kind' in result && result.kind === 'error'; } -function buildDirectIosIsResult( - predicate: Exclude, - expectedText: string, - selector: string, - session: SessionState, - node: SnapshotNode, -): Record | null { - const result = evaluateIsPredicate({ - predicate, - node, - nodes: [node], - expectedText, - platform: session.device.platform, - }); - return { - predicate, - pass: result.pass, - selector, - ...(predicate === 'text' ? { text: result.actualText } : {}), - selectorChain: [selector], - }; -} - function readDirectIosSelectorNode(data: Record): SnapshotNode | undefined { const nodes = data.nodes; if (!Array.isArray(nodes)) return undefined; diff --git a/test/integration/provider-scenarios/ios-world.ts b/test/integration/provider-scenarios/ios-world.ts index 5705e9411..5b3cc2328 100644 --- a/test/integration/provider-scenarios/ios-world.ts +++ b/test/integration/provider-scenarios/ios-world.ts @@ -142,31 +142,12 @@ export async function createIosSettingsWorld(): Promise { }, result: { transformed: true }, }, - { - command: 'ios.runner.querySelector', - deviceId: PROVIDER_SCENARIO_IOS_SIMULATOR.id, - platform: 'apple', - request: { - command: 'querySelector', - selectorKey: 'label', - selectorValue: 'General', - appBundleId: 'com.apple.Preferences', - }, - result: { - found: true, - nodes: [ - { - index: 0, - type: 'XCUIElementTypeCell', - label: 'General', - identifier: 'General', - rect: { x: 16, y: 100, width: 360, height: 44 }, - enabled: true, - hittable: true, - }, - ], - }, - }, + // `is visible label=General` answered from a direct `querySelector` here until R37 retired + // that shortcut; it now resolves through the bound capture like every other predicate, so it + // consumes a snapshot and issues no runner query at all. The second snapshot is + // `find attrs by label`. This transcript is the scripted proof that the bypass is gone: an + // unexpected `querySelector` would fail the scenario rather than pass unnoticed. + runnerSnapshot(), runnerSnapshot(), { command: 'ios.runner.findText',