From 77d68985c29881d4d6fe5083e8048ef808734b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 12:12:08 +0200 Subject: [PATCH 1/3] feat(ios): carry the presented-surface fact from capture producers to consumers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A settle confirmation that has to prove a dismissal (#1831) has nothing to compare against: the capture contract says nothing about whether a modal presentation was over the app, so a consumer can only infer it from tree shape — and a tree shape that happens to hold before a tap is exactly what makes the confirmation arm on a no-op. iOS is where it matters most, because there an overlay is usually the same app process, so it moves neither the surface identity nor the foreground-owner fact. Publish the producer's own reading instead, on the state that is already built from the capture's unfiltered nodes: - `SnapshotState.presentedSurface` takes 'presented' | 'absent' | 'unknown', and `readPresentedSurfaceFact` is the one wire reader. A producer that inspected its whole subject and saw nothing presenting says 'absent'; one whose evidence cannot answer says 'unknown'; only a producer that never classified stays silent. Anything the vocabulary cannot name reads as 'unknown', so an unrecognised value can never become a false absence. - The fact rides the response envelope on every iOS capture route, including a digest, and travels with the nodes wherever a capture crosses a layer: the backend capture and interactor snapshot results, the daemon capture cache, the interactive-retry and deferred-re-observation nodes, the ref-target frame, and the settle baseline. A consumer that adopts an interaction's own capture therefore keeps the fact that capture established rather than re-deriving a weaker one. - Both settle routes mint that baseline from one function, which moves to capture-kit beside the assembly that builds the state it reads: the interaction route lives in `commands/` and the generic route in `daemon/`, and the layering forbids the second from importing the first. So the next fact a baseline owes cannot be carried by one route and dropped by the other. - The runner forwards the reading of the tier that answered. Its accessibility tree carries no window classes, so it states 'presented' from a resolved blocking system modal and never 'absent'; the shared classifier comes next. Consumers are untouched: nothing decides on the fact yet. --- .../RunnerTests+Models.swift | 5 + .../RunnerTests+SystemModal.swift | 12 ++- ...erTests+PresentedSurfacePayloadTests.swift | 26 ++++++ .../PresentedSurface.swift | 17 ++++ packages/capture-kit/package.json | 4 + .../capture-kit/src/snapshot-baseline.test.ts | 38 ++++++++ packages/capture-kit/src/snapshot-baseline.ts | 25 +++++ packages/capture-kit/src/snapshot-state.ts | 7 ++ packages/contracts/src/backend.ts | 7 ++ packages/contracts/src/client-capture.ts | 8 ++ packages/contracts/src/interaction.ts | 19 +++- packages/contracts/src/interactor-types.ts | 7 ++ packages/daemon/src/core/snapshot-capture.ts | 6 ++ .../daemon/src/execution/generic-settle.ts | 8 +- .../internal/find-target-capture.ts | 2 +- .../execution/interaction/internal/find.ts | 9 ++ .../server/__tests__/response-views.test.ts | 12 +++ packages/daemon/src/server/response-views.ts | 1 + packages/kernel/src/presented-surface.test.ts | 32 +++++++ packages/kernel/src/record.ts | 24 ++++- packages/kernel/src/snapshot.ts | 24 +++++ packages/platform-apple/src/interactor.ts | 1 + .../__tests__/snapshot-presentation.test.ts | 25 +++++ .../src/runner/snapshot-presentation.ts | 15 ++- scripts/layering/package-boundaries.test.ts | 1 + .../client-snapshot-presented-surface.test.ts | 43 +++++++++ src/agent-device-client.ts | 9 +- .../presented-surface-carriage.test.ts | 92 +++++++++++++++++++ src/commands/capture/runtime/snapshot.ts | 13 +++ .../runtime/native-ref-interaction.ts | 2 +- .../runtime/post-action-observation.ts | 3 +- .../runtime/post-action-surface.ts | 11 --- .../runtime/ref-target-resolution.ts | 3 +- .../interaction/runtime/resolution.ts | 2 +- src/commands/interaction/runtime/settle.ts | 8 +- src/commands/output/result-serialization.ts | 1 + .../output/snapshot-serialization.test.ts | 25 +++++ src/core/__tests__/snapshot-state.test.ts | 21 +++++ 38 files changed, 536 insertions(+), 32 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+PresentedSurfacePayloadTests.swift create mode 100644 apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift create mode 100644 packages/capture-kit/src/snapshot-baseline.test.ts create mode 100644 packages/capture-kit/src/snapshot-baseline.ts create mode 100644 packages/kernel/src/presented-surface.test.ts create mode 100644 src/__tests__/client-snapshot-presented-surface.test.ts create mode 100644 src/commands/capture/runtime/presented-surface-carriage.test.ts diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift index dd35949a9b..09e412cf04 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift @@ -438,6 +438,11 @@ struct DataPayload: Codable { var snapshotQuality: SnapshotQuality? /// Set when the capture describes an in-place system surface, not the app itself (#2438). var systemSurface: SystemSurfaceProvenancePayload? + /// Whether this capture read a modal presentation over the app's own content (#3565). The runner's + /// accessibility tree reports no UIKit class names, so a runner capture states `presented` from + /// positive evidence and `unknown` otherwise; `absent` belongs to a producer that can read window + /// structure, and a tier that classified nothing publishes no key at all. + var presentedSurface: PresentedSurface? /// The keyboard band this capture measured, when the tier that answered reads keyboards at all /// (#2660). Absent means the query-sweep or private-AX tier answered, and the daemon's tap guard /// keeps deriving the band from the tree. diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SystemModal.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SystemModal.swift index c05d3fb2f9..bfc3c931eb 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SystemModal.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SystemModal.swift @@ -61,10 +61,20 @@ extension RunnerTests { nodes.append(contentNode) } - return DataPayload(nodes: nodes, truncated: false) + // The probe resolved a blocking modal over the app, so this payload states its own subject rather + // than leaving a reader to infer it from a tree that holds only the modal (#3565). + return Self.blockingSystemModalPayload(nodes: nodes) #endif } + /// The payload a resolved blocking system modal publishes: the modal's own tree, and the fact that + /// the modal is what it describes. The probe is the one capture that states `presented` without + /// reading a window structure — its tree holds only the modal, so nothing downstream could read the + /// presentation out of it (#3565). Split from the probe body because that body needs a device. + static func blockingSystemModalPayload(nodes: [PresentedNode]) -> DataPayload { + DataPayload(nodes: nodes, truncated: false, presentedSurface: .presented) + } + func firstBlockingSystemModal( in springboard: XCUIApplication, deadline: Date = .distantFuture diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+PresentedSurfacePayloadTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+PresentedSurfacePayloadTests.swift new file mode 100644 index 0000000000..353eed92ed --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+PresentedSurfacePayloadTests.swift @@ -0,0 +1,26 @@ +import XCTest +import AgentDeviceSnapshotPresentation + +#if AGENT_DEVICE_RUNNER_UNIT_TESTS +// #3565: a capture has to say whether it read a modal presentation over the app. The blocking +// system-modal probe is the one capture that can state `presented` without reading any window +// structure: the tree it publishes holds only the modal, so nothing downstream could read a +// presentation out of it. Only the payload factory is pinned here, because the probe body that calls it +// enumerates XCUIElements and needs a device. +extension RunnerTests { + func testResolvedBlockingSystemModalPayloadStatesAPresentedSurface() { + XCTAssertEqual(Self.blockingSystemModalPayload(nodes: []).presentedSurface, .presented) + } + + // Silence is the answer a consumer must not be able to confuse with absence: a capture the runner + // never classified (an older runner, a tier that published no reading) encodes no key at all. + func testUnclassifiedSnapshotPayloadEncodesNoPresentedSurfaceKey() throws { + let encoded = try JSONEncoder().encode(DataPayload(nodes: [], truncated: false)) + let object = try XCTUnwrap( + JSONSerialization.jsonObject(with: encoded) as? [String: Any], + "the snapshot payload encodes as an object" + ) + XCTAssertNil(object["presentedSurface"]) + } +} +#endif diff --git a/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift b/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift new file mode 100644 index 0000000000..dc77d705d7 --- /dev/null +++ b/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift @@ -0,0 +1,17 @@ +import Foundation + +/// Whether this capture read a modal presentation over the app's own content (#3565). The producer owns +/// the answer, and `absent` is a reading rather than a default, because a consumer arms a dismissal +/// confirmation on it: a capture whose evidence cannot answer says `unknown` instead of staying silent, +/// since silence is also what a producer that never classified publishes. +/// +/// Mirrors the TypeScript `PresentedSurface` in `@agent-device/kernel/snapshot`. The rule that arrives at +/// one of these three values is `SnapshotPresentedSurface` beside this type. +public enum PresentedSurface: String, Codable, Equatable { + /// A presentation covers the content this capture describes. + case presented + /// Every window this capture read showed only its own content container. + case absent + /// The capture supports neither answer. + case unknown +} diff --git a/packages/capture-kit/package.json b/packages/capture-kit/package.json index d4f9d14a04..0c164af2a9 100644 --- a/packages/capture-kit/package.json +++ b/packages/capture-kit/package.json @@ -226,6 +226,10 @@ "types": "./src/snapshot/scroll-edge-state.ts", "default": "./src/snapshot/scroll-edge-state.ts" }, + "./snapshot-baseline": { + "types": "./src/snapshot-baseline.ts", + "default": "./src/snapshot-baseline.ts" + }, "./snapshot-chrome": { "types": "./src/snapshot-chrome.ts", "default": "./src/snapshot-chrome.ts" diff --git a/packages/capture-kit/src/snapshot-baseline.test.ts b/packages/capture-kit/src/snapshot-baseline.test.ts new file mode 100644 index 0000000000..8aed6e04ba --- /dev/null +++ b/packages/capture-kit/src/snapshot-baseline.test.ts @@ -0,0 +1,38 @@ +import { expect, test } from 'vitest'; +import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import { surfaceScopedNodes } from './snapshot-baseline.ts'; + +const nodes = [{ index: 0, type: 'Application', ref: 'e1' }]; + +/** + * #3565 and #2438: this is the one mint both settle routes take their baseline from, so every fact a + * capture established about what it read has to leave here with the nodes. A baseline that lost the + * presented-surface fact would read to #1831's dismissal confirmation as "no producer spoke" instead of + * the reading the capture actually took. + */ +test('the mint carries the presented-surface fact beside the nodes', () => { + for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { + expect(surfaceScopedNodes(snapshot({ presentedSurface }))).toEqual({ + nodes, + presentedSurface, + }); + } +}); + +test('the mint carries no presented surface a capture never read', () => { + expect(surfaceScopedNodes(snapshot({}))).toEqual({ nodes }); +}); + +test('the mint carries the system surface the nodes describe', () => { + expect( + surfaceScopedNodes(snapshot({ iosSystemSurfaceBundleId: 'com.apple.SafariViewService' })), + ).toEqual({ nodes, surfaceBundleId: 'com.apple.SafariViewService' }); +}); + +test('a session with no stored snapshot still mints an empty baseline', () => { + expect(surfaceScopedNodes(undefined)).toEqual({ nodes: [] }); +}); + +function snapshot(fields: Partial): SnapshotState { + return { nodes, createdAt: 0, presentationKey: '', ...fields } as SnapshotState; +} diff --git a/packages/capture-kit/src/snapshot-baseline.ts b/packages/capture-kit/src/snapshot-baseline.ts new file mode 100644 index 0000000000..594f7a20f6 --- /dev/null +++ b/packages/capture-kit/src/snapshot-baseline.ts @@ -0,0 +1,25 @@ +import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import type { SurfaceScopedNodes } from '@agent-device/contracts/interaction'; + +/** + * Mints the one baseline value a post-action comparison carries: the captured nodes together with the + * surface they describe and the presentation the capture read (#2438, #3565). + * + * Both settle routes take their baseline from here — the element-targeted interaction route and the + * generic daemon route (`scroll`/`back --settle`) — so a fact a capture established cannot be carried + * by one route and dropped by the other. The pair sits beside {@link buildSnapshotState} because that + * is the layer both routes can reach: the interaction route lives in `commands/`, the generic route in + * `daemon/`, and the layering forbids the second from importing the first. + * + * One value, never two channels: every boundary that carries a baseline carries the type this returns, + * so nodes cannot arrive without the surface they describe. + */ +export function surfaceScopedNodes(snapshot: SnapshotState | undefined): SurfaceScopedNodes { + return { + nodes: snapshot?.nodes ?? [], + ...(snapshot?.iosSystemSurfaceBundleId + ? { surfaceBundleId: snapshot.iosSystemSurfaceBundleId } + : {}), + ...(snapshot?.presentedSurface ? { presentedSurface: snapshot.presentedSurface } : {}), + }; +} diff --git a/packages/capture-kit/src/snapshot-state.ts b/packages/capture-kit/src/snapshot-state.ts index c1c361a3d8..79e13e565f 100644 --- a/packages/capture-kit/src/snapshot-state.ts +++ b/packages/capture-kit/src/snapshot-state.ts @@ -10,6 +10,7 @@ import { snapshotPresentationOptionsFromFlags, snapshotRefMinting, type IosTargetActivation, + type PresentedSurface, type RawSnapshotNode, type SnapshotBackend, type SnapshotCaptureProvenance, @@ -55,6 +56,11 @@ export function buildSnapshotState( keyboard?: SnapshotKeyboardBandFact; /** The box the producer measured the rects in (#3182), carried to the response that reads this state. */ viewport?: SnapshotViewportSize; + /** + * iOS: the producer's reading of a modal presentation over the app (#3565), decided on the + * acquired tree so every projection of one screen carries the same value. + */ + presentedSurface?: PresentedSurface; } & SnapshotCaptureProvenance, flags: | (Pick & @@ -92,6 +98,7 @@ export function buildSnapshotState( ? { comparisonKey: iosSnapshotComparisonIdentityKey(data.comparisonIdentity) } : {}), ...(data.systemSurface ? { iosSystemSurfaceBundleId: data.systemSurface.bundleId } : {}), + ...(data.presentedSurface ? { presentedSurface: data.presentedSurface } : {}), ...(data.targetActivation ? { targetActivation: data.targetActivation } : {}), ...(data.keyboard ? { keyboard: data.keyboard } : {}), ...(data.viewport ? { viewport: data.viewport } : {}), diff --git a/packages/contracts/src/backend.ts b/packages/contracts/src/backend.ts index ec47635b75..76bdfbca33 100644 --- a/packages/contracts/src/backend.ts +++ b/packages/contracts/src/backend.ts @@ -23,6 +23,7 @@ import type { import type { Point, Rect, + PresentedSurface, SnapshotKeyboardBandFact, SnapshotNode, SnapshotOptions, @@ -58,6 +59,12 @@ export type BackendSnapshotResult = { * the band from `nodes`. */ keyboard?: SnapshotKeyboardBandFact; + /** + * iOS: whether this capture read a modal presentation over the app (#3565), classified by the + * producer on its acquired tree. A backend that publishes nothing classified nothing; a consumer + * treats that as `'unknown'`, never as absence. + */ + presentedSurface?: PresentedSurface; /** * The box these rects are measured in, as the producer measured it (#3182). Absent means the * producer measured no box — never a zero — since that absence is the only answer a producer with diff --git a/packages/contracts/src/client-capture.ts b/packages/contracts/src/client-capture.ts index 6be33ac0bd..6024cbb0b0 100644 --- a/packages/contracts/src/client-capture.ts +++ b/packages/contracts/src/client-capture.ts @@ -4,6 +4,7 @@ import type { SessionSurface } from './session-surface.ts'; import type { PublicSnapshotCaptureAnnotations } from './snapshot-capture-annotations.ts'; import type { SnapshotDiagnosticsSummary } from './snapshot-diagnostics.ts'; import type { + PresentedSurface, SnapshotCommandOptionFields, SnapshotKeyboardBandFact, SnapshotNode, @@ -78,6 +79,13 @@ export type CaptureSnapshotResult = { * screen size only; content-safe gesture bounds stay with #1821. */ viewport?: SnapshotViewportSize; + /** + * iOS: whether the capture read a modal presentation over the app (#3565), decided on the acquired + * tree before interactive filtering, compaction, depth, or scope. `'absent'` is a positive reading; + * a capture whose producer could not decide — or did not speak — reports `'unknown'`, and absence + * of the field means a non-iOS producer or an older runner published nothing. + */ + presentedSurface?: PresentedSurface; /** * Screenshot captured automatically when the semantic snapshot was sparse. * Remote clients receive a materialized local path through the daemon artifact channel. diff --git a/packages/contracts/src/interaction.ts b/packages/contracts/src/interaction.ts index 49a2ead963..c2e0568f5c 100644 --- a/packages/contracts/src/interaction.ts +++ b/packages/contracts/src/interaction.ts @@ -1,4 +1,9 @@ -import type { Point, SnapshotKeyboardBandFact, SnapshotNode } from '@agent-device/kernel/snapshot'; +import type { + PresentedSurface, + Point, + SnapshotKeyboardBandFact, + SnapshotNode, +} from '@agent-device/kernel/snapshot'; import type { ResponseCost } from '@agent-device/kernel/contracts'; import type { ClickButton } from './click-button.ts'; import type { FillUnconfirmedVerification } from './fill-evidence.ts'; @@ -127,6 +132,11 @@ export type PreresolvedInteractionTarget = { * its baseline came from. */ iosSystemSurfaceBundleId?: string; + /** + * Whether the capture `nodes` came from read a modal presentation over the app (#3565), carried so + * the adopting leaf's settle baseline keeps the fact its own capture established. + */ + presentedSurface?: PresentedSurface; /** * The keyboard band the capture of `nodes` measured, carried so the adopting consumer's keyboard * guard reads the same band the tree was captured with instead of re-deriving one (#2660). @@ -148,6 +158,13 @@ export type SurfaceScopedNodes = { nodes: SnapshotNode[]; /** Bundle id of the in-place iOS system surface; absent for ordinary app content. */ surfaceBundleId?: string; + /** + * Whether the capture these nodes came from read a modal presentation over the app (#3565). The + * settle baseline is carried in this type, so the fact has to travel with the nodes: a consumer + * that arms #1831's dismissal confirmation reads it here instead of guessing from the tree. + * Absent means the capture's producer published no fact. + */ + presentedSurface?: PresentedSurface; }; /** Which tree a resolved `@ref`'s node was read from (ADR 0014 step 5). */ diff --git a/packages/contracts/src/interactor-types.ts b/packages/contracts/src/interactor-types.ts index 9c3fa56533..a4b616eafa 100644 --- a/packages/contracts/src/interactor-types.ts +++ b/packages/contracts/src/interactor-types.ts @@ -17,6 +17,7 @@ import type { IosSnapshotComparisonIdentity, } from './ios-snapshot.ts'; import type { + PresentedSurface, RawSnapshotNode, Point, Rect, @@ -265,6 +266,12 @@ export type SnapshotResult = Omit & * over the session app rather than the app itself (#2438). */ systemSurface?: IosSystemSurfaceProvenance; + /** + * Whether this capture read a modal presentation over the app (#3565), computed by the producer on + * the acquired tree before any projection. Absent means the producer published no fact at all, + * which a consumer must treat as `'unknown'`. + */ + presentedSurface?: PresentedSurface; /** * Keyboard evidence acquired with this tree: measured `visible`, policy-derived `derived`, * additive `visibility-only`, rejected `tree-rejected`, proven `absent`, or unknown `unmeasurable` facts. Measured and diff --git a/packages/daemon/src/core/snapshot-capture.ts b/packages/daemon/src/core/snapshot-capture.ts index 0838d7059b..14f5231ff4 100644 --- a/packages/daemon/src/core/snapshot-capture.ts +++ b/packages/daemon/src/core/snapshot-capture.ts @@ -11,6 +11,7 @@ import { normalizeRef, SNAPSHOT_CAPTURE_OPTION_KEYS, snapshotOptionsFromFlags, + type PresentedSurface, type RawSnapshotNode, type SnapshotCaptureProvenance, type SnapshotKeyboardBandFact, @@ -54,6 +55,11 @@ type SnapshotData = { keyboard?: SnapshotKeyboardBandFact; /** The box the capture's rects are measured in (#3182), carried to the state the response reads. */ viewport?: SnapshotViewportSize; + /** + * iOS: whether the capture read a modal presentation over the app (#3565), carried to the state + * settle's presented-surface confirmation arms on. Absent means the producer published no fact. + */ + presentedSurface?: PresentedSurface; } & Omit & SnapshotCaptureProvenance; diff --git a/packages/daemon/src/execution/generic-settle.ts b/packages/daemon/src/execution/generic-settle.ts index ba812847c6..86bf1d16a2 100644 --- a/packages/daemon/src/execution/generic-settle.ts +++ b/packages/daemon/src/execution/generic-settle.ts @@ -1,5 +1,6 @@ import type { CommandFlags } from '@agent-device/contracts/command'; import type { CreateDaemonCommandSurface } from '@agent-device/contracts/daemon-command-surface'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import type { SettleObservation, SettleParams, @@ -84,12 +85,7 @@ export function planGenericSettleObservation( const settle = readSettleRequest(params.flags); if (!settle) return {}; const snapshot = params.session.snapshot; - const baseline: SurfaceScopedNodes = { - nodes: snapshot?.nodes ?? [], - ...(snapshot?.iosSystemSurfaceBundleId - ? { surfaceBundleId: snapshot.iosSystemSurfaceBundleId } - : {}), - }; + const baseline = surfaceScopedNodes(snapshot); return { observe: async () => await observeSettled(params, settle, baseline) }; } diff --git a/packages/daemon/src/execution/interaction/internal/find-target-capture.ts b/packages/daemon/src/execution/interaction/internal/find-target-capture.ts index d1ed52ee94..69336e81f7 100644 --- a/packages/daemon/src/execution/interaction/internal/find-target-capture.ts +++ b/packages/daemon/src/execution/interaction/internal/find-target-capture.ts @@ -10,7 +10,7 @@ import { errorResponse } from '@agent-device/kernel/contracts'; /** The tree a mutating find resolves its target against, plus what the capture disclosed. */ export type FindTargetTree = CaptureProvenance & - Pick; + Pick; /** * Find's target capture. A mutating find (click/fill/focus/type) resolves its target from its diff --git a/packages/daemon/src/execution/interaction/internal/find.ts b/packages/daemon/src/execution/interaction/internal/find.ts index 62be98f335..b42d780ad4 100644 --- a/packages/daemon/src/execution/interaction/internal/find.ts +++ b/packages/daemon/src/execution/interaction/internal/find.ts @@ -10,6 +10,7 @@ import { SELECTOR_PIPELINE_POLICIES } from '@agent-device/selectors/selector-pip import { coveredInteractionDetails } from '@agent-device/selectors/interaction-error'; import { centerOfRect, + type PresentedSurface, type SnapshotKeyboardBandFact, type SnapshotState, } from '@agent-device/kernel/snapshot'; @@ -69,6 +70,8 @@ type ResolvedMatch = { iosSystemSurfaceBundleId?: string; /** The keyboard band that capture's producer measured, when it measured one (#2660). */ keyboard?: SnapshotKeyboardBandFact; + /** Whether that capture read a modal presentation over the app (#3565). */ + presentedSurface?: PresentedSurface; actionFlags: Record; /** * Set when find's row refuses this match as covered. Only the focus/type @@ -213,6 +216,9 @@ export async function handleFindCommands(params: FindRouteInput): Promise { expect('viewport' in withoutViewport).toBe(false); }); +/** + * Same reason as the viewport: the digest drops the tree the fact was read on, and a reader arming on + * "was something presented" (#3565) cannot recover it from a node count. + */ +test('digest carries the presented-surface fact of the tree it collapsed (#3565)', () => { + const digest = snapshotView!({ ...SNAPSHOT_DATA, presentedSurface: 'presented' }, 'digest'); + expect(digest.presentedSurface).toBe('presented'); + + const unread = snapshotView!(SNAPSHOT_DATA, 'digest'); + expect('presentedSurface' in unread).toBe(false); +}); + test('digest tolerates missing/empty node trees', () => { const digest = snapshotView!({ truncated: true }, 'digest'); expect(digest).toMatchObject({ nodeCount: 0, refs: [], truncated: true }); diff --git a/packages/daemon/src/server/response-views.ts b/packages/daemon/src/server/response-views.ts index 10abef06ea..feae4b7374 100644 --- a/packages/daemon/src/server/response-views.ts +++ b/packages/daemon/src/server/response-views.ts @@ -35,6 +35,7 @@ function snapshotView(data: DaemonResponseData, level: ResponseLevel): DaemonRes const carriedFields = [ 'visibility', 'viewport', + 'presentedSurface', 'snapshotQuality', 'targetActivation', 'observation', diff --git a/packages/kernel/src/presented-surface.test.ts b/packages/kernel/src/presented-surface.test.ts new file mode 100644 index 0000000000..5fd2297f4b --- /dev/null +++ b/packages/kernel/src/presented-surface.test.ts @@ -0,0 +1,32 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { PRESENTED_SURFACE_STATES } from './snapshot.ts'; +import { readPresentedSurfaceFact } from './record.ts'; + +// The presented-surface fact (#3565) arrives from two producers — the Apple runner's capture payload +// and a serialized snapshot response — and both seams read it here. What the reader must never do is +// turn a value it cannot name into silence: a consumer that arms the #1831 dismissal confirmation on +// `undefined` is back to guessing from a tree shape, which is the thing this fact retires. + +test('a capture that published no fact stays silent, and only that payload is silent', () => { + assert.equal(readPresentedSurfaceFact(undefined), undefined); +}); + +test('each declared state reads back as itself', () => { + for (const state of PRESENTED_SURFACE_STATES) { + assert.equal(readPresentedSurfaceFact(state), state); + } +}); + +// The vocabulary is a closed set, so a producer that renames or invents a state must not reach a +// consumer as a state it cannot name. `absent` is the dangerous direction: reading a typo of it as +// `absent` would disarm a confirmation that is waiting for a dismissal. +test('a state this vocabulary never declared reads as unknown, never as absent', () => { + for (const payload of ['none', 'Presented', 'no-sheet', '', 'null', 0, 1, true, [], {}, null]) { + assert.equal( + readPresentedSurfaceFact(payload), + 'unknown', + `payload ${JSON.stringify(payload)}`, + ); + } +}); diff --git a/packages/kernel/src/record.ts b/packages/kernel/src/record.ts index 0140ec9038..a9edc1d5ea 100644 --- a/packages/kernel/src/record.ts +++ b/packages/kernel/src/record.ts @@ -1,6 +1,12 @@ import { AppError } from '@agent-device/kernel/errors'; import type { DeviceKind, DeviceTarget, PublicPlatform } from '@agent-device/kernel/device'; -import type { Point, Rect, SnapshotKeyboardBandFact } from '@agent-device/kernel/snapshot'; +import type { + Point, + PresentedSurface, + Rect, + SnapshotKeyboardBandFact, +} from '@agent-device/kernel/snapshot'; +import { PRESENTED_SURFACE_STATES } from '@agent-device/kernel/snapshot'; function readRequired( record: Record, @@ -153,6 +159,22 @@ function readKeyboardFrameFact( : { kind: 'visible', frame }; } +/** + * Reads a producer's presented-surface fact (#3565) out of an untyped payload, shared by the Apple + * runner's wire reader and the daemon's Node client reader. The argument is the value already read + * under the fact's own key, so `undefined` is the producer stating nothing and stays silence. A + * value under that key which is not one of the three states is restated as `'unknown'` rather than + * dropped: the reader cannot tell a malformed `'absent'` from a genuine one, and silence would leave + * a consumer that arms a dismissal confirmation on `undefined` — which is exactly the guessing this + * fact retires. Only a named state is trusted. + */ +export function readPresentedSurfaceFact(value: unknown): PresentedSurface | undefined { + if (value === undefined) return undefined; + return (PRESENTED_SURFACE_STATES as readonly unknown[]).includes(value) + ? (value as PresentedSurface) + : 'unknown'; +} + export function parsePoint(value: unknown): Point | undefined { if (!isRecord(value)) return undefined; const x = readNumberField(value, 'x'); diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index bf341244d1..b9531850f9 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -386,6 +386,23 @@ export type SnapshotKeyboardBandFact = | { kind: 'absent' } | { kind: 'unmeasurable'; reason: string }; +/** + * Whether this capture read a modal presentation over the app's own content (#3565). + * + * The capture producer owns this: an interactive capture, a full capture, a depth cut, and a scoped + * capture of ONE screen all answer the same value, because the fact is decided on the acquired tree + * before any of those projections runs. Settle's dismissal confirmation (#1831) arms on this value + * instead of reconstructing a presentation from the shape of a tree it happens to hold. + * + * `'absent'` is a positive reading, not a default: the producer walked every window it captured and + * every window showed only its own presentation container. Anything the producer could not read — + * a tree from a producer that reports no UIKit class facts, an unexplained extra container, a + * truncated capture — answers `'unknown'`, never `'absent'`. + */ +export const PRESENTED_SURFACE_STATES = ['presented', 'absent', 'unknown'] as const; + +export type PresentedSurface = (typeof PRESENTED_SURFACE_STATES)[number]; + /** * The box a capture's node rects are measured in, published beside them so a reader never has to * infer it from the tree (#3182). Two dimensions and no origin: the field answers "how big is the @@ -734,6 +751,13 @@ export type SnapshotState = { * must never be compared as the same presentation; consumers that surface the tree disclose it. */ iosSystemSurfaceBundleId?: string; + /** + * iOS: whether this capture read a modal presentation over the app's own content (#3565), decided + * on the acquired tree before interactive filtering, compaction, depth, or scope, so every view of + * one screen carries one value. `'absent'` is read, never defaulted; an unreadable or unexplained + * capture answers `'unknown'`. Settle's presented-surface confirmation arms on this fact. + */ + presentedSurface?: PresentedSurface; /** * iOS and Android: keyboard observations made during acquisition. Measured and derived frames * take precedence over reconstruction from the presented tree. Visibility-only and unknown facts diff --git a/packages/platform-apple/src/interactor.ts b/packages/platform-apple/src/interactor.ts index 4e62c2f10f..b4d258ae70 100644 --- a/packages/platform-apple/src/interactor.ts +++ b/packages/platform-apple/src/interactor.ts @@ -264,6 +264,7 @@ async function captureAppleRunnerSnapshot( ...(presentation.viewport ? { viewport: presentation.viewport } : {}), ...(result.quality ? { quality: result.quality } : {}), ...(result.systemSurface ? { systemSurface: result.systemSurface } : {}), + ...(result.presentedSurface ? { presentedSurface: result.presentedSurface } : {}), ...(result.keyboard ? { keyboard: result.keyboard } : {}), ...(result.targetActivation ? { targetActivation: result.targetActivation } : {}), ...(result.observation ? { observation: result.observation } : {}), diff --git a/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts b/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts index 2d40311e16..e5342c2fe8 100644 --- a/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts +++ b/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts @@ -284,6 +284,31 @@ test('the reader forwards each published keyboard band shape unchanged', () => { ); }); +// #3565: the runner classifies the presentation on the acquisition it is about to present, and this +// seam is the only place that fact becomes a TS value. Forwarding the raw wire string would let a +// payload spelling a state this vocabulary never declared reach settle as if a producer had read it. +test('the reader forwards the presented-surface fact through the kernel reader (#3565)', () => { + assert.equal( + readAppleSnapshotResult({ presentedSurface: 'presented' }).presentedSurface, + 'presented', + ); + assert.equal(readAppleSnapshotResult({ presentedSurface: 'absent' }).presentedSurface, 'absent'); + assert.equal( + readAppleSnapshotResult({ presentedSurface: 'unknown' }).presentedSurface, + 'unknown', + ); + assert.equal( + readAppleSnapshotResult({ presentedSurface: 'no-sheet' }).presentedSurface, + 'unknown', + ); +}); + +test('a capture the runner classified nothing publishes no presented-surface fact', () => { + // The terminals (a sparse payload, an AX-unavailable failure) and any older runner answer with no + // key. The daemon must read that as no evidence, not as a screen the producer proved clear. + assert.equal(readAppleSnapshotResult({ nodes: [] }).presentedSurface, undefined); +}); + test('a capture from a tier that never reads the keyboard publishes no fact at all', () => { // The query sweep and private-AX tiers answer with no `keyboard` key, which is how the daemon // learns to keep deriving the band from that tree instead of being told the screen is clear. diff --git a/packages/platform-apple/src/runner/snapshot-presentation.ts b/packages/platform-apple/src/runner/snapshot-presentation.ts index eac6560dea..16cd266117 100644 --- a/packages/platform-apple/src/runner/snapshot-presentation.ts +++ b/packages/platform-apple/src/runner/snapshot-presentation.ts @@ -22,9 +22,14 @@ import { buildIosSnapshotPresentationKey, } from '@agent-device/capture-kit/ios-snapshot-planning'; import { AppError } from '@agent-device/kernel/errors'; -import { isRecord, readSnapshotKeyboardBandFact } from '@agent-device/kernel/record'; +import { + isRecord, + readPresentedSurfaceFact, + readSnapshotKeyboardBandFact, +} from '@agent-device/kernel/record'; import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; import type { + PresentedSurface, RawSnapshotNode, SnapshotKeyboardBandFact, SnapshotQualityVerdict, @@ -48,6 +53,12 @@ export type AppleRunnerSnapshotResult = Readonly<{ qualityPayload?: IosRunnerQualityPayloadFacts; runnerFatal?: boolean; systemSurface?: IosSystemSurfaceProvenance; + /** + * The runner's own answer about a modal presentation over the app (#3565), read from its capture + * payload. The runner classifies the acquisition it is about to present, and only publishes a + * reading it can support: a payload with no key is a capture the runner could not decide. + */ + presentedSurface?: PresentedSurface; keyboard?: SnapshotKeyboardBandFact; /** Foreground repair this capture's own command had to perform (#2682). */ targetActivation?: IosTargetActivation; @@ -59,6 +70,7 @@ export function readAppleSnapshotResult( device: Pick, ): AppleRunnerSnapshotResult { const systemSurface = readSystemSurfaceProvenance(result.systemSurface); + const presentedSurface = readPresentedSurfaceFact(result.presentedSurface); const keyboard = readSnapshotKeyboardBandFact(result.keyboard); const observation = readSnapshotObservation(result.observation); const targetActivation = readTargetActivationFact(result[TARGET_ACTIVATION_WIRE_KEY], (detail) => @@ -82,6 +94,7 @@ export function readAppleSnapshotResult( runnerFatal: result.runnerFatal === true, ...(keyboard ? { keyboard } : {}), ...(systemSurface ? { systemSurface } : {}), + ...(presentedSurface ? { presentedSurface } : {}), ...(targetActivation ? { targetActivation } : {}), ...(observation ? { observation } : {}), message: diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index c9e6100ad5..286dc77f9d 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -528,6 +528,7 @@ test('the real tree parses, declares, and passes R11', () => { '@agent-device/capture-kit/screenshot-diff-pixels', '@agent-device/capture-kit/screenshot-overlay', '@agent-device/capture-kit/scroll-edge-state', + '@agent-device/capture-kit/snapshot-baseline', '@agent-device/capture-kit/snapshot-chrome', '@agent-device/capture-kit/snapshot-chrome-fixtures', '@agent-device/capture-kit/snapshot-desktop-projection', diff --git a/src/__tests__/client-snapshot-presented-surface.test.ts b/src/__tests__/client-snapshot-presented-surface.test.ts new file mode 100644 index 0000000000..2166dfaf62 --- /dev/null +++ b/src/__tests__/client-snapshot-presented-surface.test.ts @@ -0,0 +1,43 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { createAgentDeviceClient } from '../agent-device-client.ts'; +import { createTransport } from './client-transport-fixture.ts'; + +// The presented-surface fact (#3565) reaches Node.js callers as one optional field on the snapshot +// result, and this owns the client half of that wire. Silence and a refusal are different answers: +// a payload with no key came from a producer that never classified (every non-iOS backend, any older +// runner), while a key this vocabulary cannot name came from a producer that tried and is now stating +// `unknown` out loud — because a reader that armed a dismissal confirmation on `undefined` is the +// guessing this fact exists to retire. + +function clientAnswering(data: Record) { + const setup = createTransport(async () => ({ ok: true, data: { nodes: [], ...data } })); + return createAgentDeviceClient(setup.config, { transport: setup.transport }); +} + +test('client capture.snapshot preserves the presented-surface state the producer published (#3565)', async () => { + for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { + const client = clientAnswering({ truncated: false, presentedSurface }); + + assert.equal( + (await client.capture.snapshot()).presentedSurface, + presentedSurface, + `state ${presentedSurface}`, + ); + } +}); + +test('client capture.snapshot reports no presented surface for a capture that carried none (#3565)', async () => { + const client = clientAnswering({ truncated: false }); + + assert.equal('presentedSurface' in (await client.capture.snapshot()), false); +}); + +test('client capture.snapshot restates a state it cannot name as unknown, never as absent (#3565)', async () => { + for (const presentedSurface of ['none', 'PRESENTED', 0, true, [], {}, null]) { + const client = clientAnswering({ truncated: false, presentedSurface }); + const result = await client.capture.snapshot(); + + assert.equal(result.presentedSurface, 'unknown', `payload ${JSON.stringify(presentedSurface)}`); + } +}); diff --git a/src/agent-device-client.ts b/src/agent-device-client.ts index a23449959a..4f289e0f57 100644 --- a/src/agent-device-client.ts +++ b/src/agent-device-client.ts @@ -72,7 +72,11 @@ import { writeMetroSessionHints, type MetroSessionHints, } from './metro/metro-session-hints.ts'; -import { asOptionalRecord, readSnapshotKeyboardBandFact } from '@agent-device/kernel/record'; +import { + asOptionalRecord, + readPresentedSurfaceFact, + readSnapshotKeyboardBandFact, +} from '@agent-device/kernel/record'; import { readSnapshotViewportSize } from '@agent-device/kernel/rect'; import { readResponseWarnings } from '@agent-device/kernel/success-text'; import { createLeaseClient } from './client/lease-client.ts'; @@ -537,6 +541,7 @@ function optionalSnapshotResponseFields( | 'visibility' | 'keyboard' | 'viewport' + | 'presentedSurface' | 'warnings' | 'snapshotQuality' | 'snapshotDiagnostics' @@ -549,9 +554,11 @@ function optionalSnapshotResponseFields( const keyboard = readSnapshotKeyboardBandFact(data.keyboard); const viewport = readSnapshotViewportSize(data.viewport); const snapshotDiagnostics = readSnapshotDiagnosticsSummary(data.snapshotDiagnostics); + const presentedSurface = readPresentedSurfaceFact(data.presentedSurface); return { ...(keyboard ? { keyboard } : {}), ...(viewport ? { viewport } : {}), + ...(presentedSurface ? { presentedSurface } : {}), ...(visibility ? { visibility: visibility as CaptureSnapshotResult['visibility'] } : {}), ...readSerializedSnapshotCaptureAnnotations(data), ...(unchanged ? { unchanged: unchanged as CaptureSnapshotResult['unchanged'] } : {}), diff --git a/src/commands/capture/runtime/presented-surface-carriage.test.ts b/src/commands/capture/runtime/presented-surface-carriage.test.ts new file mode 100644 index 0000000000..f17f02494c --- /dev/null +++ b/src/commands/capture/runtime/presented-surface-carriage.test.ts @@ -0,0 +1,92 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { AgentDeviceBackend, BackendSnapshotResult } from '@agent-device/contracts/backend'; +import { localCommandPolicy } from '@agent-device/contracts/command-policy'; +import type { + CommandSessionRecord, + CommandSessionStore, +} from '@agent-device/contracts/runtime-contract'; +import { createLocalArtifactAdapter } from '../../../io.ts'; +import { createAgentDevice } from '../../../runtime.ts'; + +/** + * #3565: the presented-surface fact a capture producer publishes has to reach both places a consumer + * reads it from — the response an operator inspects, and the session record the next settle and the + * next interaction mint their baseline from. The capture route copies a declared field set, so a + * field that is not copied disappears here and shows up later as a dismissal confirmation nobody can + * confirm. A backend states the fact either beside the tree it returns or inside state it built + * itself, and both routes have to answer. + */ +for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { + test(`runtime capture carries a ${presentedSurface} presented surface beside the tree`, async () => { + const captured = await captureWith({ + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + truncated: false, + backend: 'xctest', + presentedSurface, + }); + + assert.equal(captured.response.presentedSurface, presentedSurface, 'response'); + assert.equal(captured.recorded?.snapshot?.presentedSurface, presentedSurface, 'session record'); + }); + + test(`runtime capture carries a ${presentedSurface} presented surface stated inside backend state`, async () => { + const captured = await captureWith({ + snapshot: { + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + truncated: false, + backend: 'xctest', + createdAt: 1, + presentedSurface, + }, + }); + + assert.equal(captured.response.presentedSurface, presentedSurface, 'response'); + assert.equal(captured.recorded?.snapshot?.presentedSurface, presentedSurface, 'session record'); + }); +} + +/** A producer that states nothing must stay silent on both routes, never become a false absence. */ +test('runtime capture keeps an unstated presented surface unstated', async () => { + const besideTree = await captureWith({ + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + truncated: false, + backend: 'xctest', + }); + assert.equal('presentedSurface' in besideTree.response, false, 'response'); + assert.equal(besideTree.recorded?.snapshot?.presentedSurface, undefined, 'session record'); + + const insideState = await captureWith({ + snapshot: { + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + truncated: false, + backend: 'xctest', + createdAt: 1, + }, + }); + assert.equal('presentedSurface' in insideState.response, false, 'response'); + assert.equal(insideState.recorded?.snapshot?.presentedSurface, undefined, 'session record'); +}); + +async function captureWith(result: BackendSnapshotResult) { + let recorded: CommandSessionRecord | undefined; + const sessions: CommandSessionStore = { + get: () => undefined, + set: (value) => { + recorded = value; + }, + }; + const backend: AgentDeviceBackend = { + platform: 'ios', + captureSnapshot: async () => result, + }; + const device = createAgentDevice({ + backend, + artifacts: createLocalArtifactAdapter(), + sessions, + policy: localCommandPolicy(), + }); + + const response = await device.capture.snapshot({ session: 'default' }); + return { response, recorded }; +} diff --git a/src/commands/capture/runtime/snapshot.ts b/src/commands/capture/runtime/snapshot.ts index f50af7e9e5..71579c434f 100644 --- a/src/commands/capture/runtime/snapshot.ts +++ b/src/commands/capture/runtime/snapshot.ts @@ -10,6 +10,7 @@ import { import { AppError } from '@agent-device/kernel/errors'; import { normalizeType } from '@agent-device/contracts/snapshot'; import type { + PresentedSurface, SnapshotKeyboardBandFact, SnapshotNode, SnapshotState, @@ -67,6 +68,13 @@ export type SnapshotCommandResult = { * a zero. See `CaptureSnapshotResult.viewport` for what each producer measures. */ viewport?: SnapshotViewportSize; + /** + * iOS: whether this capture read a modal presentation over the app (#3565), decided on the acquired + * tree before interactive filtering, compaction, depth, or scope. `'absent'` is read, never + * defaulted; a producer that could not decide says `'unknown'` and one that never classified — every + * non-iOS backend — publishes nothing. + */ + presentedSurface?: PresentedSurface; } & PublicSnapshotCaptureAnnotations; type SnapshotCapture = { @@ -111,6 +119,9 @@ export const snapshotCommand: RuntimeCommand< : {}), ...(capture.snapshot.keyboard ? { keyboard: capture.snapshot.keyboard } : {}), ...(capture.snapshot.viewport ? { viewport: capture.snapshot.viewport } : {}), + ...(capture.snapshot.presentedSurface + ? { presentedSurface: capture.snapshot.presentedSurface } + : {}), ...snapshotAppFields(capture), }); }; @@ -225,6 +236,7 @@ function normalizeBackendSnapshot( ...result.snapshot, ...(result.keyboard ? { keyboard: result.keyboard } : {}), ...(result.viewport ? { viewport: result.viewport } : {}), + ...(result.presentedSurface ? { presentedSurface: result.presentedSurface } : {}), }); } return { @@ -234,6 +246,7 @@ function normalizeBackendSnapshot( createdAt: now(runtime), ...(result.keyboard ? { keyboard: result.keyboard } : {}), ...(result.viewport ? { viewport: result.viewport } : {}), + ...(result.presentedSurface ? { presentedSurface: result.presentedSurface } : {}), }; } diff --git a/src/commands/interaction/runtime/native-ref-interaction.ts b/src/commands/interaction/runtime/native-ref-interaction.ts index d1e0f1efe4..8e70ceedeb 100644 --- a/src/commands/interaction/runtime/native-ref-interaction.ts +++ b/src/commands/interaction/runtime/native-ref-interaction.ts @@ -3,7 +3,7 @@ import { normalizeRef } from '@agent-device/kernel/snapshot'; import { resolveRectCenter } from '@agent-device/kernel/rect-center'; import type { AgentDeviceRuntime, CommandContext } from '@agent-device/contracts/runtime-contract'; import { SELECTOR_PIPELINE_POLICIES } from '@agent-device/selectors/selector-pipeline-policy'; -import { surfaceScopedNodes } from './post-action-surface.ts'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import type { InteractionTarget, ResolvedInteractionTarget, diff --git a/src/commands/interaction/runtime/post-action-observation.ts b/src/commands/interaction/runtime/post-action-observation.ts index 6db4baf870..49fa2b7ebf 100644 --- a/src/commands/interaction/runtime/post-action-observation.ts +++ b/src/commands/interaction/runtime/post-action-observation.ts @@ -6,7 +6,8 @@ import type { SettleParams, } from '@agent-device/contracts/interaction'; import { captureInteractionSnapshot } from './interaction-snapshot-capture.ts'; -import { summarizePostActionEvidence, surfaceScopedNodes } from './post-action-surface.ts'; +import { summarizePostActionEvidence } from './post-action-surface.ts'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import { settleAfterInteraction, settleEvidence } from './settle.ts'; type ObservedResult = T & { diff --git a/src/commands/interaction/runtime/post-action-surface.ts b/src/commands/interaction/runtime/post-action-surface.ts index aea3aaafb2..9258795e81 100644 --- a/src/commands/interaction/runtime/post-action-surface.ts +++ b/src/commands/interaction/runtime/post-action-surface.ts @@ -1,4 +1,3 @@ -import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { summarizeAxEvidence } from '@agent-device/capture-kit/snapshot-evidence'; import type { InteractionEvidence, @@ -22,16 +21,6 @@ import { * disclosure cannot hold on one route and drop on the other. */ -/** Mints the one carried value from a capture: the nodes together with the surface they describe. */ -export function surfaceScopedNodes(snapshot: SnapshotState): SurfaceScopedNodes { - return { - nodes: snapshot.nodes, - ...(snapshot.iosSystemSurfaceBundleId - ? { surfaceBundleId: snapshot.iosSystemSurfaceBundleId } - : {}), - }; -} - /** * The transition between the two compared captures, or `undefined` when both describe the same * surface and an ordinary same-surface comparison is therefore valid. An absent baseline has no diff --git a/src/commands/interaction/runtime/ref-target-resolution.ts b/src/commands/interaction/runtime/ref-target-resolution.ts index 5c08d51696..d7a8754b05 100644 --- a/src/commands/interaction/runtime/ref-target-resolution.ts +++ b/src/commands/interaction/runtime/ref-target-resolution.ts @@ -15,7 +15,7 @@ import { STALE_REF_HINT } from '@agent-device/selectors'; import type { InteractionSnapshot } from './interaction-snapshot-capture.ts'; import { requireSnapshotSession } from './selector-read-shared.ts'; import { findNodeByLabel } from '@agent-device/capture-kit/snapshot-node-lookup'; -import { surfaceScopedNodes } from './post-action-surface.ts'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import type { InteractionTarget, PreresolvedInteractionTarget, @@ -81,6 +81,7 @@ function adoptPreresolvedRefTarget( ...(preresolved.iosSystemSurfaceBundleId ? { surfaceBundleId: preresolved.iosSystemSurfaceBundleId } : {}), + ...(preresolved.presentedSurface ? { presentedSurface: preresolved.presentedSurface } : {}), }, ...(preresolved.keyboard ? { keyboard: preresolved.keyboard } : {}), resolved: buildRefResolution(ref, preresolved.node, 'exact'), diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index a2255c6569..d52e65294e 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -14,7 +14,7 @@ import { } from './interaction-snapshot-capture.ts'; import { containsPoint } from '@agent-device/kernel/rect'; import { createSnapshotVisibility } from '@agent-device/contracts/snapshot'; -import { surfaceScopedNodes } from './post-action-surface.ts'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import type { InteractionTarget, PointTarget, diff --git a/src/commands/interaction/runtime/settle.ts b/src/commands/interaction/runtime/settle.ts index f44e76226a..8ccff098d1 100644 --- a/src/commands/interaction/runtime/settle.ts +++ b/src/commands/interaction/runtime/settle.ts @@ -5,6 +5,7 @@ import type { CommandSessionRecord, } from '@agent-device/contracts/runtime-contract'; import { isSparseSnapshotQualityVerdict } from '@agent-device/capture-kit/snapshot-quality-verdict'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import { buildSnapshotDiff } from '@agent-device/capture-kit/snapshot-diff'; import { displayLabel, formatRole } from '@agent-device/capture-kit/snapshot-lines'; import { @@ -33,7 +34,6 @@ import { crossSurfaceSettleHint, resolvePostActionSurfaceChange, summarizePostActionEvidence, - surfaceScopedNodes, } from './post-action-surface.ts'; /** @@ -242,13 +242,11 @@ function authorizedRefBaseline( session: CommandSessionRecord | undefined, ): SurfaceScopedNodes | undefined { if (resolved.kind !== 'ref') return undefined; - const frame = session?.refFrameSnapshot; - return frame ? nonEmptyBaseline(surfaceScopedNodes(frame)) : undefined; + return nonEmptyBaseline(surfaceScopedNodes(session?.refFrameSnapshot)); } function sessionBaseline(session: CommandSessionRecord | undefined): SurfaceScopedNodes { - const tree = session?.refFrameSnapshot ?? session?.snapshot; - return tree ? surfaceScopedNodes(tree) : { nodes: [] }; + return surfaceScopedNodes(session?.refFrameSnapshot ?? session?.snapshot); } function nonEmptyBaseline( diff --git a/src/commands/output/result-serialization.ts b/src/commands/output/result-serialization.ts index cff68bad33..27c7e2316e 100644 --- a/src/commands/output/result-serialization.ts +++ b/src/commands/output/result-serialization.ts @@ -79,6 +79,7 @@ export function serializeSnapshotResult(result: CaptureSnapshotResult): Record { + const data = serializeSnapshotResult({ + nodes: [], + truncated: false, + presentedSurface: 'presented', + identifiers: { session: 'qa' }, + }); + + assert.equal(data.presentedSurface, 'presented'); +}); + +test('serializeSnapshotResult omits a presented surface the producer did not read (#3565)', () => { + const data = serializeSnapshotResult({ + nodes: [], + truncated: false, + identifiers: { session: 'qa' }, + }); + + assert.equal('presentedSurface' in data, false); +}); diff --git a/src/core/__tests__/snapshot-state.test.ts b/src/core/__tests__/snapshot-state.test.ts index 9830862235..d3d18b06cc 100644 --- a/src/core/__tests__/snapshot-state.test.ts +++ b/src/core/__tests__/snapshot-state.test.ts @@ -740,3 +740,24 @@ test('buildSnapshotState numbers an Android capture densely so every node is add expect(state.nodes.map((node) => node.ref)).toEqual(['e1', 'e2', 'e3']); }); + +// #3565: the presentation fact belongs to the capture, so the assembly carries it verbatim into the +// state every consumer reads. What it must never do is mint a value: a capture whose producer could +// not decide — every non-iOS backend among them — leaves the field off, and a reader treats that as +// `unknown` rather than as a screen someone proved clear. +test('buildSnapshotState carries the presented-surface fact its producer published (#3565)', () => { + for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { + const state = buildSnapshotState( + { nodes: [{ index: 0, type: 'Application' }], presentedSurface }, + undefined, + ); + + expect(state.presentedSurface).toBe(presentedSurface); + } +}); + +test('buildSnapshotState leaves an unclassified capture without a presented surface (#3565)', () => { + const state = buildSnapshotState({ nodes: [{ index: 0, type: 'Application' }] }, undefined); + + expect('presentedSurface' in state).toBe(false); +}); From a41dbf84e583baa685fa102ed96588be26a02714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 15:50:47 +0200 Subject: [PATCH 2/3] fix(kernel): keep the presented-surface read off the shared wire reader's closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #3577 found four problems with how the fact crosses the wire. `readPresentedSurfaceFact` validated against `PRESENTED_SURFACE_STATES`, which made every eager consumer of `kernel/record` evaluate `kernel/snapshot.ts` as well. ADR 0019's loading-shape probe measures exactly that: thirteen carried entries grew by one module and 70 eager statements, which `scripts/__tests__/eager-closure-budgets.test.ts` refuses — the growth is a pre-existing module made eager, so no split explains it and shrinkage elsewhere cannot fund it. The reader now decides from a table it owns, keyed by `PresentedSurface`, so a vocabulary state without a row fails to compile rather than reading as `unknown`. The lookup is own-property, which also closes the inherited-member path an `in` test would have opened: a payload naming `constructor` is not a state. `PRESENTED_SURFACE_STATES` is frozen. `as const` was only a type-level promise on an array the reader trusted payloads against, so any consumer could move the boundary between a state and `unknown` for the whole process. The client normalized the fact on the default response path and returned a digest payload verbatim, which meant the same field was read two ways depending on `--response-level`. Passing a leveled payload through without reshaping it is not a licence to skip the fact's own vocabulary, so the digest path reads it through the same reader; a payload with no key stays byte-identical. Two doc comments claimed a capture "came from read" something. Both now name the producer that did the reading. --- packages/contracts/src/interaction.ts | 8 +- packages/kernel/src/presented-surface.test.ts | 73 ++++++++++++++++++- packages/kernel/src/record.ts | 49 ++++++++++--- packages/kernel/src/snapshot.ts | 2 +- .../src/runner/snapshot-presentation.ts | 2 +- .../client-snapshot-presented-surface.test.ts | 63 ++++++++++++++++ src/agent-device-client.ts | 11 ++- 7 files changed, 185 insertions(+), 23 deletions(-) diff --git a/packages/contracts/src/interaction.ts b/packages/contracts/src/interaction.ts index c2e0568f5c..f787ffc477 100644 --- a/packages/contracts/src/interaction.ts +++ b/packages/contracts/src/interaction.ts @@ -133,8 +133,8 @@ export type PreresolvedInteractionTarget = { */ iosSystemSurfaceBundleId?: string; /** - * Whether the capture `nodes` came from read a modal presentation over the app (#3565), carried so - * the adopting leaf's settle baseline keeps the fact its own capture established. + * Whether the producer that captured `nodes` read a modal presentation over the app (#3565), + * carried so the adopting leaf's settle baseline keeps the fact its own capture established. */ presentedSurface?: PresentedSurface; /** @@ -159,8 +159,8 @@ export type SurfaceScopedNodes = { /** Bundle id of the in-place iOS system surface; absent for ordinary app content. */ surfaceBundleId?: string; /** - * Whether the capture these nodes came from read a modal presentation over the app (#3565). The - * settle baseline is carried in this type, so the fact has to travel with the nodes: a consumer + * Whether the producer that captured these nodes read a modal presentation over the app (#3565). + * The settle baseline is carried in this type, so the fact has to travel with the nodes: a consumer * that arms #1831's dismissal confirmation reads it here instead of guessing from the tree. * Absent means the capture's producer published no fact. */ diff --git a/packages/kernel/src/presented-surface.test.ts b/packages/kernel/src/presented-surface.test.ts index 5fd2297f4b..3cd17d7899 100644 --- a/packages/kernel/src/presented-surface.test.ts +++ b/packages/kernel/src/presented-surface.test.ts @@ -7,14 +7,51 @@ import { readPresentedSurfaceFact } from './record.ts'; // and a serialized snapshot response — and both seams read it here. What the reader must never do is // turn a value it cannot name into silence: a consumer that arms the #1831 dismissal confirmation on // `undefined` is back to guessing from a tree shape, which is the thing this fact retires. +// +// It takes the payload rather than the value it holds, so the lookup itself is owned here: every +// caller reading a wire fact is doing a property lookup on data the producer controls. + +// One-argument form publishes the key with that value, `undefined` included, so a test can name the +// difference between a producer that never wrote the key and one that wrote it with nothing in it. +const carrying = (presentedSurface: unknown): Record => ({ + nodeCount: 2, + presentedSurface, +}); test('a capture that published no fact stays silent, and only that payload is silent', () => { - assert.equal(readPresentedSurfaceFact(undefined), undefined); + assert.equal(readPresentedSurfaceFact({ nodeCount: 2 }), undefined); +}); + +// A key that is present with no value is not the same event as no key: the producer declared the fact +// and failed to state it, which is what `unknown` exists to say. Collapsing the two would let a +// producer that dropped the value read exactly like a backend that has never had the fact at all. +test('a published key holding no value reads unknown, not the silence of no key', () => { + assert.equal(readPresentedSurfaceFact(carrying(undefined)), 'unknown'); +}); + +// A missing key and an inherited one are not the same event, and only one of them is silence: a plain +// lookup walks to `Object.prototype`, where any module sharing the process could park a value, and +// `absent` is precisely the state that tells #1831 no dismissal is owed. +test('a payload with no own fact key is silent rather than inherited', () => { + Object.defineProperty(Object.prototype, 'presentedSurface', { + value: 'absent', + configurable: true, + writable: true, + }); + try { + assert.equal(readPresentedSurfaceFact({ nodeCount: 2 }), undefined); + assert.equal( + readPresentedSurfaceFact(JSON.parse('{"presentedSurface":"presented"}')), + 'presented', + ); + } finally { + delete (Object.prototype as unknown as Record).presentedSurface; + } }); test('each declared state reads back as itself', () => { for (const state of PRESENTED_SURFACE_STATES) { - assert.equal(readPresentedSurfaceFact(state), state); + assert.equal(readPresentedSurfaceFact(carrying(state)), state); } }); @@ -24,9 +61,39 @@ test('each declared state reads back as itself', () => { test('a state this vocabulary never declared reads as unknown, never as absent', () => { for (const payload of ['none', 'Presented', 'no-sheet', '', 'null', 0, 1, true, [], {}, null]) { assert.equal( - readPresentedSurfaceFact(payload), + readPresentedSurfaceFact(carrying(payload)), 'unknown', `payload ${JSON.stringify(payload)}`, ); } }); + +// The reader trusts only its own table's own properties, so a published value that happens to name an +// inherited `Object.prototype` member is not a state. +test('a published value naming an inherited object member is not a state', () => { + for (const payload of ['constructor', 'toString', '__proto__']) { + assert.equal(readPresentedSurfaceFact(carrying(payload)), 'unknown', payload); + } +}); + +// The reader decides from a table it owns, and the exported vocabulary is frozen rather than +// `as const` alone: a consumer that mutated the array could otherwise move the boundary between a +// trusted state and `unknown` for every other module in the process. +test('the exported vocabulary cannot be widened or narrowed at runtime', () => { + assert.ok(Object.isFrozen(PRESENTED_SURFACE_STATES)); + + for (const mutate of [ + () => (PRESENTED_SURFACE_STATES as unknown as string[]).push('not-presented'), + () => ((PRESENTED_SURFACE_STATES as unknown as string[])[0] = 'absent'), + () => (PRESENTED_SURFACE_STATES as unknown as string[]).pop(), + ]) { + try { + mutate(); + } catch { + // A strict-mode assignment or a frozen push throws; either way the array is unchanged. + } + } + + assert.deepEqual([...PRESENTED_SURFACE_STATES], ['presented', 'absent', 'unknown']); + assert.equal(readPresentedSurfaceFact(carrying('not-presented')), 'unknown'); +}); diff --git a/packages/kernel/src/record.ts b/packages/kernel/src/record.ts index a9edc1d5ea..8304bcaea1 100644 --- a/packages/kernel/src/record.ts +++ b/packages/kernel/src/record.ts @@ -6,7 +6,6 @@ import type { Rect, SnapshotKeyboardBandFact, } from '@agent-device/kernel/snapshot'; -import { PRESENTED_SURFACE_STATES } from '@agent-device/kernel/snapshot'; function readRequired( record: Record, @@ -160,18 +159,44 @@ function readKeyboardFrameFact( } /** - * Reads a producer's presented-surface fact (#3565) out of an untyped payload, shared by the Apple - * runner's wire reader and the daemon's Node client reader. The argument is the value already read - * under the fact's own key, so `undefined` is the producer stating nothing and stays silence. A - * value under that key which is not one of the three states is restated as `'unknown'` rather than - * dropped: the reader cannot tell a malformed `'absent'` from a genuine one, and silence would leave - * a consumer that arms a dismissal confirmation on `undefined` — which is exactly the guessing this - * fact retires. Only a named state is trusted. + * The declared states, each keyed by its own name so a payload string is a state only when this table + * has that exact key. Keyed by `PresentedSurface`, so a state added to the vocabulary without a row + * here fails to compile instead of silently reading as `unknown`. + * + * A local table rather than `PRESENTED_SURFACE_STATES`: this module is the shared wire reader every + * response seam imports, and a load-time edge to `kernel/snapshot.ts` from here makes that module + * eager for every consumer of `kernel/record` — which ADR 0019's loading-shape probe measures per + * entry (`scripts/__tests__/eager-closure-budgets.test.ts`). The type is the tie that keeps the two + * in agreement, and `presented-surface.test.ts` reads both against the same payloads. */ -export function readPresentedSurfaceFact(value: unknown): PresentedSurface | undefined { - if (value === undefined) return undefined; - return (PRESENTED_SURFACE_STATES as readonly unknown[]).includes(value) - ? (value as PresentedSurface) +const PRESENTED_SURFACE_BY_NAME: Readonly> = { + presented: 'presented', + absent: 'absent', + unknown: 'unknown', +}; + +/** The key a producer publishes its presented-surface fact under (#3565). */ +const PRESENTED_SURFACE_WIRE_KEY = 'presentedSurface'; + +/** + * Reads a producer's presented-surface fact (#3565) out of a response or runner payload, shared by the + * Apple runner's wire reader and the Node client's. Taking the payload rather than the value is the + * point: reading a wire fact is a property lookup on data the producer controls, so it has to be + * own-property both ways. A missing key is the producer stating nothing and stays silence — and an + * inherited one is not silence's substitute, since a plain lookup walks to `Object.prototype`, where + * anything sharing the process could park `'absent'`, the one state that disarms the confirmation + * #1831 arms. A value under the fact's own key which is not one of the three states is restated as + * `'unknown'` rather than dropped: the reader cannot tell a malformed `'absent'` from a genuine one, + * and silence would leave a consumer arming a dismissal confirmation on nothing — the guessing this + * fact retires. Only a name in this table is trusted, so a payload naming `constructor` is not. + */ +export function readPresentedSurfaceFact( + payload: Record, +): PresentedSurface | undefined { + if (!Object.hasOwn(payload, PRESENTED_SURFACE_WIRE_KEY)) return undefined; + const value = payload[PRESENTED_SURFACE_WIRE_KEY]; + return typeof value === 'string' && Object.hasOwn(PRESENTED_SURFACE_BY_NAME, value) + ? PRESENTED_SURFACE_BY_NAME[value as PresentedSurface] : 'unknown'; } diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index b9531850f9..d140a06506 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -399,7 +399,7 @@ export type SnapshotKeyboardBandFact = * a tree from a producer that reports no UIKit class facts, an unexplained extra container, a * truncated capture — answers `'unknown'`, never `'absent'`. */ -export const PRESENTED_SURFACE_STATES = ['presented', 'absent', 'unknown'] as const; +export const PRESENTED_SURFACE_STATES = Object.freeze(['presented', 'absent', 'unknown'] as const); export type PresentedSurface = (typeof PRESENTED_SURFACE_STATES)[number]; diff --git a/packages/platform-apple/src/runner/snapshot-presentation.ts b/packages/platform-apple/src/runner/snapshot-presentation.ts index 16cd266117..a139bed679 100644 --- a/packages/platform-apple/src/runner/snapshot-presentation.ts +++ b/packages/platform-apple/src/runner/snapshot-presentation.ts @@ -70,7 +70,7 @@ export function readAppleSnapshotResult( device: Pick, ): AppleRunnerSnapshotResult { const systemSurface = readSystemSurfaceProvenance(result.systemSurface); - const presentedSurface = readPresentedSurfaceFact(result.presentedSurface); + const presentedSurface = readPresentedSurfaceFact(result); const keyboard = readSnapshotKeyboardBandFact(result.keyboard); const observation = readSnapshotObservation(result.observation); const targetActivation = readTargetActivationFact(result[TARGET_ACTIVATION_WIRE_KEY], (detail) => diff --git a/src/__tests__/client-snapshot-presented-surface.test.ts b/src/__tests__/client-snapshot-presented-surface.test.ts index 2166dfaf62..1201528417 100644 --- a/src/__tests__/client-snapshot-presented-surface.test.ts +++ b/src/__tests__/client-snapshot-presented-surface.test.ts @@ -41,3 +41,66 @@ test('client capture.snapshot restates a state it cannot name as unknown, never assert.equal(result.presentedSurface, 'unknown', `payload ${JSON.stringify(presentedSurface)}`); } }); + +// A digest response is the same fact on a different shape, and the client skips its normalizer for +// that shape. Skipping the tree projection cannot make the fact's own vocabulary optional: a caller +// arming the #1831 confirmation on a digest is arming the same thing. +test('a digest capture normalizes the presented-surface fact it carries (#3565)', async () => { + for (const [presentedSurface, expected] of [ + ['presented', 'presented'], + ['absent', 'absent'], + ['none', 'unknown'], + [0, 'unknown'], + [null, 'unknown'], + ] as const) { + const client = clientAnswering({ nodeCount: 2, refs: [], presentedSurface }); + const result = (await client.capture.snapshot({ responseLevel: 'digest' })) as Record< + string, + unknown + >; + + assert.equal(result.presentedSurface, expected, `payload ${JSON.stringify(presentedSurface)}`); + assert.equal(result.nodeCount, 2, 'the digest shape itself survives'); + } +}); + +test('a digest capture with no presented-surface fact stays silent (#3565)', async () => { + const client = clientAnswering({ nodeCount: 2, refs: [] }); + const result = (await client.capture.snapshot({ responseLevel: 'digest' })) as Record< + string, + unknown + >; + + assert.equal('presentedSurface' in result, false); +}); + +// Same split at the boundary a caller actually sees, so `presentedSurface` in result keeps meaning +// something: a payload that declared the fact without a value publishes `unknown`, and one that never +// declared it publishes no key at all. Passing the first through untouched would hand back a key whose +// value is `undefined`, which reads as "the platform publishes no reading" to exactly the check that +// the docs tell callers to make. +test('a published key holding no value reaches the caller as unknown (#3565)', async () => { + const client = clientAnswering({ truncated: false, presentedSurface: undefined }); + + assert.equal((await client.capture.snapshot()).presentedSurface, 'unknown'); +}); + +// The reported shape, pinned at the client rather than only at the reader: a digest capture carries no +// fact, and a lookup that walked the prototype chain would promote whatever a co-resident module +// parked there into a trusted `absent` — the one state that tells #1831 no dismissal is owed. +test('a digest capture never reads an inherited presented-surface value (#3565)', async () => { + Object.defineProperty(Object.prototype, 'presentedSurface', { + value: 'absent', + configurable: true, + writable: true, + }); + try { + const client = clientAnswering({ nodeCount: 2, refs: [] }); + for (const responseLevel of ['digest', 'full'] as const) { + const result = (await client.capture.snapshot({ responseLevel })) as Record; + assert.equal(Object.hasOwn(result, 'presentedSurface'), false, `level ${responseLevel}`); + } + } finally { + delete (Object.prototype as unknown as Record).presentedSurface; + } +}); diff --git a/src/agent-device-client.ts b/src/agent-device-client.ts index 4f289e0f57..199662a0b3 100644 --- a/src/agent-device-client.ts +++ b/src/agent-device-client.ts @@ -388,7 +388,14 @@ export function createAgentDeviceClient( // `nodes` tree and would collapse it to an empty snapshot. Pass the // leveled payload through verbatim. (Mirrors capture.screenshot; the // caller opted into the level, so the runtime value is the leveled shape.) - if (isLeveledResponse(options)) return data as unknown as CaptureSnapshotResult; + if (isLeveledResponse(options)) { + // Verbatim is not unchecked: the digest carries the presented-surface fact (#3565), which + // the normalizer skipped above would have read. `readPresentedSurfaceFact` owns its rules. + const presentedSurface = readPresentedSurfaceFact(data); + return (presentedSurface === undefined + ? data + : { ...data, presentedSurface }) as unknown as CaptureSnapshotResult; + } return normalizeSnapshotResult(data, session); }, screenshot: async (options: CaptureScreenshotOptions = {}) => { @@ -554,7 +561,7 @@ function optionalSnapshotResponseFields( const keyboard = readSnapshotKeyboardBandFact(data.keyboard); const viewport = readSnapshotViewportSize(data.viewport); const snapshotDiagnostics = readSnapshotDiagnosticsSummary(data.snapshotDiagnostics); - const presentedSurface = readPresentedSurfaceFact(data.presentedSurface); + const presentedSurface = readPresentedSurfaceFact(data); return { ...(keyboard ? { keyboard } : {}), ...(viewport ? { viewport } : {}), From 3c785c31decb5cef56f77f2050dc59b8dd8184e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 14 Oct 2026 13:51:06 +0200 Subject: [PATCH 3/3] refactor(interaction): hand find's leaf the baseline surfaceScopedNodes minted PreresolvedInteractionTarget carries one SurfaceScopedNodes minted from find's own capture instead of mirroring nodes, surface and presentation field by field, so the adopting leaf's settle baseline is that value as-is. PresentedSurface becomes a plain union, and the fact travels only inside SnapshotState: the top-level BackendSnapshotResult field had no production writer. Moves the moving-target gate onto the capture-kit baseline mint. --- .../PresentedSurface.swift | 3 +- packages/capture-kit/src/snapshot-baseline.ts | 6 +++- packages/contracts/src/backend.ts | 7 ---- packages/contracts/src/interaction.ts | 17 +++------- .../internal/__tests__/find.test.ts | 27 ++++++++++++++++ .../__tests__/interaction-touch-fixtures.ts | 2 +- .../execution/interaction/internal/find.ts | 32 ++++++------------- packages/kernel/src/presented-surface.test.ts | 27 ++-------------- packages/kernel/src/record.ts | 6 ---- packages/kernel/src/snapshot.ts | 4 +-- .../__tests__/snapshot-presentation.test.ts | 13 +++++--- .../interaction-touch-fill.test.ts | 2 +- .../interaction-touch-press.test.ts | 6 +++- .../presented-surface-carriage.test.ts | 25 ++------------- src/commands/capture/runtime/snapshot.ts | 2 -- .../runtime/moving-target-gate.test.ts | 2 +- .../interaction/runtime/moving-target-gate.ts | 2 +- .../runtime/ref-target-resolution.ts | 10 ++---- 18 files changed, 72 insertions(+), 121 deletions(-) diff --git a/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift b/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift index dc77d705d7..94e19aca05 100644 --- a/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift +++ b/apple/snapshot-presentation/Sources/AgentDeviceSnapshotPresentation/PresentedSurface.swift @@ -5,8 +5,7 @@ import Foundation /// confirmation on it: a capture whose evidence cannot answer says `unknown` instead of staying silent, /// since silence is also what a producer that never classified publishes. /// -/// Mirrors the TypeScript `PresentedSurface` in `@agent-device/kernel/snapshot`. The rule that arrives at -/// one of these three values is `SnapshotPresentedSurface` beside this type. +/// Mirrors the TypeScript `PresentedSurface` in `@agent-device/kernel/snapshot`. public enum PresentedSurface: String, Codable, Equatable { /// A presentation covers the content this capture describes. case presented diff --git a/packages/capture-kit/src/snapshot-baseline.ts b/packages/capture-kit/src/snapshot-baseline.ts index 594f7a20f6..3535c39a0c 100644 --- a/packages/capture-kit/src/snapshot-baseline.ts +++ b/packages/capture-kit/src/snapshot-baseline.ts @@ -14,7 +14,11 @@ import type { SurfaceScopedNodes } from '@agent-device/contracts/interaction'; * One value, never two channels: every boundary that carries a baseline carries the type this returns, * so nodes cannot arrive without the surface they describe. */ -export function surfaceScopedNodes(snapshot: SnapshotState | undefined): SurfaceScopedNodes { +export function surfaceScopedNodes( + snapshot: + | Pick + | undefined, +): SurfaceScopedNodes { return { nodes: snapshot?.nodes ?? [], ...(snapshot?.iosSystemSurfaceBundleId diff --git a/packages/contracts/src/backend.ts b/packages/contracts/src/backend.ts index 76bdfbca33..ec47635b75 100644 --- a/packages/contracts/src/backend.ts +++ b/packages/contracts/src/backend.ts @@ -23,7 +23,6 @@ import type { import type { Point, Rect, - PresentedSurface, SnapshotKeyboardBandFact, SnapshotNode, SnapshotOptions, @@ -59,12 +58,6 @@ export type BackendSnapshotResult = { * the band from `nodes`. */ keyboard?: SnapshotKeyboardBandFact; - /** - * iOS: whether this capture read a modal presentation over the app (#3565), classified by the - * producer on its acquired tree. A backend that publishes nothing classified nothing; a consumer - * treats that as `'unknown'`, never as absence. - */ - presentedSurface?: PresentedSurface; /** * The box these rects are measured in, as the producer measured it (#3182). Absent means the * producer measured no box — never a zero — since that absence is the only answer a producer with diff --git a/packages/contracts/src/interaction.ts b/packages/contracts/src/interaction.ts index f787ffc477..17b8ddf1f4 100644 --- a/packages/contracts/src/interaction.ts +++ b/packages/contracts/src/interaction.ts @@ -124,21 +124,14 @@ export type PreresolvedInteractionTarget = { ref: string; /** The node the caller resolved, after the caller's own promotion. */ node: SnapshotNode; - /** The tree `node` came from — the guards read its siblings for occlusion/viewport. */ - nodes: SnapshotNode[]; - /** - * The in-place iOS system surface `nodes` describes (#2438), absent for ordinary app content. - * Travels with the tree so the adopting consumer's post-action comparison knows which surface - * its baseline came from. - */ - iosSystemSurfaceBundleId?: string; /** - * Whether the producer that captured `nodes` read a modal presentation over the app (#3565), - * carried so the adopting leaf's settle baseline keeps the fact its own capture established. + * The tree `node` came from, minted by `surfaceScopedNodes` from the caller's capture. The guards + * read its siblings for occlusion/viewport, and the adopting leaf's post-action baseline is this + * value as-is, so the surface (#2438) and presentation (#3565) facts travel with the nodes. */ - presentedSurface?: PresentedSurface; + tree: SurfaceScopedNodes; /** - * The keyboard band the capture of `nodes` measured, carried so the adopting consumer's keyboard + * The keyboard band the capture of `tree` measured, carried so the adopting consumer's keyboard * guard reads the same band the tree was captured with instead of re-deriving one (#2660). */ keyboard?: SnapshotKeyboardBandFact; diff --git a/packages/daemon/src/execution/interaction/internal/__tests__/find.test.ts b/packages/daemon/src/execution/interaction/internal/__tests__/find.test.ts index 27b4050d7c..83beed2136 100644 --- a/packages/daemon/src/execution/interaction/internal/__tests__/find.test.ts +++ b/packages/daemon/src/execution/interaction/internal/__tests__/find.test.ts @@ -161,6 +161,33 @@ test('handleFindCommands click prefers on-screen duplicate text matches', async expect(invokeCalls[0]!.positionals?.[0]).toBe('@e3'); }); +// #3565: the leaf adopts find's tree as its settle baseline, so the presentation find's capture +// read must arrive on that tree rather than be re-derived from the nodes. +test('handleFindCommands click hands the leaf the baseline its own capture minted', async () => { + const nodes = [ + { index: 0, ref: 'e1', type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + ref: 'e2', + type: 'Button', + label: 'Close', + hittable: true, + rect: { x: 20, y: 80, width: 80, height: 44 }, + parentIndex: 0, + }, + ]; + mockDispatch.mockImplementation(async (_device, command) => + command === 'snapshot' ? { nodes, presentedSurface: 'presented' } : {}, + ); + + const { response, invokeCalls } = await runFindClickScenario({ positionals: ['Close', 'click'] }); + + expect(response.ok).toBe(true); + const carried = invokeCalls[0]!.internal?.findResolvedTarget; + expect(carried?.tree.presentedSurface).toBe('presented'); + expect(carried?.tree.nodes).toContain(carried?.node); +}); + test('handleFindCommands click tries query-scoped full retry before failing sparse verdict', async () => { const session = makeSession('default'); session.snapshot = { diff --git a/packages/daemon/src/execution/interaction/internal/__tests__/interaction-touch-fixtures.ts b/packages/daemon/src/execution/interaction/internal/__tests__/interaction-touch-fixtures.ts index 1b51da7f56..141123b5f4 100644 --- a/packages/daemon/src/execution/interaction/internal/__tests__/interaction-touch-fixtures.ts +++ b/packages/daemon/src/execution/interaction/internal/__tests__/interaction-touch-fixtures.ts @@ -140,7 +140,7 @@ export function findResolvedTarget( preresolved: ReturnType, ref = '@e2', ) { - return { ref, node: preresolved.node, nodes: preresolved.nodes }; + return { ref, node: preresolved.node, tree: { nodes: preresolved.nodes } }; } /** The coordinates the leaf handed to the bound point-tap operation. */ diff --git a/packages/daemon/src/execution/interaction/internal/find.ts b/packages/daemon/src/execution/interaction/internal/find.ts index b42d780ad4..c62feb492f 100644 --- a/packages/daemon/src/execution/interaction/internal/find.ts +++ b/packages/daemon/src/execution/interaction/internal/find.ts @@ -1,4 +1,8 @@ -import type { PreresolvedInteractionTarget } from '@agent-device/contracts/interaction'; +import type { + PreresolvedInteractionTarget, + SurfaceScopedNodes, +} from '@agent-device/contracts/interaction'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import { isReadOnlyFindAction, checkFindArgs, @@ -10,7 +14,6 @@ import { SELECTOR_PIPELINE_POLICIES } from '@agent-device/selectors/selector-pip import { coveredInteractionDetails } from '@agent-device/selectors/interaction-error'; import { centerOfRect, - type PresentedSurface, type SnapshotKeyboardBandFact, type SnapshotState, } from '@agent-device/kernel/snapshot'; @@ -65,13 +68,10 @@ type ResolvedMatch = { node: SnapshotState['nodes'][number]; resolvedNode: SnapshotState['nodes'][number]; ref: string; - nodes: SnapshotState['nodes']; - /** The in-place iOS system surface the target capture described (#2438), if any. */ - iosSystemSurfaceBundleId?: string; + /** The target capture's baseline: its nodes with the surface and presentation they describe. */ + tree: SurfaceScopedNodes; /** The keyboard band that capture's producer measured, when it measured one (#2660). */ keyboard?: SnapshotKeyboardBandFact; - /** Whether that capture read a modal presentation over the app (#3565). */ - presentedSurface?: PresentedSurface; actionFlags: Record; /** * Set when find's row refuses this match as covered. Only the focus/type @@ -211,14 +211,8 @@ export async function handleFindCommands(params: FindRouteInput): Promise { }); test('each declared state reads back as itself', () => { - for (const state of PRESENTED_SURFACE_STATES) { + const states: PresentedSurface[] = ['presented', 'absent', 'unknown']; + for (const state of states) { assert.equal(readPresentedSurfaceFact(carrying(state)), state); } }); @@ -75,25 +76,3 @@ test('a published value naming an inherited object member is not a state', () => assert.equal(readPresentedSurfaceFact(carrying(payload)), 'unknown', payload); } }); - -// The reader decides from a table it owns, and the exported vocabulary is frozen rather than -// `as const` alone: a consumer that mutated the array could otherwise move the boundary between a -// trusted state and `unknown` for every other module in the process. -test('the exported vocabulary cannot be widened or narrowed at runtime', () => { - assert.ok(Object.isFrozen(PRESENTED_SURFACE_STATES)); - - for (const mutate of [ - () => (PRESENTED_SURFACE_STATES as unknown as string[]).push('not-presented'), - () => ((PRESENTED_SURFACE_STATES as unknown as string[])[0] = 'absent'), - () => (PRESENTED_SURFACE_STATES as unknown as string[]).pop(), - ]) { - try { - mutate(); - } catch { - // A strict-mode assignment or a frozen push throws; either way the array is unchanged. - } - } - - assert.deepEqual([...PRESENTED_SURFACE_STATES], ['presented', 'absent', 'unknown']); - assert.equal(readPresentedSurfaceFact(carrying('not-presented')), 'unknown'); -}); diff --git a/packages/kernel/src/record.ts b/packages/kernel/src/record.ts index 8304bcaea1..d7048f2654 100644 --- a/packages/kernel/src/record.ts +++ b/packages/kernel/src/record.ts @@ -162,12 +162,6 @@ function readKeyboardFrameFact( * The declared states, each keyed by its own name so a payload string is a state only when this table * has that exact key. Keyed by `PresentedSurface`, so a state added to the vocabulary without a row * here fails to compile instead of silently reading as `unknown`. - * - * A local table rather than `PRESENTED_SURFACE_STATES`: this module is the shared wire reader every - * response seam imports, and a load-time edge to `kernel/snapshot.ts` from here makes that module - * eager for every consumer of `kernel/record` — which ADR 0019's loading-shape probe measures per - * entry (`scripts/__tests__/eager-closure-budgets.test.ts`). The type is the tie that keeps the two - * in agreement, and `presented-surface.test.ts` reads both against the same payloads. */ const PRESENTED_SURFACE_BY_NAME: Readonly> = { presented: 'presented', diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index d140a06506..c09d74fac7 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -399,9 +399,7 @@ export type SnapshotKeyboardBandFact = * a tree from a producer that reports no UIKit class facts, an unexplained extra container, a * truncated capture — answers `'unknown'`, never `'absent'`. */ -export const PRESENTED_SURFACE_STATES = Object.freeze(['presented', 'absent', 'unknown'] as const); - -export type PresentedSurface = (typeof PRESENTED_SURFACE_STATES)[number]; +export type PresentedSurface = 'presented' | 'absent' | 'unknown'; /** * The box a capture's node rects are measured in, published beside them so a reader never has to diff --git a/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts b/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts index e5342c2fe8..6af8851091 100644 --- a/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts +++ b/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts @@ -289,16 +289,19 @@ test('the reader forwards each published keyboard band shape unchanged', () => { // payload spelling a state this vocabulary never declared reach settle as if a producer had read it. test('the reader forwards the presented-surface fact through the kernel reader (#3565)', () => { assert.equal( - readAppleSnapshotResult({ presentedSurface: 'presented' }).presentedSurface, + readAppleSnapshotResult({ presentedSurface: 'presented' }, IOS_SIMULATOR).presentedSurface, 'presented', ); - assert.equal(readAppleSnapshotResult({ presentedSurface: 'absent' }).presentedSurface, 'absent'); assert.equal( - readAppleSnapshotResult({ presentedSurface: 'unknown' }).presentedSurface, + readAppleSnapshotResult({ presentedSurface: 'absent' }, IOS_SIMULATOR).presentedSurface, + 'absent', + ); + assert.equal( + readAppleSnapshotResult({ presentedSurface: 'unknown' }, IOS_SIMULATOR).presentedSurface, 'unknown', ); assert.equal( - readAppleSnapshotResult({ presentedSurface: 'no-sheet' }).presentedSurface, + readAppleSnapshotResult({ presentedSurface: 'no-sheet' }, IOS_SIMULATOR).presentedSurface, 'unknown', ); }); @@ -306,7 +309,7 @@ test('the reader forwards the presented-surface fact through the kernel reader ( test('a capture the runner classified nothing publishes no presented-surface fact', () => { // The terminals (a sparse payload, an AX-unavailable failure) and any older runner answer with no // key. The daemon must read that as no evidence, not as a screen the producer proved clear. - assert.equal(readAppleSnapshotResult({ nodes: [] }).presentedSurface, undefined); + assert.equal(readAppleSnapshotResult({ nodes: [] }, IOS_SIMULATOR).presentedSurface, undefined); }); test('a capture from a tier that never reads the keyboard publishes no fact at all', () => { diff --git a/src/__tests__/daemon-composition/interaction/interaction-touch-fill.test.ts b/src/__tests__/daemon-composition/interaction/interaction-touch-fill.test.ts index b9b3db30c5..35087c1e0b 100644 --- a/src/__tests__/daemon-composition/interaction/interaction-touch-fill.test.ts +++ b/src/__tests__/daemon-composition/interaction/interaction-touch-fill.test.ts @@ -317,7 +317,7 @@ test("ADR 0014 blocker-2: a mutating find's internal fill from an expired frame positionals: ['@e1', 'hello'], flags: {}, internal: { - findResolvedTarget: { ref: '@e1', node: findNode, nodes: findNodes }, + findResolvedTarget: { ref: '@e1', node: findNode, tree: { nodes: findNodes } }, }, }, sessionName, diff --git a/src/__tests__/daemon-composition/interaction/interaction-touch-press.test.ts b/src/__tests__/daemon-composition/interaction/interaction-touch-press.test.ts index 73666b524b..3cb67024c7 100644 --- a/src/__tests__/daemon-composition/interaction/interaction-touch-press.test.ts +++ b/src/__tests__/daemon-composition/interaction/interaction-touch-press.test.ts @@ -387,7 +387,11 @@ test('#1654: the shared guards still run on the pre-resolved node', async () => }; const response = await runFindInternalClick(sessionStore, sessionName, { - findResolvedTarget: { ref: '@e2', node: blocked, nodes: [preresolved.nodes[0]!, blocked] }, + findResolvedTarget: { + ref: '@e2', + node: blocked, + tree: { nodes: [preresolved.nodes[0]!, blocked] }, + }, }); expect(response?.ok).toBe(false); diff --git a/src/commands/capture/runtime/presented-surface-carriage.test.ts b/src/commands/capture/runtime/presented-surface-carriage.test.ts index f17f02494c..9b6ca3ec40 100644 --- a/src/commands/capture/runtime/presented-surface-carriage.test.ts +++ b/src/commands/capture/runtime/presented-surface-carriage.test.ts @@ -14,22 +14,9 @@ import { createAgentDevice } from '../../../runtime.ts'; * reads it from — the response an operator inspects, and the session record the next settle and the * next interaction mint their baseline from. The capture route copies a declared field set, so a * field that is not copied disappears here and shows up later as a dismissal confirmation nobody can - * confirm. A backend states the fact either beside the tree it returns or inside state it built - * itself, and both routes have to answer. + * confirm. A backend states the fact inside the state it returns. */ for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { - test(`runtime capture carries a ${presentedSurface} presented surface beside the tree`, async () => { - const captured = await captureWith({ - nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], - truncated: false, - backend: 'xctest', - presentedSurface, - }); - - assert.equal(captured.response.presentedSurface, presentedSurface, 'response'); - assert.equal(captured.recorded?.snapshot?.presentedSurface, presentedSurface, 'session record'); - }); - test(`runtime capture carries a ${presentedSurface} presented surface stated inside backend state`, async () => { const captured = await captureWith({ snapshot: { @@ -46,16 +33,8 @@ for (const presentedSurface of ['presented', 'absent', 'unknown'] as const) { }); } -/** A producer that states nothing must stay silent on both routes, never become a false absence. */ +/** A producer that states nothing must stay silent, never become a false absence. */ test('runtime capture keeps an unstated presented surface unstated', async () => { - const besideTree = await captureWith({ - nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], - truncated: false, - backend: 'xctest', - }); - assert.equal('presentedSurface' in besideTree.response, false, 'response'); - assert.equal(besideTree.recorded?.snapshot?.presentedSurface, undefined, 'session record'); - const insideState = await captureWith({ snapshot: { nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], diff --git a/src/commands/capture/runtime/snapshot.ts b/src/commands/capture/runtime/snapshot.ts index 71579c434f..1a34e15744 100644 --- a/src/commands/capture/runtime/snapshot.ts +++ b/src/commands/capture/runtime/snapshot.ts @@ -236,7 +236,6 @@ function normalizeBackendSnapshot( ...result.snapshot, ...(result.keyboard ? { keyboard: result.keyboard } : {}), ...(result.viewport ? { viewport: result.viewport } : {}), - ...(result.presentedSurface ? { presentedSurface: result.presentedSurface } : {}), }); } return { @@ -246,7 +245,6 @@ function normalizeBackendSnapshot( createdAt: now(runtime), ...(result.keyboard ? { keyboard: result.keyboard } : {}), ...(result.viewport ? { viewport: result.viewport } : {}), - ...(result.presentedSurface ? { presentedSurface: result.presentedSurface } : {}), }; } diff --git a/src/commands/interaction/runtime/moving-target-gate.test.ts b/src/commands/interaction/runtime/moving-target-gate.test.ts index 2fea8904c0..635bc84b78 100644 --- a/src/commands/interaction/runtime/moving-target-gate.test.ts +++ b/src/commands/interaction/runtime/moving-target-gate.test.ts @@ -231,7 +231,7 @@ test("a mutating find's carried node is not second-guessed by position", async ( stored: carried, trees: [carried, slideTree(RESTING_X)], ...DISTURBING_HISTORY, - preresolvedTarget: { ref: '@e2', node: carried.nodes[1]!, nodes: carried.nodes }, + preresolvedTarget: { ref: '@e2', node: carried.nodes[1]!, tree: { nodes: carried.nodes } }, }); assert.deepEqual(await device.press('ref'), [centerOf(MID_FLIGHT_X)]); diff --git a/src/commands/interaction/runtime/moving-target-gate.ts b/src/commands/interaction/runtime/moving-target-gate.ts index 36cf6b620d..d10a6f92ac 100644 --- a/src/commands/interaction/runtime/moving-target-gate.ts +++ b/src/commands/interaction/runtime/moving-target-gate.ts @@ -22,7 +22,7 @@ import type { import { nodeRectByIdentity } from '@agent-device/ad-script'; import { now, sleep } from '../../runtime-common.ts'; import { captureInteractionSnapshot } from './interaction-snapshot-capture.ts'; -import { surfaceScopedNodes } from './post-action-surface.ts'; +import { surfaceScopedNodes } from '@agent-device/capture-kit/snapshot-baseline'; import { resolveInteractionTarget } from './resolution.ts'; import type { ResolveInteractionTargetParams } from './interaction-resolution-request.ts'; diff --git a/src/commands/interaction/runtime/ref-target-resolution.ts b/src/commands/interaction/runtime/ref-target-resolution.ts index d7a8754b05..9ad1b56b05 100644 --- a/src/commands/interaction/runtime/ref-target-resolution.ts +++ b/src/commands/interaction/runtime/ref-target-resolution.ts @@ -69,20 +69,14 @@ function adoptPreresolvedRefTarget( if (!ref) throw new AppError('INVALID_ARGS', `Invalid ref: ${target.ref}`); const carriedRef = normalizeRef(preresolved.ref); const nodeRef = preresolved.node.ref ? normalizeRef(preresolved.node.ref) : null; - if (carriedRef !== ref || nodeRef !== ref || !preresolved.nodes.includes(preresolved.node)) { + if (carriedRef !== ref || nodeRef !== ref || !preresolved.tree.nodes.includes(preresolved.node)) { throw new AppError( 'COMMAND_FAILED', 'Internal find target provenance does not match the interaction ref', ); } return { - tree: { - nodes: preresolved.nodes, - ...(preresolved.iosSystemSurfaceBundleId - ? { surfaceBundleId: preresolved.iosSystemSurfaceBundleId } - : {}), - ...(preresolved.presentedSurface ? { presentedSurface: preresolved.presentedSurface } : {}), - }, + tree: preresolved.tree, ...(preresolved.keyboard ? { keyboard: preresolved.keyboard } : {}), resolved: buildRefResolution(ref, preresolved.node, 'exact'), };