diff --git a/.gitignore b/.gitignore index 40ae8f1d5..e5c3a0935 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,5 @@ android/snapshot-helper/build/ android/snapshot-helper/dist/ android/ime-helper/build/ android/ime-helper/dist/ +android/multitouch-helper/build/ +android/multitouch-helper/dist/ diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index e3a048d1d..6fd2b7a25 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -38,6 +38,12 @@ Accepted (2026-07-10); partially implemented (last updated 2026-07-16). See [Mig - Decision 6 amendment, repair-segment default exclusion of observation-only commands and the `--record` opt-in — stage 1 (interim guidance only, #1287) and stage 2 (the semantic default-exclusion fix, this amendment) of #1271. See the amendment under decision 6 below. +- Decision 3 amendment (#1349), read-only step identity — landmark-mode `target-v1` evidence and + post-resolution (in-loop) verification for selector `wait`, `get`-pattern coverage for `is` (except + `exists`), the `targetIdentityVerification` descriptor trait pinning the evidence-carrying command + set, explicit deferral of read-only `find`/`is exists`/non-selector waits, and ADR 0016's destination + guard strengthened to require a `verified` landmark annotation, with a provider-level + reshuffled-screen false-pass regression. See the amendment under decision 3 below. **Accepted but NOT yet implemented** (this amendment; tracked by #1235 — repair-transaction lifecycle): the R7 repair-transaction keep-alive and its distinct `resume.repairSessionHeld` signal, the ARMED → @@ -370,6 +376,58 @@ A recorded `id` never matches a node without that id. > platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a labeled `Text` child hits > the same substitution on iOS. +> **Amendment (#1349): read-only step identity — `wait` landmark verification, `is` coverage, and the +> per-command verification-phase trait.** Decision 3's evidence and machinery extend to the eligible +> read-only steps without changing their execution semantics. The annotation format is **unchanged** +> (`target-v1`, same tuple, same bounds, same writer/parser); what is new is a per-command dispatch of +> WHEN verification runs, declared on the central `CommandDescriptor` as +> `targetIdentityVerification: 'pre-dispatch' | 'post-resolution'` and pinned by a completeness test — +> a new evidence-carrying command must choose its phase explicitly rather than silently entering the +> generic pre-dispatch path. +> +> - **`wait ` — covered, `post-resolution`, landmark (existence) semantics.** A wait's +> landmark may legitimately be absent when its step starts, so the generic pre-action verification +> (paths 2-6) must never run for it; only path 1 (recorded-`unverifiable`, which consults no screen) +> refuses up front. The recorded annotation instead travels into the wait's own polling loop +> (`internal.replayLandmarkGuard`, the same daemon-only channel as `replayTargetGuard`), which +> strengthens the success condition from "the selector matches" to "a selector match carries the +> recorded identity" — the identity tier only (local identity + leaf-anchored ancestry prefix). The +> positional disambiguation signals are deliberately **not** compared: a destination guard proves the +> landmark exists on the ready screen, not that it kept its list position, and requiring position +> would produce the inverse false-failure on a legitimately reshuffled-but-correct screen. Polling is +> preserved fail-open in time and fail-closed at the deadline: a same-selector impostor never aborts +> the wait (a transient look-alike mid-transition is exactly what a wait exists to wait through); at +> the deadline, a poll history containing selector matches that never carried the identity surfaces +> as an `identity-mismatch` divergence (`matchCount` from the last matching poll, observed identity + +> first ancestry mismatch in `mismatches`) BEFORE the wait reports success, while a selector that +> never matched at all remains the ordinary wait timeout (`action-failure` — that failure needs a +> state repair, not an identity repair, and its `repairHint` routing is already correct). +> A wait poll also rides out a capture that judged the current screen UNREADABLE (the Android +> helper's content verdicts, `isUnreadableCaptureContentError`; iOS surfaces the same state as a +> sparse verdict with no matches): live Android validation showed a guard wait replayed right after +> a navigation press deterministically dies on its first mid-transition capture otherwise. A wait +> whose screen never became readable rethrows the last capture verdict at the deadline instead of +> masking it as a generic timeout. +> Record time writes evidence in a **landmark mode**: the step-5 self-check is identity-set +> MEMBERSHIP rather than isolation (mirroring what replay will actually verify), and a matched node +> with no id and no label after #1269 demotion records **no annotation** — a role-only landmark +> identity is near-vacuous, and an unannotated wait keeps its existing selector-existence semantics +> instead of failing closed on evidence that never discriminated anything. ADR 0016's destination +> guard consumes exactly this: a qualifying guard is a selector wait with a `verified` annotation. +> - **`get` — unchanged**; already covered by the pre-dispatch path and the post-resolution guard. +> - **`is` (all predicates except `exists`) — covered, `pre-dispatch`, the `get` pattern end-to-end.** +> `is` resolves a unique node immediately, so pre-action verification is semantically valid; the +> resolved node/tree feed record-time evidence, and dispatch threads `replayTargetGuard` into +> `assertExpectedResolvedTarget` exactly like `get`. The direct-iOS `is`/`wait` fast paths are gated +> off during recording and guarded replays, mirroring `get`'s existing recording gate. +> - **Intentionally deferred, with tests proving no annotation is recorded and no identity check runs:** +> `is exists` (existence assertion with no unique winner; wait-like semantics without the +> guard-critical role), every read-only `find` variant (fuzzy-locator resolution has no +> selector-chain identity token for the classifier, and publication already refuses mutating `find` +> as non-verifiable), and `wait text`/`wait stable`/duration waits/`wait @ref` (no element target, or +> a session-local ref that ADR 0016 already refuses to publish; `wait @ref` is rejected rather than +> converted to a portable selector). + **Ancestry.** The chain is the nearest **K = 8** ancestors of the target, ordered **leaf→root** (nearest ancestor first), each entry `{ role, label? }` under the same normalization (`role` may be the empty string when the node has no type; `label` is omitted when empty). Truncation drops entries from the @@ -1254,6 +1312,7 @@ not restate or amend the decisions. | 8. Agent-supervised re-record repair, base | 6 (R1-R6) | Shipped | #1228 | | 9. Repair-transaction lifecycle | 6 (R7) | Shipped | #1235 | | 10. Repair-segment default exclusion + `--record` | 6 amendment | Shipped | #1271 stage 2 | +| 11. Read-only step identity (wait landmark verification, `is` coverage) | 3 amendment (#1349) | Shipped | — | Step 4 (#1209) added the `selector-miss`/`identity-mismatch`/`identity-unverifiable` divergence kinds and a post-resolution target guard that cross-checks the dispatched winner against the verified member. diff --git a/docs/adr/0016-active-session-script-publication.md b/docs/adr/0016-active-session-script-publication.md index fc14468fe..6bdd90065 100644 --- a/docs/adr/0016-active-session-script-publication.md +++ b/docs/adr/0016-active-session-script-publication.md @@ -87,15 +87,27 @@ the serialized guard before filesystem work and never relies on repair-only bare `session save-script` refuses publication without this **destination guard** and tells the author to record one. V1 does not infer a screen identity from a snapshot or synthesize an implicit guard. -The initial guard is selector-level. `wait` does not yet carry recorded-landmark identity through its -polling resolution, so the guard proves that an element matching its selector exists, not that it is the -same landmark element observed while authoring. Authors must choose a selective destination-specific -landmark; a reshuffled screen containing the same weak label elsewhere can false-pass. -[#1349](https://github.com/callstack/agent-device/issues/1349) owns the identity design for waits and -remaining read-only steps. It must preserve polling: ADR 0012's current pre-action `target-v1` +> **Amendment (#1349, shipped).** The guard is now identity-level, not merely selector-level. A +> selector wait recorded while ARMED captures landmark-mode `target-v1` evidence for the matched +> element (ADR 0012's read-only-step amendment), and a qualifying destination guard is exactly a +> selector wait whose annotation is `verification: "verified"` — a selector wait on an identity-empty +> element (no id and no label) records no annotation and does not qualify, so publication refuses it +> with a recovery hint naming a labeled/id-bearing landmark. On replay, polling is preserved: the wait +> keeps polling until a selector match carries the recorded identity (local identity + leaf-anchored +> ancestry prefix), and a deadline reached with only same-selector impostors fails closed as an +> `identity-mismatch` `REPLAY_DIVERGENCE` before the wait reports success. The reshuffled-screen +> false-pass below is covered by a provider-scenario regression (record → publish → replay against a +> reshuffled tree whose same-label node sits under a different id/ancestry). + +The original selector-level caveat, retained as context: the guard proved that an element matching its +selector exists, not that it is the same landmark element observed while authoring, so a reshuffled +screen containing the same weak label elsewhere could false-pass. +[#1349](https://github.com/callstack/agent-device/issues/1349) owned the identity design for waits and +remaining read-only steps. It preserves polling: ADR 0012's pre-action `target-v1` verification cannot be attached to a wait unchanged because a not-yet-present landmark is the expected -starting condition. Any identity check for a wait happens after its selector resolves, before the wait -reports success, or uses a distinct guard-specific mechanism. +starting condition, so the identity check runs after the wait's selector resolves and before the wait +reports success (see ADR 0012's #1349 amendment for the mechanism and the read-only-step coverage +classification). The phrase "last mutating action" is derived from a request-sensitive recording-effect trait on the central `CommandDescriptor`, required for every command that records session actions and guarded by a @@ -203,7 +215,9 @@ executing that script, not the artifact being saved. - The artifact replays from a cold start, completes its destination guard, returns the live session id, and accepts a subsequent command on that session. - Every action in ADR 0012's existing target-binding command set has canonical identity evidence and no - unresolved `@ref` reaches disk; the destination guard remains selector-level until #1349 lands. + unresolved `@ref` reaches disk; since #1349 the destination guard additionally requires verified + recorded landmark identity, with a reshuffled-screen false-pass regression proving replay fails + closed. - Existing-target refusal preserves the original bytes; `--force` replaces atomically; a failed publish remains retryable. - After PUBLISHED, later ordinary actions remain usable, repeated `session save-script` fails, plain diff --git a/src/cli/parser/cli-help.ts b/src/cli/parser/cli-help.ts index 5aee724a50..cb9c62b4e 100644 --- a/src/cli/parser/cli-help.ts +++ b/src/cli/parser/cli-help.ts @@ -234,7 +234,7 @@ Reusable open-to-destination scripts: agent-device press 'id="continue"' --settle agent-device wait 'role="heading" label="Screen X"' agent-device session save-script - session save-script [path] [--force] publishes the sole recorded open through the destination guard, omits close, and leaves the session active. A duration wait, wait stable, or wait @ref is not a destination guard. A second successful open aborts publication; start a fresh session to author again. + session save-script [path] [--force] publishes the sole recorded open through the destination guard, omits close, and leaves the session active. The guard is a selector wait on a labeled or id-bearing landmark: its recorded identity is captured while armed, and replay verifies that identity after the wait's selector resolves, so a reshuffled screen with the same label elsewhere fails closed instead of false-passing. A duration wait, wait stable, wait @ref, or a selector wait on an unlabeled element is not a destination guard. A second successful open aborts publication; start a fresh session to author again. Recorded fill/type inputs are written literally to the .ad file. Do not record passwords, tokens, or other secrets; use pre-authenticated test state or non-secret fixture credentials until parameterized input authoring is available. Snapshots and refs: diff --git a/src/commands/interaction/runtime/selector-read.test.ts b/src/commands/interaction/runtime/selector-read.test.ts index 3fb8a1853..476b62abf 100644 --- a/src/commands/interaction/runtime/selector-read.test.ts +++ b/src/commands/interaction/runtime/selector-read.test.ts @@ -10,7 +10,14 @@ import { } from '../../../runtime.ts'; import { ref, selector } from './selector-read.ts'; import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts'; -import { createSelectorDevice, selectorReadSnapshot } from './__tests__/test-utils/index.ts'; +import { + createFakeClock, + createSelectorDevice, + selectorReadSnapshot, +} from './__tests__/test-utils/index.ts'; +import { computeTargetEvidence } from '../../../daemon/session-target-evidence.ts'; +import { WAIT_LANDMARK_MISMATCH_REASON } from '../../../replay/target-identity-node.ts'; +import { AppError } from '../../../kernel/errors.ts'; test('runtime get reads text from a selector target', async () => { const snapshot = selectorReadSnapshot(); @@ -485,3 +492,299 @@ test('runtime selector convenience methods use explicit target helpers', async ( assert.equal(visible.pass, true); assert.deepEqual(waited, { kind: 'text', text: 'Ready', waitedMs: 0 }); }); + +// --------------------------------------------------------------------------- +// #1349: wait's in-loop landmark identity verification (replay-only, +// threaded as `target.recordedLandmark`). Polling semantics are preserved — +// a same-selector impostor never aborts the wait; only the deadline turns +// rejected candidates into the fail-closed landmark refusal. +// --------------------------------------------------------------------------- + +function landmarkScreen(parentLabel: string) { + return makeSnapshotState([ + { index: 0, depth: 0, type: 'Other', label: parentLabel }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'StaticText', + label: 'Screen X', + rect: { x: 0, y: 0, width: 100, height: 20 }, + }, + ]); +} + +function recordedLandmarkFor(snapshot: ReturnType) { + const node = snapshot.nodes[1]!; + const evidence = computeTargetEvidence( + { node, preActionNodes: snapshot.nodes }, + { mode: 'landmark' }, + ); + assert.ok(evidence); + assert.equal(evidence.verification, 'verified'); + return evidence; +} + +function landmarkWaitDevice(captures: Array>) { + let call = 0; + const initial = captures[0]!; + const device = createAgentDevice({ + backend: { + platform: 'ios', + captureSnapshot: async () => { + const snapshot = captures[Math.min(call, captures.length - 1)]!; + call += 1; + return { snapshot }; + }, + } satisfies AgentDeviceBackend, + artifacts: createLocalArtifactAdapter(), + sessions: createMemorySessionStore([{ name: 'default', snapshot: initial }]), + policy: localCommandPolicy(), + clock: createFakeClock(), + }); + return device; +} + +test('runtime wait keeps polling past a same-selector impostor and succeeds on the recorded landmark', async () => { + const recordTime = landmarkScreen('Detail Screen'); + const recorded = recordedLandmarkFor(recordTime); + const impostor = landmarkScreen('List Screen'); + const empty = makeSnapshotState([{ index: 0, depth: 0, type: 'Other', label: 'Loading' }]); + const device = landmarkWaitDevice([empty, impostor, landmarkScreen('Detail Screen')]); + + const result = await device.selectors.wait({ + session: 'default', + target: { + kind: 'selector', + selector: 'label="Screen X"', + timeoutMs: 10_000, + recordedLandmark: recorded, + }, + }); + + assert.equal(result.kind, 'selector'); + if (result.kind !== 'selector') throw new Error('unreachable'); + // Two rejected polls (absent, then impostor) before the landmark appeared. + assert.equal(result.waitedMs >= 600, true); + assert.equal(result.node?.label, 'Screen X'); + assert.equal(result.preActionNodes?.length, 2); +}); + +test('runtime wait fails closed at the deadline when only impostors matched the selector', async () => { + const recorded = recordedLandmarkFor(landmarkScreen('Detail Screen')); + const device = landmarkWaitDevice([landmarkScreen('List Screen')]); + + const error = await device.selectors + .wait({ + session: 'default', + target: { + kind: 'selector', + selector: 'label="Screen X"', + timeoutMs: 1000, + recordedLandmark: recorded, + }, + }) + .then( + () => undefined, + (thrown: unknown) => thrown, + ); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, WAIT_LANDMARK_MISMATCH_REASON); + assert.equal(error.details?.matchCount, 1); + const observed = error.details?.observed as { role: string; label?: string }; + assert.equal(observed.label, 'Screen X'); + const ancestry = error.details?.observedAncestry as Array<{ role: string; label?: string }>; + assert.equal(ancestry[0]?.label, 'List Screen'); +}); + +test('runtime wait with a recorded landmark keeps the plain timeout when the selector never matched', async () => { + const recorded = recordedLandmarkFor(landmarkScreen('Detail Screen')); + const empty = makeSnapshotState([{ index: 0, depth: 0, type: 'Other', label: 'Loading' }]); + const device = landmarkWaitDevice([empty]); + + await assert.rejects( + device.selectors.wait({ + session: 'default', + target: { + kind: 'selector', + selector: 'label="Screen X"', + timeoutMs: 1000, + recordedLandmark: recorded, + }, + }), + (thrown: unknown) => { + assert.ok(thrown instanceof AppError); + assert.match(thrown.message, /wait timed out for selector/); + assert.equal(thrown.details?.reason, undefined); + return true; + }, + ); +}); + +test('runtime wait without a recorded landmark returns the satisfying match for record-time evidence', async () => { + const device = landmarkWaitDevice([landmarkScreen('Detail Screen')]); + + const result = await device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 1000 }, + }); + + assert.equal(result.kind, 'selector'); + if (result.kind !== 'selector') throw new Error('unreachable'); + assert.equal(result.node?.label, 'Screen X'); + assert.equal(result.preActionNodes?.length, 2); +}); + +// --------------------------------------------------------------------------- +// Wait polls ride out captures that judged the screen unreadable (the +// mid-transition Android helper content verdicts) instead of aborting the +// wait — the live-validated destination-guard gap from #1349's PR review. +// --------------------------------------------------------------------------- + +function unreadableCaptureError() { + return new AppError( + 'COMMAND_FAILED', + 'Android snapshot helper returned insufficient foreground app content', + { + androidSnapshotHelperFailureReason: 'content-poor-app-window', + retriable: true, + }, + ); +} + +function waitDeviceWithCaptures(captures: Array<() => ReturnType>) { + let call = 0; + return createAgentDevice({ + backend: { + platform: 'android', + captureSnapshot: async () => { + const produce = captures[Math.min(call, captures.length - 1)]!; + call += 1; + return { snapshot: produce() }; + }, + } satisfies AgentDeviceBackend, + artifacts: createLocalArtifactAdapter(), + sessions: createMemorySessionStore([{ name: 'default' }]), + policy: localCommandPolicy(), + clock: createFakeClock(), + }); +} + +test('runtime wait rides out an unreadable mid-transition capture and succeeds on the next poll', async () => { + const device = waitDeviceWithCaptures([ + () => { + throw unreadableCaptureError(); + }, + () => landmarkScreen('Detail Screen'), + ]); + + const result = await device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 5000 }, + }); + + assert.equal(result.kind, 'selector'); + if (result.kind !== 'selector') throw new Error('unreachable'); + assert.equal(result.waitedMs >= 300, true); +}); + +test('runtime wait rethrows the capture verdict when the screen never became readable', async () => { + const device = waitDeviceWithCaptures([ + () => { + throw unreadableCaptureError(); + }, + ]); + + await assert.rejects( + device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 1000 }, + }), + /insufficient foreground app content/, + ); +}); + +test('runtime wait keeps the plain timeout when readable polls simply never matched', async () => { + const empty = () => makeSnapshotState([{ index: 0, depth: 0, type: 'Other', label: 'Loading' }]); + const device = waitDeviceWithCaptures([ + empty, + () => { + throw unreadableCaptureError(); + }, + ]); + + await assert.rejects( + device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 1000 }, + }), + /wait timed out for selector/, + ); +}); + +function failingWaitDevice(produceError: () => Error): { + device: ReturnType; + attempts: () => number; +} { + let attempts = 0; + const device = createAgentDevice({ + backend: { + platform: 'android', + captureSnapshot: async () => { + attempts += 1; + throw produceError(); + }, + } satisfies AgentDeviceBackend, + artifacts: createLocalArtifactAdapter(), + sessions: createMemorySessionStore([{ name: 'default' }]), + policy: localCommandPolicy(), + clock: createFakeClock(), + }); + return { device, attempts: () => attempts }; +} + +test('runtime wait still fails immediately on a non-content capture failure', async () => { + const { device, attempts } = failingWaitDevice( + () => new AppError('COMMAND_FAILED', 'adb device offline'), + ); + + await assert.rejects( + device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 5000 }, + }), + /adb device offline/, + ); + // Fail-FAST, not fail-at-deadline: a single capture attempt, no polling. + assert.equal(attempts(), 1); +}); + +test('runtime wait fails immediately on a helper MECHANISM failure even though it carries androidSnapshotHelperFailureReason', async () => { + // The realistic wrapper shape: androidSnapshotHelperCaptureError / + // androidSnapshotHelperUnavailableError stamp the SAME details key as the + // content verdicts, but with free-form mechanism reasons. Those must not be + // polled until the wait deadline — the broad any-string classifier would + // ride this to the deadline and rethrow the same error, so the attempt + // count (not the eventual message) is what makes this regression bite. + const { device, attempts } = failingWaitDevice( + () => + new AppError( + 'COMMAND_FAILED', + 'Android snapshot helper failed: instrumentation run timed out after 120000ms', + { + androidSnapshotHelperFailureReason: 'instrumentation run timed out after 120000ms', + hint: 'The device may be busy; retry once it settles.', + }, + ), + ); + + await assert.rejects( + device.selectors.wait({ + session: 'default', + target: { kind: 'selector', selector: 'label="Screen X"', timeoutMs: 5000 }, + }), + /instrumentation run timed out/, + ); + assert.equal(attempts(), 1); +}); diff --git a/src/commands/interaction/runtime/selector-read.ts b/src/commands/interaction/runtime/selector-read.ts index 455e5bffd..9ea97fa01 100644 --- a/src/commands/interaction/runtime/selector-read.ts +++ b/src/commands/interaction/runtime/selector-read.ts @@ -8,6 +8,7 @@ import type { SnapshotNode } from '../../../kernel/snapshot.ts'; import { findNodeByRef, normalizeRef } from '../../../kernel/snapshot.ts'; import { isSparseSnapshotQualityVerdict, + isUnreadableCaptureContentError, type SnapshotQualityVerdict, } from '../../../snapshot/snapshot-quality.ts'; import type { AgentDeviceRuntime, CommandContext } from '../../../runtime-contract.ts'; @@ -16,9 +17,25 @@ import { parseSelectorChain, type SelectorChain } from '../../../selectors/parse import { findSelectorChainMatch, formatSelectorFailure, + listSelectorChainMatches, resolveSelectorChain, selectorFailureHint, } from '../../../selectors/index.ts'; +import type { SelectorChainMatchList } from '../../../selectors/resolve.ts'; +import { + readNodeLocalIdentity, + WAIT_LANDMARK_MISMATCH_REASON, + type WaitLandmarkMismatchEvidence, +} from '../../../replay/target-identity-node.ts'; +import { + buildAncestryChain, + buildIndexMap, + filterIdentitySet, +} from '../../../replay/target-evidence-tree.ts'; +import { + annotationLocalIdentity, + type TargetAnnotationV1, +} from '../../../replay/target-identity.ts'; import { buildSelectorChainForNode } from '../../../selectors/build.ts'; import { evaluateIsPredicate, @@ -108,6 +125,8 @@ export type IsCommandOptions = CommandContext & predicate: 'visible' | 'hidden' | 'exists' | 'editable' | 'selected' | 'focused' | 'text'; selector: string; expectedText?: string; + /** ADR 0012 step 4: replay-only post-resolution guard; see resolution.ts. */ + expectedResolvedTarget?: ExpectedResolvedTarget; }; export type IsCommandResult = { @@ -117,6 +136,9 @@ export type IsCommandResult = { matches?: number; text?: string; selectorChain?: string[]; + /** ADR 0012 decision 3 / #1349: the resolved node and its tree, for record-time evidence (absent for `exists`). */ + node?: SnapshotNode; + preActionNodes?: SnapshotNode[]; }; export type WaitCommandOptions = CommandContext & @@ -125,14 +147,33 @@ export type WaitCommandOptions = CommandContext & | { kind: 'sleep'; durationMs: number } | { kind: 'text'; text: string; timeoutMs?: number | null } | { kind: 'ref'; ref: string; timeoutMs?: number | null } - | { kind: 'selector'; selector: string; timeoutMs?: number | null } + | { + kind: 'selector'; + selector: string; + timeoutMs?: number | null; + /** + * ADR 0012 / #1349, replay-only: the recorded landmark identity this + * wait must observe before reporting success. Polling is unchanged — + * the loop keeps waiting while no selector match carries this + * identity, and a timeout with rejected candidates throws the + * `WAIT_LANDMARK_MISMATCH_REASON` refusal instead of success. + */ + recordedLandmark?: TargetAnnotationV1; + } | { kind: 'stable'; quietMs?: number | null; timeoutMs?: number | null }; }; export type WaitCommandResult = | { kind: 'sleep'; waitedMs: number } | { kind: 'text'; waitedMs: number; text: string } - | { kind: 'selector'; waitedMs: number; selector: string } + | { + kind: 'selector'; + waitedMs: number; + selector: string; + /** ADR 0012 decision 3: the satisfying match and the tree it came from, for record-time evidence. */ + node?: SnapshotNode; + preActionNodes?: SnapshotNode[]; + } | { kind: 'stable'; waitedMs: number; @@ -343,6 +384,12 @@ export const isCommand: RuntimeCommand = asyn hint: selectorFailureHint([]), }); } + assertExpectedResolvedTarget( + resolved.node, + capture.snapshot.nodes, + options.expectedResolvedTarget, + 'is', + ); const result = evaluateIsPredicate({ predicate: options.predicate, node: resolved.node, @@ -369,6 +416,8 @@ export const isCommand: RuntimeCommand = asyn selector: resolved.selector.raw, ...(options.predicate === 'text' ? { text: result.actualText } : {}), selectorChain: chain.selectors.map((entry) => entry.raw), + node: resolved.node, + preActionNodes: capture.snapshot.nodes, }; }; @@ -417,6 +466,7 @@ export const waitCommand: RuntimeCommand options, options.target.selector, options.target.timeoutMs, + options.target.recordedLandmark, ); } if (options.target.kind === 'stable') { @@ -513,28 +563,134 @@ async function waitForSelector( options: WaitCommandOptions, selectorExpression: string, timeoutMs: number | null | undefined, + recordedLandmark: TargetAnnotationV1 | undefined, ): Promise { const timeout = timeoutMs ?? DEFAULT_TIMEOUT_MS; const start = now(runtime); const chain = parseSelectorChain(selectorExpression); const capturePolicy = deriveSelectorCapturePolicy({ selectorChain: chain }); + // ADR 0012 / #1349: the LAST poll whose capture matched the recorded + // selector without any match carrying the recorded landmark identity. A + // transient same-selector impostor (the previous screen mid-transition) + // must not abort a wait whose job is to wait through it, so the loop keeps + // polling; only the deadline turns this into the fail-closed refusal. + let landmarkMismatch: WaitLandmarkMismatchEvidence | undefined; + const unreadable = createUnreadablePollTracker(); while (now(runtime) - start < timeout) { // Presence-only poll: skip scroll-hint derivation (#1270), same as waitForFindMatch. - const capture = await captureSelectorSnapshot(runtime, options, { - updateSession: true, - includeHiddenContentHints: false, - ...capturePolicy, - }); - const match = findSelectorChainMatch(capture.snapshot.nodes, chain, { - platform: runtime.backend.platform, - }); - if (match) - return { kind: 'selector', selector: match.selector.raw, waitedMs: now(runtime) - start }; + const capture = await unreadable.attempt(() => + captureSelectorSnapshot(runtime, options, { + updateSession: true, + includeHiddenContentHints: false, + ...capturePolicy, + }), + ); + if (capture) { + const nodes = capture.snapshot.nodes; + const matchList = listSelectorChainMatches(nodes, chain, { + platform: runtime.backend.platform, + }); + if (matchList) { + const landmark = resolveLandmarkMatch(nodes, matchList, recordedLandmark); + if (landmark.kind === 'satisfied') { + return { + kind: 'selector', + selector: matchList.selector.raw, + waitedMs: now(runtime) - start, + node: landmark.node, + preActionNodes: nodes, + }; + } + landmarkMismatch = landmark.evidence; + } + } await sleep(runtime, POLL_INTERVAL_MS); } + if (landmarkMismatch) { + throw new AppError( + 'COMMAND_FAILED', + `wait matched selector ${selectorExpression} but no candidate carried the recorded landmark identity`, + { reason: WAIT_LANDMARK_MISMATCH_REASON, ...landmarkMismatch }, + ); + } + unreadable.rethrowIfNeverReadable(); throw new AppError('COMMAND_FAILED', `wait timed out for selector: ${selectorExpression}`); } +/** + * A wait poll rides out a capture that judged the current screen unreadable + * (`isUnreadableCaptureContentError` — the mid-transition state a wait exists + * to wait through; iOS surfaces the same state as a sparse verdict with no + * matches). Any other capture failure still throws immediately, and a wait + * whose screen NEVER became readable rethrows the last content verdict at the + * deadline so persistent breakage keeps its capture diagnosis instead of a + * generic timeout. + */ +function createUnreadablePollTracker(): { + attempt: (capture: () => Promise) => Promise; + rethrowIfNeverReadable: () => void; +} { + let sawReadableCapture = false; + let lastUnreadableError: unknown; + return { + attempt: async (capture: () => Promise): Promise => { + try { + const result = await capture(); + sawReadableCapture = true; + return result; + } catch (error) { + if (!isUnreadableCaptureContentError(error)) throw error; + lastUnreadableError = error; + return undefined; + } + }, + rethrowIfNeverReadable: () => { + if (!sawReadableCapture && lastUnreadableError !== undefined) throw lastUnreadableError; + }, + }; +} + +type LandmarkMatchOutcome = + | { kind: 'satisfied'; node: SnapshotNode } + | { kind: 'identity-mismatch'; evidence: WaitLandmarkMismatchEvidence }; + +/** + * #1349 landmark check: the wait is satisfied when SOME selector match + * carries the recorded identity (local identity + leaf-anchored ancestry + * prefix). Positional disambiguation signals are deliberately not consulted — + * a destination guard proves the landmark exists on the ready screen, not + * that it kept its list position. + */ +function resolveLandmarkMatch( + nodes: SnapshotNode[], + matchList: SelectorChainMatchList, + recorded: TargetAnnotationV1 | undefined, +): LandmarkMatchOutcome { + const firstMatch = matchList.matchedNodes[0]!; + if (!recorded) return { kind: 'satisfied', node: firstMatch }; + const byIndex = buildIndexMap(nodes); + const identitySet = filterIdentitySet( + matchList.matchedNodes, + byIndex, + annotationLocalIdentity(recorded), + recorded.ancestry, + ); + const member = identitySet[0]; + if (member) return { kind: 'satisfied', node: member }; + return { + kind: 'identity-mismatch', + evidence: { + matchCount: matchList.matchedNodes.length, + observed: readNodeLocalIdentity(firstMatch), + observedAncestry: buildAncestryChain( + firstMatch, + byIndex, + Math.max(recorded.ancestry.length, 1), + ).chain, + }, + }; +} + async function waitForText( runtime: AgentDeviceRuntime, options: WaitCommandOptions, @@ -543,13 +699,17 @@ async function waitForText( ): Promise { const timeout = timeoutMs ?? DEFAULT_TIMEOUT_MS; const start = now(runtime); + const unreadable = createUnreadablePollTracker(); while (now(runtime) - start < timeout) { - const found = runtime.backend.findText - ? (await runtime.backend.findText(toBackendContext(runtime, options), text)).found - : await snapshotContainsText(runtime, options, text); + const found = await unreadable.attempt(async () => + runtime.backend.findText + ? (await runtime.backend.findText(toBackendContext(runtime, options), text)).found + : await snapshotContainsText(runtime, options, text), + ); if (found) return { kind: 'text', text, waitedMs: now(runtime) - start }; await sleep(runtime, POLL_INTERVAL_MS); } + unreadable.rethrowIfNeverReadable(); throw new AppError('COMMAND_FAILED', `wait timed out for text: ${text}`); } diff --git a/src/core/command-descriptor/__tests__/parity.test.ts b/src/core/command-descriptor/__tests__/parity.test.ts index ad0e4021b..97bbe5fe0 100644 --- a/src/core/command-descriptor/__tests__/parity.test.ts +++ b/src/core/command-descriptor/__tests__/parity.test.ts @@ -21,6 +21,7 @@ import { listMcpExposedCommandNames, resolveCommandRecordsSessionAction, resolveCommandRecordingEffect, + resolveTargetIdentityVerification, RAW_COMMAND_DESCRIPTORS, } from '../registry.ts'; @@ -388,3 +389,26 @@ test('recordingEffect resolves request-sensitive observation and mutation subcom 'mutates-app', ); }); + +test('targetIdentityVerification pins exactly the evidence-carrying command set (ADR 0012 / #1349)', () => { + const declared = RAW_COMMAND_DESCRIPTORS.flatMap((descriptor) => { + const phase = resolveTargetIdentityVerification(descriptor.name); + return phase ? [[descriptor.name, phase] as const] : []; + }); + // A new evidence-carrying command must choose its replay verification phase + // here explicitly instead of silently entering the generic pre-dispatch + // path — wait is the only command whose target may legitimately be absent + // when its step starts. + assert.deepEqual( + [...declared].sort(([a], [b]) => a.localeCompare(b)), + [ + ['click', 'pre-dispatch'], + ['fill', 'pre-dispatch'], + ['get', 'pre-dispatch'], + ['is', 'pre-dispatch'], + ['longpress', 'pre-dispatch'], + ['press', 'pre-dispatch'], + ['wait', 'post-resolution'], + ], + ); +}); diff --git a/src/core/command-descriptor/registry.ts b/src/core/command-descriptor/registry.ts index 8ed84e615..1ea31c9e2 100644 --- a/src/core/command-descriptor/registry.ts +++ b/src/core/command-descriptor/registry.ts @@ -18,6 +18,7 @@ import type { RecordingEffect, CommandResponseDataTransform, CommandTimeoutPolicy, + TargetIdentityVerification, } from './types.ts'; type RawCommandDescriptorShape = T extends CommandDescriptor @@ -757,6 +758,9 @@ export const RAW_COMMAND_DESCRIPTORS = [ catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'observes-app', + // #1349: a wait's landmark may legitimately be absent when the step + // starts, so identity verification runs inside its polling resolution. + targetIdentityVerification: 'post-resolution', daemon: { route: 'snapshot', refFrameEffect: 'preserve' }, capability: ALL_DEVICE_COMMAND_CAPABILITY, // The wait budget travels as a positional, not a flag; parse it the same @@ -859,6 +863,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'click', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'mutates-app', @@ -876,6 +881,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'fill', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'mutates-app', @@ -894,6 +900,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'longpress', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public', key: 'longPress' }, recordsSessionAction: true, recordingEffect: 'mutates-app', @@ -917,6 +924,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'press', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'mutates-app', @@ -951,6 +959,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'get', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'observes-app', @@ -971,6 +980,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ { name: 'is', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), + targetIdentityVerification: 'pre-dispatch', catalog: { group: 'public' }, recordsSessionAction: true, recordingEffect: 'observes-app', @@ -1416,6 +1426,17 @@ export function resolveCommandRecordsSessionAction(command: string | undefined): return COMMAND_DESCRIPTOR_BY_NAME.get(command)?.recordsSessionAction ?? false; } +/** + * ADR 0012 / #1349: the replay verification phase for one command's recorded + * `target-v1` evidence, or `undefined` for a command whose steps never carry + * it (an annotation on such a step is inert, exactly like an old reader). + */ +export function resolveTargetIdentityVerification( + command: string, +): TargetIdentityVerification | undefined { + return COMMAND_DESCRIPTOR_BY_NAME.get(command)?.targetIdentityVerification; +} + /** ADR 0016 request-sensitive app-state effect for one recorded request. */ export function resolveCommandRecordingEffect( req: Pick, diff --git a/src/core/command-descriptor/types.ts b/src/core/command-descriptor/types.ts index 6efd5e8cc..797a62e0e 100644 --- a/src/core/command-descriptor/types.ts +++ b/src/core/command-descriptor/types.ts @@ -103,6 +103,17 @@ export type CommandRecordingEffect = | RecordingEffect | ((req: Pick) => RecordingEffect); +/** + * ADR 0012 / #1349: when replay verifies a recorded `target-v1` annotation. + * `pre-dispatch` — the step loop verifies against a fresh capture before + * dispatch (touch/fill/get/is expect their target present). `post-resolution` + * — the command's own resolution verifies (wait's polling loop, whose + * landmark may legitimately be absent at step start); only decision 3 path 1 + * runs up front. Declared on every evidence-carrying command and pinned by + * the parity test, so a new one must choose a phase explicitly. + */ +export type TargetIdentityVerification = 'pre-dispatch' | 'post-resolution'; + /** * The single additive command-descriptor shape (ADR-0008, Phase 1 step 1). * @@ -149,6 +160,8 @@ type CommandDescriptorBase = { responseDataTransform?: CommandResponseDataTransform; catalog: CommandCatalogFacet; dispatch?: CommandDispatchFacet; + /** ADR 0012 / #1349: present iff this command's recorded steps can carry `target-v1` evidence. */ + targetIdentityVerification?: TargetIdentityVerification; /** * Whether the command records an action into the active session replay script * by default, making `--no-record` meaningful. Declared on every raw descriptor diff --git a/src/core/press-retarget.ts b/src/core/press-retarget.ts index 9a01c5878..57acbc787 100644 --- a/src/core/press-retarget.ts +++ b/src/core/press-retarget.ts @@ -19,6 +19,7 @@ import { demoteNonUniqueLocalIdentity, readNodeLocalIdentity, } from '../replay/target-identity-node.ts'; +import { buildIndexMap } from '../replay/target-evidence-tree.ts'; import { normalizeSelectorText } from '../selectors/build.ts'; import { isSemanticTouchTarget } from './interaction-targeting.ts'; @@ -47,12 +48,6 @@ function isIdentityEmpty(node: SnapshotNode, nodes: readonly SnapshotNode[]): bo return normalizeSelectorText(node.value) === null; } -function buildIndexMap(nodes: readonly SnapshotNode[]): Map { - const map = new Map(); - for (const node of nodes) map.set(node.index, node); - return map; -} - /** True when `node` is a proper descendant of `root` — a cycle-safe parent walk, matching `buildAncestryChain`'s guard. */ function isDescendantOf( node: SnapshotNode, diff --git a/src/daemon/__tests__/session-script-active-publication.test.ts b/src/daemon/__tests__/session-script-active-publication.test.ts index e5625def1..e2f8d2359 100644 --- a/src/daemon/__tests__/session-script-active-publication.test.ts +++ b/src/daemon/__tests__/session-script-active-publication.test.ts @@ -20,12 +20,27 @@ function action(command: string, positionals: string[] = []): SessionAction { return { ts: 1, command, positionals, flags: {} }; } +/** #1349: a qualifying guard wait carries verified landmark-mode evidence. */ +function guardWait(positionals: string[]): SessionAction { + return { + ...action('wait', positionals), + targetEvidence: { + role: 'heading', + label: 'Screen X', + ancestry: [], + sibling: 0, + viewportOrder: 0, + verification: 'verified', + }, + }; +} + describe('ADR 0016 active publication contract', () => { test('accepts exactly one initial open and a selector guard after the final mutation', () => { const actions = [ action('open', ['Demo']), { ...action('press', ['id="continue"']), targetEvidence: TARGET_EVIDENCE }, - action('wait', ['role="heading" label="Screen X"']), + guardWait(['role="heading" label="Screen X"']), action('wait', ['stable']), ]; @@ -44,11 +59,34 @@ describe('ADR 0016 active publication contract', () => { ).toThrow(/portable destination guard/); }); + // #1349: the guard proves recorded landmark identity, not selector + // existence — the ADR 0016 reshuffled-screen false-pass regression begins + // at publication: an identity-less guard never ships. + test('rejects a selector wait without recorded landmark evidence as the destination guard', () => { + expect(() => + validateActivePublicationActions([ + action('open', ['Demo']), + action('wait', ['role="heading" label="Screen X"']), + ]), + ).toThrow(/portable destination guard/); + }); + + test('rejects a selector wait whose landmark evidence is unverifiable as the destination guard', () => { + const unverifiableGuard = guardWait(['role="heading" label="Screen X"']); + unverifiableGuard.targetEvidence = { + ...unverifiableGuard.targetEvidence!, + verification: 'unverifiable', + }; + expect(() => + validateActivePublicationActions([action('open', ['Demo']), unverifiableGuard]), + ).toThrow(/portable destination guard/); + }); + test('requires the guard after request-sensitive mutations', () => { expect(() => validateActivePublicationActions([ action('open', ['Demo']), - action('wait', ['id="screen-x"']), + guardWait(['id="screen-x"']), action('alert', ['accept']), ]), ).toThrow(/after the final mutating action/); @@ -57,7 +95,7 @@ describe('ADR 0016 active publication contract', () => { validateActivePublicationActions([ action('open', ['Demo']), action('keyboard', ['status']), - action('wait', ['id="screen-x"']), + guardWait(['id="screen-x"']), ]), ).not.toThrow(); }); diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts index 01d7986e7..40db36c18 100644 --- a/src/daemon/__tests__/session-target-evidence.test.ts +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -589,3 +589,88 @@ test('computeTargetEvidence: a unique id is still recorded as identity (already- assert.ok(evidence); assert.equal(evidence.id, 'save'); }); + +// --------------------------------------------------------------------------- +// #1349: landmark mode — wait's existence-semantics evidence. +// --------------------------------------------------------------------------- + +/** Two identical rows the disambiguation signals cannot fully separate, plus a labeled parent. */ +function twinRowsFixture(): SnapshotNode[] { + return toSnapshotNodes([ + { index: 0, type: 'Window', depth: 0 }, + { index: 1, type: 'List', label: 'Results', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'StaticText', + label: 'Row', + rect: { x: 0, y: 10, width: 100, height: 20 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'StaticText', + label: 'Row', + rect: { x: 0, y: 40, width: 100, height: 20 }, + depth: 2, + parentIndex: 1, + }, + ]); +} + +test('computeTargetEvidence landmark mode: identity twins stay verified (membership, not isolation)', () => { + const nodes = twinRowsFixture(); + const winner = nodes.find((node) => node.index === 3); + assert.ok(winner); + const evidence = computeTargetEvidence( + { node: winner, preActionNodes: nodes }, + { mode: 'landmark' }, + ); + assert.ok(evidence); + assert.equal(evidence.verification, 'verified'); + assert.equal(evidence.role, 'statictext'); + assert.equal(evidence.label, 'Row'); +}); + +test('computeTargetEvidence landmark mode: an identity-empty winner records no annotation', () => { + const nodes = toSnapshotNodes([ + { index: 0, type: 'Window', depth: 0 }, + { + index: 1, + type: 'Other', + depth: 1, + parentIndex: 0, + rect: { x: 0, y: 0, width: 100, height: 100 }, + }, + ]); + const winner = nodes.find((node) => node.index === 1); + assert.ok(winner); + assert.equal( + computeTargetEvidence({ node: winner, preActionNodes: nodes }, { mode: 'landmark' }), + undefined, + ); + // Action mode still writes evidence for the same node — the omission is + // landmark-specific fail-open behavior. + assert.ok(computeTargetEvidence({ node: winner, preActionNodes: nodes })); +}); + +test('computeTargetEvidence landmark mode: a broken parent walk still fails closed to unverifiable', () => { + const nodes = toSnapshotNodes([ + { + index: 1, + type: 'StaticText', + label: 'Screen X', + depth: 1, + parentIndex: 99, // dangling parent: capture anomaly + rect: { x: 0, y: 0, width: 100, height: 20 }, + }, + ]); + const winner = nodes[0]; + assert.ok(winner); + const evidence = computeTargetEvidence( + { node: winner, preActionNodes: nodes }, + { mode: 'landmark' }, + ); + assert.ok(evidence); + assert.equal(evidence.verification, 'unverifiable'); +}); diff --git a/src/daemon/handlers/__tests__/find.test.ts b/src/daemon/handlers/__tests__/find.test.ts index 2159bf054..58c0fe3b1 100644 --- a/src/daemon/handlers/__tests__/find.test.ts +++ b/src/daemon/handlers/__tests__/find.test.ts @@ -801,3 +801,51 @@ test('find rejects --record on a mutating action before any device work', async // Refused before the action dispatched. expect(invokeCalls).toHaveLength(0); }); + +test('read-only find while recording is intentionally deferred from target-v1 evidence (#1349)', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'default'; + const session = makeSession(sessionName); + session.recordSession = true; + sessionStore.set(sessionName, session); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') { + return { + nodes: [ + { + index: 0, + depth: 0, + type: 'Button', + label: 'Save', + rect: { x: 10, y: 10, width: 40, height: 20 }, + enabled: true, + hittable: true, + }, + ], + }; + } + return {}; + }); + + const response = await handleFindCommands({ + req: { + token: 't', + session: sessionName, + command: 'find', + positionals: ['text', 'Save', 'exists'], + flags: {}, + }, + sessionName, + logPath: '/tmp/test.log', + sessionStore, + invoke: async () => ({ ok: true, data: {} }), + }); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.command).toBe('find'); + // The fuzzy-locator resolution has no selector-chain identity token for + // replay verification, so read-only find records NO annotation in v1 — + // an explicit deferral, not an accident. + expect(recordedAction?.targetEvidence).toBeUndefined(); +}); diff --git a/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts index 9940fbc92..16e5d5123 100644 --- a/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts +++ b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts @@ -345,3 +345,78 @@ test('press on an identity-empty container: container-based daemon response, des ); expect(script).toContain('"role":"textview","label":"Connected devices"'); }); + +// --------------------------------------------------------------------------- +// #1349: `is` joins the evidence-carrying set (get-pattern) — a unique- +// resolving predicate records target-v1 evidence while recording; `is exists` +// stays intentionally unannotated; the direct-iOS fast path is gated during +// recording so the snapshot path supplies the evidence tree. +// --------------------------------------------------------------------------- + +function mockSnapshotWithSaveButton() { + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') { + return { backend: 'xctest', nodes: SAVE_BUTTON_NODES }; + } + return {}; + }); +} + +test('is visible while recording attaches target-v1 evidence and strips the resolution payload everywhere', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-is'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: true })); + mockSnapshotWithSaveButton(); + + const response = await runCommand(sessionStore, sessionName, 'is', ['visible', 'label="Save"']); + + expect(response?.ok).toBe(true); + if (response?.ok) { + expect(response.data).not.toHaveProperty('node'); + expect(response.data).not.toHaveProperty('preActionNodes'); + expect(response.data).not.toHaveProperty('selectorChain'); + } + // The direct-iOS querySelector fast path must be gated during recording. + expect(mockRunAppleRunnerCommand).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ command: 'querySelector' }), + expect.anything(), + ); + + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toMatchObject({ + id: 'save', + role: 'button', + label: 'Save', + verification: 'verified', + }); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('is exists while recording stays intentionally unannotated (deferred coverage)', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-is-exists'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: true })); + mockSnapshotWithSaveButton(); + + const response = await runCommand(sessionStore, sessionName, 'is', ['exists', 'label="Save"']); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.command).toBe('is'); + expect(recordedAction?.targetEvidence).toBeUndefined(); +}); + +test('is visible without recording never computes target-v1 evidence', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'non-recording-is'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: false })); + mockSnapshotWithSaveButton(); + + const response = await runCommand(sessionStore, sessionName, 'is', ['visible', 'label="Save"']); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toBeUndefined(); +}); diff --git a/src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts b/src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts index 61f0f947a..b3fb0f2cd 100644 --- a/src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-target-verification-runtime.test.ts @@ -485,3 +485,280 @@ test('a target-binding divergence carries a real computed resume (step 5 wiring, expect((resume.planDigest ?? '').length).toBeGreaterThan(0); expect(resume.reason).toBeUndefined(); }); + +// --------------------------------------------------------------------------- +// #1349: wait's post-resolution landmark verification. An annotated selector +// wait must NEVER enter the generic pre-dispatch resolution path — polling is +// the command's own job — so the step loop only threads the recorded +// landmark and converts the loop's timeout refusal afterwards. +// --------------------------------------------------------------------------- + +import { WAIT_LANDMARK_MISMATCH_REASON } from '../../../replay/target-identity-node.ts'; + +const WAIT_ANNOTATION = + '# agent-device:target-v1 {"role":"statictext","label":"Screen X","ancestry":[{"role":"other","label":"Detail Screen"}],"sibling":0,"viewportOrder":0,"verification":"verified"}'; + +const WAIT_UNVERIFIABLE_ANNOTATION = + '# agent-device:target-v1 {"role":"statictext","label":"Screen X","ancestry":[{"role":"other","label":"Detail Screen"}],"sibling":0,"viewportOrder":0,"verification":"unverifiable"}'; + +test('an annotated selector wait dispatches with the landmark guard and no eager pre-action refusal', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-wait-landmark-thread-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [WAIT_ANNOTATION, 'wait "label=\\"Screen X\\"" 2000']); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: { waitedMs: 300, selector: 'label="Screen X"' } }; + }, + }); + + expect(response.ok).toBe(true); + expect(invoked.map((req) => req.command)).toEqual(['wait']); + expect(invoked[0]?.internal?.replayLandmarkGuard).toEqual({ + role: 'statictext', + label: 'Screen X', + ancestry: [{ role: 'other', label: 'Detail Screen' }], + sibling: 0, + viewportOrder: 0, + verification: 'verified', + }); + expect(invoked[0]?.internal?.replayTargetGuard).toBeUndefined(); + // The landmark may legitimately be absent when the step starts: no + // pre-action capture, no eager refusal. + expect(mockDispatchCommand).not.toHaveBeenCalled(); +}); + +test('a recorded-unverifiable wait annotation refuses before polling with matchCount omitted', async () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-replay-wait-landmark-unverifiable-'), + ); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [ + WAIT_UNVERIFIABLE_ANNOTATION, + 'wait "label=\\"Screen X\\"" 2000', + ]); + + mockDispatchCommand.mockResolvedValue({ nodes: [], truncated: false, backend: 'xctest' }); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: {} }; + }, + }); + + expect(invoked.length).toBe(0); + expect(response.ok).toBe(false); + if (response.ok) return; + const divergence = response.error.details?.divergence as Record; + expect(divergence.kind).toBe('identity-unverifiable'); + const targetBinding = divergence.targetBinding as Record; + expect('matchCount' in targetBinding).toBe(false); +}); + +test("the wait loop's landmark refusal converts into an identity-mismatch divergence", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-wait-landmark-miss-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [WAIT_ANNOTATION, 'wait "label=\\"Screen X\\"" 2000']); + + // The divergence screen capture after the refusal. + mockDispatchCommand.mockResolvedValue({ + nodes: [ + { + index: 0, + depth: 0, + type: 'StaticText', + label: 'Screen X', + rect: { x: 0, y: 0, width: 100, height: 20 }, + }, + ], + truncated: false, + backend: 'xctest', + }); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { + ok: false, + error: { + code: 'COMMAND_FAILED', + message: + 'wait matched selector label="Screen X" but no candidate carried the recorded landmark identity', + details: { + reason: WAIT_LANDMARK_MISMATCH_REASON, + matchCount: 2, + observed: { role: 'statictext', label: 'Screen X' }, + observedAncestry: [{ role: 'other', label: 'List Screen' }], + }, + }, + }; + }, + }); + + expect(invoked.map((req) => req.command)).toEqual(['wait']); + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('REPLAY_DIVERGENCE'); + const divergence = response.error.details?.divergence as Record; + expect(divergence.kind).toBe('identity-mismatch'); + expect(divergence.repairHint).toBe('caution'); + const targetBinding = divergence.targetBinding as Record; + expect(targetBinding.classification).toBe('identity-mismatch'); + expect(targetBinding.matchCount).toBe(2); + expect(targetBinding.recorded).toEqual({ role: 'statictext', label: 'Screen X' }); + expect(targetBinding.observed).toEqual({ role: 'statictext', label: 'Screen X' }); + const mismatches = targetBinding.mismatches as string[]; + expect(mismatches.some((entry) => entry.startsWith('ancestry[0]'))).toBe(true); +}); + +test('a plain wait timeout on an annotated wait stays an action-failure divergence', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-wait-plain-timeout-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [WAIT_ANNOTATION, 'wait "label=\\"Screen X\\"" 2000']); + + mockDispatchCommand.mockResolvedValue({ nodes: [], truncated: false, backend: 'xctest' }); + + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async () => ({ + ok: false, + error: { + code: 'COMMAND_FAILED', + message: 'wait timed out for selector: label="Screen X"', + }, + }), + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + const divergence = response.error.details?.divergence as Record; + expect(divergence.kind).toBe('action-failure'); +}); + +test('an annotation on a duration wait is inert (no guard, no refusal)', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-wait-duration-inert-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [WAIT_UNVERIFIABLE_ANNOTATION, 'wait 500']); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: { waitedMs: 500 } }; + }, + }); + + expect(response.ok).toBe(true); + expect(invoked.map((req) => req.command)).toEqual(['wait']); + expect(invoked[0]?.internal?.replayLandmarkGuard).toBeUndefined(); + expect(mockDispatchCommand).not.toHaveBeenCalled(); +}); + +// --------------------------------------------------------------------------- +// #1349: `is` joins the generic pre-dispatch verification path (get-pattern). +// --------------------------------------------------------------------------- + +const IS_ANNOTATION = + '# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[],"sibling":0,"viewportOrder":0,"verification":"verified"}'; + +test('an annotated is step verifies pre-dispatch and threads the post-resolution guard', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-is-verified-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [IS_ANNOTATION, 'is visible id="save"']); + + mockDispatchCommand.mockResolvedValue({ + nodes: [ + { + index: 0, + depth: 0, + type: 'Button', + identifier: 'save', + label: 'Save', + rect: { x: 10, y: 10, width: 40, height: 20 }, + }, + ], + truncated: false, + backend: 'xctest', + }); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: { predicate: 'visible', pass: true } }; + }, + }); + + expect(response.ok).toBe(true); + expect(invoked.map((req) => req.command)).toEqual(['is']); + expect(invoked[0]?.internal?.replayTargetGuard).toMatchObject({ + identity: { id: 'save', role: 'button', label: 'Save' }, + }); +}); + +test('an annotated is step whose recorded identity vanished diverges before dispatch', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-is-mismatch-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [IS_ANNOTATION, 'is visible label="Save"']); + + mockDispatchCommand.mockResolvedValue({ + nodes: [ + { + index: 0, + depth: 0, + type: 'Button', + identifier: 'save-v2', + label: 'Save', + rect: { x: 10, y: 10, width: 40, height: 20 }, + }, + ], + truncated: false, + backend: 'xctest', + }); + + const invoked: DaemonRequest[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: {} }; + }, + }); + + expect(invoked.length).toBe(0); + expect(response.ok).toBe(false); + if (response.ok) return; + const divergence = response.error.details?.divergence as Record; + expect(divergence.kind).toBe('identity-mismatch'); +}); diff --git a/src/daemon/handlers/__tests__/session-script-publication.test.ts b/src/daemon/handlers/__tests__/session-script-publication.test.ts index 4569b545a..cff5a86e6 100644 --- a/src/daemon/handlers/__tests__/session-script-publication.test.ts +++ b/src/daemon/handlers/__tests__/session-script-publication.test.ts @@ -19,6 +19,17 @@ const TARGET_EVIDENCE: TargetAnnotationV1 = { verification: 'verified', }; +/** #1349: a destination guard qualifies only with verified landmark evidence. */ +const GUARD_EVIDENCE: TargetAnnotationV1 = { + id: 'screen-x', + role: 'heading', + label: 'Screen X', + ancestry: [], + sibling: 0, + viewportOrder: 0, + verification: 'verified', +}; + let root: string; let store: SessionStore; @@ -40,7 +51,13 @@ function armedSession(overrides: Partial = {}): SessionState { flags: {}, targetEvidence: TARGET_EVIDENCE, }, - { ts: 3, command: 'wait', positionals: ['id="screen-x"'], flags: {} }, + { + ts: 3, + command: 'wait', + positionals: ['id="screen-x"'], + flags: {}, + targetEvidence: GUARD_EVIDENCE, + }, ], ...overrides, }); @@ -207,7 +224,15 @@ test('invalid destination guard remains armed and creates no target directory', test('missing initial open is non-retriable within the armed session', () => { const outputPath = path.join(root, 'missing', 'screen-x.ad'); const session = armedSession({ - actions: [{ ts: 1, command: 'wait', positionals: ['id="screen-x"'], flags: {} }], + actions: [ + { + ts: 1, + command: 'wait', + positionals: ['id="screen-x"'], + flags: {}, + targetEvidence: GUARD_EVIDENCE, + }, + ], }); store.set('authoring', session); @@ -248,7 +273,13 @@ test('publishes leading-at typed values without mistaking them for session refs' positionals: ['text', '@handle', 'get', 'text'], flags: {}, }, - { ts: 5, command: 'wait', positionals: ['id="screen-x"'], flags: {} }, + { + ts: 5, + command: 'wait', + positionals: ['id="screen-x"'], + flags: {}, + targetEvidence: GUARD_EVIDENCE, + }, ], }); store.set('authoring', session); @@ -277,7 +308,13 @@ test('refuses mutating find steps that cannot enforce target identity on replay' positionals: ['text', 'Continue', 'click'], flags: {}, }, - { ts: 3, command: 'wait', positionals: ['id="screen-x"'], flags: {} }, + { + ts: 3, + command: 'wait', + positionals: ['id="screen-x"'], + flags: {}, + targetEvidence: GUARD_EVIDENCE, + }, ], }); store.set('authoring', session); diff --git a/src/daemon/handlers/__tests__/wait-landmark-recording.test.ts b/src/daemon/handlers/__tests__/wait-landmark-recording.test.ts new file mode 100644 index 000000000..dd8e0ea65 --- /dev/null +++ b/src/daemon/handlers/__tests__/wait-landmark-recording.test.ts @@ -0,0 +1,159 @@ +/** + * #1349 (ADR 0012 decision 3 extension): the wait dispatch seam. + * + * Record time: a selector wait that succeeds while the session is recording + * attaches landmark-mode `target-v1` evidence to the recorded action, and the + * resolution payload (`node`/`preActionNodes`) reaches neither the public + * response nor session history. + * + * Replay time: `internal.replayLandmarkGuard` threads the recorded landmark + * into the real polling loop — success requires an identity-carrying match, + * and a timeout with only impostor matches surfaces the + * `WAIT_LANDMARK_MISMATCH_REASON` refusal. + */ +import { test, expect, vi, beforeEach } from 'vitest'; +import { dispatchWaitViaRuntime } from '../../selector-runtime.ts'; +import type { DaemonRequest } from '../../types.ts'; +import { WAIT_LANDMARK_MISMATCH_REASON } from '../../../replay/target-identity-node.ts'; +import type { TargetAnnotationV1 } from '../../../replay/target-identity.ts'; +import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; +import { makeAndroidSession } from '../../../__tests__/test-utils/session-factories.ts'; + +vi.mock('../../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchCommand: vi.fn(async () => ({})), + resolveTargetDevice: vi.fn(actual.resolveTargetDevice), + }; +}); + +vi.mock('../../device-ready.ts', () => ({ + ensureDeviceReady: vi.fn(async () => {}), +})); + +import { dispatchCommand } from '../../../core/dispatch.ts'; + +const mockDispatch = vi.mocked(dispatchCommand); + +function screenSnapshot(parentLabel: string) { + return { + backend: 'android', + nodes: [ + { + index: 0, + depth: 0, + type: 'FrameLayout', + label: parentLabel, + rect: { x: 0, y: 0, width: 390, height: 844 }, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'TextView', + label: 'Screen X', + rect: { x: 0, y: 100, width: 390, height: 40 }, + }, + ], + }; +} + +beforeEach(() => { + mockDispatch.mockReset(); + mockDispatch.mockImplementation(async (_device: unknown, command: string) => { + return command === 'snapshot' ? screenSnapshot('Detail Screen') : {}; + }); +}); + +function waitReq(overrides: Partial = {}): DaemonRequest { + return { + token: 't', + session: 'default', + command: 'wait', + positionals: ['label="Screen X"', '1000'], + flags: {}, + ...overrides, + } as DaemonRequest; +} + +async function runWait(options: { recordSession?: boolean; req?: DaemonRequest } = {}) { + const sessionStore = makeSessionStore(); + const session = makeAndroidSession('default'); + session.recordSession = options.recordSession ?? false; + sessionStore.set('default', session); + const response = await dispatchWaitViaRuntime({ + req: options.req ?? waitReq(), + sessionName: 'default', + logPath: '/tmp/test.log', + sessionStore, + }); + return { response, sessionStore }; +} + +test('a recorded selector wait attaches landmark-mode target-v1 evidence and strips the resolution payload', async () => { + const { response, sessionStore } = await runWait({ recordSession: true }); + + expect(response.ok).toBe(true); + if (response.ok) { + expect(response.data).not.toHaveProperty('node'); + expect(response.data).not.toHaveProperty('preActionNodes'); + } + + const recordedAction = sessionStore.get('default')?.actions[0]; + expect(recordedAction?.command).toBe('wait'); + expect(recordedAction?.targetEvidence).toMatchObject({ + role: 'textview', + label: 'Screen X', + ancestry: [{ role: 'framelayout', label: 'Detail Screen' }], + verification: 'verified', + }); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('a selector wait without recording never computes target-v1 evidence', async () => { + const { response, sessionStore } = await runWait({ recordSession: false }); + + expect(response.ok).toBe(true); + const recordedAction = sessionStore.get('default')?.actions[0]; + expect(recordedAction?.command).toBe('wait'); + expect(recordedAction?.targetEvidence).toBeUndefined(); +}); + +function recordedLandmark(): TargetAnnotationV1 { + return { + role: 'textview', + label: 'Screen X', + ancestry: [{ role: 'framelayout', label: 'Detail Screen' }], + sibling: 0, + viewportOrder: 0, + verification: 'verified', + }; +} + +test('a replayed wait with a landmark guard succeeds when a match carries the recorded identity', async () => { + const { response } = await runWait({ + req: waitReq({ internal: { replayLandmarkGuard: recordedLandmark() } }), + }); + + expect(response.ok).toBe(true); +}); + +test('a replayed wait with a landmark guard refuses at the deadline when only impostors matched', async () => { + mockDispatch.mockImplementation(async (_device: unknown, command: string) => { + return command === 'snapshot' ? screenSnapshot('List Screen') : {}; + }); + + const { response } = await runWait({ + req: waitReq({ + positionals: ['label="Screen X"', '600'], + internal: { replayLandmarkGuard: recordedLandmark() }, + }), + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.details?.reason).toBe(WAIT_LANDMARK_MISMATCH_REASON); + expect(response.error.details?.matchCount).toBe(1); +}); diff --git a/src/daemon/handlers/session-replay-repair-hint.ts b/src/daemon/handlers/session-replay-repair-hint.ts index 90739e5b8..c7876a53e 100644 --- a/src/daemon/handlers/session-replay-repair-hint.ts +++ b/src/daemon/handlers/session-replay-repair-hint.ts @@ -28,12 +28,8 @@ import { type TargetAnnotationV1, type TargetScrollRegion, } from '../../replay/target-identity.ts'; -import { - buildAncestryChain, - buildIndexMap, - computeScrollRegionKey, - scrollRegionKeysEqual, -} from '../session-target-evidence.ts'; +import { buildAncestryChain, buildIndexMap } from '../../replay/target-evidence-tree.ts'; +import { computeScrollRegionKey, scrollRegionKeysEqual } from '../session-target-evidence.ts'; export type ReplayRepairHintCapture = | { state: 'available'; nodes: SnapshotNode[] } diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index 62e5576da..c58846b8e 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -18,6 +18,7 @@ import { SessionStore } from '../session-store.ts'; import { expandSessionPath } from '../session-paths.ts'; import { applySaveScriptRetarget } from '../session-action-recorder.ts'; import { computeReplayPlanDigest } from '../../replay/plan-digest.ts'; +import type { TargetAnnotationV1 } from '../../replay/target-identity.ts'; import { errorResponse, noActiveSessionError } from './response.ts'; import { invokeReplayAction } from './session-replay-action-runtime.ts'; import { tryParseSelectorChain } from '../../selectors/index.ts'; @@ -46,8 +47,11 @@ import { } from './session-replay-runtime-plan.ts'; import { buildReplayTargetGuardMismatchResponse, + buildWaitLandmarkMismatchResponse, isReplayTargetGuardMismatchResponse, + isWaitLandmarkMismatchResponse, verifyReplayActionTarget, + type ReplayVerifiedTargetGuard, } from './session-replay-target-verification.ts'; import { buildReplayBuiltinVars } from './session-replay-vars.ts'; import { @@ -106,11 +110,14 @@ async function resolveReplayStepResponse( }); if (!verification.verified) return verification.response; const guard = verification.guard; - const guardedReq = guard - ? { - ...ctx.replayReq, - internal: { ...ctx.replayReq.internal, replayTargetGuard: guard.expected }, - } + const deferredLandmark = verification.deferredLandmark; + const guardInternal = guard + ? { replayTargetGuard: guard.expected } + : deferredLandmark + ? { replayLandmarkGuard: deferredLandmark } + : undefined; + const guardedReq = guardInternal + ? { ...ctx.replayReq, internal: { ...ctx.replayReq.internal, ...guardInternal } } : ctx.replayReq; const response = await invokeReplayAction({ req: guardedReq, @@ -124,11 +131,39 @@ async function resolveReplayStepResponse( tracePath: ctx.actionTracePath, invoke: ctx.invoke, }); - if (!guard || !isReplayTargetGuardMismatchResponse(response)) return response; - return await buildReplayTargetGuardMismatchResponse({ + return await convertIdentityRefusalResponse({ + ctx, action, - scope: ctx.scope, + index, + artifactPaths, + sourcePath, + sourceLine, + response, guard, + deferredLandmark, + }); +} + +/** + * Converts a dispatch-time identity refusal — the post-resolution guard + * mismatch, or wait's landmark timeout — into its identity-mismatch + * divergence; every other response passes through unchanged. + */ +async function convertIdentityRefusalResponse(params: { + ctx: ReplayStepContext; + action: SessionAction; + index: number; + artifactPaths: string[]; + sourcePath: string; + sourceLine: number; + response: DaemonResponse; + guard: ReplayVerifiedTargetGuard | undefined; + deferredLandmark: TargetAnnotationV1 | undefined; +}): Promise { + const { ctx, action, index, artifactPaths, sourcePath, sourceLine, response } = params; + const mismatchParams = { + action, + scope: ctx.scope, failedResponse: response, sourcePath, sourceLine, @@ -141,7 +176,14 @@ async function resolveReplayStepResponse( responseLevel: ctx.responseLevel, planActions: ctx.actions, planDigest: ctx.planDigest, - }); + }; + if (params.guard && isReplayTargetGuardMismatchResponse(response)) { + return await buildReplayTargetGuardMismatchResponse({ ...mismatchParams, guard: params.guard }); + } + if (params.deferredLandmark && isWaitLandmarkMismatchResponse(response)) { + return await buildWaitLandmarkMismatchResponse(mismatchParams); + } + return response; } export async function runReplayScriptFile(params: { diff --git a/src/daemon/handlers/session-replay-target-classification.ts b/src/daemon/handlers/session-replay-target-classification.ts index e4f80d916..3e8404feb 100644 --- a/src/daemon/handlers/session-replay-target-classification.ts +++ b/src/daemon/handlers/session-replay-target-classification.ts @@ -41,16 +41,19 @@ import { tryParseSelectorChain, } from '../../selectors/index.ts'; import { + buildAncestryChain, buildIndexMap, + filterIdentitySet, +} from '../../replay/target-evidence-tree.ts'; +import { boundedLocalIdentity, - buildAncestryChain, computeSiblingOrdinal, computeScrollRegionKey, scrollRegionKeysEqual, orderByViewportPosition, - filterIdentitySet, } from '../session-target-evidence.ts'; import { + annotationLocalIdentity, classifyTargetBindingMatch, type LocalIdentity, type TargetAnnotationV1, @@ -108,7 +111,7 @@ export function classifyReplayTarget(params: { }); const byIndex = buildIndexMap(nodes); - const identity = identityAsLocalIdentity(recorded); + const identity = annotationLocalIdentity(recorded); const identitySet = filterIdentitySet( matching.matchedNodes, byIndex, @@ -238,14 +241,6 @@ function resolveSelectorTargetMatches( return { matchedNodes, winnerRef: resolved.node.ref }; } -function identityAsLocalIdentity(recorded: TargetAnnotationV1): LocalIdentity { - return { - ...(recorded.id !== undefined ? { id: recorded.id } : {}), - role: recorded.role, - ...(recorded.label !== undefined ? { label: recorded.label } : {}), - }; -} - type MappedVerificationFailure = Omit; /** Decision 3 paths 2/3/5/6 (excluding verified), mapped onto a wire divergence kind. */ @@ -371,7 +366,7 @@ function ancestryEntryMismatches( } /** Leaf-anchored prefix: the first divergence explains everything after it. */ -function firstAncestryMismatch( +export function firstAncestryMismatch( recordedAncestry: readonly { role: string; label?: string }[], observedAncestry: readonly { role: string; label?: string }[], ): string[] { diff --git a/src/daemon/handlers/session-replay-target-token.ts b/src/daemon/handlers/session-replay-target-token.ts index e71b07e3a..36aef55b3 100644 --- a/src/daemon/handlers/session-replay-target-token.ts +++ b/src/daemon/handlers/session-replay-target-token.ts @@ -1,10 +1,15 @@ import { isTouchTargetCommand } from '../../replay/script-utils.ts'; +import { splitIsSelectorArgs } from '../../selectors/index.ts'; import type { SessionAction } from '../types.ts'; /** Returns the resolved-target token carried by an eligible replay action. */ export function extractReplayTargetToken(action: SessionAction): string | undefined { const positionals = action.positionals ?? []; if (action.command === 'get') return positionals[1]; + // #1349: `is [expected]` — the selector expression is + // the target token (an `is exists` step is never annotated, so this only + // runs for unique-resolving predicates). + if (action.command === 'is') return splitIsSelectorArgs(positionals).split?.selectorExpression; if (!isTouchTargetCommand(action.command) && action.command !== 'fill') return undefined; const first = positionals[0]; if (first === undefined) return undefined; diff --git a/src/daemon/handlers/session-replay-target-verification.ts b/src/daemon/handlers/session-replay-target-verification.ts index b11bf64ac..82fc36163 100644 --- a/src/daemon/handlers/session-replay-target-verification.ts +++ b/src/daemon/handlers/session-replay-target-verification.ts @@ -7,7 +7,11 @@ import { resolveReplayAction, type ReplayVarScope, } from '../../replay/vars.ts'; -import type { LocalIdentity, TargetAnnotationV1 } from '../../replay/target-identity.ts'; +import { + annotationLocalIdentity, + type LocalIdentity, + type TargetAnnotationV1, +} from '../../replay/target-identity.ts'; import { createReplayDivergenceSanitizer, type ReplayDivergence, @@ -18,8 +22,11 @@ import { import { readNodeStructuralDenotation, REPLAY_TARGET_GUARD_MISMATCH_REASON, + WAIT_LANDMARK_MISMATCH_REASON, type ReplayTargetGuardDenotation, } from '../../replay/target-identity-node.ts'; +import { resolveTargetIdentityVerification } from '../../core/command-descriptor/registry.ts'; +import { parseWaitPositionals } from '../../core/wait-positionals.ts'; import type { DaemonResponse, SessionAction } from '../types.ts'; import { SessionStore } from '../session-store.ts'; import { boundedLocalIdentity } from '../session-target-evidence.ts'; @@ -39,6 +46,7 @@ import { buildReplayDivergenceFailureResponse } from './session-replay-runtime-f import { buildAndPersistReplayDivergenceResume } from './session-replay-resume.ts'; import { classifyReplayTarget, + firstAncestryMismatch, identityFieldMismatches, } from './session-replay-target-classification.ts'; import { extractReplayTargetToken, readRefLabel } from './session-replay-target-token.ts'; @@ -62,7 +70,19 @@ export type ReplayVerifiedTargetGuard = { }; export type ReplayTargetVerificationOutcome = - | { verified: true; guard?: ReplayVerifiedTargetGuard } + | { + verified: true; + guard?: ReplayVerifiedTargetGuard; + /** + * #1349 post-resolution phase (`wait`): the recorded landmark to thread + * into the command's own resolution (`internal.replayLandmarkGuard`). + * `verified: true` here means only "nothing to refuse pre-dispatch" — + * the identity check runs inside the wait's polling loop, and the step + * loop converts its timeout refusal into an identity-mismatch + * divergence (`buildWaitLandmarkMismatchResponse`). + */ + deferredLandmark?: TargetAnnotationV1; + } | { verified: false; response: DaemonResponse }; type TargetBindingDivergenceContext = { @@ -120,7 +140,7 @@ function buildTargetBindingDivergenceResponse( const targetBinding = { classification: built.kind, ...(built.matchCount !== undefined ? { matchCount: built.matchCount } : {}), - recorded: sanitizeIdentity(identityFromAnnotation(recorded), sanitize), + recorded: sanitizeIdentity(annotationLocalIdentity(recorded), sanitize), ...(built.observed ? { observed: sanitizeIdentity(built.observed, sanitize) } : {}), mismatches: built.mismatches.slice(0, 5).map((entry) => sanitize(entry)), candidates: built.candidateNodes.slice(0, 5).map((node) => describeCandidate(node, sanitize)), @@ -226,14 +246,6 @@ export async function verifyReplayActionTarget(params: { // every other replay divergence, so an expanded `${VAR}` never leaks // through an un-scrubbed positional). const resolvedAction = resolveReplayAction(action, scope, { file: sourcePath, line: sourceLine }); - const token = extractReplayTargetToken(resolvedAction); - if (token === undefined) return { verified: true }; - if (!token.startsWith('@') && !tryParseSelectorChain(token)) { - // A malformed recorded selector is not this module's concern — the real - // dispatch will parse (and fail) it the same way an unannotated action - // would. - return { verified: true }; - } const scrubVars = collectReplayScrubbableVarValues(scope); const sanitize = createReplayDivergenceSanitizer(scrubVars); @@ -252,10 +264,9 @@ export async function verifyReplayActionTarget(params: { planActions, planDigest, }; - - // Decision 3 path 1: a recorded-`unverifiable` annotation fires before any - // resolution — matchCount is omitted (never computed). - if (recorded.verification === 'unverifiable') { + const buildRecordedUnverifiableResponse = async (): Promise => { + // Decision 3 path 1: a recorded-`unverifiable` annotation fires before + // any resolution — matchCount is omitted (never computed). const observation = await captureDivergenceObservation({ session, sessionName, @@ -263,21 +274,47 @@ export async function verifyReplayActionTarget(params: { logPath, action, }); - return { - verified: false, - response: buildTargetBindingDivergenceResponse(context, { - kind: 'identity-unverifiable', - matchCount: undefined, - observed: undefined, - candidateNodes: [], - mismatches: [], - causeCode: 'IDENTITY_UNVERIFIABLE', - causeMessage: - 'The recorded target evidence could not verify itself when it was captured (a structural capture anomaly), so replay cannot trust it before acting.', - screen: buildDivergenceScreen(observation, sanitize), - repairCapture: toReplayRepairHintCapture(observation), - }), - }; + return buildTargetBindingDivergenceResponse(context, { + kind: 'identity-unverifiable', + matchCount: undefined, + observed: undefined, + candidateNodes: [], + mismatches: [], + causeCode: 'IDENTITY_UNVERIFIABLE', + causeMessage: + 'The recorded target evidence could not verify itself when it was captured (a structural capture anomaly), so replay cannot trust it before acting.', + screen: buildDivergenceScreen(observation, sanitize), + repairCapture: toReplayRepairHintCapture(observation), + }); + }; + + // #1349 post-resolution phase (`wait`): NEVER the generic pre-dispatch + // resolution below — an absent landmark is a wait's expected starting + // condition, so refusing on the current screen would break polling. Only + // path 1 (recorded-`unverifiable`, no resolution involved) refuses up + // front; a verifiable landmark is deferred into the wait's own loop. + if (resolveTargetIdentityVerification(action.command) === 'post-resolution') { + const parsed = parseWaitPositionals(resolvedAction.positionals ?? []); + // Only a selector wait names a landmark; an annotation on any other wait + // form is inert, like an old reader. + if (parsed?.kind !== 'selector') return { verified: true }; + if (recorded.verification === 'unverifiable') { + return { verified: false, response: await buildRecordedUnverifiableResponse() }; + } + return { verified: true, deferredLandmark: recorded }; + } + + const token = extractReplayTargetToken(resolvedAction); + if (token === undefined) return { verified: true }; + if (!token.startsWith('@') && !tryParseSelectorChain(token)) { + // A malformed recorded selector is not this module's concern — the real + // dispatch will parse (and fail) it the same way an unannotated action + // would. + return { verified: true }; + } + + if (recorded.verification === 'unverifiable') { + return { verified: false, response: await buildRecordedUnverifiableResponse() }; } const observation = await captureDivergenceObservation({ @@ -365,10 +402,9 @@ export function isReplayTargetGuardMismatchResponse(response: DaemonResponse): b return !response.ok && response.error.details?.reason === REPLAY_TARGET_GUARD_MISMATCH_REASON; } -export async function buildReplayTargetGuardMismatchResponse(params: { +type PostDispatchMismatchParams = { action: SessionAction; scope: ReplayVarScope; - guard: ReplayVerifiedTargetGuard; failedResponse: DaemonResponse; sourcePath: string; sourceLine: number; @@ -381,40 +417,38 @@ export async function buildReplayTargetGuardMismatchResponse(params: { responseLevel: ResponseLevel | undefined; planActions: SessionAction[]; planDigest: string; -}): Promise { - const { - action, - scope, - guard, - failedResponse, - sourcePath, - sourceLine, - replayPath, - step, - sessionName, - sessionStore, - logPath, - artifactPaths, - responseLevel, - planActions, - planDigest, - } = params; - // The guard is only ever attached to an annotated action; fall back to the - // original failure if the invariant is somehow violated. +}; + +type PostDispatchMismatchEvidence = { + matchCount: number | undefined; + observed: LocalIdentity | undefined; + mismatches: string[]; + causeMessage: string; +}; + +/** + * The shared post-dispatch identity-mismatch shaping: both refusal markers — + * the guard mismatch and wait's landmark refusal — arrive as a failed dispatch + * response whose details carry the observed evidence, and both become the same + * bounded identity-mismatch divergence around their marker-specific evidence. + */ +async function buildPostDispatchIdentityMismatchResponse( + params: PostDispatchMismatchParams, + deriveEvidence: ( + recorded: TargetAnnotationV1, + details: Record | undefined, + ) => PostDispatchMismatchEvidence, +): Promise { + const { action, scope, failedResponse, sessionName, sessionStore, logPath } = params; + // The refusal markers are only ever attached to an annotated action; fall + // back to the original failure if the invariant is somehow violated. const recorded = action.targetEvidence; if (!recorded) return failedResponse; const scrubVars = collectReplayScrubbableVarValues(scope); const sanitize = createReplayDivergenceSanitizer(scrubVars); const details = failedResponse.ok ? undefined : failedResponse.error.details; - const observed = readGuardMismatchObservedIdentity(details?.observed); - // The guard fires even when local identity is identical (a same-identity - // duplicate resolved by structural position) — surface the structural - // difference so `mismatches` is never empty on a real divergence. - const structuralMismatch = describeStructuralMismatch( - details?.expectedStructural, - details?.observedStructural, - ); + const evidence = deriveEvidence(recorded, details); const session = sessionStore.get(sessionName); const observation = session @@ -429,34 +463,106 @@ export async function buildReplayTargetGuardMismatchResponse(params: { { recorded, action, - step, - sourcePath, - sourceLine, - replayPath, - artifactPaths, + step: params.step, + sourcePath: params.sourcePath, + sourceLine: params.sourceLine, + replayPath: params.replayPath, + artifactPaths: params.artifactPaths, sessionName, sessionStore, - responseLevel, + responseLevel: params.responseLevel, scrubVars, - planActions, - planDigest, + planActions: params.planActions, + planDigest: params.planDigest, }, { kind: 'identity-mismatch', - matchCount: guard.matchCount, - observed, + matchCount: evidence.matchCount, + observed: evidence.observed, candidateNodes: [], + mismatches: evidence.mismatches, + causeCode: 'IDENTITY_MISMATCH', + causeMessage: evidence.causeMessage, + screen: buildDivergenceScreen(observation, sanitize), + repairCapture: toReplayRepairHintCapture(observation), + }, + ); +} + +export async function buildReplayTargetGuardMismatchResponse( + params: PostDispatchMismatchParams & { guard: ReplayVerifiedTargetGuard }, +): Promise { + return await buildPostDispatchIdentityMismatchResponse(params, (recorded, details) => { + const observed = readGuardMismatchObservedIdentity(details?.observed); + // The guard fires even when local identity is identical (a same-identity + // duplicate resolved by structural position) — surface the structural + // difference so `mismatches` is never empty on a real divergence. + const structuralMismatch = describeStructuralMismatch( + details?.expectedStructural, + details?.observedStructural, + ); + return { + matchCount: params.guard.matchCount, + observed, mismatches: [ ...(observed ? identityFieldMismatches(recorded, observed) : []), ...(structuralMismatch ? [structuralMismatch] : []), ], - causeCode: 'IDENTITY_MISMATCH', causeMessage: 'Dispatch resolution (with occlusion/visibility guards) resolved a different element than pre-action verification isolated; the action was not sent.', - screen: buildDivergenceScreen(observation, sanitize), - repairCapture: toReplayRepairHintCapture(observation), - }, - ); + }; + }); +} + +// --------------------------------------------------------------------------- +// #1349 deferred (post-resolution) landmark verification for `wait`: the +// polling loop refuses at its deadline when selector candidates appeared but +// none carried the recorded landmark identity; the replay loop converts that +// refusal into an identity-mismatch target-binding divergence here. A plain +// wait timeout (the selector never matched at all) is NOT this marker — it +// stays an ordinary action-failure divergence, because "the landmark never +// appeared" needs a state repair, not an identity repair. +// --------------------------------------------------------------------------- + +export function isWaitLandmarkMismatchResponse(response: DaemonResponse): boolean { + return !response.ok && response.error.details?.reason === WAIT_LANDMARK_MISMATCH_REASON; +} + +export async function buildWaitLandmarkMismatchResponse( + params: PostDispatchMismatchParams, +): Promise { + return await buildPostDispatchIdentityMismatchResponse(params, (recorded, details) => { + const observed = readGuardMismatchObservedIdentity(details?.observed); + const observedAncestry = readAncestryEntries(details?.observedAncestry); + return { + matchCount: typeof details?.matchCount === 'number' ? details.matchCount : undefined, + observed, + mismatches: observed + ? [ + ...identityFieldMismatches(recorded, observed), + ...firstAncestryMismatch(recorded.ancestry, observedAncestry), + ] + : [], + causeMessage: + 'Candidates matched the recorded wait selector during polling, but none carried the recorded landmark identity before the timeout; the wait did not report success.', + }; + }); +} + +/** The wait refusal's `observedAncestry` entries, defensively re-read off error details. */ +function readAncestryEntries(value: unknown): { role: string; label?: string }[] { + if (!Array.isArray(value)) return []; + const entries: { role: string; label?: string }[] = []; + for (const entry of value) { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return []; + const record = entry as Record; + if (typeof record.role !== 'string') return []; + entries.push({ + role: record.role, + ...(typeof record.label === 'string' ? { label: record.label } : {}), + }); + } + return entries; } function readGuardMismatchObservedIdentity(value: unknown): LocalIdentity | undefined { @@ -490,14 +596,6 @@ function readStructuralDenotation( return { documentOrder: record.documentOrder, sibling: record.sibling }; } -function identityFromAnnotation(recorded: TargetAnnotationV1): ReplayDivergenceTargetIdentity { - return { - ...(recorded.id !== undefined ? { id: recorded.id } : {}), - role: recorded.role, - ...(recorded.label !== undefined ? { label: recorded.label } : {}), - }; -} - function sanitizeIdentity( identity: ReplayDivergenceTargetIdentity, sanitize: (value: string, limit?: number) => string, diff --git a/src/daemon/selector-recording.ts b/src/daemon/selector-recording.ts index 628fecc0d..00c1143bb 100644 --- a/src/daemon/selector-recording.ts +++ b/src/daemon/selector-recording.ts @@ -3,7 +3,11 @@ import type { SnapshotNode } from '../kernel/snapshot.ts'; import { stripAndroidSystemChromeProvenanceFromNode } from '../contracts/android-system-chrome.ts'; import { SessionStore } from './session-store.ts'; import { isInteractiveObservation } from './session-action-recorder.ts'; -import { computeTargetEvidence, type RecordedTargetCapture } from './session-target-evidence.ts'; +import { + computeTargetEvidence, + type RecordedTargetCapture, + type TargetEvidenceMode, +} from './session-target-evidence.ts'; export function buildFindRecordResult( result: Record, @@ -80,6 +84,16 @@ export function toDaemonGetData(result: DaemonGetResult): Record>(result: T): T { + const { node: _node, preActionNodes: _preActionNodes, ...recordResult } = result; + return recordResult as T; +} + export function toDaemonWaitData(result: Record): Record { return { waitedMs: result.waitedMs, @@ -103,11 +117,15 @@ export function recordIfSession( result: Record, /** ADR 0012 decision 3: record-time input for the `target-v1` annotation. */ recordedTarget?: RecordedTargetCapture, + /** #1349: `landmark` for wait's existence-semantics evidence; defaults to `action`. */ + evidenceMode?: TargetEvidenceMode, ): void { const session = sessionStore.get(sessionName); if (!session) return; const targetEvidence = - session.recordSession && recordedTarget ? computeTargetEvidence(recordedTarget) : undefined; + session.recordSession && recordedTarget + ? computeTargetEvidence(recordedTarget, { mode: evidenceMode }) + : undefined; sessionStore.recordAction(session, { command: req.command, positionals: req.positionals ?? [], diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 2c00a5c11..7faa3f2cf 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -27,11 +27,14 @@ import { buildFindRecordResult, buildGetRecordResult, recordIfSession, + stripResolutionPayload, stripSelectorChain, toDaemonFindData, toDaemonGetData, toDaemonWaitData, } from './selector-recording.ts'; +import type { RecordedTargetCapture } from './session-target-evidence.ts'; +import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import { maybeWaitTimeoutSurfaceResponse } from './wait-current-surface.ts'; import { withSystemSurfaceDisclosure } from './handlers/system-surface-disclosure.ts'; import { @@ -220,13 +223,20 @@ export async function dispatchIsViaRuntime( if (predicate !== 'text' && split.rest.length > 0) { return errorResponse('INVALID_ARGS', `is ${predicate} does not accept trailing values`); } - const directResponse = await dispatchDirectIosSelectorIs( - params, - predicate as IsPredicate, - split.selectorExpression, - expectedText, - ); - if (directResponse) return directResponse; + // 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 replayTargetGuard = req.internal?.replayTargetGuard; + const recordingSession = params.sessionStore.get(params.sessionName)?.recordSession === true; + if (!replayTargetGuard && !recordingSession) { + const directResponse = await dispatchDirectIosSelectorIs( + params, + predicate as IsPredicate, + split.selectorExpression, + expectedText, + ); + if (directResponse) return directResponse; + } const resolvedRuntime = await createSelectorRuntime(params, { requireSession: true, @@ -241,9 +251,12 @@ export async function dispatchIsViaRuntime( predicate: predicate as IsPredicate, selector: split.selectorExpression, expectedText, + expectedResolvedTarget: replayTargetGuard, }); - recordIfSession(params.sessionStore, params.sessionName, req, result); - return stripSelectorChain(result); + const recordedTarget = readRecordedResolutionTarget(result); + const strippedResult = stripResolutionPayload(result); + recordIfSession(params.sessionStore, params.sessionName, req, strippedResult, recordedTarget); + return stripSelectorChain(strippedResult); }); return withSystemSurfaceDisclosure( await maybeAndroidForegroundBlockerResponse(params, response, `is ${predicate}`), @@ -262,7 +275,11 @@ export async function dispatchWaitViaRuntime( const unsupported = requireCommandSupported('wait', device); if (unsupported) return unsupported; } - if (parsed.kind === 'selector') { + // ADR 0012 / #1349: recording and a replayed landmark check both need the + // snapshot polling path — evidence and the identity comparison are computed + // from the resolution tree the direct runner query never captures. + const recordedLandmark = req.internal?.replayLandmarkGuard; + if (parsed.kind === 'selector' && !recordedLandmark && !session?.recordSession) { const directResponse = await dispatchDirectIosSelectorWait({ ...params, session, @@ -304,9 +321,17 @@ export async function dispatchWaitViaRuntime( const result = await runtime.selectors.wait({ session: sessionName, requestId: req.meta?.requestId, - target: toWaitTarget(waitParsed, session), + target: toWaitTarget(waitParsed, session, recordedLandmark), }); - recordIfSession(sessionStore, sessionName, req, result); + const recordedTarget = readRecordedResolutionTarget(result); + recordIfSession( + sessionStore, + sessionName, + req, + stripResolutionPayload(result), + recordedTarget, + 'landmark', + ); const data = toDaemonWaitData(result); return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data; }); @@ -327,6 +352,16 @@ export async function dispatchWaitViaRuntime( ); } +/** ADR 0012 decision 3 / #1349: a wait/is result's resolution payload, when the tree path produced one. */ +function readRecordedResolutionTarget( + result: Record, +): RecordedTargetCapture | undefined { + const node = result.node; + const preActionNodes = result.preActionNodes; + if (!node || typeof node !== 'object' || !Array.isArray(preActionNodes)) return undefined; + return { node: node as SnapshotNode, preActionNodes: preActionNodes as SnapshotNode[] }; +} + function readDirectIosGetSelector( session: SessionState | undefined, property: 'text' | 'attrs', @@ -569,13 +604,18 @@ function parseGetTarget(req: DaemonRequest): return { ok: true, target: { kind: 'selector', selector } }; } -function toWaitTarget(parsed: WaitParsed, session: SessionState | undefined) { +function toWaitTarget( + parsed: WaitParsed, + session: SessionState | undefined, + recordedLandmark?: TargetAnnotationV1, +) { if (parsed.kind === 'sleep') return { kind: 'sleep' as const, durationMs: parsed.durationMs }; if (parsed.kind === 'selector') { return { kind: 'selector' as const, selector: parsed.selectorExpression, timeoutMs: parsed.timeoutMs, + ...(recordedLandmark ? { recordedLandmark } : {}), }; } if (parsed.kind === 'ref') { diff --git a/src/daemon/session-script-active-publication.ts b/src/daemon/session-script-active-publication.ts index c241e68c1..8feff351f 100644 --- a/src/daemon/session-script-active-publication.ts +++ b/src/daemon/session-script-active-publication.ts @@ -48,7 +48,7 @@ export function validateActivePublicationActions(actions: SessionAction[]): void 'Cannot publish this session without a portable destination guard after the final mutating action.', { retriable: true, - hint: 'Record a selective selector-targeted wait, for example wait \'role="heading" label="Screen X"\', then retry session save-script.', + hint: 'Record a selector-targeted wait on a labeled or id-bearing landmark, for example wait \'role="heading" label="Screen X"\', so its recorded identity is captured, then retry session save-script.', }, ); } @@ -127,7 +127,15 @@ export function toActivePublicationFailure( function isPortableDestinationGuard(action: SessionAction): boolean { if (action.command !== 'wait') return false; const parsed = parseWaitPositionals(action.positionals); - return parsed?.kind === 'selector' && tryParseSelectorChain(parsed.selectorExpression) !== null; + if (parsed?.kind !== 'selector' || tryParseSelectorChain(parsed.selectorExpression) === null) { + return false; + } + // #1349: a guard proves recorded landmark identity, not bare selector + // existence — a reshuffled screen containing the same label elsewhere must + // fail replay closed instead of false-passing. A selector wait recorded + // without verified evidence (an identity-empty landmark, or a capture + // anomaly) therefore does not qualify. + return action.targetEvidence?.verification === 'verified'; } function readTargetBindingToken(action: SessionAction): string | undefined { diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index bce33793c..8cad1e6f4 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -22,9 +22,13 @@ import { readNodeLocalIdentity, siblingOrdinal, } from '../replay/target-identity-node.ts'; +import { + buildAncestryChain, + buildIndexMap, + filterIdentitySet, +} from '../replay/target-evidence-tree.ts'; import { classifyTargetBindingMatch, - matchesAncestryPrefix, matchesLocalIdentity, serializeTargetAnnotationV1, utf8ByteLength, @@ -43,13 +47,27 @@ export type RecordedTargetCapture = { preActionNodes: SnapshotNode[]; }; +/** + * #1349 (ADR 0012 decision 3 amendment): what replay verification will prove. + * `action` — isolation of the recorded winner (decision 3's original + * contract). `landmark` (wait) — existence of an identity-carrying selector + * match; an identity-empty winner (no id, no label after #1269 demotion) + * yields no annotation, keeping the wait's selector-existence semantics. + */ +export type TargetEvidenceMode = 'action' | 'landmark'; + export function computeTargetEvidence( capture: RecordedTargetCapture, + options: { mode?: TargetEvidenceMode } = {}, ): TargetAnnotationV1 | undefined { + const mode = options.mode ?? 'action'; const { node, preActionNodes: nodes } = capture; if (typeof node.index !== 'number') return undefined; const byIndex = buildIndexMap(nodes); const identity = demoteNonUniqueId(boundedLocalIdentity(node), nodes); + if (mode === 'landmark' && identity.id === undefined && identity.label === undefined) { + return undefined; + } const ancestryWalk = buildAncestryChain(node, byIndex, TARGET_ANNOTATION_MAX_ANCESTRY); const fullAncestry = ancestryWalk.chain; const sibling = computeSiblingOrdinal(nodes, node); @@ -85,82 +103,65 @@ export function computeTargetEvidence( for (let ancestryLength = fullAncestry.length; ancestryLength >= floor; ancestryLength -= 1) { const { candidate, domain } = buildCandidate(ancestryLength); - // Size against the longest verification value so the payload fits - // whichever one the self-check returns. - if ( - utf8ByteLength(serializeTargetAnnotationV1({ ...candidate, verification: 'unverifiable' })) <= - TARGET_ANNOTATION_MAX_PAYLOAD_BYTES - ) { - // A broken parent walk is a capture anomaly: fail closed instead of - // self-checking against structural signals that cannot be trusted. - candidate.verification = ancestryWalk.broken - ? 'unverifiable' - : runRecordTimeSelfCheck({ node, domain }); - return candidate; - } - if (ancestryLength === floor) { - // Decision 3's terminal fail-closed downgrade; rect is diagnostic-only - // and is dropped before ever emitting an over-cap payload. - candidate.verification = 'unverifiable'; - if ( - utf8ByteLength(serializeTargetAnnotationV1(candidate)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES - ) { - delete candidate.rect; - } + if (fitsPayloadCeiling(candidate)) { + candidate.verification = resolveRecordTimeVerification({ + node, + domain, + mode, + brokenWalk: ancestryWalk.broken, + }); return candidate; } + if (ancestryLength === floor) return finalizeOversizedCandidate(candidate); } return undefined; } +/** + * A broken parent walk is a capture anomaly: fail closed instead of + * self-checking against structural signals that cannot be trusted. Landmark + * evidence needs no isolation self-check — the winner is a member of its own + * identity set by construction, so the broken walk is its only record-time + * failure mode. Action evidence runs decision 3's step-5 self-check. + */ +function resolveRecordTimeVerification(params: { + node: SnapshotNode; + domain: DisambiguationDomain; + mode: TargetEvidenceMode; + brokenWalk: boolean; +}): TargetVerification { + if (params.brokenWalk) return 'unverifiable'; + if (params.mode === 'landmark') return 'verified'; + return runRecordTimeSelfCheck({ node: params.node, domain: params.domain }); +} + +/** Size against the longest verification value so the payload fits whichever one the self-check returns. */ +function fitsPayloadCeiling(candidate: TargetAnnotationV1): boolean { + return ( + utf8ByteLength(serializeTargetAnnotationV1({ ...candidate, verification: 'unverifiable' })) <= + TARGET_ANNOTATION_MAX_PAYLOAD_BYTES + ); +} + +/** Decision 3's terminal fail-closed downgrade; rect is diagnostic-only and dropped before emitting an over-cap payload. */ +function finalizeOversizedCandidate(candidate: TargetAnnotationV1): TargetAnnotationV1 { + candidate.verification = 'unverifiable'; + if ( + utf8ByteLength(serializeTargetAnnotationV1(candidate)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES + ) { + delete candidate.rect; + } + return candidate; +} + // --------------------------------------------------------------------------- // Identity / ancestry / sibling / scroll region — decision 3's structural // primitives, computed once per candidate node against the record-time tree. // --------------------------------------------------------------------------- -export function buildIndexMap(nodes: readonly SnapshotNode[]): Map { - const map = new Map(); - for (const node of nodes) map.set(node.index, node); - return map; -} - /** The one identity reader (normalized AND field-capped, on every path): shared with dispatch's post-resolution guard. */ export const boundedLocalIdentity = readNodeLocalIdentity; -export type AncestryWalk = { - chain: TargetAncestryEntry[]; - /** - * Decision 3 capture anomaly: a `parentIndex` that resolves to no node, or - * a parent cycle. A broken walk fails the annotation closed to - * `unverifiable` — the structural signals cannot be trusted. - */ - broken: boolean; -}; - -/** Decision 3 "Ancestry": nearest K ancestors, leaf→root, {role,label?}. */ -export function buildAncestryChain( - node: SnapshotNode, - byIndex: Map, - limit: number, -): AncestryWalk { - const chain: TargetAncestryEntry[] = []; - const visited = new Set([node.index]); - let current = node; - while (chain.length < limit) { - if (typeof current.parentIndex !== 'number') return { chain, broken: false }; - const parent = byIndex.get(current.parentIndex); - if (!parent || visited.has(parent.index)) return { chain, broken: true }; - visited.add(parent.index); - const identity = boundedLocalIdentity(parent); - chain.push({ - role: identity.role, - ...(identity.label !== undefined ? { label: identity.label } : {}), - }); - current = parent; - } - return { chain, broken: false }; -} - /** * Decision 3 record-time write step 3: the winner's zero-based index among * its OWN parent's children, in document order. Root-level nodes (no parent) @@ -219,28 +220,6 @@ type DisambiguationDomain = { viewportOrder: number; }; -/** - * Decision 3 record-time write step 2 / replay-time verification's identity - * set I: candidates sharing the recorded local identity with a matching - * leaf-anchored ancestry prefix. Shared by the writer's self-check (over the - * full record-time tree) and replay-time verification (over the recorded - * selector/ref's matched-node domain) — "record and replay compute this - * ordinal identically by definition" applies to this filter too. - */ -export function filterIdentitySet( - candidates: readonly SnapshotNode[], - byIndex: Map, - identity: LocalIdentity, - ancestry: readonly TargetAncestryEntry[], -): SnapshotNode[] { - return candidates.filter((candidate) => { - if (!matchesLocalIdentity(boundedLocalIdentity(candidate), identity)) return false; - const observed = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); - // A candidate with a broken parent walk cannot prove the prefix. - return !observed.broken && matchesAncestryPrefix(observed.chain, ancestry); - }); -} - /** * ADR 0012 decision 3 amendment (#1269): an id is identity only when it * uniquely denotes the target in the record-time tree. `boundedLocalIdentity` diff --git a/src/daemon/types.ts b/src/daemon/types.ts index b1482b9bd..c59058996 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -75,6 +75,18 @@ type DaemonRequestInternal = { * different same-identity duplicate. */ replayTargetGuard?: ReplayTargetGuardDenotation; + /** + * ADR 0012 / #1349 deferred (post-resolution) identity verification: the + * recorded `target-v1` landmark of an annotated selector `wait`, set ONLY + * by the replay step loop. The wait dispatch threads it into the polling + * loop as `recordedLandmark`; success then requires a selector match + * carrying this identity, and a timeout with rejected candidates surfaces + * the `WAIT_LANDMARK_MISMATCH_REASON` refusal the step loop converts into + * an identity-mismatch divergence. Never used by the generic pre-dispatch + * verification path — a wait's landmark may legitimately be absent when + * the step starts. + */ + replayLandmarkGuard?: TargetAnnotationV1; /** * ADR 0014: set when a mutating `find` re-enters the interaction leaf with the * ref it just resolved by locator against a fresh capture. That ref is find's diff --git a/src/platforms/android/snapshot-content-recovery.ts b/src/platforms/android/snapshot-content-recovery.ts index 8dd59dbc9..6998405e9 100644 --- a/src/platforms/android/snapshot-content-recovery.ts +++ b/src/platforms/android/snapshot-content-recovery.ts @@ -1,6 +1,7 @@ import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; import { isAndroidInputMethodOwnedNode } from '../../contracts/android-input-ownership.ts'; import { hasAndroidSystemChromeProvenance } from '../../contracts/android-system-chrome.ts'; +import type { AndroidContentRecoveryReason } from '../../snapshot/snapshot-quality.ts'; import { classifyAndroidAlertIdentifier } from './alert-detection.ts'; import { androidUiNodes, type AndroidUiNodeMetadata } from './ui-hierarchy.ts'; @@ -14,7 +15,8 @@ const INSUFFICIENT_APP_CONTENT_REASON = 'Android snapshot helper returned insufficient application window content'; export type AndroidHelperContentRecoveryDecision = { - reason: 'empty-helper-output' | 'system-window-only' | 'content-poor-app-window'; + /** ADR 0012 #1349: waits classify these as ride-out-able content verdicts (`isUnreadableCaptureContentError`). */ + reason: AndroidContentRecoveryReason; failureReason: string; diagnostics: { helperNodeCount: number; diff --git a/src/replay/target-evidence-tree.ts b/src/replay/target-evidence-tree.ts new file mode 100644 index 000000000..0ceb15c12 --- /dev/null +++ b/src/replay/target-evidence-tree.ts @@ -0,0 +1,82 @@ +/** + * ADR 0012 decision 3: the tree-structural evidence primitives shared by the + * record-time writer (`src/daemon/session-target-evidence.ts`), replay-time + * verification (`src/daemon/handlers/session-replay-target-classification.ts`), + * and the command-resolution landmark check `wait` runs inside its polling + * loop (#1349, `src/commands/interaction/runtime/selector-read.ts`). They are + * pure functions over `SnapshotNode` trees; keeping them in the shared + * `replay/` zone lets the commands runtime consume them without importing the + * daemon. + */ + +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { readNodeLocalIdentity } from './target-identity-node.ts'; +import { + matchesAncestryPrefix, + matchesLocalIdentity, + type LocalIdentity, + type TargetAncestryEntry, +} from './target-identity.ts'; + +export function buildIndexMap(nodes: readonly SnapshotNode[]): Map { + const map = new Map(); + for (const node of nodes) map.set(node.index, node); + return map; +} + +export type AncestryWalk = { + chain: TargetAncestryEntry[]; + /** + * Decision 3 capture anomaly: a `parentIndex` that resolves to no node, or + * a parent cycle. A broken walk fails the annotation closed to + * `unverifiable` — the structural signals cannot be trusted. + */ + broken: boolean; +}; + +/** Decision 3 "Ancestry": nearest K ancestors, leaf→root, {role,label?}. */ +export function buildAncestryChain( + node: SnapshotNode, + byIndex: Map, + limit: number, +): AncestryWalk { + const chain: TargetAncestryEntry[] = []; + const visited = new Set([node.index]); + let current = node; + while (chain.length < limit) { + if (typeof current.parentIndex !== 'number') return { chain, broken: false }; + const parent = byIndex.get(current.parentIndex); + if (!parent || visited.has(parent.index)) return { chain, broken: true }; + visited.add(parent.index); + const identity = readNodeLocalIdentity(parent); + chain.push({ + role: identity.role, + ...(identity.label !== undefined ? { label: identity.label } : {}), + }); + current = parent; + } + return { chain, broken: false }; +} + +/** + * Decision 3 record-time write step 2 / replay-time verification's identity + * set I: candidates sharing the recorded local identity with a matching + * leaf-anchored ancestry prefix. Shared by the writer's self-check (over the + * full record-time tree), replay-time verification (over the recorded + * selector/ref's matched-node domain), and wait's in-loop landmark check — + * "record and replay compute this ordinal identically by definition" applies + * to this filter too. + */ +export function filterIdentitySet( + candidates: readonly SnapshotNode[], + byIndex: Map, + identity: LocalIdentity, + ancestry: readonly TargetAncestryEntry[], +): SnapshotNode[] { + return candidates.filter((candidate) => { + if (!matchesLocalIdentity(readNodeLocalIdentity(candidate), identity)) return false; + const observed = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); + // A candidate with a broken parent walk cannot prove the prefix. + return !observed.broken && matchesAncestryPrefix(observed.chain, ancestry); + }); +} diff --git a/src/replay/target-identity-node.ts b/src/replay/target-identity-node.ts index ba09217f2..1f2881d30 100644 --- a/src/replay/target-identity-node.ts +++ b/src/replay/target-identity-node.ts @@ -161,3 +161,26 @@ export type ReplayTargetGuardDenotation = { * layers can share it without a layering back-edge. */ export const REPLAY_TARGET_GUARD_MISMATCH_REASON = 'replay_target_guard_mismatch'; + +/** + * #1349: `details.reason` marker on the timeout refusal `wait` throws when + * candidates matching the recorded selector appeared during polling but none + * ever carried the recorded landmark identity (local identity + leaf-anchored + * ancestry prefix). The replay step loop converts it into an + * identity-mismatch target-binding divergence; the wait never reports + * success. Lives here (replay zone) for the same layering reason as the + * guard-mismatch marker above. + */ +export const WAIT_LANDMARK_MISMATCH_REASON = 'wait_landmark_identity_mismatch'; + +/** + * The compact evidence `wait` retains from its LAST poll whose capture + * matched the recorded selector: the domain size and the first match's + * identity/ancestry, enough for the daemon to build the divergence's + * `targetBinding` payload without shipping node trees through error details. + */ +export type WaitLandmarkMismatchEvidence = { + matchCount: number; + observed: LocalIdentity; + observedAncestry: { role: string; label?: string }[]; +}; diff --git a/src/replay/target-identity.ts b/src/replay/target-identity.ts index 1740d7bec..1fae5c40a 100644 --- a/src/replay/target-identity.ts +++ b/src/replay/target-identity.ts @@ -339,6 +339,17 @@ function parseVerificationField(value: unknown): TargetVerification { export type LocalIdentity = { id?: string; role: string; label?: string }; +/** The recorded annotation's identity tier as a bare `LocalIdentity` (drop-empty-keys form). */ +export function annotationLocalIdentity( + recorded: Pick, +): LocalIdentity { + return { + ...(recorded.id !== undefined ? { id: recorded.id } : {}), + role: recorded.role, + ...(recorded.label !== undefined ? { label: recorded.label } : {}), + }; +} + /** * Decision 3 "Local identity": id match wins outright when the recording * carries one ("a recorded id never matches a node without that id"); with diff --git a/src/snapshot/snapshot-quality.ts b/src/snapshot/snapshot-quality.ts index ead6591b4..aba46c7a8 100644 --- a/src/snapshot/snapshot-quality.ts +++ b/src/snapshot/snapshot-quality.ts @@ -116,3 +116,47 @@ function collapsedLeafWarnings( } return warnings; } + +/** + * The Android helper's content-recovery verdicts: the capture mechanism + * worked but judged the current screen's CONTENT unreadable. The single + * enumeration behind both `AndroidHelperContentRecoveryDecision['reason']` + * (`src/platforms/android/snapshot-content-recovery.ts` derives its union + * from this list) and `isUnreadableCaptureContentError` below, so a new + * content verdict cannot be added to one without the other. + */ +const ANDROID_CONTENT_RECOVERY_REASONS = [ + 'empty-helper-output', + 'system-window-only', + 'content-poor-app-window', +] as const; + +export type AndroidContentRecoveryReason = (typeof ANDROID_CONTENT_RECOVERY_REASONS)[number]; + +const ANDROID_CONTENT_RECOVERY_REASON_SET: ReadonlySet = new Set( + ANDROID_CONTENT_RECOVERY_REASONS, +); + +/** + * True when a thrown capture failure is a CONTENT verdict — the capture + * mechanism worked but judged the current screen unreadable (the Android + * helper's content-recovery refusals) — rather than a mechanism failure. A + * polling wait rides these out exactly like a sparse-verdict capture that + * returned no match (iOS already yields a verdict instead of throwing): a + * mid-transition screen is the state a wait exists to wait through. One-shot + * reads keep failing loudly, and a wait whose screen never becomes readable + * rethrows the capture failure at its deadline instead of masking it as a + * generic timeout. + * + * Matches ONLY the enumerated content-recovery reasons: Android stamps + * `androidSnapshotHelperFailureReason` on mechanism failures too (helper + * timeouts, adb failures, a missing helper artifact — with free-form reason + * strings), and those must keep failing a wait immediately rather than being + * polled until its deadline. + */ +export function isUnreadableCaptureContentError(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + const details = (error as { details?: Record }).details; + const reason = details?.androidSnapshotHelperFailureReason; + return typeof reason === 'string' && ANDROID_CONTENT_RECOVERY_REASON_SET.has(reason); +} diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index ee5a68c7e..6b30c3c8b 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -4,7 +4,7 @@ import os from 'node:os'; import path from 'node:path'; import { test } from 'vitest'; import { assertRpcError, assertRpcOk } from './assertions.ts'; -import { createAndroidSettingsWorld } from './android-world.ts'; +import { androidSettingsXml, createAndroidSettingsWorld } from './android-world.ts'; import { withProviderScenarioResource } from './harness.ts'; test('provider route publishes and replays an open-to-destination script with a live handoff', async () => { @@ -44,6 +44,68 @@ test('provider route publishes and replays an open-to-destination script with a }); }, 20_000); +// #1349 / ADR 0016 reshuffled-screen false-pass regression: the destination +// guard must prove recorded landmark IDENTITY, not selector existence. The +// replay lands on a reshuffled screen that still contains a node labeled +// "Search" — under a different id and ancestry — so the selector alone would +// have passed the old guard. With landmark verification the wait must fail +// closed through the bounded REPLAY_DIVERGENCE contract instead of +// reporting the destination ready. +test('replaying a published script against a reshuffled screen fails closed instead of false-passing the guard', async () => { + let phase: 'record' | 'replay' = 'record'; + const reshuffledXml = () => + [ + '', + '', + ' ', + ' ', + ' ', + '', + ].join('\n'); + await withProviderScenarioResource( + () => + createAndroidSettingsWorld({ + snapshotXml: () => (phase === 'record' ? androidSettingsXml('') : reshuffledXml()), + }), + async (world) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-reshuffle-guard-provider-')); + const scriptPath = path.join(root, 'settings-search.ad'); + const client = world.daemon.client(); + try { + await client.apps.open({ + app: 'settings', + saveScript: scriptPath, + ...world.selection, + }); + await client.command.wait({ + selector: 'label="Search"', + timeoutMs: 1500, + ...world.selection, + }); + const published = await client.sessions.saveScript({ path: scriptPath }); + assert.equal(published.savedScript, scriptPath); + // The published guard carries recorded landmark identity. + assert.match(fs.readFileSync(scriptPath, 'utf8'), /agent-device:target-v1/); + await client.sessions.close(); + + phase = 'replay'; + const replay = await world.daemon.callCommand('replay', [scriptPath], world.selection); + const errorData = assertRpcError(replay, 'REPLAY_DIVERGENCE', /wait/i); + const divergence = (errorData.details as Record | undefined)?.divergence as + | { kind?: string; targetBinding?: { classification?: string; matchCount?: number } } + | undefined; + assert.equal(divergence?.kind, 'identity-mismatch'); + // matchCount >= 1 is the false-pass proof: the selector alone DID + // match the reshuffled screen; only identity refused it. + assert.ok((divergence?.targetBinding?.matchCount ?? 0) >= 1); + await world.daemon.callCommand('close'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, + ); +}, 30_000); + test('a second successful open aborts publication and terminal save flags fail before close', async () => { await withProviderScenarioResource(createAndroidSettingsWorld, async (world) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-aborted-script-provider-'));