From b08498fd56a718dd12a6f49ba84b32ea4b4e04d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 12:24:08 +0200 Subject: [PATCH 1/9] feat: parse and preserve .ad target-v1 evidence (ADR 0012 migration step 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recording now emits a `# agent-device:target-v1 {...}` comment immediately before every click/press/longpress/fill action that resolves through the tree (ref or selector), carrying the identity/ancestry/sibling/scrollRegion/ viewportOrder tuple from decision 3's record-time write algorithm plus a record-time self-check (verified/unverifiable). The parser accepts known fields in any order, NFC-normalizes, rejects malformed/oversized annotations with INVALID_ARGS, binds an annotation only to the immediately-next action line, and treats unknown future target-vN comments as ordinary comments. writeReplayScript's read-then-rewrite (heal) path preserves v1 annotations in canonical form. Nothing consumes the parsed evidence yet beyond preservation — replay-time enforcement is a later migration step. The identity tuple's own node/tree only ever lived on the internal visualization/session-history payload (never the public response); it is converted to the compact target-v1 evidence and stripped before anything is persisted or returned. --- .../interaction/runtime/interactions.test.ts | 8 +- .../interaction/runtime/resolution.ts | 20 +- src/daemon/__tests__/session-store.test.ts | 54 +++ .../__tests__/session-target-evidence.test.ts | 321 ++++++++++++ .../handlers/__tests__/interaction.test.ts | 109 +++++ src/daemon/handlers/interaction-common.ts | 38 +- .../handlers/interaction-touch-response.ts | 11 + src/daemon/session-action-recorder.ts | 3 + src/daemon/session-script-writer.ts | 6 +- src/daemon/session-target-evidence.ts | 302 ++++++++++++ src/daemon/types.ts | 9 + src/replay/__tests__/script.test.ts | 114 +++++ src/replay/__tests__/target-identity.test.ts | 446 +++++++++++++++++ src/replay/script-formatting.ts | 14 + src/replay/script.ts | 54 ++- src/replay/target-identity.ts | 459 ++++++++++++++++++ 16 files changed, 1958 insertions(+), 10 deletions(-) create mode 100644 src/daemon/__tests__/session-target-evidence.test.ts create mode 100644 src/daemon/session-target-evidence.ts create mode 100644 src/replay/__tests__/target-identity.test.ts create mode 100644 src/replay/target-identity.ts diff --git a/src/commands/interaction/runtime/interactions.test.ts b/src/commands/interaction/runtime/interactions.test.ts index 4e35d61a7..00e07d9bc 100644 --- a/src/commands/interaction/runtime/interactions.test.ts +++ b/src/commands/interaction/runtime/interactions.test.ts @@ -127,7 +127,13 @@ test('runtime fill uses backend ref primitive without resolving snapshot geometr assert.deepEqual(calls, [{ ref: '@e1', text: 'hello', delayMs: 25 }]); assert.equal(result.kind, 'ref'); assert.equal(result.point, undefined); - assert.equal(result.node, undefined); + // ADR 0012 decision 3: the native-ref preflight already looked this node up + // (for the offscreen/occlusion guards) and now reuses that lookup to carry + // `node`/`preActionNodes` for record-time target-binding evidence — at zero + // extra capture cost. These never reach the public response (stripped by + // `finalizeTouchInteraction`); this is the internal runtime result shape. + assert.equal(result.node?.ref, 'e1'); + assert.ok(Array.isArray(result.preActionNodes)); assert.deepEqual(result.target, { kind: 'ref', ref: '@e1' }); assert.equal(result.text, 'hello'); assert.deepEqual(result.backendResult, { ref: 'e1', text: 'hello' }); diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index f2b4c4441..6a4ce601a 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -492,7 +492,12 @@ export async function preflightNativeRefInteraction( options: CommandContext, target: Extract, action: InteractionAction, -): Promise<{ targetHittable?: boolean; hint?: string }> { +): Promise<{ + targetHittable?: boolean; + hint?: string; + node?: SnapshotNode; + preActionNodes?: SnapshotNode[]; +}> { const session = await runtime.sessions.get(options.session ?? 'default'); const nodes = session?.snapshot?.nodes; if (!nodes || normalizeRef(target.ref) === null) return {}; @@ -502,7 +507,18 @@ export async function preflightNativeRefInteraction( if (!resolved) return {}; assertInteractionNotBlocked(resolved.node, `Ref ${target.ref}`, action); assertVisibleRefTarget(resolved.node, nodes, target.ref, action); - return describeNonHittableTarget(resolved.node, action); + return { + ...describeNonHittableTarget(resolved.node, action), + // Reuses the lookup above (zero extra capture cost) so the native-ref + // fast path — `click @ref`/`fill @ref` with default options — can still + // record ADR 0012 decision-3 target-binding evidence from the stored + // session tree, not only from the full resolution path. Stripped back + // out of the response before it reaches any caller (see + // `interaction-touch-response.ts`/`finalizeTouchInteraction`); never + // exposed publicly. + node: resolved.node, + preActionNodes: nodes, + }; } // isNodeVisibleOnScreen (not the effective-viewport form): items inside an diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 622e8600b..6318c4fdb 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/src/daemon/__tests__/session-store.test.ts @@ -6,6 +6,7 @@ import path from 'node:path'; import { SessionStore } from '../session-store.ts'; import type { SessionState } from '../types.ts'; import { buildRequestFinishedEvent } from '../session-event-log.ts'; +import type { TargetAnnotationV1 } from '../../replay/target-identity.ts'; type RecordActionEntry = Parameters[1]; @@ -592,3 +593,56 @@ test('writeSessionLog preserves significant whitespace and empty string argument /--metro-host " host\\t" --launch-url "myapp:\/\/dev "/, ]); }); + +// --------------------------------------------------------------------------- +// ADR 0012 decision 3: recorded target-v1 evidence flows end to end through +// `recordAction` → `SessionScriptWriter` into the `.ad` file, immediately +// before the action it annotates. +// --------------------------------------------------------------------------- + +const SAVE_TARGET_EVIDENCE: TargetAnnotationV1 = { + id: 'save', + role: 'button', + label: 'Save', + ancestry: [{ role: 'toolbar', label: 'Editor' }], + sibling: 0, + viewportOrder: 0, + verification: 'verified', +}; + +test('writeSessionLog emits the target-v1 annotation immediately before its action line', () => { + const fixture = makeFixture('agent-device-session-log-target-evidence-'); + recordOpen(fixture.store, fixture.session); + fixture.store.recordAction(fixture.session, { + command: 'click', + positionals: ['@e12'], + flags: { platform: 'ios' }, + result: {}, + targetEvidence: SAVE_TARGET_EVIDENCE, + }); + recordClose(fixture.store, fixture.session); + + const script = writeScript(fixture); + const lines = script.trim().split('\n'); + const clickLineIndex = lines.findIndex((line) => line.startsWith('click ')); + assert.ok(clickLineIndex > 0); + assert.equal( + lines[clickLineIndex - 1], + '# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[{"role":"toolbar","label":"Editor"}],"sibling":0,"viewportOrder":0,"verification":"verified"}', + ); +}); + +test('writeSessionLog never fabricates a target-v1 annotation for actions recorded without evidence', () => { + const fixture = makeFixture('agent-device-session-log-no-target-evidence-'); + recordOpen(fixture.store, fixture.session); + fixture.store.recordAction(fixture.session, { + command: 'click', + positionals: ['@e12'], + flags: { platform: 'ios' }, + result: {}, + }); + recordClose(fixture.store, fixture.session); + + const script = writeScript(fixture); + assert.equal(/agent-device:target-v1/.test(script), false); +}); diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts new file mode 100644 index 000000000..2ace59065 --- /dev/null +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -0,0 +1,321 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import type { RawSnapshotNode, SnapshotNode } from '../../kernel/snapshot.ts'; +import { computeTargetEvidence } from '../session-target-evidence.ts'; +import { + parseTargetAnnotationV1Payload, + serializeTargetAnnotationV1, + utf8ByteLength, + TARGET_ANNOTATION_MAX_ANCESTRY, + TARGET_ANNOTATION_MAX_FIELD_BYTES, + TARGET_ANNOTATION_MAX_PAYLOAD_BYTES, +} from '../../replay/target-identity.ts'; + +function toSnapshotNodes(raw: RawSnapshotNode[]): SnapshotNode[] { + return raw.map((node, position) => ({ ...node, ref: `e${position + 1}` })); +} + +function findByLabel(nodes: SnapshotNode[], label: string): SnapshotNode { + const found = nodes.find((node) => node.label === label); + if (!found) throw new Error(`fixture missing node with label ${label}`); + return found; +} + +// --------------------------------------------------------------------------- +// Basic record-time write algorithm (decision 3 steps 1-5) +// --------------------------------------------------------------------------- + +function toolbarFixture(): SnapshotNode[] { + return toSnapshotNodes([ + { index: 0, type: 'Window', depth: 0 }, + { index: 1, type: 'Toolbar', label: 'Editor', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'Button', + identifier: 'save', + label: 'Save', + rect: { x: 10, y: 10, width: 40, height: 20 }, + depth: 2, + parentIndex: 1, + }, + ]); +} + +test('computeTargetEvidence: single unambiguous node is verified with the expected identity/ancestry/rect', () => { + const nodes = toolbarFixture(); + const winner = findByLabel(nodes, 'Save'); + const evidence = computeTargetEvidence({ node: winner, nodes }); + assert.ok(evidence); + assert.deepEqual(evidence, { + id: 'save', + role: 'button', + label: 'Save', + ancestry: [{ role: 'toolbar', label: 'Editor' }, { role: 'window' }], + sibling: 0, + viewportOrder: 0, + rect: { x: 10, y: 10, width: 40, height: 20 }, + verification: 'verified', + }); +}); + +// --------------------------------------------------------------------------- +// Scroll region + viewport order +// --------------------------------------------------------------------------- + +function scrollableListFixture(): SnapshotNode[] { + return toSnapshotNodes([ + { index: 0, type: 'Window', depth: 0 }, + { + index: 1, + type: 'ScrollView', + identifier: 'editor-scroll', + depth: 1, + parentIndex: 0, + }, + { + index: 2, + type: 'Cell', + label: 'Row', + rect: { x: 0, y: 100, width: 100, height: 20 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'Cell', + label: 'Row', + rect: { x: 0, y: 50, width: 100, height: 20 }, + depth: 2, + parentIndex: 1, + }, + { + index: 4, + type: 'Cell', + label: 'Row', + rect: { x: 0, y: 0, width: 100, height: 20 }, + depth: 2, + parentIndex: 1, + }, + ]); +} + +test('computeTargetEvidence: scrollRegion is the nearest scrollable ancestor local identity, viewportOrder is top-to-bottom within it', () => { + const nodes = scrollableListFixture(); + const winner = nodes[2]!; // rect y:100 -> should be last (index 2) in top-to-bottom order + const evidence = computeTargetEvidence({ node: winner, nodes }); + assert.ok(evidence); + assert.deepEqual(evidence.scrollRegion, { role: 'scrollview', id: 'editor-scroll' }); + // rows are at y=0,50,100 -> the y:100 row is ordinal 2 (0-based) top-to-bottom + assert.equal(evidence.viewportOrder, 2); + assert.equal(evidence.verification, 'verified'); +}); + +// --------------------------------------------------------------------------- +// Duplicate identity resolved by sibling / viewportOrder, still verified +// (decision 3's self-check succeeds by construction whenever the capture +// supplies the needed structural data). +// --------------------------------------------------------------------------- + +test('computeTargetEvidence: duplicate identity across two parents is still verified via the sibling ordinal', () => { + const nodes = toSnapshotNodes([ + { index: 0, type: 'Window', depth: 0 }, + { index: 1, type: 'Row', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'Button', + label: 'Go back', + rect: { x: 0, y: 0, width: 40, height: 20 }, + depth: 2, + parentIndex: 1, + }, + { index: 3, type: 'Row', depth: 1, parentIndex: 0 }, + { + index: 4, + type: 'Button', + label: 'Go back', + rect: { x: 0, y: 200, width: 40, height: 20 }, + depth: 2, + parentIndex: 3, + }, + ]); + const winner = nodes[4]!; // the second "Go back" button, under the second Row + const evidence = computeTargetEvidence({ node: winner, nodes }); + assert.ok(evidence); + assert.equal(evidence.id, undefined); + assert.equal(evidence.role, 'button'); + assert.equal(evidence.label, 'Go back'); + // Both buttons are child index 0 of their own (identical, unlabeled) Row — + // ancestry alone does not disambiguate them, and sibling is identical (0) + // for both. Only document-order-based viewportOrder within the (no + // scroll-region) partition can isolate the winner. + assert.equal(evidence.sibling, 0); + assert.equal(evidence.verification, 'verified'); +}); + +// --------------------------------------------------------------------------- +// Writer-parser invariant: max-size (256-byte) labels x K=8 ancestry reduces +// rather than rejecting its own output. +// --------------------------------------------------------------------------- + +test('computeTargetEvidence: max-size labels across K=8 ancestors reduce ancestry to fit 4 KiB, and the writer never rejects its own output', () => { + const maxLabel = 'x'.repeat(TARGET_ANNOTATION_MAX_FIELD_BYTES); + const raw: RawSnapshotNode[] = []; + // Build a deep chain of 9 ancestors (root .. depth 8) with BOTH role and + // label maxed at 256 bytes, plus the winning leaf at depth 9. 8 such + // entries alone exceed the 4 KiB payload cap, forcing root-side reduction. + for (let depth = 0; depth < 9; depth += 1) { + raw.push({ + index: depth, + type: maxLabel, + label: maxLabel, + depth, + parentIndex: depth > 0 ? depth - 1 : undefined, + }); + } + raw.push({ + index: 9, + type: 'Button', + label: maxLabel, + identifier: maxLabel, + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 9, + parentIndex: 8, + }); + const nodes = toSnapshotNodes(raw); + const winner = nodes.at(-1)!; + + const evidence = computeTargetEvidence({ node: winner, nodes }); + assert.ok(evidence); + assert.ok(evidence.ancestry.length <= TARGET_ANNOTATION_MAX_ANCESTRY); + assert.ok(evidence.ancestry.length < TARGET_ANNOTATION_MAX_ANCESTRY, 'ancestry must have been reduced'); + // Nearest-ancestor-first: the first (kept) entries are the leaf-side ones. + assert.equal(evidence.ancestry[0]?.label, maxLabel); + + const json = serializeTargetAnnotationV1(evidence); + assert.ok(utf8ByteLength(json) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES); + // The writer-parser invariant: the writer's own output must never be + // rejected by its own parser. + const reparsed = parseTargetAnnotationV1Payload(json); + assert.deepEqual(reparsed, evidence); +}); + +test('computeTargetEvidence: a root node with no ancestors has empty ancestry and is still verified', () => { + const nodes = toSnapshotNodes([ + { + index: 0, + type: 'Button', + identifier: 'root-button', + label: 'Root', + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 0, + }, + ]); + const evidence = computeTargetEvidence({ node: nodes[0]!, nodes }); + assert.ok(evidence); + assert.deepEqual(evidence.ancestry, []); + assert.equal(evidence.verification, 'verified'); +}); + +// --------------------------------------------------------------------------- +// Fixture realism: a real-capture-shaped tree (react-navigation-style bottom +// tab bar) with undefined/false `hittable` and anonymous wrapper nodes — see +// test/integration/provider-scenarios/ios-world.ts (PR #1172 pattern). +// --------------------------------------------------------------------------- + +function bottomTabsRealCaptureFixture(): SnapshotNode[] { + return toSnapshotNodes([ + { + index: 0, + type: 'Application', + label: 'React Navigation Example', + rect: { x: 0, y: 0, width: 402, height: 874 }, + enabled: true, + hittable: true, + depth: 0, + }, + { + index: 1, + type: 'Window', + rect: { x: 0, y: 0, width: 402, height: 874 }, + enabled: true, + hittable: true, + depth: 1, + parentIndex: 0, + }, + { + index: 2, + type: 'ScrollView', + label: 'Contacts', + rect: { x: 0, y: 116, width: 402, height: 675 }, + enabled: true, + hittable: false, + hiddenContentBelow: true, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'StaticText', + label: 'Marissa Castillo', + rect: { x: 52, y: 132, width: 110, height: 17 }, + enabled: true, + // Real captures commonly omit `hittable` entirely on plain text nodes + // rather than reporting it `false` — anonymous, undefined-hittable. + depth: 3, + parentIndex: 2, + }, + { + index: 4, + // Anonymous wrapper: no identifier/label of its own — a real shape + // this ADR's ancestry entries must tolerate (`role` present, `label` + // omitted). + type: 'Other', + rect: { x: 0, y: 791, width: 402, height: 83 }, + enabled: true, + hittable: false, + depth: 2, + parentIndex: 1, + }, + { + index: 5, + type: 'Button', + label: 'Article, unselected', + identifier: 'article', + rect: { x: 0, y: 791, width: 101, height: 49 }, + enabled: true, + hittable: false, + depth: 3, + parentIndex: 4, + }, + { + index: 6, + type: 'Button', + label: 'Chat, unselected', + identifier: 'chat', + rect: { x: 101, y: 791, width: 100, height: 49 }, + enabled: true, + hittable: false, + depth: 3, + parentIndex: 4, + }, + ]); +} + +test('computeTargetEvidence: real-capture-shaped tree (undefined hittable, anonymous wrapper ancestor)', () => { + const nodes = bottomTabsRealCaptureFixture(); + const winner = findByLabel(nodes, 'Article, unselected'); + const evidence = computeTargetEvidence({ node: winner, nodes }); + assert.ok(evidence); + assert.equal(evidence.id, 'article'); + assert.equal(evidence.role, 'button'); + // The anonymous "Other" wrapper ancestor carries a role but no label. + assert.deepEqual(evidence.ancestry[0], { role: 'other' }); + assert.equal(evidence.ancestry[1]?.role, 'window'); + assert.equal(evidence.ancestry[2]?.role, 'application'); + assert.equal(evidence.scrollRegion, undefined); // tab bar sits outside the ScrollView + assert.equal(evidence.sibling, 0); // first child of the "Other" tab-bar row + assert.equal(evidence.verification, 'verified'); + + const json = serializeTargetAnnotationV1(evidence); + assert.deepEqual(parseTargetAnnotationV1Payload(json), evidence); +}); diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index 03a6fbbe4..cded144a1 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -3560,3 +3560,112 @@ test('after a session reopen, a pin from the previous lifetime warns (reseeded g expect(String(response.data?.warning)).toContain(`minted from snapshot s${oldGeneration}`); } }); + +// --------------------------------------------------------------------------- +// ADR 0012 decision 3: target-v1 evidence is computed only while the session +// is being recorded, lands on the recorded action (never the public +// response), and the raw resolved node/tree never reaches either the client +// or session history. +// --------------------------------------------------------------------------- + +test('press @ref while recording attaches target-v1 evidence to the recorded action, never to the public response', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-press'; + const session = makeSession(sessionName); + session.recordSession = true; + session.snapshot = { + nodes: attachRefs([ + { + index: 0, + type: 'XCUIElementTypeButton', + identifier: 'save', + label: 'Save', + rect: { x: 10, y: 20, width: 100, height: 40 }, + enabled: true, + hittable: true, + }, + ]), + createdAt: Date.now(), + backend: 'xctest', + }; + sessionStore.set(sessionName, session); + + const response = await handleInteractionCommands({ + req: { + token: 't', + session: sessionName, + command: 'press', + positionals: ['@e1'], + // verify:true forces full runtime resolution (captures node/tree), + // sidestepping the native-ref fast path exactly like the sibling + // --verify tests above. + flags: { verify: true }, + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + 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('targetEvidence'); + } + + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toEqual({ + id: 'save', + role: 'button', + label: 'Save', + ancestry: [], + sibling: 0, + viewportOrder: 0, + rect: { x: 10, y: 20, width: 100, height: 40 }, + verification: 'verified', + }); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('press @ref without recording never computes target-v1 evidence', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'non-recording-press'; + const session = makeSession(sessionName); + session.recordSession = false; + session.snapshot = { + nodes: attachRefs([ + { + index: 0, + type: 'XCUIElementTypeButton', + identifier: 'save', + label: 'Save', + rect: { x: 10, y: 20, width: 100, height: 40 }, + enabled: true, + hittable: true, + }, + ]), + createdAt: Date.now(), + backend: 'xctest', + }; + sessionStore.set(sessionName, session); + + const response = await handleInteractionCommands({ + req: { + token: 't', + session: sessionName, + command: 'press', + positionals: ['@e1'], + flags: { verify: true }, + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toBeUndefined(); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index bacf1ae8c..8ae771cb0 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -1,5 +1,5 @@ import type { CommandFlags } from '../../core/dispatch.ts'; -import type { SnapshotState } from '../../kernel/snapshot.ts'; +import type { SnapshotNode, SnapshotState } from '../../kernel/snapshot.ts'; import type { DaemonCommandContext } from '../context.ts'; import { recordTouchVisualizationEvent } from '../recording-gestures.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; @@ -13,6 +13,7 @@ import { stripInternalInteractionFlags, } from '../interaction-outcome-policy.ts'; import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; +import { computeTargetEvidence } from '../session-target-evidence.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -55,11 +56,16 @@ export function finalizeTouchInteraction(params: { androidFreshnessBaseline, } = params; const actionFlags = stripInternalInteractionFlags(flags); + const { result: recordedResult, targetEvidence } = extractTargetEvidenceForRecording( + session, + result, + ); sessionStore.recordAction(session, { command, positionals, flags: actionFlags ?? {}, - result, + result: recordedResult, + ...(targetEvidence ? { targetEvidence } : {}), }); markPendingInteractionOutcome({ session, @@ -76,10 +82,36 @@ export function finalizeTouchInteraction(params: { session, command, positionals, - result, + recordedResult, (actionFlags ?? {}) as Record, actionStartedAt, actionFinishedAt, ); return { ok: true, data: responseData }; } + +/** + * ADR 0012 decision 3: `result.node`/`result.preActionNodes` (attached only + * to the internal visualization/session payload, see + * `interaction-touch-response.ts`) are the record-time winner and tree. When + * the session is being recorded (`--save-script`), turn them into the + * compact `target-v1` evidence the script writer emits; either way, strip the + * raw node/tree back out so no downstream consumer (session history, touch + * visualization telemetry) ever holds a full AX subtree per action. + */ +function extractTargetEvidenceForRecording( + session: SessionState, + result: Record, +): { result: Record; targetEvidence: ReturnType } { + if (!('node' in result) && !('preActionNodes' in result)) { + return { result, targetEvidence: undefined }; + } + const { node, preActionNodes, ...rest } = result as Record & { + node?: SnapshotNode; + preActionNodes?: SnapshotNode[]; + }; + if (!session.recordSession || !node || !preActionNodes) { + return { result: rest, targetEvidence: undefined }; + } + return { result: rest, targetEvidence: computeTargetEvidence({ node, nodes: preActionNodes }) }; +} \ No newline at end of file diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index 3bdb03227..c2e849133 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -125,6 +125,17 @@ export function buildInteractionResponseData(params: { visualization.warning = warning; responseData.warning = warning; } + // ADR 0012 decision 3: the resolved node and the record-time tree it came + // from ride ONLY on `visualization` (session history / touch overlay + // input), never on `responseData` (the public payload) — attaching them + // here, not via `commonExtra`, is what keeps them off the wire. + // `finalizeTouchInteraction` (interaction-common.ts) turns them into the + // compact `targetEvidence` annotation when the session is being recorded + // and strips the raw tree back out either way. + if ('node' in result && result.node) visualization.node = result.node; + if ('preActionNodes' in result && result.preActionNodes) { + visualization.preActionNodes = result.preActionNodes; + } return { result: visualization, responseData }; } diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 60fd67c18..d40493520 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -3,6 +3,7 @@ import { SCREENSHOT_ACTION_FLAG_KEYS } from '../contracts/screenshot.ts'; import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { SessionAction, SessionRuntimeHints, SessionState } from './types.ts'; import { expandSessionPath } from './session-paths.ts'; +import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; export type RecordActionEntry = { command: string; @@ -10,6 +11,7 @@ export type RecordActionEntry = { flags: CommandFlags; runtime?: SessionRuntimeHints; result?: Record; + targetEvidence?: TargetAnnotationV1; }; export function recordActionEntry( @@ -30,6 +32,7 @@ export function recordActionEntry( runtime: entry.runtime, flags: sanitizeFlags(entry.flags), result: entry.result, + ...(entry.targetEvidence ? { targetEvidence: entry.targetEvidence } : {}), }; session.actions.push(action); emitDiagnostic({ diff --git a/src/daemon/session-script-writer.ts b/src/daemon/session-script-writer.ts index af1738b52..b66e85206 100644 --- a/src/daemon/session-script-writer.ts +++ b/src/daemon/session-script-writer.ts @@ -3,7 +3,10 @@ import path from 'node:path'; import { publicPlatformString } from '../kernel/device.ts'; import { inferFillText } from './action-utils.ts'; import { emitDiagnostic } from '../utils/diagnostics.ts'; -import { formatPortableActionLine } from '../replay/script-formatting.ts'; +import { + formatPortableActionLine, + formatTargetAnnotationLines, +} from '../replay/script-formatting.ts'; import { expandSessionPath, safeSessionName } from './session-paths.ts'; import { appendScriptSeriesFlags, @@ -164,6 +167,7 @@ function formatScript(session: SessionState, actions: SessionAction[]): string { ); for (const action of actions) { if (action.flags?.noRecord) continue; + lines.push(...formatTargetAnnotationLines(action)); lines.push(formatActionLine(action)); } return `${lines.join('\n')}\n`; diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts new file mode 100644 index 000000000..600b9ce04 --- /dev/null +++ b/src/daemon/session-target-evidence.ts @@ -0,0 +1,302 @@ +/** + * ADR 0012 decision 3: record-time computation of `.ad` target-binding + * evidence (the `# agent-device:target-v1 {...}` annotation). + * + * `computeTargetEvidence` implements decision 3's "Record-time write" + * algorithm steps 1-5 against the SAME record-time tree the interaction + * resolver just captured (`ResolvedInteractionTarget.node` + + * `.preActionNodes`, see `src/commands/interaction/runtime/resolution.ts`). + * It never captures anything itself — callers own gating this on whether the + * session is actually being recorded (`session.recordSession`), since the + * ancestry/identity-set scan is O(nodes) and pointless otherwise. + * + * This module lives under `src/daemon/` (not `src/replay/`) because it needs + * `SnapshotNode`/`findNearestScrollableContainer` from the daemon's + * snapshot-presentation layer, which sits above `replay` in the import DAG; + * `replay` (below `daemon`) owns only the tree-agnostic spec pieces + * (`src/replay/target-identity.ts`) that both the writer here and the parser + * share. + */ + +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { resolveRectCenter } from '../utils/rect-center.ts'; +import { normalizeType } from '../snapshot/snapshot-processing.ts'; +import { findNearestScrollableContainer } from './snapshot-presentation/tree.ts'; +import { + classifyTargetBindingMatch, + matchesAncestryPrefix, + matchesLocalIdentity, + normalizeIdentifierField, + normalizeLabelField, + normalizeRoleField, + serializeTargetAnnotationV1, + truncateToUtf8Bytes, + utf8ByteLength, + TARGET_ANNOTATION_MAX_ANCESTRY, + TARGET_ANNOTATION_MAX_FIELD_BYTES, + TARGET_ANNOTATION_MAX_PAYLOAD_BYTES, + type LocalIdentity, + type TargetAncestryEntry, + type TargetAnnotationV1, + type TargetScrollRegion, + type TargetVerification, +} from '../replay/target-identity.ts'; + +export function computeTargetEvidence(params: { + node: SnapshotNode; + nodes: readonly SnapshotNode[]; +}): TargetAnnotationV1 | undefined { + const { node, nodes } = params; + if (typeof node.index !== 'number') return undefined; + const byIndex = buildIndexMap(nodes); + const identity = boundedLocalIdentity(node); + const fullAncestry = buildAncestryChain(node, byIndex, TARGET_ANNOTATION_MAX_ANCESTRY); + const sibling = computeSiblingOrdinal(nodes, node); + const scrollRegion = computeScrollRegionKey(node, byIndex); + const rect = boundedRect(node); + + // Writer-parser invariant (decision 3): reduce ancestry from the root side + // until the canonical serialization fits the 4 KiB payload cap. Every + // string field is already bounded to 256 bytes by `boundedLocalIdentity`/ + // `buildAncestryChain`/`computeScrollRegionKey`, so this loop only ever + // needs to shed ancestry entries. Decision 3 stops reducing once only + // `ancestry[0]` (the parent) is retained — the floor is 1 when the winner + // has any ancestor at all, else 0 (a root node has none to keep). + const floor = fullAncestry.length > 0 ? 1 : 0; + const buildCandidate = (ancestryLength: number) => { + const ancestry = fullAncestry.slice(0, ancestryLength); + const domain = computeDisambiguationDomain({ + nodes, + byIndex, + node, + identity, + ancestry, + sibling, + scrollRegion, + }); + const candidate: TargetAnnotationV1 = { + ...identity, + ancestry, + sibling, + viewportOrder: domain.viewportOrder, + ...(scrollRegion ? { scrollRegion } : {}), + ...(rect ? { rect } : {}), + verification: 'verified', + }; + return { candidate, domain }; + }; + + for (let ancestryLength = fullAncestry.length; ancestryLength >= floor; ancestryLength -= 1) { + const { candidate, domain } = buildCandidate(ancestryLength); + if (utf8ByteLength(serializeTargetAnnotationV1(candidate)) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES) { + candidate.verification = runRecordTimeSelfCheck({ node, domain }); + return candidate; + } + if (ancestryLength === floor) { + // Decision 3's terminal fail-closed guarantee: a parent-only (or, for a + // root node, ancestry-less) payload fits arithmetically once every + // field is already capped at 256 bytes, so this branch is not expected + // to run. If it somehow still doesn't fit, drop the diagnostic-only + // rect (never compared) as one last, spec-consistent reduction rather + // than emit a payload the parser would reject. + candidate.verification = 'unverifiable'; + if (utf8ByteLength(serializeTargetAnnotationV1(candidate)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES) { + delete candidate.rect; + } + return candidate; + } + } + return undefined; +} + +// --------------------------------------------------------------------------- +// Identity / ancestry / sibling / scroll region — decision 3's structural +// primitives, computed once per candidate node against the record-time tree. +// --------------------------------------------------------------------------- + +function buildIndexMap(nodes: readonly SnapshotNode[]): Map { + const map = new Map(); + for (const node of nodes) map.set(node.index, node); + return map; +} + +function computeLocalIdentity(node: SnapshotNode): LocalIdentity { + const role = normalizeRoleField(normalizeType(node.type ?? '')); + const id = normalizeIdentifierField(node.identifier); + const label = normalizeLabelField(node.label); + return { ...(id !== undefined ? { id } : {}), role, ...(label !== undefined ? { label } : {}) }; +} + +function boundedLocalIdentity(node: SnapshotNode): LocalIdentity { + const identity = computeLocalIdentity(node); + return { + ...(identity.id !== undefined + ? { id: truncateToUtf8Bytes(identity.id, TARGET_ANNOTATION_MAX_FIELD_BYTES) } + : {}), + role: truncateToUtf8Bytes(identity.role, TARGET_ANNOTATION_MAX_FIELD_BYTES), + ...(identity.label !== undefined + ? { label: truncateToUtf8Bytes(identity.label, TARGET_ANNOTATION_MAX_FIELD_BYTES) } + : {}), + }; +} + +/** Decision 3 "Ancestry": nearest K ancestors, leaf→root, {role,label?}. */ +function buildAncestryChain( + node: SnapshotNode, + byIndex: Map, + limit: number, +): TargetAncestryEntry[] { + const chain: TargetAncestryEntry[] = []; + const visited = new Set(); + let current = typeof node.parentIndex === 'number' ? byIndex.get(node.parentIndex) : undefined; + while (current && !visited.has(current.index) && chain.length < limit) { + visited.add(current.index); + const identity = boundedLocalIdentity(current); + chain.push({ role: identity.role, ...(identity.label !== undefined ? { label: identity.label } : {}) }); + current = typeof current.parentIndex === 'number' ? byIndex.get(current.parentIndex) : undefined; + } + return chain; +} + +/** + * 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) + * are siblings of every other root-level node. + */ +function computeSiblingOrdinal(nodes: readonly SnapshotNode[], node: SnapshotNode): number { + const parentIndex = node.parentIndex; + let ordinal = 0; + for (const candidate of nodes) { + if (candidate.parentIndex !== parentIndex) continue; + if (candidate.index === node.index) return ordinal; + ordinal += 1; + } + return 0; +} + +/** Decision 3 record-time write step 4: nearest scrollable ancestor's local identity, or `undefined` for *none*. */ +function computeScrollRegionKey( + node: SnapshotNode, + byIndex: Map, +): TargetScrollRegion | undefined { + // `findNearestScrollableContainer` is typed generically over + // `RawSnapshotNode`; every value in `byIndex` is actually a `SnapshotNode` + // (built from the same `nodes` array), so the cast back is safe. + const container = findNearestScrollableContainer(node, byIndex) as SnapshotNode | null; + if (!container) return undefined; + const identity = boundedLocalIdentity(container); + return { + role: identity.role, + ...(identity.id !== undefined ? { id: identity.id } : {}), + ...(identity.label !== undefined ? { label: identity.label } : {}), + }; +} + +function scrollRegionKeysEqual( + a: TargetScrollRegion | undefined, + b: TargetScrollRegion | undefined, +): boolean { + if (!a && !b) return true; + if (!a || !b) return false; + return a.role === b.role && a.id === b.id && a.label === b.label; +} + +function boundedRect(node: SnapshotNode): TargetAnnotationV1['rect'] { + const rect = node.rect; + if (!rect) return undefined; + return { x: rect.x, y: rect.y, width: rect.width, height: rect.height }; +} + +// --------------------------------------------------------------------------- +// Disambiguation domain: decision 3 record-time write step 2 (identity set) +// + step 4 (scroll-region partition + viewportOrder), and everything the +// step-5 self-check needs to classify the result. +// --------------------------------------------------------------------------- + +type DisambiguationDomain = { + identitySet: SnapshotNode[]; + siblingMatches: SnapshotNode[]; + regionMembers: SnapshotNode[] | undefined; + orderedRegion: SnapshotNode[]; + viewportOrder: number; +}; + +function computeDisambiguationDomain(params: { + nodes: readonly SnapshotNode[]; + byIndex: Map; + node: SnapshotNode; + identity: LocalIdentity; + ancestry: TargetAncestryEntry[]; + sibling: number; + scrollRegion: TargetScrollRegion | undefined; +}): DisambiguationDomain { + const { nodes, byIndex, node, identity, ancestry, sibling, scrollRegion } = params; + + // Step 2: all nodes sharing the winner's local identity with a matching + // leaf-anchored ancestry prefix. + const identitySet = nodes.filter((candidate) => { + if (!matchesLocalIdentity(computeLocalIdentity(candidate), identity)) return false; + const observedAncestry = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); + return matchesAncestryPrefix(observedAncestry, ancestry); + }); + + const siblingMatches = identitySet.filter( + (candidate) => computeSiblingOrdinal(nodes, candidate) === sibling, + ); + + const regionMembers = + identitySet.length > 0 + ? identitySet.filter((candidate) => + scrollRegionKeysEqual(computeScrollRegionKey(candidate, byIndex), scrollRegion), + ) + : undefined; + + const orderedRegion = regionMembers ? orderByViewportPosition(regionMembers) : []; + const viewportOrder = Math.max( + orderedRegion.findIndex((candidate) => candidate.index === node.index), + 0, + ); + + return { identitySet, siblingMatches, regionMembers, orderedRegion, viewportOrder }; +} + +/** Decision 3: rect center top-to-bottom then left-to-right; ties by document order; rect-less last, in document order. */ +function orderByViewportPosition(members: readonly SnapshotNode[]): SnapshotNode[] { + return members + .map((node, documentOrder) => ({ node, documentOrder, center: resolveRectCenter(node.rect) })) + .sort((a, b) => { + if (!a.center && !b.center) return a.documentOrder - b.documentOrder; + if (!a.center) return 1; + if (!b.center) return -1; + if (a.center.y !== b.center.y) return a.center.y - b.center.y; + if (a.center.x !== b.center.x) return a.center.x - b.center.x; + return a.documentOrder - b.documentOrder; + }) + .map((entry) => entry.node); +} + +/** + * Decision 3 record-time write step 5: run the replay-time classification + * (decision 3's paths 2-6, shared via `classifyTargetBindingMatch`) against + * the record-time tree itself. Paths 2/3 are unreachable here by + * construction — the winner always matched itself and is always a member of + * its own identity set — but are still fed through the shared classifier so + * the exact same function runs at record and (in a future step) replay time. + */ +function runRecordTimeSelfCheck(params: { + node: SnapshotNode; + domain: DisambiguationDomain; +}): TargetVerification { + const { node, domain } = params; + const winnerRef = node.ref; + const identitySetRefs = domain.identitySet.map((n) => n.ref); + const classification = classifyTargetBindingMatch({ + winnerRef, + matchedRefs: identitySetRefs, + identitySetRefs, + siblingMatchRefs: domain.siblingMatches.map((n) => n.ref), + regionMemberRefs: domain.regionMembers?.map((n) => n.ref), + viewportCandidateRef: domain.orderedRegion[domain.viewportOrder]?.ref, + }); + return classification.outcome; +} diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 70a84efcd..e362682d9 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -19,6 +19,7 @@ import type { RecordingScope } from '../core/recording-scope.ts'; import type { DeviceInfo, Platform, PlatformSelector } from '../kernel/device.ts'; import type { ExecBackgroundResult, ExecResult } from '../utils/exec.ts'; import type { SnapshotState } from '../kernel/snapshot.ts'; +import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import type { AppLogFailure, AppLogState } from './app-log-process.ts'; import type { DeviceLease } from './lease-registry.ts'; import type { AndroidNativePerfSession } from '../platforms/android/perf.ts'; @@ -398,4 +399,12 @@ export type SessionAction = { noRecord?: boolean; }; result?: Record; + /** + * ADR 0012 decision 3: parsed or record-time-computed `target-v1` + * evidence, emitted as a `# agent-device:target-v1 {...}` comment + * immediately before this action's line. Inert — migration step 3 parses + * and preserves it, but nothing enforces it yet (that lands in a later + * migration step). + */ + targetEvidence?: TargetAnnotationV1; }; diff --git a/src/replay/__tests__/script.test.ts b/src/replay/__tests__/script.test.ts index d4e1eb994..e75617d60 100644 --- a/src/replay/__tests__/script.test.ts +++ b/src/replay/__tests__/script.test.ts @@ -10,6 +10,7 @@ import { REPLAY_METADATA_PLATFORMS, writeReplayScript, } from '../script.ts'; +import type { TargetAnnotationV1 } from '../target-identity.ts'; import type { SessionAction, SessionState } from '../../daemon/types.ts'; function makeSession(): SessionState { @@ -373,3 +374,116 @@ test('replay parsing strips versioned-ref pins from recorded refs (#1076)', () = const malformed = parseReplayScriptDetailed('press @e2~x3').actions[0]; assert.deepEqual(malformed?.positionals, ['@e2~x3']); }); + +// --------------------------------------------------------------------------- +// ADR 0012 decision 3: `.ad` target-v1 annotation parsing/binding/preservation +// (migration step 3 — parser/writer only, no replay-time enforcement). +// --------------------------------------------------------------------------- + +const SAVE_EVIDENCE: TargetAnnotationV1 = { + id: 'save', + role: 'button', + label: 'Save', + ancestry: [{ role: 'toolbar', label: 'Editor' }, { role: 'window' }], + sibling: 0, + viewportOrder: 0, + scrollRegion: { role: 'scrollview', id: 'editor-scroll' }, + verification: 'verified', +}; + +const SAVE_EVIDENCE_LINE = + '# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[{"role":"toolbar","label":"Editor"},{"role":"window"}],"sibling":0,"viewportOrder":0,"scrollRegion":{"role":"scrollview","id":"editor-scroll"},"verification":"verified"}'; + +test('a target-v1 annotation immediately preceding an action line attaches to that action', () => { + const script = ['context platform=ios device=iPhone', SAVE_EVIDENCE_LINE, 'click @e12 "Save"'].join( + '\n', + ); + const { actions } = parseReplayScriptDetailed(script); + assert.equal(actions.length, 1); + assert.deepEqual(actions[0]?.targetEvidence, SAVE_EVIDENCE); +}); + +test('a target-v1 annotation followed by a blank line before the action is rejected as INVALID_ARGS', () => { + const script = [SAVE_EVIDENCE_LINE, '', 'click @e12 "Save"'].join('\n'); + assert.throws( + () => parseReplayScriptDetailed(script), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + /must be immediately followed by its action line/.test(error.message), + ); +}); + +test('a target-v1 annotation followed by another comment before the action is rejected as INVALID_ARGS', () => { + const script = [SAVE_EVIDENCE_LINE, '# note', 'click @e12 "Save"'].join('\n'); + assert.throws( + () => parseReplayScriptDetailed(script), + (error: unknown) => error instanceof AppError && error.code === 'INVALID_ARGS', + ); +}); + +test('a target-v1 annotation as the last line of the script (no action follows) is rejected as INVALID_ARGS', () => { + assert.throws( + () => parseReplayScriptDetailed(SAVE_EVIDENCE_LINE), + (error: unknown) => error instanceof AppError && error.code === 'INVALID_ARGS', + ); +}); + +test('a malformed target-v1 payload is rejected as INVALID_ARGS, not silently dropped', () => { + const script = ['# agent-device:target-v1 {not json', 'click @e12 "Save"'].join('\n'); + assert.throws( + () => parseReplayScriptDetailed(script), + (error: unknown) => error instanceof AppError && error.code === 'INVALID_ARGS', + ); +}); + +test('an unknown future target-vN comment is an ordinary comment: no binding requirement, no evidence attached', () => { + const script = [ + '# agent-device:target-v2 {"whatever":true}', + '', + 'click @e12 "Save"', + ].join('\n'); + const { actions } = parseReplayScriptDetailed(script); + assert.equal(actions.length, 1); + assert.equal(actions[0]?.targetEvidence, undefined); +}); + +test('old readers ignoring the comment execute the action unchanged: an action without a preceding annotation carries no targetEvidence', () => { + const { actions } = parseReplayScriptDetailed('click @e12 "Save"'); + assert.equal(actions[0]?.targetEvidence, undefined); +}); + +test('writeReplayScript (read-then-rewrite / heal path) preserves a v1 annotation in canonical form', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-script-target-v1-')); + const replayPath = path.join(root, 'flow.ad'); + const actions: SessionAction[] = [ + { + ts: Date.now(), + command: 'click', + positionals: ['@e12'], + flags: {}, + targetEvidence: SAVE_EVIDENCE, + }, + ]; + + writeReplayScript(replayPath, actions, makeSession()); + const script = fs.readFileSync(replayPath, 'utf8'); + const lines = script.trim().split('\n'); + assert.equal(lines.at(-2), SAVE_EVIDENCE_LINE); + assert.equal(lines.at(-1), 'click @e12'); + + // Round trip: re-parsing the rewritten script recovers the same evidence. + const reparsed = parseReplayScriptDetailed(script); + assert.deepEqual(reparsed.actions[0]?.targetEvidence, SAVE_EVIDENCE); +}); + +test('session-recorded actions without target evidence never gain a fabricated annotation on rewrite', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-script-target-v1-none-')); + const replayPath = path.join(root, 'flow.ad'); + const actions: SessionAction[] = [ + { ts: Date.now(), command: 'click', positionals: ['@e12'], flags: {} }, + ]; + writeReplayScript(replayPath, actions, makeSession()); + const script = fs.readFileSync(replayPath, 'utf8'); + assert.equal(/agent-device:target-v1/.test(script), false); +}); diff --git a/src/replay/__tests__/target-identity.test.ts b/src/replay/__tests__/target-identity.test.ts new file mode 100644 index 000000000..a9714adac --- /dev/null +++ b/src/replay/__tests__/target-identity.test.ts @@ -0,0 +1,446 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { AppError } from '../../kernel/errors.ts'; +import { + classifyTargetBindingMatch, + formatTargetAnnotationCommentLine, + matchesAncestryPrefix, + matchesLocalIdentity, + normalizeLabelField, + parseTargetAnnotationCommentLine, + parseTargetAnnotationV1Payload, + serializeTargetAnnotationV1, + truncateToUtf8Bytes, + TARGET_ANNOTATION_MAX_ANCESTRY, + TARGET_ANNOTATION_MAX_FIELD_BYTES, + TARGET_ANNOTATION_MAX_PAYLOAD_BYTES, + type TargetAnnotationV1, +} from '../target-identity.ts'; + +function baseEvidence(overrides: Partial = {}): TargetAnnotationV1 { + return { + id: 'save', + role: 'button', + label: 'Save', + ancestry: [ + { role: 'toolbar', label: 'Editor' }, + { role: 'window' }, + ], + sibling: 0, + viewportOrder: 0, + scrollRegion: { role: 'scrollview', id: 'editor-scroll' }, + verification: 'verified', + ...overrides, + }; +} + +function assertInvalidArgs(fn: () => unknown, messagePattern?: RegExp): void { + assert.throws( + fn, + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + (!messagePattern || messagePattern.test(error.message)), + ); +} + +// --------------------------------------------------------------------------- +// Canonical serialization / field order +// --------------------------------------------------------------------------- + +test('serializeTargetAnnotationV1 uses the exact canonical field order from decision 3', () => { + const json = serializeTargetAnnotationV1(baseEvidence()); + assert.equal( + json, + '{"id":"save","role":"button","label":"Save","ancestry":[{"role":"toolbar","label":"Editor"},{"role":"window"}],"sibling":0,"viewportOrder":0,"scrollRegion":{"role":"scrollview","id":"editor-scroll"},"verification":"verified"}', + ); +}); + +test('formatTargetAnnotationCommentLine emits the ASCII # agent-device:target-v1 prefix', () => { + const line = formatTargetAnnotationCommentLine(baseEvidence()); + assert.ok(line.startsWith('# agent-device:target-v1 {')); +}); + +// --------------------------------------------------------------------------- +// Parse-write-parse round trip / semantic equality +// --------------------------------------------------------------------------- + +test('parse(serialize(evidence)) round trips to a semantically equal object', () => { + const evidence = baseEvidence({ rect: { x: 1, y: 2, width: 3, height: 4 } }); + const parsedBack = parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(evidence)); + assert.deepEqual(parsedBack, evidence); +}); + +test('parseTargetAnnotationCommentLine accepts known fields in any JSON key order', () => { + const line = + '# agent-device:target-v1 {"verification":"verified","sibling":0,"role":"button","viewportOrder":2,"ancestry":[],"id":"save"}'; + const result = parseTargetAnnotationCommentLine(line); + assert.equal(result.kind, 'v1'); + if (result.kind !== 'v1') throw new Error('unreachable'); + assert.deepEqual(result.evidence, { + id: 'save', + role: 'button', + ancestry: [], + sibling: 0, + viewportOrder: 2, + verification: 'verified', + }); +}); + +test('parseTargetAnnotationCommentLine ignores unknown fields', () => { + const line = + '# agent-device:target-v1 {"role":"button","verification":"verified","futureField":{"nested":true}}'; + const result = parseTargetAnnotationCommentLine(line); + assert.equal(result.kind, 'v1'); + if (result.kind !== 'v1') throw new Error('unreachable'); + assert.equal((result.evidence as Record).futureField, undefined); +}); + +test('an unknown future target-vN annotation is an ordinary comment to a v1 reader', () => { + const result = parseTargetAnnotationCommentLine('# agent-device:target-v2 {"anything":"goes"}'); + assert.deepEqual(result, { kind: 'future-version' }); +}); + +test('a line that merely mentions the tag in prose is an ordinary comment', () => { + assert.deepEqual(parseTargetAnnotationCommentLine('# see agent-device:target-v1 docs'), { + kind: 'none', + }); + assert.deepEqual(parseTargetAnnotationCommentLine('# just a comment'), { kind: 'none' }); +}); + +// --------------------------------------------------------------------------- +// Normalization: NFC, label trim/collapse, normalized-role source +// --------------------------------------------------------------------------- + +test('normalizeLabelField NFC-normalizes, trims, and collapses internal whitespace', () => { + // "é" as e + combining acute (NFD) must normalize to the precomposed (NFC) form. + const nfd = 'Café'; + assert.equal(normalizeLabelField(` ${nfd} au lait `), 'Café au lait'); +}); + +test('normalizeLabelField treats a whitespace-only label as absent', () => { + assert.equal(normalizeLabelField(' '), undefined); +}); + +test('embedded quotes and backslashes in labels round trip losslessly', () => { + const evidence = baseEvidence({ label: 'Say "hi" \\ backslash', id: undefined }); + const json = serializeTargetAnnotationV1(evidence); + assert.ok(json.includes('\\"hi\\"')); + const parsed = parseTargetAnnotationV1Payload(json); + assert.equal(parsed.label, 'Say "hi" \\ backslash'); +}); + +test('Unicode labels (including astral code points) round trip losslessly', () => { + const evidence = baseEvidence({ label: '\u{1F600} café résumé', id: undefined }); + const parsed = parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(evidence)); + assert.equal(parsed.label, '\u{1F600} café résumé'); +}); + +// --------------------------------------------------------------------------- +// Old/new reader compatibility +// --------------------------------------------------------------------------- + +test('a v1 reader treats an annotation with only role + verification as valid, defaulting the rest', () => { + const result = parseTargetAnnotationCommentLine( + '# agent-device:target-v1 {"role":"button","verification":"verified"}', + ); + assert.equal(result.kind, 'v1'); + if (result.kind !== 'v1') throw new Error('unreachable'); + assert.deepEqual(result.evidence, { + role: 'button', + ancestry: [], + sibling: 0, + viewportOrder: 0, + verification: 'verified', + }); +}); + +// --------------------------------------------------------------------------- +// Leaf-anchored ancestry prefix matching: root-side truncation + inserted +// wrapper mismatch +// --------------------------------------------------------------------------- + +test('matchesAncestryPrefix accepts an observed chain that is a superset on the root side (truncation)', () => { + const recorded = [{ role: 'toolbar', label: 'Editor' }]; + const observedFullDepth = [ + { role: 'toolbar', label: 'Editor' }, + { role: 'window' }, + { role: 'application' }, + ]; + assert.equal(matchesAncestryPrefix(observedFullDepth, recorded), true); +}); + +test('matchesAncestryPrefix rejects an inserted wrapper ancestor (structure is part of identity)', () => { + const recorded = [ + { role: 'toolbar', label: 'Editor' }, + { role: 'window' }, + ]; + // A new wrapper view inserted directly above the target shifts every entry + // one level down — the leaf-anchored prefix no longer matches. + const observedWithInsertedWrapper = [ + { role: 'view' }, + { role: 'toolbar', label: 'Editor' }, + { role: 'window' }, + ]; + assert.equal(matchesAncestryPrefix(observedWithInsertedWrapper, recorded), false); +}); + +test('matchesAncestryPrefix rejects a shorter observed chain', () => { + const recorded = [{ role: 'toolbar' }, { role: 'window' }]; + assert.equal(matchesAncestryPrefix([{ role: 'toolbar' }], recorded), false); +}); + +test('matchesAncestryPrefix leaves an unconstrained (absent) recorded label unconstrained', () => { + const recorded = [{ role: 'toolbar' }]; + assert.equal(matchesAncestryPrefix([{ role: 'toolbar', label: 'anything' }], recorded), true); +}); + +// --------------------------------------------------------------------------- +// Local identity +// --------------------------------------------------------------------------- + +test('matchesLocalIdentity: a recorded id never matches a node without that id', () => { + assert.equal(matchesLocalIdentity({ role: 'button' }, { id: 'save', role: 'button' }), false); +}); + +test('matchesLocalIdentity: with no recorded id, role+label must both match, absent-absent counts as equal', () => { + assert.equal(matchesLocalIdentity({ role: 'button' }, { role: 'button' }), true); + assert.equal( + matchesLocalIdentity({ role: 'button', label: 'Save' }, { role: 'button' }), + false, + ); +}); + +// --------------------------------------------------------------------------- +// Bounds: 256-byte fields, 4 KiB payload, 8-entry ancestry — parser REJECTS, +// never truncates. +// --------------------------------------------------------------------------- + +test('parser rejects a string field exceeding the 256-byte cap', () => { + const oversizedLabel = 'x'.repeat(TARGET_ANNOTATION_MAX_FIELD_BYTES + 1); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', label: oversizedLabel, verification: 'verified' }), + ), + /256-byte field cap/, + ); +}); + +test('parser rejects a payload exceeding the 4 KiB cap', () => { + // Every individual field stays within the 256-byte field cap, but 8 + // maxed-out ancestry entries plus maxed top-level/scrollRegion fields blow + // the 4 KiB payload ceiling collectively. + const maxLabel = 'x'.repeat(TARGET_ANNOTATION_MAX_FIELD_BYTES); + const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY }, () => ({ + role: maxLabel, + label: maxLabel, + })); + const json = JSON.stringify({ + id: maxLabel, + role: maxLabel, + label: maxLabel, + ancestry, + scrollRegion: { role: maxLabel, id: maxLabel, label: maxLabel }, + verification: 'verified', + }); + assert.ok(Buffer.byteLength(json, 'utf8') > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES); + assertInvalidArgs(() => parseTargetAnnotationV1Payload(json), /4096-byte payload cap/); +}); + +test('parser rejects more than 8 ancestry entries', () => { + const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY + 1 }, () => ({ role: 'view' })); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', ancestry, verification: 'verified' }), + ), + /8-entry cap/, + ); +}); + +test('truncateToUtf8Bytes never splits a surrogate pair', () => { + const emoji = '\u{1F600}'; // 4 UTF-8 bytes, a surrogate pair in UTF-16 + const truncated = truncateToUtf8Bytes(`ab${emoji}`, 3); + assert.equal(Buffer.byteLength(truncated, 'utf8') <= 3, true); + // The budget (3 bytes) fits "ab" but not the 4-byte emoji — the whole + // surrogate pair must be dropped together, never split. + assert.equal(truncated, 'ab'); + assert.equal(/[\ud800-\udbff]$/.test(truncated), false); +}); + +// --------------------------------------------------------------------------- +// Malformed / unbound annotations +// --------------------------------------------------------------------------- + +test('parser rejects non-JSON payloads', () => { + assertInvalidArgs(() => parseTargetAnnotationV1Payload('{not json'), /valid JSON/); +}); + +test('parser rejects a JSON array or scalar payload', () => { + assertInvalidArgs(() => parseTargetAnnotationV1Payload('[]')); + assertInvalidArgs(() => parseTargetAnnotationV1Payload('"button"')); +}); + +test('parser rejects a wrong-typed known field', () => { + assertInvalidArgs(() => + parseTargetAnnotationV1Payload(JSON.stringify({ role: 42, verification: 'verified' })), + ); +}); + +test('parser rejects an invalid verification value', () => { + assertInvalidArgs(() => + parseTargetAnnotationV1Payload(JSON.stringify({ role: 'button', verification: 'maybe' })), + ); +}); + +test('parser rejects a negative or non-integer sibling/viewportOrder', () => { + assertInvalidArgs(() => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', sibling: -1, verification: 'verified' }), + ), + ); + assertInvalidArgs(() => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', viewportOrder: 1.5, verification: 'verified' }), + ), + ); +}); + +// --------------------------------------------------------------------------- +// rect is diagnostic only: parsed, bounded, but never a comparison input at +// this parser layer (there is no comparator here yet — decision 3's +// enforcement lands in a later migration step — this just proves the parser +// accepts/round-trips it as inert data). +// --------------------------------------------------------------------------- + +test('rect parses and round trips but carries no comparison semantics here', () => { + const evidence = baseEvidence({ rect: { x: 10, y: 20, width: 30, height: 40 } }); + const parsed = parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(evidence)); + assert.deepEqual(parsed.rect, { x: 10, y: 20, width: 30, height: 40 }); +}); + +test('parser rejects a malformed rect', () => { + assertInvalidArgs(() => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', rect: { x: 1, y: 2 }, verification: 'verified' }), + ), + ); +}); + +// --------------------------------------------------------------------------- +// Classification core (decision 3 "Replay-time verification" paths 2-6): +// this is the exact function the writer's record-time self-check calls, and +// what a future replay-enforcement step will reuse — so duplicate/unverifiable +// evidence is exercised directly and generically here. +// --------------------------------------------------------------------------- + +test('classifyTargetBindingMatch path 2: matchCount 0 is unverifiable (selector-miss)', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: [], + identitySetRefs: [], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 2, outcome: 'unverifiable', reason: 'selector-miss' }); +}); + +test('classifyTargetBindingMatch path 3: empty identity set is unverifiable', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: [], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 3, outcome: 'unverifiable', reason: 'identity-set-empty' }); +}); + +test('classifyTargetBindingMatch path 4: unique identity-set member equal to winner is verified', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: ['e1'], + siblingMatchRefs: ['e1'], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 4, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 5: unique identity-set member that is NOT the winner (unique-but-wrong rebind)', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: ['e9'], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }); +}); + +test('classifyTargetBindingMatch path 6: duplicate identity resolved by a unique sibling match', () => { + const verified = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1'], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, + }); + assert.deepEqual(verified, { path: 6, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 6: same child index recurring under different parents falls through sibling, resolved by region-scoped viewportOrder', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + // Both members are "child 0" under different parents — sibling alone + // does not isolate. + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs: ['e2', 'e1'], + viewportCandidateRef: 'e1', + }); + assert.deepEqual(result, { path: 6, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 6: a scroll region that no longer exists falls through to unverifiable, never a silent pick', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs: undefined, // region unavailable + viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); +}); + +test('classifyTargetBindingMatch path 6: an out-of-range recorded viewportOrder falls through to unverifiable', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs: ['e1', 'e2'], + viewportCandidateRef: undefined, // ordinal was out of range + }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); +}); + +test('classifyTargetBindingMatch path 6: viewportOrder resolves to a DIFFERENT node than the winner is unverifiable, never a silent pick', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs: ['e1', 'e2'], + viewportCandidateRef: 'e2', + }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); +}); diff --git a/src/replay/script-formatting.ts b/src/replay/script-formatting.ts index 19453fc7b..ca76c1cda 100644 --- a/src/replay/script-formatting.ts +++ b/src/replay/script-formatting.ts @@ -6,6 +6,7 @@ import { appendScreenshotActionScriptArgs, appendSnapshotActionScriptArgs, } from './script-utils.ts'; +import { formatTargetAnnotationCommentLine } from './target-identity.ts'; import type { SessionAction } from '../daemon/types.ts'; export function formatPortableActionLine( @@ -30,3 +31,16 @@ export function formatPortableActionLine( } return parts.join(' '); } + +/** + * ADR 0012 decision 3: the `# agent-device:target-v1 {...}` annotation line + * that must immediately precede this action's line (no blank/intervening + * line — decision 3's binding rule), or `[]` when the action carries no + * target evidence. Shared by the live session-script writer + * (`src/daemon/session-script-writer.ts`) and `writeReplayScript`'s + * read-then-rewrite preservation (`script.ts`) so both emit the exact same + * canonical form. + */ +export function formatTargetAnnotationLines(action: SessionAction): string[] { + return action.targetEvidence ? [formatTargetAnnotationCommentLine(action.targetEvidence)] : []; +} diff --git a/src/replay/script.ts b/src/replay/script.ts index e9feb4e4d..1e8b5f876 100644 --- a/src/replay/script.ts +++ b/src/replay/script.ts @@ -5,7 +5,7 @@ import { readScreenshotScriptFlag } from '../contracts/screenshot.ts'; import type { DeviceTarget, PlatformSelector } from '../kernel/device.ts'; import { PLATFORM_SELECTORS, publicPlatformString } from '../kernel/device.ts'; import { parseReplayOpenFlags } from './open-script.ts'; -import { formatPortableActionLine } from './script-formatting.ts'; +import { formatPortableActionLine, formatTargetAnnotationLines } from './script-formatting.ts'; import type { SessionAction, SessionState } from '../daemon/types.ts'; import { formatScriptStringLiteral, @@ -14,6 +14,7 @@ import { parseReplayRuntimeFlags, stripRecordedRefGeneration, } from './script-utils.ts'; +import { parseTargetAnnotationCommentLine } from './target-identity.ts'; import { REPLAY_VAR_KEY_RE } from './vars.ts'; // Replay metadata `context platform=` lines support every accepted `--platform` @@ -41,15 +42,45 @@ export type ParsedReplayScript = { actionLines: number[]; }; +type PendingTargetAnnotation = { evidence: SessionAction['targetEvidence']; line: number }; + +// fallow-ignore-next-line complexity export function parseReplayScriptDetailed(script: string): ParsedReplayScript { const actions: SessionAction[] = []; const actionLines: number[] = []; const lines = script.split(/\r?\n/); let sawAction = false; + let pending: PendingTargetAnnotation | undefined; + + const rejectUnbound = (index: number, why: string): never => { + throw new AppError( + 'INVALID_ARGS', + // pending is only read here when set, right after the guard below. + `target-v1 annotation on line ${pending!.line} must be immediately followed by its action line (line ${index + 1} ${why}).`, + ); + }; + for (const [index, rawLine] of lines.entries()) { const trimmed = rawLine.trim(); - if (trimmed.length === 0 || trimmed.startsWith('#')) continue; + if (trimmed.length === 0) { + if (pending) rejectUnbound(index, 'is blank'); + continue; + } + if (trimmed.startsWith('#')) { + const annotation = parseTargetAnnotationCommentLine(trimmed); + if (annotation.kind === 'v1') { + if (pending) rejectUnbound(index, 'is another target-v1 annotation'); + pending = { evidence: annotation.evidence, line: index + 1 }; + continue; + } + // A plain comment or an unknown future `target-vN` is an ordinary + // comment to this (v1) reader — but it is still an intervening line + // for any annotation still pending binding. + if (pending) rejectUnbound(index, 'is a comment'); + continue; + } if (isReplayEnvLine(trimmed)) { + if (pending) rejectUnbound(index, 'is an env directive'); if (sawAction) { throw new AppError( 'INVALID_ARGS', @@ -59,11 +90,24 @@ export function parseReplayScriptDetailed(script: string): ParsedReplayScript { continue; } const parsed = parseReplayScriptLine(rawLine); - if (!parsed) continue; + if (!parsed) { + if (pending) rejectUnbound(index, 'did not parse as an action'); + continue; + } + if (pending) { + parsed.targetEvidence = pending.evidence; + pending = undefined; + } actions.push(parsed); actionLines.push(index + 1); sawAction = true; } + if (pending) { + throw new AppError( + 'INVALID_ARGS', + `target-v1 annotation on line ${pending.line} must be immediately followed by its action line (end of script reached).`, + ); + } return { actions, actionLines }; } @@ -475,6 +519,10 @@ export function writeReplayScript( ); } for (const action of actions) { + // ADR 0012 decision 3: a writer that reads then rewrites a script (this + // heal/`--update` path) must preserve v1 annotations in canonical form, + // never silently discard them. + lines.push(...formatTargetAnnotationLines(action)); lines.push(formatReplayActionLine(action)); } const serialized = `${lines.join('\n')}\n`; diff --git a/src/replay/target-identity.ts b/src/replay/target-identity.ts new file mode 100644 index 000000000..eb8d54512 --- /dev/null +++ b/src/replay/target-identity.ts @@ -0,0 +1,459 @@ +/** + * ADR 0012 decision 3: versioned `.ad` target-binding evidence. + * + * This module is the shared, tree-agnostic spine consumed by both the writer + * (`src/daemon/session-target-evidence.ts`, which has an actual snapshot tree + * to compute the payload from) and the parser (`src/replay/script.ts`, which + * only ever sees the JSON comment text). It owns: + * + * - the wire type and its canonical JSON.stringify field order, + * - Unicode/whitespace normalization and the 256-byte-field / 4 KiB-payload + * caps, + * - parsing + validation of a `target-v1` comment payload (`INVALID_ARGS` on + * anything malformed or oversized — the parser REJECTS, it never + * truncates), and + * - the record/replay-shared classification core (decision 3's six-path + * verification algorithm), expressed generically over node refs so it has + * no dependency on `SnapshotNode`/the daemon tree. The writer's record-time + * self-check (decision 3, record-time write step 5) calls this; a future + * migration step (4) reuses the exact same function at replay time so the + * two can never drift. + * + * Nothing here enforces anything at replay time yet (migration step 3 is + * parser/writer only) — parsed evidence is inert until step 4 lands. + */ + +import { AppError } from '../kernel/errors.ts'; + +const TARGET_ANNOTATION_TAG = 'agent-device:target-v1'; +// Captures the version and the REST of the line verbatim, even when it isn't +// well-formed JSON — a line that claims the tag but carries garbage must be +// rejected as a malformed v1 annotation (INVALID_ARGS), not silently treated +// as an ordinary comment because it failed to look like `{...}` up front. +const TARGET_ANNOTATION_LINE_RE = /^#\s*agent-device:target-v(\d+)(?:\s+(.*))?$/; + +export const TARGET_ANNOTATION_MAX_FIELD_BYTES = 256; +export const TARGET_ANNOTATION_MAX_PAYLOAD_BYTES = 4096; +export const TARGET_ANNOTATION_MAX_ANCESTRY = 8; + +export type TargetAncestryEntry = { role: string; label?: string }; +export type TargetScrollRegion = { role: string; id?: string; label?: string }; +export type TargetRect = { x: number; y: number; width: number; height: number }; +export type TargetVerification = 'verified' | 'unverifiable'; + +export type TargetAnnotationV1 = { + id?: string; + role: string; + label?: string; + ancestry: TargetAncestryEntry[]; + sibling: number; + viewportOrder: number; + scrollRegion?: TargetScrollRegion; + rect?: TargetRect; + verification: TargetVerification; +}; + +// --------------------------------------------------------------------------- +// Normalization (decision 3 "Normalization"): all strings NFC; `label` fields +// additionally trim and collapse internal whitespace runs. A string that is +// empty after normalization is omitted (writer) / treated as absent +// (comparator). Applies to every string field: top-level id/role/label, +// ancestry entry role/label, and scrollRegion role/id/label. +// --------------------------------------------------------------------------- + +function nfc(value: string): string { + return value.normalize('NFC'); +} + +/** id/role fields: NFC only (never trimmed/collapsed — see decision 3). */ +export function normalizeIdentifierField(value: string | undefined): string | undefined { + if (value === undefined) return undefined; + const normalized = nfc(value); + return normalized.length > 0 ? normalized : undefined; +} + +/** `role` is always required (may be the empty string per decision 3's ancestry note). */ +export function normalizeRoleField(value: string): string { + return nfc(value); +} + +/** label fields: NFC, trim, collapse internal whitespace runs to one space. */ +export function normalizeLabelField(value: string | undefined): string | undefined { + if (value === undefined) return undefined; + const collapsed = nfc(value) + .trim() + .replace(/\s+/g, ' '); + return collapsed.length > 0 ? collapsed : undefined; +} + +export function utf8ByteLength(value: string): number { + return Buffer.byteLength(value, 'utf8'); +} + +/** + * Writer-side field truncation to the 256-byte cap ("per-field truncation", + * decision 3's writer-parser invariant). Trims on a code-point boundary so a + * surrogate pair is never split. The parser never calls this — it REJECTS + * oversized fields instead (see `parseTargetAnnotationV1Payload`). + */ +export function truncateToUtf8Bytes(value: string, maxBytes: number): string { + if (utf8ByteLength(value) <= maxBytes) return value; + let end = value.length; + while (end > 0 && utf8ByteLength(value.slice(0, end)) > maxBytes) { + end -= 1; + } + if (end > 0) { + const code = value.charCodeAt(end - 1); + if (code >= 0xd8_00 && code <= 0xdb_ff) end -= 1; // don't split a surrogate pair + } + return value.slice(0, end); +} + +// --------------------------------------------------------------------------- +// Canonical serialization (decision 3's exact field order + nested-object +// key order from the example payload). +// --------------------------------------------------------------------------- + +function buildCanonicalTargetAnnotationObject( + evidence: TargetAnnotationV1, +): Record { + const obj: Record = {}; + if (evidence.id !== undefined) obj.id = evidence.id; + obj.role = evidence.role; + if (evidence.label !== undefined) obj.label = evidence.label; + obj.ancestry = evidence.ancestry.map(buildAncestryEntryObject); + obj.sibling = evidence.sibling; + obj.viewportOrder = evidence.viewportOrder; + if (evidence.scrollRegion) obj.scrollRegion = buildScrollRegionObject(evidence.scrollRegion); + if (evidence.rect) obj.rect = buildRectObject(evidence.rect); + obj.verification = evidence.verification; + return obj; +} + +function buildAncestryEntryObject(entry: TargetAncestryEntry): Record { + const obj: Record = { role: entry.role }; + if (entry.label !== undefined) obj.label = entry.label; + return obj; +} + +function buildScrollRegionObject(region: TargetScrollRegion): Record { + const obj: Record = { role: region.role }; + if (region.id !== undefined) obj.id = region.id; + if (region.label !== undefined) obj.label = region.label; + return obj; +} + +function buildRectObject(rect: TargetRect): Record { + return { x: rect.x, y: rect.y, width: rect.width, height: rect.height }; +} + +export function serializeTargetAnnotationV1(evidence: TargetAnnotationV1): string { + return JSON.stringify(buildCanonicalTargetAnnotationObject(evidence)); +} + +export function formatTargetAnnotationCommentLine(evidence: TargetAnnotationV1): string { + return `# ${TARGET_ANNOTATION_TAG} ${serializeTargetAnnotationV1(evidence)}`; +} + +// --------------------------------------------------------------------------- +// Parsing (decision 3's parser bullet + "Replay-time verification" intro). +// --------------------------------------------------------------------------- + +export type TargetAnnotationLineParseResult = + | { kind: 'none' } + | { kind: 'future-version' } + | { kind: 'v1'; evidence: TargetAnnotationV1 }; + +/** + * Recognizes a `# agent-device:target-vN {...}` comment line. `N !== 1` is an + * ordinary comment to this (v1) reader, per decision 3: "An unknown future + * `target-vN` comment is an ordinary comment to a v1 reader." Any other `#` + * line (including one that merely mentions the tag inside prose) is `none`. + */ +export function parseTargetAnnotationCommentLine(rawLine: string): TargetAnnotationLineParseResult { + const trimmed = rawLine.trim(); + if (!trimmed.startsWith('#')) return { kind: 'none' }; + const match = TARGET_ANNOTATION_LINE_RE.exec(trimmed); + if (!match) return { kind: 'none' }; + const version = Number(match[1]); + if (version !== 1) return { kind: 'future-version' }; + const evidence = parseTargetAnnotationV1Payload((match[2] ?? '').trim()); + return { kind: 'v1', evidence }; +} + +/** + * Parses and validates the JSON payload of a `target-v1` annotation. + * Accepts known fields in any order, ignores unknown fields, NFC-normalizes + * known strings, and rejects malformed/oversized payloads with + * `INVALID_ARGS` (decision 3: "The parser rejects a v1 annotation exceeding + * these bounds with INVALID_ARGS"). + */ +// fallow-ignore-next-line complexity +export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotationV1 { + if (utf8ByteLength(jsonText) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES) { + throw new AppError( + 'INVALID_ARGS', + `target-v1 annotation exceeds the ${TARGET_ANNOTATION_MAX_PAYLOAD_BYTES}-byte payload cap.`, + ); + } + let parsed: unknown; + try { + parsed = JSON.parse(jsonText); + } catch { + throw new AppError('INVALID_ARGS', 'target-v1 annotation is not valid JSON.'); + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + throw new AppError('INVALID_ARGS', 'target-v1 annotation must be a JSON object.'); + } + const raw = parsed as Record; + + const role = parseRequiredRoleField(raw.role); + const id = parseOptionalIdentifierField(raw.id, 'id'); + const label = parseOptionalLabelField(raw.label, 'label'); + const ancestry = parseAncestryField(raw.ancestry); + const sibling = parseNonNegativeIntField(raw.sibling, 'sibling', 0); + const viewportOrder = parseNonNegativeIntField(raw.viewportOrder, 'viewportOrder', 0); + const scrollRegion = parseScrollRegionField(raw.scrollRegion); + const rect = parseRectField(raw.rect); + const verification = parseVerificationField(raw.verification); + + return { + ...(id !== undefined ? { id } : {}), + role, + ...(label !== undefined ? { label } : {}), + ancestry, + sibling, + viewportOrder, + ...(scrollRegion ? { scrollRegion } : {}), + ...(rect ? { rect } : {}), + verification, + }; +} + +function parseRequiredRoleField(value: unknown): string { + if (value === undefined) return ''; + if (typeof value !== 'string') { + throw new AppError('INVALID_ARGS', 'target-v1 "role" must be a string.'); + } + return boundField(normalizeRoleField(value), 'role'); +} + +function parseOptionalIdentifierField(value: unknown, field: 'id'): string | undefined { + if (value === undefined) return undefined; + if (typeof value !== 'string') { + throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a string.`); + } + const normalized = normalizeIdentifierField(value); + return normalized === undefined ? undefined : boundField(normalized, field); +} + +function parseOptionalLabelField(value: unknown, field: string): string | undefined { + if (value === undefined) return undefined; + if (typeof value !== 'string') { + throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a string.`); + } + const normalized = normalizeLabelField(value); + return normalized === undefined ? undefined : boundField(normalized, field); +} + +function boundField(value: string, field: string): string { + if (utf8ByteLength(value) > TARGET_ANNOTATION_MAX_FIELD_BYTES) { + throw new AppError( + 'INVALID_ARGS', + `target-v1 "${field}" exceeds the ${TARGET_ANNOTATION_MAX_FIELD_BYTES}-byte field cap.`, + ); + } + return value; +} + +function parseAncestryField(value: unknown): TargetAncestryEntry[] { + if (value === undefined) return []; + if (!Array.isArray(value)) { + throw new AppError('INVALID_ARGS', 'target-v1 "ancestry" must be an array.'); + } + if (value.length > TARGET_ANNOTATION_MAX_ANCESTRY) { + throw new AppError( + 'INVALID_ARGS', + `target-v1 "ancestry" exceeds the ${TARGET_ANNOTATION_MAX_ANCESTRY}-entry cap.`, + ); + } + return value.map((entry, index) => parseAncestryEntry(entry, index)); +} + +function parseAncestryEntry(entry: unknown, index: number): TargetAncestryEntry { + if (typeof entry !== 'object' || entry === null || Array.isArray(entry)) { + throw new AppError('INVALID_ARGS', `target-v1 "ancestry[${index}]" must be an object.`); + } + const record = entry as Record; + const role = parseRequiredRoleField(record.role); + const label = parseOptionalLabelField(record.label, `ancestry[${index}].label`); + return { role, ...(label !== undefined ? { label } : {}) }; +} + +function parseNonNegativeIntField(value: unknown, field: string, fallback: number): number { + if (value === undefined) return fallback; + if (typeof value !== 'number' || !Number.isSafeInteger(value) || value < 0) { + throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a non-negative integer.`); + } + return value; +} + +function parseScrollRegionField(value: unknown): TargetScrollRegion | undefined { + if (value === undefined) return undefined; + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new AppError('INVALID_ARGS', 'target-v1 "scrollRegion" must be an object.'); + } + const record = value as Record; + const role = parseRequiredRoleField(record.role); + const id = parseOptionalIdentifierField(record.id, 'id'); + const label = parseOptionalLabelField(record.label, 'scrollRegion.label'); + return { + role, + ...(id !== undefined ? { id } : {}), + ...(label !== undefined ? { label } : {}), + }; +} + +function parseRectField(value: unknown): TargetRect | undefined { + if (value === undefined) return undefined; + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new AppError('INVALID_ARGS', 'target-v1 "rect" must be an object.'); + } + const record = value as Record; + const x = parseFiniteNumberField(record.x, 'rect.x'); + const y = parseFiniteNumberField(record.y, 'rect.y'); + const width = parseFiniteNumberField(record.width, 'rect.width'); + const height = parseFiniteNumberField(record.height, 'rect.height'); + return { x, y, width, height }; +} + +function parseFiniteNumberField(value: unknown, field: string): number { + if (typeof value !== 'number' || !Number.isFinite(value)) { + throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a finite number.`); + } + return value; +} + +function parseVerificationField(value: unknown): TargetVerification { + if (value === 'verified' || value === 'unverifiable') return value; + throw new AppError( + 'INVALID_ARGS', + 'target-v1 "verification" must be "verified" or "unverifiable".', + ); +} + +// --------------------------------------------------------------------------- +// Local identity + ancestry-prefix matching (decision 3 "Local identity" / +// "Ancestry"). Pure over the small structural shapes above — no tree +// dependency, so both the writer (over `SnapshotNode`-derived values) and a +// future replay verifier can share it verbatim. +// --------------------------------------------------------------------------- + +export type LocalIdentity = { id?: string; role: string; label?: string }; + +/** + * Decision 3 "Local identity": id match wins outright when the recording + * carries one ("a recorded id never matches a node without that id"); with + * no recorded id, role+label must both match (label absent on both sides + * counts as equal; present on exactly one side is a mismatch). + */ +export function matchesLocalIdentity(candidate: LocalIdentity, recorded: LocalIdentity): boolean { + if (recorded.id !== undefined) return candidate.id === recorded.id; + return candidate.role === recorded.role && candidate.label === recorded.label; +} + +/** + * Decision 3 "Ancestry": leaf-anchored prefix match. `observed` must be at + * least as long as `recorded`; each recorded entry's role must match exactly + * and, when the recorded entry carries a label, so must the observed one (an + * absent recorded label is unconstrained). + */ +export function matchesAncestryPrefix( + observed: readonly TargetAncestryEntry[], + recorded: readonly TargetAncestryEntry[], +): boolean { + if (observed.length < recorded.length) return false; + for (const [index, entry] of recorded.entries()) { + const candidate = observed[index]; + if (!candidate) return false; + if (candidate.role !== entry.role) return false; + if (entry.label !== undefined && candidate.label !== entry.label) return false; + } + return true; +} + +// --------------------------------------------------------------------------- +// Classification core (decision 3 "Replay-time verification", paths 2-6; +// path 1 — a recorded-`unverifiable` annotation — is checked by the caller +// before any resolution and never reaches this function). Expressed +// generically over node refs: the caller resolves matches/identity-set/ +// sibling-filter/region partition using its own tree, then hands the +// resulting ref sets here for the total-order classification. This is what +// both the writer's record-time self-check (decision 3 step 5) and — in a +// future migration step — replay-time enforcement call, so the two decision +// points can never diverge. +// --------------------------------------------------------------------------- + +export type TargetBindingClassificationInput = { + /** The node the resolver actually picked. */ + winnerRef: string; + /** `matchCount`'s domain: nodes matching the recorded selector/ref. */ + matchedRefs: readonly string[]; + /** Members of `matchedRefs` sharing the recorded local identity + ancestry prefix (decision 3 set I). */ + identitySetRefs: readonly string[]; + /** Members of `identitySetRefs` whose same-parent sibling ordinal equals the recorded `sibling`. */ + siblingMatchRefs: readonly string[]; + /** + * Members of `identitySetRefs` in the partition whose scroll-region key + * equals the recorded `scrollRegion` (the *none* partition when none was + * recorded). `undefined` when that region no longer exists at all. + */ + regionMemberRefs: readonly string[] | undefined; + /** `regionMemberRefs` ordered by decision 3's viewport ordering; the ref at the recorded `viewportOrder`, if in range. */ + viewportCandidateRef: string | undefined; +}; + +export type TargetBindingClassification = + | { path: 2; outcome: 'unverifiable'; reason: 'selector-miss' } + | { path: 3; outcome: 'unverifiable'; reason: 'identity-set-empty' } + | { path: 4; outcome: 'verified' } + | { path: 5; outcome: 'unverifiable'; reason: 'unique-but-wrong' } + | { path: 6; outcome: 'verified' | 'unverifiable' }; + +/** + * Decision 3 "Replay-time verification", paths 2-6. `matchCount == 0` (path + * 2), an empty identity set (path 3), a unique identity-set member that is or + * isn't the winner (paths 4/5), and the sibling → region-scoped-viewportOrder + * disambiguation cascade (path 6) — falling through to unverifiable, never a + * silent pick, exactly as decision 3 specifies. + */ +export function classifyTargetBindingMatch( + input: TargetBindingClassificationInput, +): TargetBindingClassification { + if (input.matchedRefs.length === 0) { + return { path: 2, outcome: 'unverifiable', reason: 'selector-miss' }; + } + if (input.identitySetRefs.length === 0) { + return { path: 3, outcome: 'unverifiable', reason: 'identity-set-empty' }; + } + if (input.identitySetRefs.length === 1) { + return input.identitySetRefs[0] === input.winnerRef + ? { path: 4, outcome: 'verified' } + : { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }; + } + if (input.siblingMatchRefs.length === 1) { + return input.siblingMatchRefs[0] === input.winnerRef + ? { path: 6, outcome: 'verified' } + : { path: 6, outcome: 'unverifiable' }; + } + if ( + input.regionMemberRefs !== undefined && + input.regionMemberRefs.length > 0 && + input.viewportCandidateRef !== undefined + ) { + return input.viewportCandidateRef === input.winnerRef + ? { path: 6, outcome: 'verified' } + : { path: 6, outcome: 'unverifiable' }; + } + return { path: 6, outcome: 'unverifiable' }; +} From 11b3e8801a68444c1fbe518f70f9562b2c3d542e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 12:25:03 +0200 Subject: [PATCH 2/9] fix: bound candidate identity before self-check comparison The identity-set scan in computeTargetEvidence compared an untruncated candidate identity against the 256-byte-bounded recorded identity, so a node whose own id/label exceeds the field cap could fail to match itself, corrupting the record-time self-check. Compare through the same bounding on both sides instead. --- .../__tests__/session-target-evidence.test.ts | 26 +++++++++++++++++++ src/daemon/session-target-evidence.ts | 8 +++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts index 2ace59065..96e069430 100644 --- a/src/daemon/__tests__/session-target-evidence.test.ts +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -319,3 +319,29 @@ test('computeTargetEvidence: real-capture-shaped tree (undefined hittable, anony const json = serializeTargetAnnotationV1(evidence); assert.deepEqual(parseTargetAnnotationV1Payload(json), evidence); }); + +// --------------------------------------------------------------------------- +// Self-consistency: a node whose own id/label exceeds the 256-byte field cap +// must still match ITSELF during the identity-set scan. The recorded +// identity is truncated (writer-parser invariant); comparing an untruncated +// candidate against it would spuriously exclude the winner from its own +// identity set and produce a false 'unverifiable'. +// --------------------------------------------------------------------------- + +test('computeTargetEvidence: a node with an over-cap id still matches itself and is verified', () => { + const overCapId = 'save-'.repeat(100); // 500 bytes, well over the 256-byte field cap + const nodes = toSnapshotNodes([ + { + index: 0, + type: 'Button', + identifier: overCapId, + label: 'Save', + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 0, + }, + ]); + const evidence = computeTargetEvidence({ node: nodes[0]!, nodes }); + assert.ok(evidence); + assert.equal(evidence.id, overCapId.slice(0, TARGET_ANNOTATION_MAX_FIELD_BYTES)); + assert.equal(evidence.verification, 'verified'); +}); diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index 600b9ce04..ccda8a226 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -235,7 +235,13 @@ function computeDisambiguationDomain(params: { // Step 2: all nodes sharing the winner's local identity with a matching // leaf-anchored ancestry prefix. const identitySet = nodes.filter((candidate) => { - if (!matchesLocalIdentity(computeLocalIdentity(candidate), identity)) return false; + // Compare through the SAME 256-byte bounding the recorded `identity` and + // ancestry entries already went through — otherwise a node whose own + // id/label exceeds the field cap would spuriously fail to match ITSELF + // (the recorded value is truncated; the raw candidate value is not), + // corrupting both this self-check and, if the replay-time matcher ever + // skipped the same bounding, a real node's identity check later. + if (!matchesLocalIdentity(boundedLocalIdentity(candidate), identity)) return false; const observedAncestry = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); return matchesAncestryPrefix(observedAncestry, ancestry); }); From 32731c1f31df272e3727dda1b170bc57281d0776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 13:00:55 +0200 Subject: [PATCH 3/9] =?UTF-8?q?fix:=20address=20PR=20#1196=20review=20?= =?UTF-8?q?=E2=80=94=20contract=20boundary,=20formatting,=20path-6=20reaso?= =?UTF-8?q?ns,=20worst-case=20sizing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - native-ref contract test: deliberately updated to assert the new ADR 0012 boundary — the preflight's node/preActionNodes ride the INTERNAL runtime result and visualization payload only; the public responseData never carries node/preActionNodes/targetEvidence (asserted directly against buildInteractionResponseData). - oxfmt formatting on all touched files. - classifyTargetBindingMatch path 6 now distinguishes decision 3's two spec-distinct outcomes: a signal isolating a member that differs from the winner ('signal-isolated-wrong', the paths-4/5 comparison class, future identity-mismatch) vs true fall-through ('no-signal-isolation', future identity-unverifiable), so step 4 can consume the reasons directly. - writer reduction loop sizes every candidate against the worst-case verification value ("unverifiable" is 4 bytes longer than "verified"), so a fail-closed self-check downgrade can never push an accepted payload over the 4 KiB cap; pinned by a 1-byte-granularity sweep across the boundary that fails against the old placeholder-sized check. --- .../__tests__/session-target-evidence.test.ts | 64 ++++++++++++++++++- src/daemon/handlers/interaction-common.ts | 2 +- src/daemon/session-target-evidence.ts | 24 +++++-- src/replay/__tests__/script.test.ts | 14 ++-- src/replay/__tests__/target-identity.test.ts | 52 +++++++++------ src/replay/target-identity.ts | 29 +++++++-- .../native-ref.contract.test.ts | 25 +++++++- 7 files changed, 170 insertions(+), 40 deletions(-) diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts index 96e069430..f46b81ec5 100644 --- a/src/daemon/__tests__/session-target-evidence.test.ts +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -187,7 +187,10 @@ test('computeTargetEvidence: max-size labels across K=8 ancestors reduce ancestr const evidence = computeTargetEvidence({ node: winner, nodes }); assert.ok(evidence); assert.ok(evidence.ancestry.length <= TARGET_ANNOTATION_MAX_ANCESTRY); - assert.ok(evidence.ancestry.length < TARGET_ANNOTATION_MAX_ANCESTRY, 'ancestry must have been reduced'); + assert.ok( + evidence.ancestry.length < TARGET_ANNOTATION_MAX_ANCESTRY, + 'ancestry must have been reduced', + ); // Nearest-ancestor-first: the first (kept) entries are the leaf-side ones. assert.equal(evidence.ancestry[0]?.label, maxLabel); @@ -345,3 +348,62 @@ test('computeTargetEvidence: a node with an over-cap id still matches itself and assert.equal(evidence.id, overCapId.slice(0, TARGET_ANNOTATION_MAX_FIELD_BYTES)); assert.equal(evidence.verification, 'verified'); }); + +// --------------------------------------------------------------------------- +// Worst-case verification sizing: "unverifiable" serializes 4 bytes longer +// than "verified". The reduction loop must size each candidate against the +// worst-case value so a fail-closed self-check downgrade can never push an +// already-accepted payload over the 4 KiB cap. The window is only 4 bytes +// wide, so sweep a tunable root-side label length across the boundary — some +// length in the sweep necessarily lands inside the window, where a +// placeholder-sized ("verified") check would accept a payload whose +// downgraded form overflows. +// --------------------------------------------------------------------------- + +test('computeTargetEvidence: reduction sizes against the worst-case verification value across the 4 KiB boundary', () => { + const maxField = 'x'.repeat(TARGET_ANNOTATION_MAX_FIELD_BYTES); + + const buildChain = (tunableLabelLength: number): SnapshotNode[] => { + const raw: RawSnapshotNode[] = []; + // Root (index 0) .. leaf's parent (index 7): 8 ancestors total (K=8). + // Root-side entries are dropped first, so the tunable label sits at the + // root to sweep total payload size in 1-byte steps. + raw.push({ index: 0, type: 'Window', label: 'w'.repeat(tunableLabelLength), depth: 0 }); + raw.push({ index: 1, type: 'View', label: 'v', depth: 1, parentIndex: 0 }); + for (let depth = 2; depth < 8; depth += 1) { + raw.push({ index: depth, type: maxField, label: maxField, depth, parentIndex: depth - 1 }); + } + raw.push({ + index: 8, + type: maxField, + label: maxField, + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 8, + parentIndex: 7, + }); + return toSnapshotNodes(raw); + }; + + let sawFullAncestry = false; + let sawReducedAncestry = false; + for (let tunable = 0; tunable <= TARGET_ANNOTATION_MAX_FIELD_BYTES; tunable += 1) { + const nodes = buildChain(tunable); + const evidence = computeTargetEvidence({ node: nodes.at(-1)!, nodes }); + assert.ok(evidence); + if (evidence.ancestry.length === TARGET_ANNOTATION_MAX_ANCESTRY) sawFullAncestry = true; + else sawReducedAncestry = true; + // The emitted payload must fit even re-serialized with the longer + // verification value — the invariant the worst-case sizing guarantees. + const worstCase = serializeTargetAnnotationV1({ ...evidence, verification: 'unverifiable' }); + assert.ok( + utf8ByteLength(worstCase) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES, + `worst-case payload overflows at tunable label length ${tunable}`, + ); + // And the parser accepts the writer's actual output, as always. + parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(evidence)); + } + // The sweep must actually cross the reduction boundary for the window to + // have been exercised. + assert.ok(sawFullAncestry, 'sweep never produced an unreduced payload — fixture too large'); + assert.ok(sawReducedAncestry, 'sweep never forced a reduction — fixture too small'); +}); diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index 8ae771cb0..6f6792c61 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -114,4 +114,4 @@ function extractTargetEvidenceForRecording( return { result: rest, targetEvidence: undefined }; } return { result: rest, targetEvidence: computeTargetEvidence({ node, nodes: preActionNodes }) }; -} \ No newline at end of file +} diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index ccda8a226..f2a06cb9b 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -88,7 +88,17 @@ export function computeTargetEvidence(params: { for (let ancestryLength = fullAncestry.length; ancestryLength >= floor; ancestryLength -= 1) { const { candidate, domain } = buildCandidate(ancestryLength); - if (utf8ByteLength(serializeTargetAnnotationV1(candidate)) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES) { + // Size against the WORST-CASE verification value ("unverifiable" is 4 + // serialized bytes longer than "verified") so the payload provably fits + // no matter what the self-check below returns — otherwise a payload + // within 4 bytes of the cap could pass this check as "verified" and then + // overflow once a fail-closed self-check downgraded it, violating the + // writer-parser invariant exactly in the rare capture-anomaly case it + // exists for. + if ( + utf8ByteLength(serializeTargetAnnotationV1({ ...candidate, verification: 'unverifiable' })) <= + TARGET_ANNOTATION_MAX_PAYLOAD_BYTES + ) { candidate.verification = runRecordTimeSelfCheck({ node, domain }); return candidate; } @@ -100,7 +110,9 @@ export function computeTargetEvidence(params: { // rect (never compared) as one last, spec-consistent reduction rather // than emit a payload the parser would reject. candidate.verification = 'unverifiable'; - if (utf8ByteLength(serializeTargetAnnotationV1(candidate)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES) { + if ( + utf8ByteLength(serializeTargetAnnotationV1(candidate)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES + ) { delete candidate.rect; } return candidate; @@ -152,8 +164,12 @@ function buildAncestryChain( while (current && !visited.has(current.index) && chain.length < limit) { visited.add(current.index); const identity = boundedLocalIdentity(current); - chain.push({ role: identity.role, ...(identity.label !== undefined ? { label: identity.label } : {}) }); - current = typeof current.parentIndex === 'number' ? byIndex.get(current.parentIndex) : undefined; + chain.push({ + role: identity.role, + ...(identity.label !== undefined ? { label: identity.label } : {}), + }); + current = + typeof current.parentIndex === 'number' ? byIndex.get(current.parentIndex) : undefined; } return chain; } diff --git a/src/replay/__tests__/script.test.ts b/src/replay/__tests__/script.test.ts index e75617d60..5c75a57fc 100644 --- a/src/replay/__tests__/script.test.ts +++ b/src/replay/__tests__/script.test.ts @@ -395,9 +395,11 @@ const SAVE_EVIDENCE_LINE = '# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[{"role":"toolbar","label":"Editor"},{"role":"window"}],"sibling":0,"viewportOrder":0,"scrollRegion":{"role":"scrollview","id":"editor-scroll"},"verification":"verified"}'; test('a target-v1 annotation immediately preceding an action line attaches to that action', () => { - const script = ['context platform=ios device=iPhone', SAVE_EVIDENCE_LINE, 'click @e12 "Save"'].join( - '\n', - ); + const script = [ + 'context platform=ios device=iPhone', + SAVE_EVIDENCE_LINE, + 'click @e12 "Save"', + ].join('\n'); const { actions } = parseReplayScriptDetailed(script); assert.equal(actions.length, 1); assert.deepEqual(actions[0]?.targetEvidence, SAVE_EVIDENCE); @@ -438,11 +440,7 @@ test('a malformed target-v1 payload is rejected as INVALID_ARGS, not silently dr }); test('an unknown future target-vN comment is an ordinary comment: no binding requirement, no evidence attached', () => { - const script = [ - '# agent-device:target-v2 {"whatever":true}', - '', - 'click @e12 "Save"', - ].join('\n'); + const script = ['# agent-device:target-v2 {"whatever":true}', '', 'click @e12 "Save"'].join('\n'); const { actions } = parseReplayScriptDetailed(script); assert.equal(actions.length, 1); assert.equal(actions[0]?.targetEvidence, undefined); diff --git a/src/replay/__tests__/target-identity.test.ts b/src/replay/__tests__/target-identity.test.ts index a9714adac..187dfbb96 100644 --- a/src/replay/__tests__/target-identity.test.ts +++ b/src/replay/__tests__/target-identity.test.ts @@ -22,10 +22,7 @@ function baseEvidence(overrides: Partial = {}): TargetAnnota id: 'save', role: 'button', label: 'Save', - ancestry: [ - { role: 'toolbar', label: 'Editor' }, - { role: 'window' }, - ], + ancestry: [{ role: 'toolbar', label: 'Editor' }, { role: 'window' }], sibling: 0, viewportOrder: 0, scrollRegion: { role: 'scrollview', id: 'editor-scroll' }, @@ -171,10 +168,7 @@ test('matchesAncestryPrefix accepts an observed chain that is a superset on the }); test('matchesAncestryPrefix rejects an inserted wrapper ancestor (structure is part of identity)', () => { - const recorded = [ - { role: 'toolbar', label: 'Editor' }, - { role: 'window' }, - ]; + const recorded = [{ role: 'toolbar', label: 'Editor' }, { role: 'window' }]; // A new wrapper view inserted directly above the target shifts every entry // one level down — the leaf-anchored prefix no longer matches. const observedWithInsertedWrapper = [ @@ -205,10 +199,7 @@ test('matchesLocalIdentity: a recorded id never matches a node without that id', test('matchesLocalIdentity: with no recorded id, role+label must both match, absent-absent counts as equal', () => { assert.equal(matchesLocalIdentity({ role: 'button' }, { role: 'button' }), true); - assert.equal( - matchesLocalIdentity({ role: 'button', label: 'Save' }, { role: 'button' }), - false, - ); + assert.equal(matchesLocalIdentity({ role: 'button', label: 'Save' }, { role: 'button' }), false); }); // --------------------------------------------------------------------------- @@ -249,7 +240,9 @@ test('parser rejects a payload exceeding the 4 KiB cap', () => { }); test('parser rejects more than 8 ancestry entries', () => { - const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY + 1 }, () => ({ role: 'view' })); + const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY + 1 }, () => ({ + role: 'view', + })); assertInvalidArgs( () => parseTargetAnnotationV1Payload( @@ -418,7 +411,7 @@ test('classifyTargetBindingMatch path 6: a scroll region that no longer exists f regionMemberRefs: undefined, // region unavailable viewportCandidateRef: undefined, }); - assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); }); test('classifyTargetBindingMatch path 6: an out-of-range recorded viewportOrder falls through to unverifiable', () => { @@ -430,11 +423,16 @@ test('classifyTargetBindingMatch path 6: an out-of-range recorded viewportOrder regionMemberRefs: ['e1', 'e2'], viewportCandidateRef: undefined, // ordinal was out of range }); - assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); }); -test('classifyTargetBindingMatch path 6: viewportOrder resolves to a DIFFERENT node than the winner is unverifiable, never a silent pick', () => { - const result = classifyTargetBindingMatch({ +test('classifyTargetBindingMatch path 6: a signal isolating a DIFFERENT node than the winner is the paths-4/5 comparison class, not fall-through', () => { + // Decision 3 path 6.i/6.ii: when a signal denotes exactly one member, + // "compare with W as in paths 4/5" — an isolated-but-different member is + // the same class as path 5's unique-but-wrong rebind (a future + // identity-mismatch), distinct from true no-isolation fall-through (a + // future identity-unverifiable with candidates). + const viaViewportOrder = classifyTargetBindingMatch({ winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], @@ -442,5 +440,23 @@ test('classifyTargetBindingMatch path 6: viewportOrder resolves to a DIFFERENT n regionMemberRefs: ['e1', 'e2'], viewportCandidateRef: 'e2', }); - assert.deepEqual(result, { path: 6, outcome: 'unverifiable' }); + assert.deepEqual(viaViewportOrder, { + path: 6, + outcome: 'unverifiable', + reason: 'signal-isolated-wrong', + }); + + const viaSibling = classifyTargetBindingMatch({ + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e2'], // sibling isolates e2, not the winner + regionMemberRefs: ['e1', 'e2'], + viewportCandidateRef: 'e1', + }); + assert.deepEqual(viaSibling, { + path: 6, + outcome: 'unverifiable', + reason: 'signal-isolated-wrong', + }); }); diff --git a/src/replay/target-identity.ts b/src/replay/target-identity.ts index eb8d54512..79bc152c0 100644 --- a/src/replay/target-identity.ts +++ b/src/replay/target-identity.ts @@ -80,9 +80,7 @@ export function normalizeRoleField(value: string): string { /** label fields: NFC, trim, collapse internal whitespace runs to one space. */ export function normalizeLabelField(value: string | undefined): string | undefined { if (value === undefined) return undefined; - const collapsed = nfc(value) - .trim() - .replace(/\s+/g, ' '); + const collapsed = nfc(value).trim().replace(/\s+/g, ' '); return collapsed.length > 0 ? collapsed : undefined; } @@ -413,12 +411,25 @@ export type TargetBindingClassificationInput = { viewportCandidateRef: string | undefined; }; +/** + * Decision 3 keeps two spec-distinct failure classes inside path 6, and the + * `reason` field preserves the distinction for migration step 4's divergence + * `kind` mapping: + * + * - a disambiguation signal ISOLATING exactly one member that differs from + * the winner is "compare with W as in paths 4/5" — the same class as path + * 5's unique-but-wrong rebind, i.e. a future `identity-mismatch` + * (`signal-isolated-wrong`); + * - neither signal isolating any member is the true fall-through — a future + * `identity-unverifiable` with up to 5 candidates (`no-signal-isolation`). + */ export type TargetBindingClassification = | { path: 2; outcome: 'unverifiable'; reason: 'selector-miss' } | { path: 3; outcome: 'unverifiable'; reason: 'identity-set-empty' } | { path: 4; outcome: 'verified' } | { path: 5; outcome: 'unverifiable'; reason: 'unique-but-wrong' } - | { path: 6; outcome: 'verified' | 'unverifiable' }; + | { path: 6; outcome: 'verified' } + | { path: 6; outcome: 'unverifiable'; reason: 'signal-isolated-wrong' | 'no-signal-isolation' }; /** * Decision 3 "Replay-time verification", paths 2-6. `matchCount == 0` (path @@ -442,18 +453,22 @@ export function classifyTargetBindingMatch( : { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }; } if (input.siblingMatchRefs.length === 1) { + // The sibling signal isolates exactly one member: the evidence denotes + // it — compare with the winner as in paths 4/5 (decision 3, path 6.i). return input.siblingMatchRefs[0] === input.winnerRef ? { path: 6, outcome: 'verified' } - : { path: 6, outcome: 'unverifiable' }; + : { path: 6, outcome: 'unverifiable', reason: 'signal-isolated-wrong' }; } if ( input.regionMemberRefs !== undefined && input.regionMemberRefs.length > 0 && input.viewportCandidateRef !== undefined ) { + // Region-scoped viewportOrder denotes a member: compare as in paths 4/5 + // (decision 3, path 6.ii). return input.viewportCandidateRef === input.winnerRef ? { path: 6, outcome: 'verified' } - : { path: 6, outcome: 'unverifiable' }; + : { path: 6, outcome: 'unverifiable', reason: 'signal-isolated-wrong' }; } - return { path: 6, outcome: 'unverifiable' }; + return { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }; } diff --git a/test/integration/interaction-contract/native-ref.contract.test.ts b/test/integration/interaction-contract/native-ref.contract.test.ts index a167b1637..6d1774a0f 100644 --- a/test/integration/interaction-contract/native-ref.contract.test.ts +++ b/test/integration/interaction-contract/native-ref.contract.test.ts @@ -4,6 +4,7 @@ import type { InteractionGuarantee } from '../../../src/contracts/interaction-gu import type { SnapshotState } from '../../../src/kernel/snapshot.ts'; import { ref } from '../../../src/commands/index.ts'; import { scenarioName } from './coverage-manifest.ts'; +import { buildInteractionResponseData } from '../../../src/daemon/handlers/interaction-touch-response.ts'; import { NATIVE_REF_COVERAGE } from './native-ref.coverage.ts'; import { closedDrawerSnapshot, @@ -169,5 +170,27 @@ test(scenario('responseConstruction'), async () => { assert.deepEqual(result.target, { kind: 'ref', ref: '@e1' }); assert.deepEqual(result.backendResult, { ref: 'e1' }); assert.equal(result.point, undefined); - assert.equal(result.node, undefined); + // ADR 0012 decision 3: the preflight's already-fetched stored-snapshot node + // and tree now ride on the INTERNAL runtime result so record-time + // target-binding evidence can be computed at zero extra capture cost. This + // is the internal boundary only — the shared construction site + // (buildInteractionResponseData) attaches them exclusively to the internal + // visualization payload, and the public responseData never carries them + // (asserted below). + assert.equal(result.node?.ref, 'e1'); + assert.ok(Array.isArray(result.preActionNodes)); + + const { result: visualization, responseData } = buildInteractionResponseData({ + source: { kind: 'runtime', result }, + referenceFrame: undefined, + }); + // Internal visualization payload: carries the node/tree for the recording + // pipeline (stripped again before session-history persistence by + // extractTargetEvidenceForRecording). + assert.ok(visualization.node); + assert.ok(visualization.preActionNodes); + // Public payload: the raw node/tree must never reach the wire. + assert.equal('node' in responseData, false); + assert.equal('preActionNodes' in responseData, false); + assert.equal('targetEvidence' in responseData, false); }); From f78658597299ac7ed72e67d7c7072a164fab8537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 13:06:42 +0200 Subject: [PATCH 4/9] fix: reject missing role keys in target-v1 annotations, correct data-flow comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-review nits: the writer emits `role` unconditionally (top level, ancestry entries, scrollRegion) — possibly as the empty string for typeless nodes, which stays accepted — so a MISSING role key can only come from a hand-edited/adversarial annotation and is now rejected with INVALID_ARGS instead of silently parsing as an implicit empty role, which step-4 enforcement could otherwise match against anonymous wrapper nodes. Also corrects the interaction-touch-response comment that overstated where the raw node/tree flows (finalizeTouchInteraction strips it before both session history and touch overlay telemetry). --- .../handlers/interaction-touch-response.ts | 14 +++--- src/replay/__tests__/target-identity.test.ts | 45 +++++++++++++++++++ src/replay/target-identity.ts | 26 ++++++++--- 3 files changed, 72 insertions(+), 13 deletions(-) diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index c2e849133..51ba2e323 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -126,12 +126,14 @@ export function buildInteractionResponseData(params: { responseData.warning = warning; } // ADR 0012 decision 3: the resolved node and the record-time tree it came - // from ride ONLY on `visualization` (session history / touch overlay - // input), never on `responseData` (the public payload) — attaching them - // here, not via `commonExtra`, is what keeps them off the wire. - // `finalizeTouchInteraction` (interaction-common.ts) turns them into the - // compact `targetEvidence` annotation when the session is being recorded - // and strips the raw tree back out either way. + // from ride ONLY on `visualization` — a hand-off channel to + // `finalizeTouchInteraction` (interaction-common.ts), which turns them into + // the compact `targetEvidence` annotation when the session is being + // recorded and strips the raw tree back out BEFORE anything downstream + // consumes the payload (session history via recordAction and touch overlay + // telemetry both receive the stripped form). They are never attached to + // `responseData` (the public payload) — attaching them here, not via + // `commonExtra`, is what keeps them off the wire. if ('node' in result && result.node) visualization.node = result.node; if ('preActionNodes' in result && result.preActionNodes) { visualization.preActionNodes = result.preActionNodes; diff --git a/src/replay/__tests__/target-identity.test.ts b/src/replay/__tests__/target-identity.test.ts index 187dfbb96..0243b0e1d 100644 --- a/src/replay/__tests__/target-identity.test.ts +++ b/src/replay/__tests__/target-identity.test.ts @@ -321,6 +321,51 @@ test('parser rejects a malformed rect', () => { ); }); +// --------------------------------------------------------------------------- +// Role presence: the writer emits `role` unconditionally (top level, every +// ancestry entry, scrollRegion) — possibly as the empty string for a +// typeless node, which stays accepted. A MISSING role key can only come from +// a hand-edited/adversarial annotation and must be rejected, or step-4 +// enforcement could match anonymous wrapper nodes through an implicit +// empty-role identity. +// --------------------------------------------------------------------------- + +test('parser rejects a missing top-level role', () => { + assertInvalidArgs( + () => parseTargetAnnotationV1Payload(JSON.stringify({ verification: 'verified' })), + /"role" is required/, + ); +}); + +test('parser rejects a missing role in an ancestry entry and in scrollRegion', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ + role: 'button', + ancestry: [{ label: 'Editor' }], + verification: 'verified', + }), + ), + /"ancestry\[0\]\.role" is required/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', scrollRegion: { id: 'list' }, verification: 'verified' }), + ), + /"scrollRegion\.role" is required/, + ); +}); + +test('parser accepts an explicit empty-string role (writer-legal for typeless nodes)', () => { + const parsed = parseTargetAnnotationV1Payload( + JSON.stringify({ role: '', ancestry: [{ role: '' }], verification: 'verified' }), + ); + assert.equal(parsed.role, ''); + assert.deepEqual(parsed.ancestry, [{ role: '' }]); +}); + // --------------------------------------------------------------------------- // Classification core (decision 3 "Replay-time verification" paths 2-6): // this is the exact function the writer's record-time self-check calls, and diff --git a/src/replay/target-identity.ts b/src/replay/target-identity.ts index 79bc152c0..fb801eb5d 100644 --- a/src/replay/target-identity.ts +++ b/src/replay/target-identity.ts @@ -205,7 +205,7 @@ export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotati } const raw = parsed as Record; - const role = parseRequiredRoleField(raw.role); + const role = parseRequiredRoleField(raw.role, 'role'); const id = parseOptionalIdentifierField(raw.id, 'id'); const label = parseOptionalLabelField(raw.label, 'label'); const ancestry = parseAncestryField(raw.ancestry); @@ -228,12 +228,24 @@ export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotati }; } -function parseRequiredRoleField(value: unknown): string { - if (value === undefined) return ''; +/** + * `role` keys are always PRESENT in writer output — the writer emits `role` + * unconditionally at the top level, in every ancestry entry, and in + * `scrollRegion` (possibly as the empty string for a typeless node, which + * decision 3 explicitly allows). A MISSING role key can therefore only come + * from a hand-edited/adversarial annotation and is rejected: once step-4 + * enforcement consumes this evidence, an implicitly-empty role in + * `matchesLocalIdentity` could match anonymous wrapper nodes and produce a + * false verified/rebind. + */ +function parseRequiredRoleField(value: unknown, field: string): string { + if (value === undefined) { + throw new AppError('INVALID_ARGS', `target-v1 "${field}" is required.`); + } if (typeof value !== 'string') { - throw new AppError('INVALID_ARGS', 'target-v1 "role" must be a string.'); + throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a string.`); } - return boundField(normalizeRoleField(value), 'role'); + return boundField(normalizeRoleField(value), field); } function parseOptionalIdentifierField(value: unknown, field: 'id'): string | undefined { @@ -283,7 +295,7 @@ function parseAncestryEntry(entry: unknown, index: number): TargetAncestryEntry throw new AppError('INVALID_ARGS', `target-v1 "ancestry[${index}]" must be an object.`); } const record = entry as Record; - const role = parseRequiredRoleField(record.role); + const role = parseRequiredRoleField(record.role, `ancestry[${index}].role`); const label = parseOptionalLabelField(record.label, `ancestry[${index}].label`); return { role, ...(label !== undefined ? { label } : {}) }; } @@ -302,7 +314,7 @@ function parseScrollRegionField(value: unknown): TargetScrollRegion | undefined throw new AppError('INVALID_ARGS', 'target-v1 "scrollRegion" must be an object.'); } const record = value as Record; - const role = parseRequiredRoleField(record.role); + const role = parseRequiredRoleField(record.role, 'scrollRegion.role'); const id = parseOptionalIdentifierField(record.id, 'id'); const label = parseOptionalLabelField(record.label, 'scrollRegion.label'); return { From 37dfb8be76a4945fecc90e8a2bf5a9923693fdcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 13:42:19 +0200 Subject: [PATCH 5/9] fix: record iOS selector target evidence --- .../__tests__/session-target-evidence.test.ts | 15 ++ src/daemon/handlers/interaction-touch.ts | 2 + src/daemon/session-target-evidence.ts | 9 +- .../target-identity-classification.test.ts | 79 ++++++++++ src/replay/__tests__/target-identity.test.ts | 141 ------------------ .../interaction-contract/daemon-harness.ts | 2 + .../direct-ios-selector.contract.test.ts | 68 +++++++++ 7 files changed, 172 insertions(+), 144 deletions(-) create mode 100644 src/replay/__tests__/target-identity-classification.test.ts diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts index f46b81ec5..faf024281 100644 --- a/src/daemon/__tests__/session-target-evidence.test.ts +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -110,6 +110,21 @@ test('computeTargetEvidence: scrollRegion is the nearest scrollable ancestor loc assert.equal(evidence.verification, 'verified'); }); +test('computeTargetEvidence: a stable scroll-region ID takes precedence over a changed label', () => { + const recordedNodes = scrollableListFixture(); + recordedNodes[1]!.label = 'Inbox'; + const recorded = computeTargetEvidence({ node: recordedNodes[2]!, nodes: recordedNodes }); + + const currentNodes = scrollableListFixture(); + currentNodes[1]!.label = 'Messages'; + const current = computeTargetEvidence({ node: currentNodes[2]!, nodes: currentNodes }); + + assert.ok(recorded); + assert.ok(current); + assert.deepEqual(recorded.scrollRegion, { role: 'scrollview', id: 'editor-scroll' }); + assert.deepEqual(current.scrollRegion, recorded.scrollRegion); +}); + // --------------------------------------------------------------------------- // Duplicate identity resolved by sibling / viewportOrder, still verified // (decision 3's self-check succeeds by construction whenever the capture diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index db2a804b2..c376b0b04 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -314,6 +314,7 @@ function readDirectIosSelectorTapTarget(params: { const { session, commandLabel, target, flags } = params; if (commandLabel !== 'click') return null; if (target.kind !== 'selector') return null; + if (session.recordSession) return null; if (hasNonDefaultClickOptions(flags)) return null; if (commandSupportsVerifyEvidence(commandLabel) && flags?.verify === true) return null; if (commandSupportsSettleObservation(commandLabel) && flags?.settle === true) return null; @@ -634,6 +635,7 @@ function readDirectIosSelectorFillTarget(params: { }): DirectIosSelectorTarget | null { const { session, target, flags } = params; if (target.kind !== 'selector') return null; + if (session.recordSession) return null; if (commandSupportsVerifyEvidence('fill') && flags?.verify === true) return null; if (commandSupportsSettleObservation('fill') && flags?.settle === true) return null; return readDirectSelectorWithMaestroFallback(session, target.selector, flags); diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index f2a06cb9b..cc5688909 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -203,8 +203,11 @@ function computeScrollRegionKey( const identity = boundedLocalIdentity(container); return { role: identity.role, - ...(identity.id !== undefined ? { id: identity.id } : {}), - ...(identity.label !== undefined ? { label: identity.label } : {}), + ...(identity.id !== undefined + ? { id: identity.id } + : identity.label !== undefined + ? { label: identity.label } + : {}), }; } @@ -214,7 +217,7 @@ function scrollRegionKeysEqual( ): boolean { if (!a && !b) return true; if (!a || !b) return false; - return a.role === b.role && a.id === b.id && a.label === b.label; + return matchesLocalIdentity(a, b); } function boundedRect(node: SnapshotNode): TargetAnnotationV1['rect'] { diff --git a/src/replay/__tests__/target-identity-classification.test.ts b/src/replay/__tests__/target-identity-classification.test.ts new file mode 100644 index 000000000..1aa42be89 --- /dev/null +++ b/src/replay/__tests__/target-identity-classification.test.ts @@ -0,0 +1,79 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { classifyTargetBindingMatch } from '../target-identity.ts'; + +// Decision 3's replay-time verification paths 2-6 are shared with the +// writer's record-time self-check and stay isolated from parser coverage. + +test('classifyTargetBindingMatch path 2: matchCount 0 is unverifiable (selector-miss)', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: [], identitySetRefs: [], siblingMatchRefs: [], + regionMemberRefs: undefined, viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 2, outcome: 'unverifiable', reason: 'selector-miss' }); +}); + +test('classifyTargetBindingMatch path 3: empty identity set is unverifiable', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: [], siblingMatchRefs: [], + regionMemberRefs: undefined, viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 3, outcome: 'unverifiable', reason: 'identity-set-empty' }); +}); + +test('classifyTargetBindingMatch path 4: unique identity-set member equal to winner is verified', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: ['e1'], siblingMatchRefs: ['e1'], + regionMemberRefs: undefined, viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 4, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 5: unique identity-set member that is not the winner is unverifiable', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: ['e9'], siblingMatchRefs: [], + regionMemberRefs: undefined, viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }); +}); + +test('classifyTargetBindingMatch path 6: duplicate identity resolved by a unique sibling match', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1'], regionMemberRefs: undefined, viewportCandidateRef: undefined, + }); + assert.deepEqual(result, { path: 6, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 6: viewport order resolves repeated sibling ordinals', () => { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], regionMemberRefs: ['e2', 'e1'], viewportCandidateRef: 'e1', + }); + assert.deepEqual(result, { path: 6, outcome: 'verified' }); +}); + +test('classifyTargetBindingMatch path 6: unavailable or out-of-range viewport evidence falls through', () => { + for (const [regionMemberRefs, viewportCandidateRef] of [ + [undefined, undefined], + [['e1', 'e2'], undefined], + ] as const) { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], regionMemberRefs, viewportCandidateRef, + }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); + } +}); + +test('classifyTargetBindingMatch path 6: a signal isolating a different node than the winner is distinct from fall-through', () => { + for (const params of [ + { siblingMatchRefs: ['e1', 'e2'], regionMemberRefs: ['e1', 'e2'], viewportCandidateRef: 'e2' }, + { siblingMatchRefs: ['e2'], regionMemberRefs: ['e1', 'e2'], viewportCandidateRef: 'e1' }, + ]) { + const result = classifyTargetBindingMatch({ + winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], ...params, + }); + assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'signal-isolated-wrong' }); + } +}); diff --git a/src/replay/__tests__/target-identity.test.ts b/src/replay/__tests__/target-identity.test.ts index 0243b0e1d..c0966da52 100644 --- a/src/replay/__tests__/target-identity.test.ts +++ b/src/replay/__tests__/target-identity.test.ts @@ -2,7 +2,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { AppError } from '../../kernel/errors.ts'; import { - classifyTargetBindingMatch, formatTargetAnnotationCommentLine, matchesAncestryPrefix, matchesLocalIdentity, @@ -365,143 +364,3 @@ test('parser accepts an explicit empty-string role (writer-legal for typeless no assert.equal(parsed.role, ''); assert.deepEqual(parsed.ancestry, [{ role: '' }]); }); - -// --------------------------------------------------------------------------- -// Classification core (decision 3 "Replay-time verification" paths 2-6): -// this is the exact function the writer's record-time self-check calls, and -// what a future replay-enforcement step will reuse — so duplicate/unverifiable -// evidence is exercised directly and generically here. -// --------------------------------------------------------------------------- - -test('classifyTargetBindingMatch path 2: matchCount 0 is unverifiable (selector-miss)', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: [], - identitySetRefs: [], - siblingMatchRefs: [], - regionMemberRefs: undefined, - viewportCandidateRef: undefined, - }); - assert.deepEqual(result, { path: 2, outcome: 'unverifiable', reason: 'selector-miss' }); -}); - -test('classifyTargetBindingMatch path 3: empty identity set is unverifiable', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1'], - identitySetRefs: [], - siblingMatchRefs: [], - regionMemberRefs: undefined, - viewportCandidateRef: undefined, - }); - assert.deepEqual(result, { path: 3, outcome: 'unverifiable', reason: 'identity-set-empty' }); -}); - -test('classifyTargetBindingMatch path 4: unique identity-set member equal to winner is verified', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1'], - identitySetRefs: ['e1'], - siblingMatchRefs: ['e1'], - regionMemberRefs: undefined, - viewportCandidateRef: undefined, - }); - assert.deepEqual(result, { path: 4, outcome: 'verified' }); -}); - -test('classifyTargetBindingMatch path 5: unique identity-set member that is NOT the winner (unique-but-wrong rebind)', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1'], - identitySetRefs: ['e9'], - siblingMatchRefs: [], - regionMemberRefs: undefined, - viewportCandidateRef: undefined, - }); - assert.deepEqual(result, { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }); -}); - -test('classifyTargetBindingMatch path 6: duplicate identity resolved by a unique sibling match', () => { - const verified = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1'], - regionMemberRefs: undefined, - viewportCandidateRef: undefined, - }); - assert.deepEqual(verified, { path: 6, outcome: 'verified' }); -}); - -test('classifyTargetBindingMatch path 6: same child index recurring under different parents falls through sibling, resolved by region-scoped viewportOrder', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - // Both members are "child 0" under different parents — sibling alone - // does not isolate. - siblingMatchRefs: ['e1', 'e2'], - regionMemberRefs: ['e2', 'e1'], - viewportCandidateRef: 'e1', - }); - assert.deepEqual(result, { path: 6, outcome: 'verified' }); -}); - -test('classifyTargetBindingMatch path 6: a scroll region that no longer exists falls through to unverifiable, never a silent pick', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1', 'e2'], - regionMemberRefs: undefined, // region unavailable - viewportCandidateRef: undefined, - }); - assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); -}); - -test('classifyTargetBindingMatch path 6: an out-of-range recorded viewportOrder falls through to unverifiable', () => { - const result = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1', 'e2'], - regionMemberRefs: ['e1', 'e2'], - viewportCandidateRef: undefined, // ordinal was out of range - }); - assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); -}); - -test('classifyTargetBindingMatch path 6: a signal isolating a DIFFERENT node than the winner is the paths-4/5 comparison class, not fall-through', () => { - // Decision 3 path 6.i/6.ii: when a signal denotes exactly one member, - // "compare with W as in paths 4/5" — an isolated-but-different member is - // the same class as path 5's unique-but-wrong rebind (a future - // identity-mismatch), distinct from true no-isolation fall-through (a - // future identity-unverifiable with candidates). - const viaViewportOrder = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1', 'e2'], - regionMemberRefs: ['e1', 'e2'], - viewportCandidateRef: 'e2', - }); - assert.deepEqual(viaViewportOrder, { - path: 6, - outcome: 'unverifiable', - reason: 'signal-isolated-wrong', - }); - - const viaSibling = classifyTargetBindingMatch({ - winnerRef: 'e1', - matchedRefs: ['e1', 'e2'], - identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e2'], // sibling isolates e2, not the winner - regionMemberRefs: ['e1', 'e2'], - viewportCandidateRef: 'e1', - }); - assert.deepEqual(viaSibling, { - path: 6, - outcome: 'unverifiable', - reason: 'signal-isolated-wrong', - }); -}); diff --git a/test/integration/interaction-contract/daemon-harness.ts b/test/integration/interaction-contract/daemon-harness.ts index 3c609a4d7..a1b686d07 100644 --- a/test/integration/interaction-contract/daemon-harness.ts +++ b/test/integration/interaction-contract/daemon-harness.ts @@ -30,6 +30,7 @@ const CONTRACT_DEVICE_ID = PROVIDER_SCENARIO_IOS_SIMULATOR.id; export async function withIosContractDaemon( entries: readonly ProviderScenarioProviderEntry[], run: (daemon: ProviderScenarioHarness, transcript: ProviderScenarioTranscript) => Promise, + options: { saveScript?: boolean | string } = {}, ): Promise { const transcript = createProviderTranscript(entries); const appleRunnerProvider = createAppleRunnerProviderFromTranscript(transcript, 'ios.runner'); @@ -50,6 +51,7 @@ export async function withIosContractDaemon( const open = await daemon.callCommand('open', [CONTRACT_APP], { platform: 'ios', udid: CONTRACT_DEVICE_ID, + ...(options.saveScript !== undefined ? { saveScript: options.saveScript } : {}), }); assertRpcOk(open); await run(daemon, transcript); diff --git a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts index 21a26cce4..1a467a3b3 100644 --- a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts +++ b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts @@ -16,6 +16,7 @@ import { runnerSnapshotEntry, runnerTapEntry, runnerTapErrorEntry, + runnerTypeEntry, withIosContractDaemon, } from './daemon-harness.ts'; @@ -28,6 +29,30 @@ import { const scenario = (guarantee: InteractionGuarantee): string => scenarioName(DIRECT_IOS_SELECTOR_COVERAGE, guarantee); +const RECORDING_TARGET_NODES = [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'continue', + label: 'Continue', + rect: { x: 40, y: 160, width: 120, height: 44 }, + enabled: true, + hittable: true, + }, + { + index: 2, + parentIndex: 0, + type: 'TextField', + identifier: 'email', + label: 'Email', + rect: { x: 40, y: 240, width: 280, height: 44 }, + enabled: true, + hittable: true, + }, +] as const; + test(scenario('responseConstruction'), async () => { await withIosContractDaemon([runnerTapEntry({ x: 150, y: 200 })], async (daemon, transcript) => { const click = await daemon.callCommand('click', ['label=Continue']); @@ -48,6 +73,49 @@ test(scenario('responseConstruction'), async () => { }); }); +test('recorded simple iOS selector click and fill use runtime resolution and persist target-v1 evidence', async () => { + await withIosContractDaemon( + [ + runnerSnapshotEntry(RECORDING_TARGET_NODES), + runnerTapEntry({ x: 100, y: 182 }), + runnerSnapshotEntry(RECORDING_TARGET_NODES), + runnerTypeEntry({ x: 180, y: 262 }), + ], + async (daemon, transcript) => { + assertRpcOk(await daemon.callCommand('click', ['id=continue'])); + assertRpcOk(await daemon.callCommand('fill', ['id=email', 'ada@example.com'])); + + assert.equal(transcript.calls[0]?.command, 'ios.runner.snapshot'); + assert.equal(transcript.calls[1]?.command, 'ios.runner.tap'); + assert.equal(transcript.calls[2]?.command, 'ios.runner.snapshot'); + assert.equal(transcript.calls[3]?.command, 'ios.runner.type'); + for (const call of [transcript.calls[1], transcript.calls[3]]) { + const request = call?.request as Record | undefined; + assert.equal(request?.selectorKey, undefined); + } + + const actions = daemon + .session() + ?.actions.filter((action) => action.command === 'click' || action.command === 'fill'); + assert.deepEqual(actions?.map((action) => action.command), ['click', 'fill']); + const targetIdentities = actions?.map((action) => { + const evidence = action.targetEvidence as Record | undefined; + return { + id: evidence?.id, + role: evidence?.role, + label: evidence?.label, + verification: evidence?.verification, + }; + }); + assert.deepEqual(targetIdentities, [ + { id: 'continue', role: 'button', label: 'Continue', verification: 'verified' }, + { id: 'email', role: 'textfield', label: 'Email', verification: 'verified' }, + ]); + }, + { saveScript: true }, + ); +}); + test(scenario('offscreen'), async () => { await withIosContractDaemon( [ From c9dc58312ea8938d4a08ce2f14a8c632ce7ab2f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 14:16:40 +0200 Subject: [PATCH 6/9] refactor: make the record-time evidence channel structural, drop argument-comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer directive: comments that argue a workaround is acceptable mark code to redesign. Applied to the whole PR diff: - The record-time node/tree now travel on a typed `recordedTarget` side channel of InteractionResponsePayloads instead of being smuggled through the visualization Record and stripped later. The construction site routes them there exclusively, finalizeTouchInteraction consumes the channel directly, and extractTargetEvidenceForRecording (the strip helper and its justifying doc) is deleted — the public/internal split is now enforced by the type shape, so the contract test asserts it in two lines instead of a paragraph. - findNearestScrollableContainer/findNearestAncestor made generic over the node type, removing a cast plus its safety-argument comment. - computeLocalIdentity/boundedLocalIdentity collapsed into one always-capped identity reader — the raw/bounded split was the root of the earlier self-match bug and existed only to be explained. - parseReplayScriptDetailed's rejectUnbound takes the pending annotation as a parameter, removing a non-null assertion and its comment. - Remaining paragraph-length argument-comments trimmed to one-line constraint statements (module docs, sizing/floor comments, regex/role rationales, test comments); spec-mapping docs stay. --- .../interaction/runtime/interactions.test.ts | 7 +- .../interaction/runtime/resolution.ts | 9 +- .../handlers/__tests__/interaction.test.ts | 4 +- src/daemon/handlers/interaction-common.ts | 44 +++------- .../handlers/interaction-touch-response.ts | 32 +++---- src/daemon/handlers/interaction-touch.ts | 7 +- src/daemon/session-target-evidence.ts | 83 +++++-------------- src/daemon/snapshot-presentation/tree.ts | 25 +++--- src/daemon/types.ts | 6 +- src/replay/script-formatting.ts | 10 +-- src/replay/script.ts | 28 +++---- src/replay/target-identity.ts | 56 ++++--------- .../native-ref.contract.test.ts | 32 ++++--- 13 files changed, 117 insertions(+), 226 deletions(-) diff --git a/src/commands/interaction/runtime/interactions.test.ts b/src/commands/interaction/runtime/interactions.test.ts index 00e07d9bc..61a337900 100644 --- a/src/commands/interaction/runtime/interactions.test.ts +++ b/src/commands/interaction/runtime/interactions.test.ts @@ -127,11 +127,8 @@ test('runtime fill uses backend ref primitive without resolving snapshot geometr assert.deepEqual(calls, [{ ref: '@e1', text: 'hello', delayMs: 25 }]); assert.equal(result.kind, 'ref'); assert.equal(result.point, undefined); - // ADR 0012 decision 3: the native-ref preflight already looked this node up - // (for the offscreen/occlusion guards) and now reuses that lookup to carry - // `node`/`preActionNodes` for record-time target-binding evidence — at zero - // extra capture cost. These never reach the public response (stripped by - // `finalizeTouchInteraction`); this is the internal runtime result shape. + // ADR 0012 decision 3: the preflight's guard lookup supplies the + // record-time evidence node on the runtime result. assert.equal(result.node?.ref, 'e1'); assert.ok(Array.isArray(result.preActionNodes)); assert.deepEqual(result.target, { kind: 'ref', ref: '@e1' }); diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index 6a4ce601a..797c9113a 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -509,13 +509,8 @@ export async function preflightNativeRefInteraction( assertVisibleRefTarget(resolved.node, nodes, target.ref, action); return { ...describeNonHittableTarget(resolved.node, action), - // Reuses the lookup above (zero extra capture cost) so the native-ref - // fast path — `click @ref`/`fill @ref` with default options — can still - // record ADR 0012 decision-3 target-binding evidence from the stored - // session tree, not only from the full resolution path. Stripped back - // out of the response before it reaches any caller (see - // `interaction-touch-response.ts`/`finalizeTouchInteraction`); never - // exposed publicly. + // ADR 0012 decision 3: the guard lookup above doubles as the record-time + // evidence source for the fast path, at zero extra capture cost. node: resolved.node, preActionNodes: nodes, }; diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index cded144a1..ccf04335c 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -3596,9 +3596,7 @@ test('press @ref while recording attaches target-v1 evidence to the recorded act session: sessionName, command: 'press', positionals: ['@e1'], - // verify:true forces full runtime resolution (captures node/tree), - // sidestepping the native-ref fast path exactly like the sibling - // --verify tests above. + // verify:true forces full runtime resolution, which captures node/tree. flags: { verify: true }, }, sessionName, diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index 6f6792c61..51117b1a9 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -1,5 +1,5 @@ import type { CommandFlags } from '../../core/dispatch.ts'; -import type { SnapshotNode, SnapshotState } from '../../kernel/snapshot.ts'; +import type { SnapshotState } from '../../kernel/snapshot.ts'; import type { DaemonCommandContext } from '../context.ts'; import { recordTouchVisualizationEvent } from '../recording-gestures.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; @@ -14,6 +14,7 @@ import { } from '../interaction-outcome-policy.ts'; import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; import { computeTargetEvidence } from '../session-target-evidence.ts'; +import type { RecordedTargetCapture } from './interaction-touch-response.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -38,6 +39,8 @@ export function finalizeTouchInteraction(params: { flags: CommandFlags | undefined; result: Record; responseData: Record; + /** ADR 0012 decision 3: record-time input for the `target-v1` annotation. */ + recordedTarget?: RecordedTargetCapture; actionStartedAt: number; actionFinishedAt: number; androidFreshnessBaseline?: SnapshotState | undefined; @@ -51,20 +54,21 @@ export function finalizeTouchInteraction(params: { flags, result, responseData, + recordedTarget, actionStartedAt, actionFinishedAt, androidFreshnessBaseline, } = params; const actionFlags = stripInternalInteractionFlags(flags); - const { result: recordedResult, targetEvidence } = extractTargetEvidenceForRecording( - session, - result, - ); + const targetEvidence = + session.recordSession && recordedTarget + ? computeTargetEvidence({ node: recordedTarget.node, nodes: recordedTarget.preActionNodes }) + : undefined; sessionStore.recordAction(session, { command, positionals, flags: actionFlags ?? {}, - result: recordedResult, + result, ...(targetEvidence ? { targetEvidence } : {}), }); markPendingInteractionOutcome({ @@ -82,36 +86,10 @@ export function finalizeTouchInteraction(params: { session, command, positionals, - recordedResult, + result, (actionFlags ?? {}) as Record, actionStartedAt, actionFinishedAt, ); return { ok: true, data: responseData }; } - -/** - * ADR 0012 decision 3: `result.node`/`result.preActionNodes` (attached only - * to the internal visualization/session payload, see - * `interaction-touch-response.ts`) are the record-time winner and tree. When - * the session is being recorded (`--save-script`), turn them into the - * compact `target-v1` evidence the script writer emits; either way, strip the - * raw node/tree back out so no downstream consumer (session history, touch - * visualization telemetry) ever holds a full AX subtree per action. - */ -function extractTargetEvidenceForRecording( - session: SessionState, - result: Record, -): { result: Record; targetEvidence: ReturnType } { - if (!('node' in result) && !('preActionNodes' in result)) { - return { result, targetEvidence: undefined }; - } - const { node, preActionNodes, ...rest } = result as Record & { - node?: SnapshotNode; - preActionNodes?: SnapshotNode[]; - }; - if (!session.recordSession || !node || !preActionNodes) { - return { result: rest, targetEvidence: undefined }; - } - return { result: rest, targetEvidence: computeTargetEvidence({ node, nodes: preActionNodes }) }; -} diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index 51ba2e323..6f3972c3b 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -5,6 +5,7 @@ import type { PressCommandResult, SettleObservation, } from '../../contracts/interaction.ts'; +import type { SnapshotNode } from '../../kernel/snapshot.ts'; import { successText } from '../../utils/success-text.ts'; import { interactionResultExtra } from './interaction-touch-targets.ts'; @@ -38,11 +39,19 @@ export type InteractionResponseSource = point: { x: number; y: number }; }; +/** ADR 0012 decision 3: record-time input for `computeTargetEvidence`. */ +export type RecordedTargetCapture = { + node: SnapshotNode; + preActionNodes: SnapshotNode[]; +}; + export type InteractionResponsePayloads = { /** Recorded in session history and used for touch visualization. */ result: Record; /** The public payload returned to the client. */ responseData: Record; + /** Typed side channel — never part of either serialized payload. */ + recordedTarget?: RecordedTargetCapture; }; export function buildInteractionResponseData(params: { @@ -125,20 +134,15 @@ export function buildInteractionResponseData(params: { visualization.warning = warning; responseData.warning = warning; } - // ADR 0012 decision 3: the resolved node and the record-time tree it came - // from ride ONLY on `visualization` — a hand-off channel to - // `finalizeTouchInteraction` (interaction-common.ts), which turns them into - // the compact `targetEvidence` annotation when the session is being - // recorded and strips the raw tree back out BEFORE anything downstream - // consumes the payload (session history via recordAction and touch overlay - // telemetry both receive the stripped form). They are never attached to - // `responseData` (the public payload) — attaching them here, not via - // `commonExtra`, is what keeps them off the wire. - if ('node' in result && result.node) visualization.node = result.node; - if ('preActionNodes' in result && result.preActionNodes) { - visualization.preActionNodes = result.preActionNodes; - } - return { result: visualization, responseData }; + return { result: visualization, responseData, ...recordedTargetCapture(result) }; +} + +function recordedTargetCapture( + result: InteractionRuntimeResult, +): Pick { + const node = 'node' in result ? result.node : undefined; + const preActionNodes = 'preActionNodes' in result ? result.preActionNodes : undefined; + return node && preActionNodes ? { recordedTarget: { node, preActionNodes } } : {}; } // Attaches refsGeneration inside the settle payload when the response is diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index c376b0b04..00e0fbd4e 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -670,9 +670,7 @@ async function dispatchRuntimeInteraction< afterRun?(result: TResult): Promise; buildPayloads( result: TResult, - ): - | { result: Record; responseData: Record } - | Promise<{ result: Record; responseData: Record }>; + ): InteractionResponsePayloads | Promise; }, ): Promise { const session = params.sessionStore.get(params.sessionName); @@ -690,7 +688,7 @@ async function dispatchRuntimeInteraction< }, ); const actionFinishedAt = Date.now(); - const { result, responseData } = await options.buildPayloads(runtimeResult); + const { result, responseData, recordedTarget } = await options.buildPayloads(runtimeResult); if (readiness.status === 'recovered') { // Append, don't clobber — the builder may already carry a warning // (e.g. stale-refs, #1076). @@ -710,6 +708,7 @@ async function dispatchRuntimeInteraction< flags: params.req.flags, result, responseData, + recordedTarget, actionStartedAt, actionFinishedAt, androidFreshnessBaseline: options.androidFreshnessBaseline, diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index cc5688909..c527b4644 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -2,20 +2,10 @@ * ADR 0012 decision 3: record-time computation of `.ad` target-binding * evidence (the `# agent-device:target-v1 {...}` annotation). * - * `computeTargetEvidence` implements decision 3's "Record-time write" - * algorithm steps 1-5 against the SAME record-time tree the interaction - * resolver just captured (`ResolvedInteractionTarget.node` + - * `.preActionNodes`, see `src/commands/interaction/runtime/resolution.ts`). - * It never captures anything itself — callers own gating this on whether the - * session is actually being recorded (`session.recordSession`), since the - * ancestry/identity-set scan is O(nodes) and pointless otherwise. - * - * This module lives under `src/daemon/` (not `src/replay/`) because it needs - * `SnapshotNode`/`findNearestScrollableContainer` from the daemon's - * snapshot-presentation layer, which sits above `replay` in the import DAG; - * `replay` (below `daemon`) owns only the tree-agnostic spec pieces - * (`src/replay/target-identity.ts`) that both the writer here and the parser - * share. + * `computeTargetEvidence` runs decision 3's "Record-time write" steps 1-5 + * against the tree the resolver already captured; it never captures, and + * callers gate it on `session.recordSession`. Tree-agnostic spec pieces live + * in `src/replay/target-identity.ts`, shared with the parser. */ import type { SnapshotNode } from '../kernel/snapshot.ts'; @@ -55,13 +45,9 @@ export function computeTargetEvidence(params: { const scrollRegion = computeScrollRegionKey(node, byIndex); const rect = boundedRect(node); - // Writer-parser invariant (decision 3): reduce ancestry from the root side - // until the canonical serialization fits the 4 KiB payload cap. Every - // string field is already bounded to 256 bytes by `boundedLocalIdentity`/ - // `buildAncestryChain`/`computeScrollRegionKey`, so this loop only ever - // needs to shed ancestry entries. Decision 3 stops reducing once only - // `ancestry[0]` (the parent) is retained — the floor is 1 when the winner - // has any ancestor at all, else 0 (a root node has none to keep). + // Decision 3's writer-parser invariant: reduce ancestry from the root side + // until the payload fits, stopping once only `ancestry[0]` is retained + // (floor 0 for a root node with no ancestors). const floor = fullAncestry.length > 0 ? 1 : 0; const buildCandidate = (ancestryLength: number) => { const ancestry = fullAncestry.slice(0, ancestryLength); @@ -88,13 +74,8 @@ export function computeTargetEvidence(params: { for (let ancestryLength = fullAncestry.length; ancestryLength >= floor; ancestryLength -= 1) { const { candidate, domain } = buildCandidate(ancestryLength); - // Size against the WORST-CASE verification value ("unverifiable" is 4 - // serialized bytes longer than "verified") so the payload provably fits - // no matter what the self-check below returns — otherwise a payload - // within 4 bytes of the cap could pass this check as "verified" and then - // overflow once a fail-closed self-check downgraded it, violating the - // writer-parser invariant exactly in the rare capture-anomaly case it - // exists for. + // 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 @@ -103,12 +84,8 @@ export function computeTargetEvidence(params: { return candidate; } if (ancestryLength === floor) { - // Decision 3's terminal fail-closed guarantee: a parent-only (or, for a - // root node, ancestry-less) payload fits arithmetically once every - // field is already capped at 256 bytes, so this branch is not expected - // to run. If it somehow still doesn't fit, drop the diagnostic-only - // rect (never compared) as one last, spec-consistent reduction rather - // than emit a payload the parser would reject. + // 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 @@ -132,22 +109,16 @@ function buildIndexMap(nodes: readonly SnapshotNode[]): Map, ): TargetScrollRegion | undefined { - // `findNearestScrollableContainer` is typed generically over - // `RawSnapshotNode`; every value in `byIndex` is actually a `SnapshotNode` - // (built from the same `nodes` array), so the cast back is safe. - const container = findNearestScrollableContainer(node, byIndex) as SnapshotNode | null; + const container = findNearestScrollableContainer(node, byIndex); if (!container) return undefined; const identity = boundedLocalIdentity(container); return { @@ -254,12 +222,6 @@ function computeDisambiguationDomain(params: { // Step 2: all nodes sharing the winner's local identity with a matching // leaf-anchored ancestry prefix. const identitySet = nodes.filter((candidate) => { - // Compare through the SAME 256-byte bounding the recorded `identity` and - // ancestry entries already went through — otherwise a node whose own - // id/label exceeds the field cap would spuriously fail to match ITSELF - // (the recorded value is truncated; the raw candidate value is not), - // corrupting both this self-check and, if the replay-time matcher ever - // skipped the same bounding, a real node's identity check later. if (!matchesLocalIdentity(boundedLocalIdentity(candidate), identity)) return false; const observedAncestry = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); return matchesAncestryPrefix(observedAncestry, ancestry); @@ -301,12 +263,9 @@ function orderByViewportPosition(members: readonly SnapshotNode[]): SnapshotNode } /** - * Decision 3 record-time write step 5: run the replay-time classification - * (decision 3's paths 2-6, shared via `classifyTargetBindingMatch`) against - * the record-time tree itself. Paths 2/3 are unreachable here by - * construction — the winner always matched itself and is always a member of - * its own identity set — but are still fed through the shared classifier so - * the exact same function runs at record and (in a future step) replay time. + * Decision 3 record-time write step 5: run the shared replay-time + * classification (`classifyTargetBindingMatch`) against the record-time tree + * itself. */ function runRecordTimeSelfCheck(params: { node: SnapshotNode; diff --git a/src/daemon/snapshot-presentation/tree.ts b/src/daemon/snapshot-presentation/tree.ts index e8b259056..8f4a87b69 100644 --- a/src/daemon/snapshot-presentation/tree.ts +++ b/src/daemon/snapshot-presentation/tree.ts @@ -71,11 +71,8 @@ function getDescendantEndPositions(nodes: RawSnapshotNode[]): number[] { return endPositions; } -function collectAncestors( - node: RawSnapshotNode, - byIndex: Map, -): RawSnapshotNode[] { - const ancestors: RawSnapshotNode[] = []; +function collectAncestors(node: T, byIndex: Map): T[] { + const ancestors: T[] = []; let current = typeof node.parentIndex === 'number' ? byIndex.get(node.parentIndex) : undefined; const visited = new Set(); while (current && !visited.has(current.index)) { @@ -87,11 +84,11 @@ function collectAncestors( return ancestors; } -export function findNearestAncestor( - node: RawSnapshotNode, - byIndex: Map, - predicate: (ancestor: RawSnapshotNode) => boolean, -): RawSnapshotNode | null { +export function findNearestAncestor( + node: T, + byIndex: Map, + predicate: (ancestor: T) => boolean, +): T | null { for (const ancestor of collectAncestors(node, byIndex)) { if (predicate(ancestor)) { return ancestor; @@ -100,11 +97,11 @@ export function findNearestAncestor( return null; } -export function findNearestScrollableContainer( - node: RawSnapshotNode, - byIndex: Map, +export function findNearestScrollableContainer( + node: T, + byIndex: Map, options: { includeSelf?: boolean } = {}, -): RawSnapshotNode | null { +): T | null { const self = options.includeSelf === true && isScrollableSnapshotType(node.type) ? node : null; return ( self ?? diff --git a/src/daemon/types.ts b/src/daemon/types.ts index e362682d9..9098084be 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -401,10 +401,8 @@ export type SessionAction = { result?: Record; /** * ADR 0012 decision 3: parsed or record-time-computed `target-v1` - * evidence, emitted as a `# agent-device:target-v1 {...}` comment - * immediately before this action's line. Inert — migration step 3 parses - * and preserves it, but nothing enforces it yet (that lands in a later - * migration step). + * evidence, written as a comment immediately before this action's line. + * Inert until migration step 4 adds enforcement. */ targetEvidence?: TargetAnnotationV1; }; diff --git a/src/replay/script-formatting.ts b/src/replay/script-formatting.ts index ca76c1cda..cdcb2575d 100644 --- a/src/replay/script-formatting.ts +++ b/src/replay/script-formatting.ts @@ -33,13 +33,9 @@ export function formatPortableActionLine( } /** - * ADR 0012 decision 3: the `# agent-device:target-v1 {...}` annotation line - * that must immediately precede this action's line (no blank/intervening - * line — decision 3's binding rule), or `[]` when the action carries no - * target evidence. Shared by the live session-script writer - * (`src/daemon/session-script-writer.ts`) and `writeReplayScript`'s - * read-then-rewrite preservation (`script.ts`) so both emit the exact same - * canonical form. + * ADR 0012 decision 3: the `# agent-device:target-v1 {...}` line that must + * immediately precede this action's line, or `[]` when the action carries no + * target evidence. Shared by both script writers for one canonical form. */ export function formatTargetAnnotationLines(action: SessionAction): string[] { return action.targetEvidence ? [formatTargetAnnotationCommentLine(action.targetEvidence)] : []; diff --git a/src/replay/script.ts b/src/replay/script.ts index 1e8b5f876..3e597323c 100644 --- a/src/replay/script.ts +++ b/src/replay/script.ts @@ -52,35 +52,37 @@ export function parseReplayScriptDetailed(script: string): ParsedReplayScript { let sawAction = false; let pending: PendingTargetAnnotation | undefined; - const rejectUnbound = (index: number, why: string): never => { + const rejectUnbound = ( + annotation: PendingTargetAnnotation, + index: number, + why: string, + ): never => { throw new AppError( 'INVALID_ARGS', - // pending is only read here when set, right after the guard below. - `target-v1 annotation on line ${pending!.line} must be immediately followed by its action line (line ${index + 1} ${why}).`, + `target-v1 annotation on line ${annotation.line} must be immediately followed by its action line (line ${index + 1} ${why}).`, ); }; for (const [index, rawLine] of lines.entries()) { const trimmed = rawLine.trim(); if (trimmed.length === 0) { - if (pending) rejectUnbound(index, 'is blank'); + if (pending) rejectUnbound(pending, index, 'is blank'); continue; } if (trimmed.startsWith('#')) { const annotation = parseTargetAnnotationCommentLine(trimmed); if (annotation.kind === 'v1') { - if (pending) rejectUnbound(index, 'is another target-v1 annotation'); + if (pending) rejectUnbound(pending, index, 'is another target-v1 annotation'); pending = { evidence: annotation.evidence, line: index + 1 }; continue; } - // A plain comment or an unknown future `target-vN` is an ordinary - // comment to this (v1) reader — but it is still an intervening line - // for any annotation still pending binding. - if (pending) rejectUnbound(index, 'is a comment'); + // An ordinary or future-target-vN comment still counts as an + // intervening line for a pending annotation. + if (pending) rejectUnbound(pending, index, 'is a comment'); continue; } if (isReplayEnvLine(trimmed)) { - if (pending) rejectUnbound(index, 'is an env directive'); + if (pending) rejectUnbound(pending, index, 'is an env directive'); if (sawAction) { throw new AppError( 'INVALID_ARGS', @@ -91,7 +93,7 @@ export function parseReplayScriptDetailed(script: string): ParsedReplayScript { } const parsed = parseReplayScriptLine(rawLine); if (!parsed) { - if (pending) rejectUnbound(index, 'did not parse as an action'); + if (pending) rejectUnbound(pending, index, 'did not parse as an action'); continue; } if (pending) { @@ -519,9 +521,7 @@ export function writeReplayScript( ); } for (const action of actions) { - // ADR 0012 decision 3: a writer that reads then rewrites a script (this - // heal/`--update` path) must preserve v1 annotations in canonical form, - // never silently discard them. + // ADR 0012 decision 3: rewrites preserve v1 annotations in canonical form. lines.push(...formatTargetAnnotationLines(action)); lines.push(formatReplayActionLine(action)); } diff --git a/src/replay/target-identity.ts b/src/replay/target-identity.ts index fb801eb5d..1740d7bec 100644 --- a/src/replay/target-identity.ts +++ b/src/replay/target-identity.ts @@ -1,35 +1,18 @@ /** - * ADR 0012 decision 3: versioned `.ad` target-binding evidence. - * - * This module is the shared, tree-agnostic spine consumed by both the writer - * (`src/daemon/session-target-evidence.ts`, which has an actual snapshot tree - * to compute the payload from) and the parser (`src/replay/script.ts`, which - * only ever sees the JSON comment text). It owns: - * - * - the wire type and its canonical JSON.stringify field order, - * - Unicode/whitespace normalization and the 256-byte-field / 4 KiB-payload - * caps, - * - parsing + validation of a `target-v1` comment payload (`INVALID_ARGS` on - * anything malformed or oversized — the parser REJECTS, it never - * truncates), and - * - the record/replay-shared classification core (decision 3's six-path - * verification algorithm), expressed generically over node refs so it has - * no dependency on `SnapshotNode`/the daemon tree. The writer's record-time - * self-check (decision 3, record-time write step 5) calls this; a future - * migration step (4) reuses the exact same function at replay time so the - * two can never drift. - * - * Nothing here enforces anything at replay time yet (migration step 3 is - * parser/writer only) — parsed evidence is inert until step 4 lands. + * ADR 0012 decision 3: versioned `.ad` target-binding evidence — the + * tree-agnostic spine shared by the writer + * (`src/daemon/session-target-evidence.ts`) and the parser + * (`src/replay/script.ts`). Owns the wire type, canonical field order, + * normalization, size caps, payload parsing/validation, and the record/ + * replay-shared classification core. Inert in migration step 3: nothing + * enforces parsed evidence at replay time until step 4. */ import { AppError } from '../kernel/errors.ts'; const TARGET_ANNOTATION_TAG = 'agent-device:target-v1'; -// Captures the version and the REST of the line verbatim, even when it isn't -// well-formed JSON — a line that claims the tag but carries garbage must be -// rejected as a malformed v1 annotation (INVALID_ARGS), not silently treated -// as an ordinary comment because it failed to look like `{...}` up front. +// Captures the rest of the line verbatim: a line claiming the tag with a +// garbage payload is a malformed v1 annotation, never an ordinary comment. const TARGET_ANNOTATION_LINE_RE = /^#\s*agent-device:target-v(\d+)(?:\s+(.*))?$/; export const TARGET_ANNOTATION_MAX_FIELD_BYTES = 256; @@ -229,14 +212,9 @@ export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotati } /** - * `role` keys are always PRESENT in writer output — the writer emits `role` - * unconditionally at the top level, in every ancestry entry, and in - * `scrollRegion` (possibly as the empty string for a typeless node, which - * decision 3 explicitly allows). A MISSING role key can therefore only come - * from a hand-edited/adversarial annotation and is rejected: once step-4 - * enforcement consumes this evidence, an implicitly-empty role in - * `matchesLocalIdentity` could match anonymous wrapper nodes and produce a - * false verified/rebind. + * The writer emits `role` unconditionally (possibly as the empty string for + * a typeless node), so a missing role key is always foreign input and is + * rejected rather than defaulted. */ function parseRequiredRoleField(value: unknown, field: string): string { if (value === undefined) { @@ -394,14 +372,8 @@ export function matchesAncestryPrefix( // --------------------------------------------------------------------------- // Classification core (decision 3 "Replay-time verification", paths 2-6; -// path 1 — a recorded-`unverifiable` annotation — is checked by the caller -// before any resolution and never reaches this function). Expressed -// generically over node refs: the caller resolves matches/identity-set/ -// sibling-filter/region partition using its own tree, then hands the -// resulting ref sets here for the total-order classification. This is what -// both the writer's record-time self-check (decision 3 step 5) and — in a -// future migration step — replay-time enforcement call, so the two decision -// points can never diverge. +// path 1 is the caller's pre-resolution check). Generic over node refs so +// the record-time self-check and future replay-time enforcement share it. // --------------------------------------------------------------------------- export type TargetBindingClassificationInput = { diff --git a/test/integration/interaction-contract/native-ref.contract.test.ts b/test/integration/interaction-contract/native-ref.contract.test.ts index 6d1774a0f..4a3d70d18 100644 --- a/test/integration/interaction-contract/native-ref.contract.test.ts +++ b/test/integration/interaction-contract/native-ref.contract.test.ts @@ -170,27 +170,25 @@ test(scenario('responseConstruction'), async () => { assert.deepEqual(result.target, { kind: 'ref', ref: '@e1' }); assert.deepEqual(result.backendResult, { ref: 'e1' }); assert.equal(result.point, undefined); - // ADR 0012 decision 3: the preflight's already-fetched stored-snapshot node - // and tree now ride on the INTERNAL runtime result so record-time - // target-binding evidence can be computed at zero extra capture cost. This - // is the internal boundary only — the shared construction site - // (buildInteractionResponseData) attaches them exclusively to the internal - // visualization payload, and the public responseData never carries them - // (asserted below). + // ADR 0012 decision 3: the preflight's guard lookup supplies the + // record-time evidence node on the runtime result. assert.equal(result.node?.ref, 'e1'); assert.ok(Array.isArray(result.preActionNodes)); - const { result: visualization, responseData } = buildInteractionResponseData({ + const { + result: visualization, + responseData, + recordedTarget, + } = buildInteractionResponseData({ source: { kind: 'runtime', result }, referenceFrame: undefined, }); - // Internal visualization payload: carries the node/tree for the recording - // pipeline (stripped again before session-history persistence by - // extractTargetEvidenceForRecording). - assert.ok(visualization.node); - assert.ok(visualization.preActionNodes); - // Public payload: the raw node/tree must never reach the wire. - assert.equal('node' in responseData, false); - assert.equal('preActionNodes' in responseData, false); - assert.equal('targetEvidence' in responseData, false); + // The construction site routes node/tree onto the typed recordedTarget + // channel only; neither serialized payload carries them. + assert.equal(recordedTarget?.node.ref, 'e1'); + for (const payload of [visualization, responseData]) { + assert.equal('node' in payload, false); + assert.equal('preActionNodes' in payload, false); + assert.equal('targetEvidence' in payload, false); + } }); From 83c946019726cfcf67bd3dfd52acd8b83bf844d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 14:20:30 +0200 Subject: [PATCH 7/9] chore: oxfmt the selector-evidence test files --- .../target-identity-classification.test.ts | 61 ++++++++++++++----- .../direct-ios-selector.contract.test.ts | 5 +- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/replay/__tests__/target-identity-classification.test.ts b/src/replay/__tests__/target-identity-classification.test.ts index 1aa42be89..ba689680a 100644 --- a/src/replay/__tests__/target-identity-classification.test.ts +++ b/src/replay/__tests__/target-identity-classification.test.ts @@ -7,48 +7,72 @@ import { classifyTargetBindingMatch } from '../target-identity.ts'; test('classifyTargetBindingMatch path 2: matchCount 0 is unverifiable (selector-miss)', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: [], identitySetRefs: [], siblingMatchRefs: [], - regionMemberRefs: undefined, viewportCandidateRef: undefined, + winnerRef: 'e1', + matchedRefs: [], + identitySetRefs: [], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, }); assert.deepEqual(result, { path: 2, outcome: 'unverifiable', reason: 'selector-miss' }); }); test('classifyTargetBindingMatch path 3: empty identity set is unverifiable', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: [], siblingMatchRefs: [], - regionMemberRefs: undefined, viewportCandidateRef: undefined, + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: [], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, }); assert.deepEqual(result, { path: 3, outcome: 'unverifiable', reason: 'identity-set-empty' }); }); test('classifyTargetBindingMatch path 4: unique identity-set member equal to winner is verified', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: ['e1'], siblingMatchRefs: ['e1'], - regionMemberRefs: undefined, viewportCandidateRef: undefined, + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: ['e1'], + siblingMatchRefs: ['e1'], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, }); assert.deepEqual(result, { path: 4, outcome: 'verified' }); }); test('classifyTargetBindingMatch path 5: unique identity-set member that is not the winner is unverifiable', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1'], identitySetRefs: ['e9'], siblingMatchRefs: [], - regionMemberRefs: undefined, viewportCandidateRef: undefined, + winnerRef: 'e1', + matchedRefs: ['e1'], + identitySetRefs: ['e9'], + siblingMatchRefs: [], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, }); assert.deepEqual(result, { path: 5, outcome: 'unverifiable', reason: 'unique-but-wrong' }); }); test('classifyTargetBindingMatch path 6: duplicate identity resolved by a unique sibling match', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1'], regionMemberRefs: undefined, viewportCandidateRef: undefined, + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1'], + regionMemberRefs: undefined, + viewportCandidateRef: undefined, }); assert.deepEqual(result, { path: 6, outcome: 'verified' }); }); test('classifyTargetBindingMatch path 6: viewport order resolves repeated sibling ordinals', () => { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1', 'e2'], regionMemberRefs: ['e2', 'e1'], viewportCandidateRef: 'e1', + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs: ['e2', 'e1'], + viewportCandidateRef: 'e1', }); assert.deepEqual(result, { path: 6, outcome: 'verified' }); }); @@ -59,8 +83,12 @@ test('classifyTargetBindingMatch path 6: unavailable or out-of-range viewport ev [['e1', 'e2'], undefined], ] as const) { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], - siblingMatchRefs: ['e1', 'e2'], regionMemberRefs, viewportCandidateRef, + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + siblingMatchRefs: ['e1', 'e2'], + regionMemberRefs, + viewportCandidateRef, }); assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'no-signal-isolation' }); } @@ -72,7 +100,10 @@ test('classifyTargetBindingMatch path 6: a signal isolating a different node tha { siblingMatchRefs: ['e2'], regionMemberRefs: ['e1', 'e2'], viewportCandidateRef: 'e1' }, ]) { const result = classifyTargetBindingMatch({ - winnerRef: 'e1', matchedRefs: ['e1', 'e2'], identitySetRefs: ['e1', 'e2'], ...params, + winnerRef: 'e1', + matchedRefs: ['e1', 'e2'], + identitySetRefs: ['e1', 'e2'], + ...params, }); assert.deepEqual(result, { path: 6, outcome: 'unverifiable', reason: 'signal-isolated-wrong' }); } diff --git a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts index 1a467a3b3..09cfb4311 100644 --- a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts +++ b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts @@ -97,7 +97,10 @@ test('recorded simple iOS selector click and fill use runtime resolution and per const actions = daemon .session() ?.actions.filter((action) => action.command === 'click' || action.command === 'fill'); - assert.deepEqual(actions?.map((action) => action.command), ['click', 'fill']); + assert.deepEqual( + actions?.map((action) => action.command), + ['click', 'fill'], + ); const targetIdentities = actions?.map((action) => { const evidence = action.targetEvidence as Record | undefined; return { From 68b569840ef238ceb870c6c1876a6a1e41f694a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 17:52:01 +0200 Subject: [PATCH 8/9] fix: record get target evidence, fail closed on broken parent linkage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer blockers on ADR 0012 step 3: - get text/attrs now records target-v1 evidence: GetCommandResult carries the resolution tree (preActionNodes, internal — neither the recorded result nor the public payload copies it), recordIfSession gains the typed recordedTarget channel, and dispatchGetViaRuntime threads the capture through. The direct-iOS get query is gated during recording so the snapshot path supplies the evidence tree, mirroring the tap/fill gating. find/is stay uncovered: their results are match-set shaped (found flags, predicate booleans over possibly-many matches), not a single resolved winner — noted in the PR. - buildAncestryChain now reports a broken parent walk (dangling parentIndex or cycle) instead of silently producing a root-like chain; the writer fails the annotation closed to 'unverifiable' per decision 3's capture-anomaly rule, and broken candidates cannot prove an ancestry prefix. Regression tests for both anomaly shapes. - New ADR 0012 recording tests live in focused files (interaction-target-evidence.test.ts) instead of growing the interaction.test.ts aggregation; the earlier additions moved out. --- .../interaction/runtime/selector-read.ts | 11 +- .../__tests__/session-target-evidence.test.ts | 64 ++++- .../interaction-target-evidence.test.ts | 231 ++++++++++++++++++ .../handlers/__tests__/interaction.test.ts | 107 -------- src/daemon/handlers/interaction-common.ts | 7 +- .../handlers/interaction-touch-response.ts | 8 +- src/daemon/selector-recording.ts | 6 + src/daemon/selector-runtime.ts | 7 + src/daemon/session-target-evidence.ts | 57 +++-- 9 files changed, 350 insertions(+), 148 deletions(-) create mode 100644 src/daemon/handlers/__tests__/interaction-target-evidence.test.ts diff --git a/src/commands/interaction/runtime/selector-read.ts b/src/commands/interaction/runtime/selector-read.ts index 814f30970..3ec4ee563 100644 --- a/src/commands/interaction/runtime/selector-read.ts +++ b/src/commands/interaction/runtime/selector-read.ts @@ -78,12 +78,16 @@ export type GetCommandResult = text: string; node: SnapshotNode; selectorChain?: string[]; + /** ADR 0012 decision 3: the tree `node` was resolved from, for record-time evidence. */ + preActionNodes: SnapshotNode[]; } | { kind: 'attrs'; target: ResolvedTarget; node: SnapshotNode; selectorChain?: string[]; + /** ADR 0012 decision 3: the tree `node` was resolved from, for record-time evidence. */ + preActionNodes: SnapshotNode[]; }; export type GetTextCommandOptions = CommandContext & @@ -199,11 +203,12 @@ export const getCommand: RuntimeCommand = a action: 'get', }); const target = { kind: 'ref' as const, ref: `@${resolved.ref}` }; + const preActionNodes = capture.snapshot.nodes; if (options.property === 'attrs') { - return { kind: 'attrs', target, node: resolved.node, selectorChain }; + return { kind: 'attrs', target, node: resolved.node, selectorChain, preActionNodes }; } const text = await readText(runtime, capture, resolved.node); - return { kind: 'text', target, text, node: resolved.node, selectorChain }; + return { kind: 'text', target, text, node: resolved.node, selectorChain, preActionNodes }; } const resolved = await resolveSelectorNode(runtime, options, options.session ?? 'default', { @@ -221,6 +226,7 @@ export const getCommand: RuntimeCommand = a target: { kind: 'selector', selector: resolved.selector }, node: resolved.node, selectorChain, + preActionNodes: resolved.capture.snapshot.nodes, }; } @@ -231,6 +237,7 @@ export const getCommand: RuntimeCommand = a text, node: resolved.node, selectorChain, + preActionNodes: resolved.capture.snapshot.nodes, }; }; diff --git a/src/daemon/__tests__/session-target-evidence.test.ts b/src/daemon/__tests__/session-target-evidence.test.ts index faf024281..81d278db7 100644 --- a/src/daemon/__tests__/session-target-evidence.test.ts +++ b/src/daemon/__tests__/session-target-evidence.test.ts @@ -44,7 +44,7 @@ function toolbarFixture(): SnapshotNode[] { test('computeTargetEvidence: single unambiguous node is verified with the expected identity/ancestry/rect', () => { const nodes = toolbarFixture(); const winner = findByLabel(nodes, 'Save'); - const evidence = computeTargetEvidence({ node: winner, nodes }); + const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes }); assert.ok(evidence); assert.deepEqual(evidence, { id: 'save', @@ -102,7 +102,7 @@ function scrollableListFixture(): SnapshotNode[] { test('computeTargetEvidence: scrollRegion is the nearest scrollable ancestor local identity, viewportOrder is top-to-bottom within it', () => { const nodes = scrollableListFixture(); const winner = nodes[2]!; // rect y:100 -> should be last (index 2) in top-to-bottom order - const evidence = computeTargetEvidence({ node: winner, nodes }); + const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes }); assert.ok(evidence); assert.deepEqual(evidence.scrollRegion, { role: 'scrollview', id: 'editor-scroll' }); // rows are at y=0,50,100 -> the y:100 row is ordinal 2 (0-based) top-to-bottom @@ -113,11 +113,14 @@ test('computeTargetEvidence: scrollRegion is the nearest scrollable ancestor loc test('computeTargetEvidence: a stable scroll-region ID takes precedence over a changed label', () => { const recordedNodes = scrollableListFixture(); recordedNodes[1]!.label = 'Inbox'; - const recorded = computeTargetEvidence({ node: recordedNodes[2]!, nodes: recordedNodes }); + const recorded = computeTargetEvidence({ + node: recordedNodes[2]!, + preActionNodes: recordedNodes, + }); const currentNodes = scrollableListFixture(); currentNodes[1]!.label = 'Messages'; - const current = computeTargetEvidence({ node: currentNodes[2]!, nodes: currentNodes }); + const current = computeTargetEvidence({ node: currentNodes[2]!, preActionNodes: currentNodes }); assert.ok(recorded); assert.ok(current); @@ -154,7 +157,7 @@ test('computeTargetEvidence: duplicate identity across two parents is still veri }, ]); const winner = nodes[4]!; // the second "Go back" button, under the second Row - const evidence = computeTargetEvidence({ node: winner, nodes }); + const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes }); assert.ok(evidence); assert.equal(evidence.id, undefined); assert.equal(evidence.role, 'button'); @@ -199,7 +202,7 @@ test('computeTargetEvidence: max-size labels across K=8 ancestors reduce ancestr const nodes = toSnapshotNodes(raw); const winner = nodes.at(-1)!; - const evidence = computeTargetEvidence({ node: winner, nodes }); + const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes }); assert.ok(evidence); assert.ok(evidence.ancestry.length <= TARGET_ANNOTATION_MAX_ANCESTRY); assert.ok( @@ -228,7 +231,7 @@ test('computeTargetEvidence: a root node with no ancestors has empty ancestry an depth: 0, }, ]); - const evidence = computeTargetEvidence({ node: nodes[0]!, nodes }); + const evidence = computeTargetEvidence({ node: nodes[0]!, preActionNodes: nodes }); assert.ok(evidence); assert.deepEqual(evidence.ancestry, []); assert.equal(evidence.verification, 'verified'); @@ -322,7 +325,7 @@ function bottomTabsRealCaptureFixture(): SnapshotNode[] { test('computeTargetEvidence: real-capture-shaped tree (undefined hittable, anonymous wrapper ancestor)', () => { const nodes = bottomTabsRealCaptureFixture(); const winner = findByLabel(nodes, 'Article, unselected'); - const evidence = computeTargetEvidence({ node: winner, nodes }); + const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes }); assert.ok(evidence); assert.equal(evidence.id, 'article'); assert.equal(evidence.role, 'button'); @@ -358,7 +361,7 @@ test('computeTargetEvidence: a node with an over-cap id still matches itself and depth: 0, }, ]); - const evidence = computeTargetEvidence({ node: nodes[0]!, nodes }); + const evidence = computeTargetEvidence({ node: nodes[0]!, preActionNodes: nodes }); assert.ok(evidence); assert.equal(evidence.id, overCapId.slice(0, TARGET_ANNOTATION_MAX_FIELD_BYTES)); assert.equal(evidence.verification, 'verified'); @@ -403,7 +406,7 @@ test('computeTargetEvidence: reduction sizes against the worst-case verification let sawReducedAncestry = false; for (let tunable = 0; tunable <= TARGET_ANNOTATION_MAX_FIELD_BYTES; tunable += 1) { const nodes = buildChain(tunable); - const evidence = computeTargetEvidence({ node: nodes.at(-1)!, nodes }); + const evidence = computeTargetEvidence({ node: nodes.at(-1)!, preActionNodes: nodes }); assert.ok(evidence); if (evidence.ancestry.length === TARGET_ANNOTATION_MAX_ANCESTRY) sawFullAncestry = true; else sawReducedAncestry = true; @@ -422,3 +425,44 @@ test('computeTargetEvidence: reduction sizes against the worst-case verification assert.ok(sawFullAncestry, 'sweep never produced an unreduced payload — fixture too large'); assert.ok(sawReducedAncestry, 'sweep never forced a reduction — fixture too small'); }); + +// --------------------------------------------------------------------------- +// Broken parent linkage is a capture anomaly (decision 3): fail closed to +// 'unverifiable', never 'verified' on structural data that cannot be trusted. +// --------------------------------------------------------------------------- + +test('computeTargetEvidence: a dangling parentIndex records unverifiable, never verified', () => { + const nodes = toSnapshotNodes([ + { + index: 0, + type: 'Button', + identifier: 'save', + label: 'Save', + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 1, + parentIndex: 42, // no node with index 42 exists + }, + ]); + const evidence = computeTargetEvidence({ node: nodes[0]!, preActionNodes: nodes }); + assert.ok(evidence); + assert.equal(evidence.verification, 'unverifiable'); +}); + +test('computeTargetEvidence: a parent cycle records unverifiable, never verified', () => { + const nodes = toSnapshotNodes([ + { index: 0, type: 'View', depth: 0, parentIndex: 1 }, + { index: 1, type: 'View', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'Button', + identifier: 'save', + label: 'Save', + rect: { x: 0, y: 0, width: 10, height: 10 }, + depth: 2, + parentIndex: 1, + }, + ]); + const evidence = computeTargetEvidence({ node: nodes[2]!, preActionNodes: nodes }); + assert.ok(evidence); + assert.equal(evidence.verification, 'unverifiable'); +}); diff --git a/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts new file mode 100644 index 000000000..23e40be42 --- /dev/null +++ b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts @@ -0,0 +1,231 @@ +import { test, expect, vi, beforeEach } from 'vitest'; +import { handleInteractionCommands } from '../interaction.ts'; +import { attachRefs, type RawSnapshotNode } from '../../../kernel/snapshot.ts'; +import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; +import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import type { CommandFlags } from '../../../core/dispatch.ts'; +import type { SessionState } from '../../types.ts'; + +// ADR 0012 decision 3, daemon-routed recording: target-v1 evidence is +// computed only while the session is being recorded, lands on the recorded +// action (never the public response), and the raw resolved node/tree reaches +// neither the client nor session history. Covers the touch path +// (finalizeTouchInteraction) and the get path (recordIfSession), including +// the recording gate on the direct-iOS get fast path. + +const { mockRunAppleRunnerCommand } = vi.hoisted(() => ({ + mockRunAppleRunnerCommand: vi.fn(), +})); + +vi.mock('../../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchCommand: vi.fn(async () => ({})), + }; +}); + +vi.mock('../../../platforms/apple/core/runner/runner-client.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + runAppleRunnerCommand: mockRunAppleRunnerCommand, + }; +}); + +import { dispatchCommand } from '../../../core/dispatch.ts'; +const mockDispatch = vi.mocked(dispatchCommand); + +const contextFromFlags = (_flags: CommandFlags | undefined) => ({}); + +beforeEach(() => { + mockDispatch.mockReset(); + mockDispatch.mockResolvedValue({}); + mockRunAppleRunnerCommand.mockReset(); + mockRunAppleRunnerCommand.mockResolvedValue({}); +}); + +const SAVE_BUTTON_NODES: RawSnapshotNode[] = [ + { + index: 0, + type: 'XCUIElementTypeButton', + identifier: 'save', + label: 'Save', + rect: { x: 10, y: 20, width: 100, height: 40 }, + enabled: true, + hittable: true, + }, +]; + +function makeSessionWithSnapshot( + sessionName: string, + options: { recordSession: boolean }, +): SessionState { + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + session.recordSession = options.recordSession; + session.snapshot = { + nodes: attachRefs(SAVE_BUTTON_NODES), + createdAt: Date.now(), + backend: 'xctest', + }; + return session; +} + +async function runCommand( + sessionStore: ReturnType, + sessionName: string, + command: string, + positionals: string[], + flags: CommandFlags = {}, +) { + return await handleInteractionCommands({ + req: { token: 't', session: sessionName, command, positionals, flags }, + sessionName, + sessionStore, + contextFromFlags, + }); +} + +test('press @ref while recording attaches target-v1 evidence to the recorded action, never to the public response', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-press'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: true })); + + // verify:true forces full runtime resolution, which captures node/tree. + const response = await runCommand(sessionStore, sessionName, 'press', ['@e1'], { verify: true }); + + 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('targetEvidence'); + } + + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toEqual({ + id: 'save', + role: 'button', + label: 'Save', + ancestry: [], + sibling: 0, + viewportOrder: 0, + rect: { x: 10, y: 20, width: 100, height: 40 }, + verification: 'verified', + }); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('press @ref without recording never computes target-v1 evidence', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'non-recording-press'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: false })); + + const response = await runCommand(sessionStore, sessionName, 'press', ['@e1'], { verify: true }); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toBeUndefined(); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('get text @ref while recording attaches target-v1 evidence to the recorded action, never to session history payloads', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-get-ref'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: true })); + + const response = await runCommand(sessionStore, sessionName, 'get', ['text', '@e1']); + + expect(response?.ok).toBe(true); + if (response?.ok) { + expect(response.data).not.toHaveProperty('preActionNodes'); + expect(response.data).not.toHaveProperty('targetEvidence'); + } + + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toEqual({ + id: 'save', + role: 'button', + label: 'Save', + ancestry: [], + sibling: 0, + viewportOrder: 0, + rect: { x: 10, y: 20, width: 100, height: 40 }, + verification: 'verified', + }); + expect(recordedAction?.result).not.toHaveProperty('node'); + expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); +}); + +test('get text @ref without recording never computes target-v1 evidence', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'non-recording-get-ref'; + sessionStore.set(sessionName, makeSessionWithSnapshot(sessionName, { recordSession: false })); + + const response = await runCommand(sessionStore, sessionName, 'get', ['text', '@e1']); + + expect(response?.ok).toBe(true); + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toBeUndefined(); +}); + +test('get text simple iOS id selector while recording skips the direct runner query and records evidence from the snapshot path', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-get-direct-gate'; + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + session.recordSession = true; + sessionStore.set(sessionName, session); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') { + return { + backend: 'xctest', + nodes: [ + { + index: 0, + depth: 0, + type: 'Application', + rect: { x: 0, y: 0, width: 393, height: 852 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'TextField', + label: 'Name', + identifier: 'field-name', + value: 'Ada Lovelace', + rect: { x: 24, y: 220, width: 320, height: 48 }, + enabled: true, + hittable: true, + }, + ], + }; + } + return {}; + }); + + const response = await runCommand(sessionStore, sessionName, 'get', ['text', 'id="field-name"']); + + expect(response?.ok).toBe(true); + // The direct-iOS querySelector fast path must be gated during recording so + // the snapshot path supplies the evidence tree. + expect(mockRunAppleRunnerCommand).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ command: 'querySelector' }), + expect.anything(), + ); + expect(mockDispatch.mock.calls.map((call) => call[1])).toContain('snapshot'); + + const recordedAction = sessionStore.get(sessionName)?.actions[0]; + expect(recordedAction?.targetEvidence).toMatchObject({ + id: 'field-name', + role: 'textfield', + label: 'Name', + ancestry: [{ role: 'application' }], + verification: 'verified', + }); +}); diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index ccf04335c..03a6fbbe4 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -3560,110 +3560,3 @@ test('after a session reopen, a pin from the previous lifetime warns (reseeded g expect(String(response.data?.warning)).toContain(`minted from snapshot s${oldGeneration}`); } }); - -// --------------------------------------------------------------------------- -// ADR 0012 decision 3: target-v1 evidence is computed only while the session -// is being recorded, lands on the recorded action (never the public -// response), and the raw resolved node/tree never reaches either the client -// or session history. -// --------------------------------------------------------------------------- - -test('press @ref while recording attaches target-v1 evidence to the recorded action, never to the public response', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'recording-press'; - const session = makeSession(sessionName); - session.recordSession = true; - session.snapshot = { - nodes: attachRefs([ - { - index: 0, - type: 'XCUIElementTypeButton', - identifier: 'save', - label: 'Save', - rect: { x: 10, y: 20, width: 100, height: 40 }, - enabled: true, - hittable: true, - }, - ]), - createdAt: Date.now(), - backend: 'xctest', - }; - sessionStore.set(sessionName, session); - - const response = await handleInteractionCommands({ - req: { - token: 't', - session: sessionName, - command: 'press', - positionals: ['@e1'], - // verify:true forces full runtime resolution, which captures node/tree. - flags: { verify: true }, - }, - sessionName, - sessionStore, - contextFromFlags, - }); - - 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('targetEvidence'); - } - - const recordedAction = sessionStore.get(sessionName)?.actions[0]; - expect(recordedAction?.targetEvidence).toEqual({ - id: 'save', - role: 'button', - label: 'Save', - ancestry: [], - sibling: 0, - viewportOrder: 0, - rect: { x: 10, y: 20, width: 100, height: 40 }, - verification: 'verified', - }); - expect(recordedAction?.result).not.toHaveProperty('node'); - expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); -}); - -test('press @ref without recording never computes target-v1 evidence', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'non-recording-press'; - const session = makeSession(sessionName); - session.recordSession = false; - session.snapshot = { - nodes: attachRefs([ - { - index: 0, - type: 'XCUIElementTypeButton', - identifier: 'save', - label: 'Save', - rect: { x: 10, y: 20, width: 100, height: 40 }, - enabled: true, - hittable: true, - }, - ]), - createdAt: Date.now(), - backend: 'xctest', - }; - sessionStore.set(sessionName, session); - - const response = await handleInteractionCommands({ - req: { - token: 't', - session: sessionName, - command: 'press', - positionals: ['@e1'], - flags: { verify: true }, - }, - sessionName, - sessionStore, - contextFromFlags, - }); - - expect(response?.ok).toBe(true); - const recordedAction = sessionStore.get(sessionName)?.actions[0]; - expect(recordedAction?.targetEvidence).toBeUndefined(); - expect(recordedAction?.result).not.toHaveProperty('node'); - expect(recordedAction?.result).not.toHaveProperty('preActionNodes'); -}); diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index 51117b1a9..218b9abf9 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -13,8 +13,7 @@ import { stripInternalInteractionFlags, } from '../interaction-outcome-policy.ts'; import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; -import { computeTargetEvidence } from '../session-target-evidence.ts'; -import type { RecordedTargetCapture } from './interaction-touch-response.ts'; +import { computeTargetEvidence, type RecordedTargetCapture } from '../session-target-evidence.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -61,9 +60,7 @@ export function finalizeTouchInteraction(params: { } = params; const actionFlags = stripInternalInteractionFlags(flags); const targetEvidence = - session.recordSession && recordedTarget - ? computeTargetEvidence({ node: recordedTarget.node, nodes: recordedTarget.preActionNodes }) - : undefined; + session.recordSession && recordedTarget ? computeTargetEvidence(recordedTarget) : undefined; sessionStore.recordAction(session, { command, positionals, diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index 6f3972c3b..7a50e51f7 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -5,7 +5,7 @@ import type { PressCommandResult, SettleObservation, } from '../../contracts/interaction.ts'; -import type { SnapshotNode } from '../../kernel/snapshot.ts'; +import type { RecordedTargetCapture } from '../session-target-evidence.ts'; import { successText } from '../../utils/success-text.ts'; import { interactionResultExtra } from './interaction-touch-targets.ts'; @@ -39,12 +39,6 @@ export type InteractionResponseSource = point: { x: number; y: number }; }; -/** ADR 0012 decision 3: record-time input for `computeTargetEvidence`. */ -export type RecordedTargetCapture = { - node: SnapshotNode; - preActionNodes: SnapshotNode[]; -}; - export type InteractionResponsePayloads = { /** Recorded in session history and used for touch visualization. */ result: Record; diff --git a/src/daemon/selector-recording.ts b/src/daemon/selector-recording.ts index 1536c9236..367bffb95 100644 --- a/src/daemon/selector-recording.ts +++ b/src/daemon/selector-recording.ts @@ -1,5 +1,6 @@ import type { DaemonRequest } from './types.ts'; import { SessionStore } from './session-store.ts'; +import { computeTargetEvidence, type RecordedTargetCapture } from './session-target-evidence.ts'; export function buildFindRecordResult( result: Record, @@ -86,14 +87,19 @@ export function recordIfSession( sessionName: string, req: DaemonRequest, result: Record, + /** ADR 0012 decision 3: record-time input for the `target-v1` annotation. */ + recordedTarget?: RecordedTargetCapture, ): void { const session = sessionStore.get(sessionName); if (!session) return; + const targetEvidence = + session.recordSession && recordedTarget ? computeTargetEvidence(recordedTarget) : undefined; sessionStore.recordAction(session, { command: req.command, positionals: req.positionals ?? [], flags: req.flags ?? {}, result, + ...(targetEvidence ? { targetEvidence } : {}), }); } diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index a5e682bb7..2f64ac3ea 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -171,6 +171,10 @@ export async function dispatchGetViaRuntime( params.sessionName, req, buildGetRecordResult(result, sub), + { + node: result.node, + preActionNodes: result.preActionNodes, + }, ); const data = toDaemonGetData(result); return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data; @@ -296,6 +300,9 @@ async function dispatchDirectIosSelectorGet( selectorExpression: string, ): Promise { const session = params.sessionStore.get(params.sessionName); + // ADR 0012 decision 3: recording requires the snapshot path so target + // evidence can be computed from the resolution tree. + if (session?.recordSession) return null; const selector = readSimpleIosSelectorTarget({ session, selectorExpression }); if (!session || !selector) return null; // get text intentionally disambiguates label/text/value triplets from snapshots; the runner diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index c527b4644..3ac772901 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -32,15 +32,21 @@ import { type TargetVerification, } from '../replay/target-identity.ts'; -export function computeTargetEvidence(params: { +/** ADR 0012 decision 3: the resolved winner and the tree it was resolved from. */ +export type RecordedTargetCapture = { node: SnapshotNode; - nodes: readonly SnapshotNode[]; -}): TargetAnnotationV1 | undefined { - const { node, nodes } = params; + preActionNodes: SnapshotNode[]; +}; + +export function computeTargetEvidence( + capture: RecordedTargetCapture, +): TargetAnnotationV1 | undefined { + const { node, preActionNodes: nodes } = capture; if (typeof node.index !== 'number') return undefined; const byIndex = buildIndexMap(nodes); const identity = boundedLocalIdentity(node); - const fullAncestry = buildAncestryChain(node, byIndex, TARGET_ANNOTATION_MAX_ANCESTRY); + const ancestryWalk = buildAncestryChain(node, byIndex, TARGET_ANNOTATION_MAX_ANCESTRY); + const fullAncestry = ancestryWalk.chain; const sibling = computeSiblingOrdinal(nodes, node); const scrollRegion = computeScrollRegionKey(node, byIndex); const rect = boundedRect(node); @@ -80,7 +86,11 @@ export function computeTargetEvidence(params: { utf8ByteLength(serializeTargetAnnotationV1({ ...candidate, verification: 'unverifiable' })) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES ) { - candidate.verification = runRecordTimeSelfCheck({ node, domain }); + // 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) { @@ -123,26 +133,38 @@ function boundedLocalIdentity(node: SnapshotNode): LocalIdentity { }; } +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?}. */ function buildAncestryChain( node: SnapshotNode, byIndex: Map, limit: number, -): TargetAncestryEntry[] { +): AncestryWalk { const chain: TargetAncestryEntry[] = []; - const visited = new Set(); - let current = typeof node.parentIndex === 'number' ? byIndex.get(node.parentIndex) : undefined; - while (current && !visited.has(current.index) && chain.length < limit) { - visited.add(current.index); - const identity = boundedLocalIdentity(current); + 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 = - typeof current.parentIndex === 'number' ? byIndex.get(current.parentIndex) : undefined; + current = parent; } - return chain; + return { chain, broken: false }; } /** @@ -223,8 +245,9 @@ function computeDisambiguationDomain(params: { // leaf-anchored ancestry prefix. const identitySet = nodes.filter((candidate) => { if (!matchesLocalIdentity(boundedLocalIdentity(candidate), identity)) return false; - const observedAncestry = buildAncestryChain(candidate, byIndex, Math.max(ancestry.length, 1)); - return matchesAncestryPrefix(observedAncestry, ancestry); + 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); }); const siblingMatches = identitySet.filter( From 5b6e932ffd57c5efeeb01ea3b77fc19b958a86c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 18:00:02 +0200 Subject: [PATCH 9/9] refactor: extract direct-iOS get selector guards below the complexity threshold --- src/daemon/selector-runtime.ts | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 2f64ac3ea..7dff9880e 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -294,26 +294,34 @@ export async function dispatchWaitViaRuntime( return await withSessionlessRunnerCleanup(session, device, execute); } -async function dispatchDirectIosSelectorGet( - params: SelectorRuntimeParams, +function readDirectIosGetSelector( + session: SessionState | undefined, property: 'text' | 'attrs', selectorExpression: string, -): Promise { - const session = params.sessionStore.get(params.sessionName); +): DirectIosSelectorTarget | null { // ADR 0012 decision 3: recording requires the snapshot path so target // evidence can be computed from the resolution tree. - if (session?.recordSession) return null; + if (!session || session.recordSession) return null; const selector = readSimpleIosSelectorTarget({ session, selectorExpression }); - if (!session || !selector) return null; // get text intentionally disambiguates label/text/value triplets from snapshots; the runner // direct query rejects those ambiguous matches before the shared selector resolver can rank them. - if (property === 'text' && selector.key !== 'id') return null; + if (property === 'text' && selector?.key !== 'id') return null; + return selector; +} + +async function dispatchDirectIosSelectorGet( + params: SelectorRuntimeParams, + property: 'text' | 'attrs', + selectorExpression: string, +): Promise { + const session = params.sessionStore.get(params.sessionName); + const selector = readDirectIosGetSelector(session, property, selectorExpression); + if (!session || !selector) return null; const result = await queryDirectIosSelectorOrFallback(params, session, selector); if (isDirectIosSelectorErrorResult(result)) return result.response; if (!result) return null; - const directQuery = { session, selector, result }; - const payload = buildDirectIosGetResult(property, directQuery.selector.raw, directQuery.result); + const payload = buildDirectIosGetResult(property, selector.raw, result); if (!payload) return null; recordIfSession( params.sessionStore,