diff --git a/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/DisplayExtent.java b/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/DisplayExtent.java new file mode 100644 index 0000000000..05c99fc493 --- /dev/null +++ b/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/DisplayExtent.java @@ -0,0 +1,87 @@ +package com.callstack.agentdevice.snapshothelper; + +import android.content.Context; +import android.graphics.Rect; +import android.os.Build; +import android.util.DisplayMetrics; +import android.view.Display; +import android.view.WindowManager; + +/** + * The physical display the captured bounds were measured on, in physical pixels, in the current + * rotation (#3182). + * + * Node bounds reach the host through {@code AccessibilityNodeInfo.getBoundsInScreen()}, which is + * absolute screen space across every interactive window, SystemUI's navigation bar included. The + * extent therefore has to be the real display extent rather than {@code Resources.getSystem()}'s + * app display size: the latter excludes a persistent navigation bar, so a nav-bar node's rect would + * land outside the published box. On API 30+ that extent is + * {@code WindowManager.getMaximumWindowMetrics().getBounds()}; below it, {@code + * Display.getRealMetrics}, which likewise carries the full panel and not the app-usable inset. + * + * The caller reads this twice around the tree dump and publishes only what {@link #whenBothAgree} + * returns. A rotation landing inside the dump changes the extent between the reads, and the capture + * then says nothing about its box rather than pairing one rotation's bounds with the other's + * dimensions. A display this process cannot address answers with null. Either way the host publishes + * the absence it reads as unknown, and the host's own guard is what refuses a zero extent. + */ +final class DisplayExtent { + private DisplayExtent() {} + + /** The extent one read of the display answered with, in the current rotation. */ + static final class Extent { + final int width; + final int height; + + Extent(int width, int height) { + this.width = width; + this.height = height; + } + } + + /** + * The extent of the display {@code context} is attached to, or null when this process could not + * address it. A zero extent is passed through as read: refusing an unusable box is the host's job, + * and {@code snapshotViewportSizeFrom} already refuses one, so a second refusal here would only + * be a second place that decides what a usable display is. + */ + static Extent read(Context context) { + try { + WindowManager windowManager = + (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); + if (windowManager == null) { + return null; + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + Rect bounds = windowManager.getMaximumWindowMetrics().getBounds(); + return new Extent(bounds.width(), bounds.height()); + } + return legacyExtent(windowManager); + } catch (Throwable error) { + return null; + } + } + + @SuppressWarnings("deprecation") + private static Extent legacyExtent(WindowManager windowManager) { + Display display = windowManager.getDefaultDisplay(); + DisplayMetrics metrics = new DisplayMetrics(); + display.getRealMetrics(metrics); + return new Extent(metrics.widthPixels, metrics.heightPixels); + } + + /** + * The extent both reads of one capture agreed on, or null when either read failed or the display + * changed underneath the tree dump. Omitting the viewport on disagreement is what keeps the + * published box a fact about the capture rather than about two different rotations. + */ + static Extent whenBothAgree(Extent before, Extent after) { + if (before == null || after == null) { + return null; + } + if (before.width != after.width || before.height != after.height) { + return null; + } + return before; + } +} diff --git a/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/SnapshotInstrumentation.java b/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/SnapshotInstrumentation.java index 6f7742a186..6026f881e0 100644 --- a/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/SnapshotInstrumentation.java +++ b/android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/SnapshotInstrumentation.java @@ -5,6 +5,7 @@ import android.content.res.Resources; import android.os.Bundle; import android.util.Base64; +import android.util.DisplayMetrics; import java.io.BufferedReader; import java.io.File; import java.io.FileOutputStream; @@ -77,14 +78,17 @@ public void onStart() { return; } long startedAtMs = System.currentTimeMillis(); + DisplayExtent.Extent beforeDump = DisplayExtent.read(getContext()); AccessibilityTreeCapture.Result capture = captureXml(waitForIdleQuietMs, waitForIdleTimeoutMs, timeoutMs, maxDepth, maxNodes); + DisplayExtent.Extent display = + DisplayExtent.whenBothAgree(beforeDump, DisplayExtent.read(getContext())); writeOutputFile(outputPath, capture.xml); if (emitChunks) { emitChunks(capture.xml); } result.putString("ok", "true"); - putCaptureMetadata(result, capture, System.currentTimeMillis() - startedAtMs); + putCaptureMetadata(result, capture, display, System.currentTimeMillis() - startedAtMs); finishSafely(0, result); } catch (Throwable error) { result.putString("ok", "false"); @@ -114,7 +118,10 @@ private static void putBaseMetadata( } private static void putCaptureMetadata( - Bundle result, AccessibilityTreeCapture.Result capture, long elapsedMs) { + Bundle result, + AccessibilityTreeCapture.Result capture, + DisplayExtent.Extent display, + long elapsedMs) { result.putString("rootPresent", Boolean.toString(capture.rootPresent)); result.putString("captureMode", capture.captureMode); result.putString("windowCount", Integer.toString(capture.windowCount)); @@ -124,8 +131,17 @@ private static void putCaptureMetadata( // Physical pixels per dp of the display the bounds above were measured on, from the same // configuration the framework lays out with (a `wm density` override included), so a host that // works in dp has the factor beside the pixels instead of a second adb round trip. - result.putString( - "pixelDensity", Float.toString(Resources.getSystem().getDisplayMetrics().density)); + DisplayMetrics metrics = Resources.getSystem().getDisplayMetrics(); + result.putString("pixelDensity", Float.toString(metrics.density)); + // The real extent of the display those bounds are measured in, read around the tree dump and + // published only when both reads agreed, so the host can publish the box the bounds live in + // without reading the tree for it. Published by absence: a display this helper could not + // address, or one that rotated mid-dump, adds no keys, which the host reads as unknown rather + // than as a screen of no size or of another rotation's size (#3182). + if (display != null) { + result.putString("displayWidth", Integer.toString(display.width)); + result.putString("displayHeight", Integer.toString(display.height)); + } } private void runOneShotViewport(Bundle result) { @@ -232,10 +248,13 @@ private void writeSessionSnapshot( result.putString("requestId", requestId); try { long startedAtMs = System.currentTimeMillis(); + DisplayExtent.Extent beforeDump = DisplayExtent.read(getContext()); AccessibilityTreeCapture.Result capture = captureXml(waitForIdleQuietMs, waitForIdleTimeoutMs, timeoutMs, maxDepth, maxNodes); + DisplayExtent.Extent display = + DisplayExtent.whenBothAgree(beforeDump, DisplayExtent.read(getContext())); result.putString("ok", "true"); - putCaptureMetadata(result, capture, System.currentTimeMillis() - startedAtMs); + putCaptureMetadata(result, capture, display, System.currentTimeMillis() - startedAtMs); result.putString("byteLength", Integer.toString(capture.xml.getBytes(StandardCharsets.UTF_8).length)); SessionResponseWriter.writeSessionResponse(output, result, capture.xml); } catch (Throwable error) { diff --git a/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/DisplayExtentTest.java b/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/DisplayExtentTest.java new file mode 100644 index 0000000000..892f68f91a --- /dev/null +++ b/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/DisplayExtentTest.java @@ -0,0 +1,66 @@ +package com.callstack.agentdevice.snapshothelper; + +public final class DisplayExtentTest { + private DisplayExtentTest() {} + + private static final DisplayExtent.Extent PORTRAIT = new DisplayExtent.Extent(1080, 2400); + private static final DisplayExtent.Extent LANDSCAPE = new DisplayExtent.Extent(2400, 1080); + + static void run() { + assertTwoReadsOfTheSameDisplayPublishItsExtent(); + assertADisplayThatMovedUnderTheDumpPublishesNothing(); + assertAReadThatFailedPublishesNothing(); + } + + private static void assertTwoReadsOfTheSameDisplayPublishItsExtent() { + // The rotation matters: a landscape capture publishes the box its bounds are actually measured + // in, which is why the extent is read per capture rather than kept across a helper session. + assertExtent( + DisplayExtent.whenBothAgree(PORTRAIT, new DisplayExtent.Extent(1080, 2400)), + 1080, + 2400, + "both reads of one portrait display"); + assertExtent( + DisplayExtent.whenBothAgree(LANDSCAPE, LANDSCAPE), + 2400, + 1080, + "both reads of one landscape display"); + } + + private static void assertADisplayThatMovedUnderTheDumpPublishesNothing() { + // A rotation landing inside the tree dump would pair that rotation's node bounds with the other + // rotation's dimensions. The capture then names no box at all rather than a wrong one (#3182). + assertNull( + DisplayExtent.whenBothAgree(PORTRAIT, LANDSCAPE), "display rotated mid-dump"); + assertNull( + DisplayExtent.whenBothAgree( + PORTRAIT, new DisplayExtent.Extent(1076, 2400)), + "display resized mid-dump"); + } + + private static void assertAReadThatFailedPublishesNothing() { + // A read that failed on either side of the dump leaves the box unknown: the surviving read says + // nothing about what the tree between the two was measured against. + assertNull(DisplayExtent.whenBothAgree(null, PORTRAIT), "read before the dump failed"); + assertNull(DisplayExtent.whenBothAgree(PORTRAIT, null), "read after the dump failed"); + assertNull(DisplayExtent.whenBothAgree(null, null), "both reads failed"); + } + + private static void assertExtent( + DisplayExtent.Extent extent, int width, int height, String label) { + if (extent == null) { + throw new AssertionError("expected an extent for " + label); + } + if (extent.width != width || extent.height != height) { + throw new AssertionError( + "wrong extent for " + label + ": got " + extent.width + "x" + extent.height); + } + } + + private static void assertNull(DisplayExtent.Extent extent, String label) { + if (extent != null) { + throw new AssertionError( + "expected no extent for " + label + " but got " + extent.width + "x" + extent.height); + } + } +} diff --git a/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/SnapshotHelperTestSuite.java b/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/SnapshotHelperTestSuite.java index 868e1f0211..e7913ac8b4 100644 --- a/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/SnapshotHelperTestSuite.java +++ b/android/snapshot-helper/src/test/java/com/callstack/agentdevice/snapshothelper/SnapshotHelperTestSuite.java @@ -8,5 +8,6 @@ public static void main(String[] args) throws Exception { AccessibilityCaptureStabilizerTest.run(); BoundedUiAutomationConnectionTest.run(); GestureViewportReaderTest.run(); + DisplayExtentTest.run(); } } diff --git a/packages/capture-kit/src/ios-snapshot-engine/engine.ts b/packages/capture-kit/src/ios-snapshot-engine/engine.ts index 5a22ae4a7b..918960f2e3 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/engine.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/engine.ts @@ -46,6 +46,9 @@ export function publishIosSnapshot( comparisonIdentity: buildIosSnapshotComparisonIdentity(input, request), residue: input.stage === 'acquired' ? [...input.acquisition.residue] : [...input.validation.residue], + ...(presentation.validatedViewport + ? { validatedViewport: presentation.validatedViewport } + : {}), }; } @@ -143,6 +146,7 @@ function presentAcquiredSnapshot( }).nodes; return { nodes: compacted.nodes, + validatedViewport: viewport, ...(qualityNodes ? { qualityNodes } : {}), presentedIndexesBySourceIndex: remapPresentedIndexes( acquisition.nodes, diff --git a/packages/capture-kit/src/ios-snapshot-engine/runner-presentation.ts b/packages/capture-kit/src/ios-snapshot-engine/runner-presentation.ts index cbae8a042d..9025886219 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/runner-presentation.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/runner-presentation.ts @@ -34,6 +34,7 @@ export function presentIosRunnerSnapshot( ); return { nodes: compacted.nodes, + ...(viewport ? { validatedViewport: viewport } : {}), ...(input.presentation.qualityPayload ? { qualityNodes: [...input.presentation.qualityPayload.nodes] } : {}), diff --git a/packages/capture-kit/src/ios-snapshot-engine/types.ts b/packages/capture-kit/src/ios-snapshot-engine/types.ts index 91fd4392bc..68c0e5d1fc 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/types.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/types.ts @@ -26,6 +26,13 @@ export type IosSnapshotPresentationStats = Readonly<{ export type IosSnapshotEnginePresentation = Readonly<{ nodes: RawSnapshotNode[]; + /** + * The box the regular projection validated this tree against (#3182). The fold already resolves it + * (`resolveIosViewport` / `resolveViewportEvidence`), so the engine hands it over instead of making + * every caller re-derive "regular validates a box, raw does not". A raw projection validates no box + * and carries none. + */ + validatedViewport?: Rect; qualityNodes?: RawSnapshotNode[]; presentedIndexesBySourceIndex: ReadonlyMap; stats: IosSnapshotPresentationStats; diff --git a/packages/capture-kit/src/snapshot-state.ts b/packages/capture-kit/src/snapshot-state.ts index ab00e11bc8..7fe70d7eda 100644 --- a/packages/capture-kit/src/snapshot-state.ts +++ b/packages/capture-kit/src/snapshot-state.ts @@ -15,6 +15,7 @@ import { type SnapshotKeyboardBandFact, snapshotStateProvenance, type SnapshotState, + type SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; import { annotateCoveredSnapshotNodes, @@ -51,6 +52,8 @@ export function buildSnapshotState( targetActivation?: IosTargetActivation; /** The keyboard band the producer measured, carried to the state the tap guards read (#2660). */ keyboard?: SnapshotKeyboardBandFact; + /** The box the producer measured the rects in (#3182), carried to the response that reads this state. */ + viewport?: SnapshotViewportSize; } & SnapshotCaptureProvenance, flags: | (Pick & @@ -89,6 +92,7 @@ export function buildSnapshotState( ...(data.systemSurface ? { iosSystemSurfaceBundleId: data.systemSurface.bundleId } : {}), ...(data.targetActivation ? { targetActivation: data.targetActivation } : {}), ...(data.keyboard ? { keyboard: data.keyboard } : {}), + ...(data.viewport ? { viewport: data.viewport } : {}), presentationKey: buildSnapshotPresentationKey(snapshotPresentationOptionsFromFlags(flags)), // Only broad Android snapshots become freshness baselines. If the user asked for a scoped // or filtered view, preserve that output contract but avoid pretending it is safe for diff --git a/packages/capture-kit/src/snapshot/__tests__/ios-snapshot-runtime-viewport.test.ts b/packages/capture-kit/src/snapshot/__tests__/ios-snapshot-runtime-viewport.test.ts new file mode 100644 index 0000000000..5c5e68f1a6 --- /dev/null +++ b/packages/capture-kit/src/snapshot/__tests__/ios-snapshot-runtime-viewport.test.ts @@ -0,0 +1,84 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { createIosSnapshotAcquisition } from '@agent-device/capture-kit/ios-snapshot-acquisition'; +import { presentIosSnapshotAcquisition } from '../ios-snapshot-runtime.ts'; +import type { IosViewportEvidence } from '@agent-device/contracts/ios-snapshot'; +import { AppError } from '@agent-device/kernel/errors'; +import type { RawSnapshotNode } from '@agent-device/kernel/snapshot'; + +// #3182: the provider acquisitions — the simulator AX bridge, an Appium page source, a Limrun tree — +// already resolve the box their nodes are measured in, and this presenter is the one seam that turns +// an acquisition into a snapshot result. Publishing nothing here left the default local-iOS producer +// without a viewport while it held the evidence, which is the issue's first `Done when` line. + +const SCREEN = { x: 0, y: 0, width: 390, height: 844 }; + +const NODES: readonly RawSnapshotNode[] = [ + { index: 0, type: 'Application', rect: SCREEN }, + { index: 1, parentIndex: 0, type: 'Button', label: 'Open', rect: { ...SCREEN, width: 80 } }, +]; + +function acquire(viewport: IosViewportEvidence) { + return createIosSnapshotAcquisition({ + producer: 'appium-source', + nodes: NODES, + viewport, + lineage: { targetId: 'sim-1:com.example.app' }, + }); +} + +test('an acquisition publishes the viewport box the regular fold measured against (#3182)', () => { + const presented = presentIosSnapshotAcquisition(acquire({ kind: 'reported', rect: SCREEN })); + + assert.deepEqual(presented.viewport, { width: 390, height: 844 }); +}); + +// The regular fold refuses a capture with no viewport, so absence is only reachable on a raw +// projection — which is also the projection that validates no box at all. +test('a raw acquisition publishes no viewport, since nothing checked it (#3182)', () => { + const presented = presentIosSnapshotAcquisition(acquire({ kind: 'reported', rect: SCREEN }), { + raw: true, + }); + + assert.equal('viewport' in presented, false); +}); + +test('a raw acquisition carrying the failed-read box publishes no viewport (#3182)', () => { + const presented = presentIosSnapshotAcquisition( + acquire({ + kind: 'reported', + rect: { + x: -Number.MAX_VALUE / 2, + y: -Number.MAX_VALUE / 2, + width: Number.MAX_VALUE, + height: Number.MAX_VALUE, + }, + }), + { raw: true }, + ); + + assert.equal('viewport' in presented, false); +}); + +test('a regular acquisition with no viewport evidence still refuses rather than minting one', () => { + assert.throws( + () => presentIosSnapshotAcquisition(acquire({ kind: 'missing', reason: 'not-provided' })), + (error: unknown) => error instanceof AppError && error.code === 'COMMAND_FAILED', + ); +}); + +// The issue's second Done-when line: an empty screen still reports the box. The box is a property of +// the surface the producer read, so a tree that names nothing on it still answers the question. +test('an empty iOS acquisition still publishes the viewport it read (#3182)', () => { + const empty = createIosSnapshotAcquisition({ + producer: 'appium-source', + nodes: [], + viewport: { kind: 'reported', rect: SCREEN }, + lineage: { targetId: 'sim-1:com.example.app' }, + }); + + const presented = presentIosSnapshotAcquisition(empty); + + assert.deepEqual(presented.nodes, []); + assert.deepEqual(presented.viewport, { width: 390, height: 844 }); +}); diff --git a/packages/capture-kit/src/snapshot/ios-snapshot-runtime.ts b/packages/capture-kit/src/snapshot/ios-snapshot-runtime.ts index d905b815d9..89677873f9 100644 --- a/packages/capture-kit/src/snapshot/ios-snapshot-runtime.ts +++ b/packages/capture-kit/src/snapshot/ios-snapshot-runtime.ts @@ -15,6 +15,7 @@ import type { SnapshotRuntimeAcquiredResult, } from '@agent-device/contracts/interactor-types'; import { AppError } from '@agent-device/kernel/errors'; +import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; const IOS_SNAPSHOT_FACT_WARNINGS: Partial> = { 'acquisition-depth': @@ -41,6 +42,9 @@ export function presentIosSnapshotAcquisition( try { const presentation = publishIosSnapshot(input, request); + // The engine returns the box its regular fold validated against and no box for raw; this seam + // only passes it through the shared construction guard (#3182). + const validatedViewport = snapshotViewportSizeFrom(presentation.validatedViewport); return { backend: 'xctest', producer: acquired.acquisition.producer, @@ -49,6 +53,7 @@ export function presentIosSnapshotAcquisition( ...(acquired.acquisition.truncated === undefined ? {} : { truncated: acquired.acquisition.truncated }), + ...(validatedViewport ? { viewport: validatedViewport } : {}), ...snapshotWarnings(acquired.acquisition.residue), }; } catch (error) { diff --git a/packages/contracts/src/client-capture.ts b/packages/contracts/src/client-capture.ts index 844897f725..6be33ac0bd 100644 --- a/packages/contracts/src/client-capture.ts +++ b/packages/contracts/src/client-capture.ts @@ -8,6 +8,7 @@ import type { SnapshotKeyboardBandFact, SnapshotNode, SnapshotUnchanged, + SnapshotViewportSize, SnapshotVisibility, } from '@agent-device/kernel/snapshot'; import type { ScreenshotResultData } from './snapshot-types.ts'; @@ -64,6 +65,19 @@ export type CaptureSnapshotResult = { * the producer measured no band and the tap guard derived one from the tree. */ keyboard?: SnapshotKeyboardBandFact; + /** + * The box the node rects are measured in, as the producer measured it (#3182). Same coordinate + * space and orientation as the rects beside it, so a consumer scales and clips against the screen + * it is being shown instead of inferring it from the largest rect on screen. + * + * Which surface it names is the producer's answer, not always the physical panel: the app window + * for iOS (so iPad Split View and a foldable panel do not inflate it), the screen the bounds were + * measured on for Android and Apple TV. Absent means the producer measured no box — a desktop + * capture whose rects are absolute in window space, a backend that reads a tree without reading a + * screen, or a raw projection nothing validated a box against — and never a zero. This is the full + * screen size only; content-safe gesture bounds stay with #1821. + */ + viewport?: SnapshotViewportSize; /** * 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/ios-snapshot.ts b/packages/contracts/src/ios-snapshot.ts index 5f84381f26..07bdb2b68f 100644 --- a/packages/contracts/src/ios-snapshot.ts +++ b/packages/contracts/src/ios-snapshot.ts @@ -219,6 +219,11 @@ export type IosSnapshotPublication = Readonly<{ presentationKey: IosSnapshotPresentationKey; comparisonIdentity: IosSnapshotComparisonIdentity; residue: readonly IosAcquisitionResidue[]; + /** + * The box the regular projection validated this tree against (#3182), handed over by the engine + * that measured against it. A raw projection validates no box and carries none. + */ + validatedViewport?: Rect; }>; export type IosSnapshotEngine = Readonly<{ diff --git a/packages/contracts/src/snapshot-types.ts b/packages/contracts/src/snapshot-types.ts index 9c43d6b92d..087c3a53b0 100644 --- a/packages/contracts/src/snapshot-types.ts +++ b/packages/contracts/src/snapshot-types.ts @@ -4,6 +4,7 @@ import type { SnapshotOptions, SnapshotQualityVerdict, ScreenshotOverlayRef, + SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; import type { DeviceRotation } from './device-rotation.ts'; import type { SnapshotDiagnosticsSummary } from './snapshot-diagnostics.ts'; @@ -28,6 +29,11 @@ export type BackendSnapshotResult = { appName?: string; appBundleId?: string; snapshotDiagnostics?: SnapshotDiagnosticsSummary; + /** + * The box the node rects are measured in, as the producer measured it (#3182). Absent means the + * producer measured no box; see {@link SnapshotViewportSize} for what each producer measures. + */ + viewport?: SnapshotViewportSize; analysis?: { rawNodeCount: number; maxDepth: number }; androidSnapshot?: AndroidSnapshotBackendMetadata; freshness?: { diff --git a/packages/kernel/src/record.ts b/packages/kernel/src/record.ts index a033463795..88f0309c81 100644 --- a/packages/kernel/src/record.ts +++ b/packages/kernel/src/record.ts @@ -109,6 +109,8 @@ export function readSnapshotKeyboardBandFact(value: unknown): SnapshotKeyboardBa const frame = parseRect(value.frame); // Same rule as `isPositiveFiniteRect` in kernel/rect, inlined: this module is a leaf that // many facades evaluate, and a rect import here would land in every one of their closures. + // The viewport re-read (#3182) is the other side of that seam and lives in kernel/rect as + // `readSnapshotViewportSize`, because a viewport IS a rect; readers take the rect module there. const plottable = frame !== undefined && [frame.x, frame.y, frame.width, frame.height].every(Number.isFinite) && diff --git a/packages/kernel/src/rect.test.ts b/packages/kernel/src/rect.test.ts index 7f9bfdd781..4979592b3b 100644 --- a/packages/kernel/src/rect.test.ts +++ b/packages/kernel/src/rect.test.ts @@ -7,6 +7,8 @@ import { isPositiveFiniteRect, isRectVisibleInViewport, pickLargestRect, + readSnapshotViewportSize, + snapshotViewportSizeFrom, } from './rect.ts'; const VIEWPORT: Rect = { x: 0, y: 0, width: 300, height: 500 }; @@ -60,6 +62,83 @@ test('the sentinel would survive any check that only looks at components and ext ); }); +// #3182: the viewport a response publishes has exactly one construction path and exactly one wire +// re-read, and both answer a box they cannot accept with absence. A zero the producer answered with +// has to become "unknown", never a claim that the screen has no width. +test('snapshotViewportSizeFrom publishes only a box the rect guard accepts (#3182)', () => { + assert.deepEqual(snapshotViewportSizeFrom({ x: 12, y: -40, width: 390, height: 844 }), { + width: 390, + height: 844, + }); + assert.equal(snapshotViewportSizeFrom(undefined), undefined, 'a producer that read nothing'); + assert.equal( + snapshotViewportSizeFrom({ x: 0, y: 0, width: 0, height: 844 }), + undefined, + 'a zero width is unknown, never a screen of no size', + ); + assert.equal( + snapshotViewportSizeFrom({ x: 0, y: 0, width: Number.NaN, height: 844 }), + undefined, + 'a non-finite extent', + ); + assert.equal(snapshotViewportSizeFrom(CG_RECT_INFINITE), undefined, 'the failed-read sentinel'); + // A viewport carries no origin, so the producer that hands over the box it still holds after a + // refused read arrives with plausible coordinates beside the sentinel's extents. The extents alone + // have to refuse it, or the largest number on the wire becomes the screen (#2891). + assert.equal( + snapshotViewportSizeFrom({ x: 0, y: 0, width: Number.MAX_VALUE, height: Number.MAX_VALUE }), + undefined, + 'failed-read extents beside a plausible origin', + ); + assert.equal( + snapshotViewportSizeFrom({ x: 0, y: 0, width: 390, height: Number.MAX_VALUE }), + undefined, + 'one failed-read extent', + ); +}); + +test('readSnapshotViewportSize accepts only a guard-approved pair from a wire payload (#3182)', () => { + assert.deepEqual(readSnapshotViewportSize({ width: 1080, height: 2400 }), { + width: 1080, + height: 2400, + }); + for (const unusable of [ + undefined, + null, + 'screen', + [], + {}, + { width: 1080 }, + { width: '1080', height: 2400 }, + { width: 0, height: 2400 }, + { width: 1080, height: -1 }, + { width: Number.NaN, height: 2400 }, + ]) { + assert.equal(readSnapshotViewportSize(unusable), undefined, String(unusable)); + } + // A producer shipping the failed-read box whole ships the sentinel origin beside maximal extents; + // an origin on the wire is inspected, not flattened to (0, 0). + assert.equal( + readSnapshotViewportSize({ ...CG_RECT_INFINITE }), + undefined, + 'the infinite sentinel with its origin', + ); + // The published shape has no origin at all, so the ordinary broken payload is the originless one: + // the extents have to be enough to recognise a failed read, or `x ?? 0` would mint the largest + // finite box on the wire as the screen every rect is measured in. + assert.equal( + readSnapshotViewportSize({ width: Number.MAX_VALUE, height: Number.MAX_VALUE }), + undefined, + 'failed-read extents with no origin', + ); + assert.equal( + readSnapshotViewportSize({ width: 390, height: Number.MAX_VALUE }), + undefined, + 'one failed-read extent', + ); + assert.equal(readSnapshotViewportSize({ width: 1, height: 1, x: '0' }), undefined); +}); + test('containsPoint is inclusive on every edge and requires all four bounds', () => { assert.equal(containsPoint(VIEWPORT, 0, 0), true); assert.equal(containsPoint(VIEWPORT, 300, 500), true); diff --git a/packages/kernel/src/rect.ts b/packages/kernel/src/rect.ts index 8321eddce8..3b48ab6185 100644 --- a/packages/kernel/src/rect.ts +++ b/packages/kernel/src/rect.ts @@ -1,4 +1,4 @@ -import type { Rect } from './snapshot.ts'; +import type { Rect, SnapshotViewportSize } from './snapshot.ts'; /** * CoreGraphics' `CGRectInfinite`, spelled in the four doubles Apple builds it from. This is what a @@ -40,6 +40,48 @@ export function isPositiveFiniteRect(rect: Rect | undefined): rect is Rect { return !isCGRectInfinite(rect); } +/** + * A failed read keeps the maximal extents of `CGRectInfinite` whatever happened to its origin, and a + * size field carries no origin of its own: a producer that hands over the box it still holds after a + * refused read, or a wire payload that arrives with the extents and no coordinates, would otherwise + * publish the largest number on the wire as the screen every rect is measured in (#2891, #3182). + */ +function reportsFailedReadExtents(width: number, height: number): boolean { + return width >= CG_RECT_INFINITE.width || height >= CG_RECT_INFINITE.height; +} + +/** + * The ONE construction path for the viewport a snapshot response publishes (#3182): a producer hands + * over the box it read, and a box this guard refuses yields `undefined` — the absence that means + * unknown — instead of a size with a zero in it. A producer that published a zero would be + * answering "this screen has no width", which no producer measured; a reader that has to tell the + * two apart can only do it when one of them is unrepresentable. + */ +export function snapshotViewportSizeFrom(box: Rect | undefined): SnapshotViewportSize | undefined { + if (!isPositiveFiniteRect(box)) return undefined; + if (reportsFailedReadExtents(box.width, box.height)) return undefined; + return { width: box.width, height: box.height } as SnapshotViewportSize; +} + +/** + * Re-read of a viewport this repo already published, out of an untyped wire payload: only a + * `{ width, height }` pair the guard accepts survives, and anything else is the absence that means + * unknown. A reader that trusted the payload could otherwise hand a consumer the `width: 0` a broken + * producer wrote, which is the claim {@link snapshotViewportSizeFrom} exists to make unrepresentable. + * + * The published shape carries no origin, so the failed read has to be recognised from its extents + * alone: the extents check covers the payload with no coordinates as well as one that arrived with + * the `CGRectInfinite` origin beside them. + */ +export function readSnapshotViewportSize(value: unknown): SnapshotViewportSize | undefined { + if (typeof value !== 'object' || value === null || Array.isArray(value)) return undefined; + const { x, y, width, height } = value as Record; + if (typeof width !== 'number' || typeof height !== 'number') return undefined; + if (x !== undefined && typeof x !== 'number') return undefined; + if (y !== undefined && typeof y !== 'number') return undefined; + return snapshotViewportSizeFrom({ x: x ?? 0, y: y ?? 0, width, height }); +} + export function rectContains(container: Rect, nested: Rect): boolean { return ( nested.x >= container.x && diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index 526b44ab53..6691445f25 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -338,6 +338,39 @@ export type SnapshotKeyboardBandFact = */ | { kind: 'unmeasurable'; reason: string }; +/** + * 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 + * surface these numbers describe", which is the only question a consumer that places points on the + * tree cannot answer from the tree itself when the tree is empty or sparse. + * + * It is the box of the surface the producer read, which is not always the physical panel. iOS + * reports the app window in the app's orientation space (ADR 0004), which is smaller than the panel + * under iPad Split View and is never the foldable panel `fold` reports (ADR 0025); Android and the + * Apple TV runner report the screen the bounds were measured on. A producer that has no box of its + * own to answer with leaves the field off: the macOS desktop, whose rects are absolute in window + * space and answer to no single frame, and the web and Linux backends, which read a tree without + * reading a screen. A consumer that needs a *gesture* band inside those bounds still reads `keyboard` + * and the app window, which is #1821's remaining scope. + * + * Absent means the producer measured no box. Absence is never `0`: `snapshotViewportSizeFrom` from + * `@agent-device/kernel/rect` is the sole construction path and refuses a box + * `isPositiveFiniteRect` refuses. The brand makes that invariant part of the type: a plain + * `{ width, height }` literal — including one with a zero in it — is not assignable here, so only + * modules that import the brand token from `kernel/rect` can build one, and they build it through + * the guard. + */ +export type SnapshotViewportSize = { + width: number; + height: number; +} & SnapshotViewportSizeBrand; + +/** @internal Exported only so `kernel/rect` can mint values of {@link SnapshotViewportSize}. */ +export declare const SNAPSHOT_VIEWPORT_SIZE_BRAND: unique symbol; +export type SnapshotViewportSizeBrand = { + readonly [SNAPSHOT_VIEWPORT_SIZE_BRAND]: 'validated'; +}; + export type SnapshotNode = RawSnapshotNode & { ref: string; /** @@ -642,6 +675,13 @@ export type SnapshotState = { * needs no geometry to be plausible (#2660). Absent means the guard measures the tree as before. */ keyboard?: SnapshotKeyboardBandFact; + /** + * The box these rects are measured in, as the producer measured it (#3182). The state is the carrier + * the response reads, so the stored tree and the published `viewport` are one fact rather than two: + * a consumer of a stored capture — a later diff, a re-read of the session's tree — gets the box the + * producer measured instead of inferring one from the largest rect still on screen. + */ + viewport?: SnapshotViewportSize; /** * iOS: this capture's own command found the session app out of foreground and the runner * activated it before answering, so an earlier observation in the session described whatever held diff --git a/packages/platform-android/src/__tests__/snapshot-capture.test.ts b/packages/platform-android/src/__tests__/snapshot-capture.test.ts new file mode 100644 index 0000000000..3853338b2f --- /dev/null +++ b/packages/platform-android/src/__tests__/snapshot-capture.test.ts @@ -0,0 +1,176 @@ +import assert from 'node:assert/strict'; +import { afterEach, beforeEach, test } from 'vitest'; +import './test-utils/android-host-test-setup.ts'; +import { + androidSnapshotPublicationInput, + androidSnapshotViewportFromHelperMetadata, +} from '../snapshot-capture.ts'; +import { snapshotAndroid } from '../snapshot.ts'; +import { resetAndroidSnapshotHelperInstallCache } from '../snapshot-helper-install.ts'; +import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts'; +import type { AndroidAdbExecutor } from '../snapshot-helper.ts'; +import { + androidSnapshotQualityDevice as device, + androidSnapshotQualityHelperArtifact as helperArtifact, +} from './snapshot-quality-fixtures.ts'; + +const SCREEN_XML = ''; + +beforeEach(async () => { + await resetAndroidSnapshotHelperSessions(); + resetAndroidSnapshotHelperInstallCache(); +}); + +afterEach(async () => { + await resetAndroidSnapshotHelperSessions(); +}); + +function helperAdbServing( + display: { width?: number; height?: number } = {}, + xml: string = SCREEN_XML, +): AndroidAdbExecutor { + const displayKeys = + display.width !== undefined && display.height !== undefined + ? [ + `INSTRUMENTATION_RESULT: displayWidth=${display.width}`, + `INSTRUMENTATION_RESULT: displayHeight=${display.height}`, + ] + : []; + const stdout = [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_STATUS: helperApiVersion=1', + 'INSTRUMENTATION_STATUS: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_STATUS: chunkIndex=0', + 'INSTRUMENTATION_STATUS: chunkCount=1', + `INSTRUMENTATION_STATUS: payloadBase64=${Buffer.from(xml, 'utf8').toString('base64')}`, + 'INSTRUMENTATION_STATUS_CODE: 1', + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: helperApiVersion=1', + 'INSTRUMENTATION_RESULT: ok=true', + 'INSTRUMENTATION_RESULT: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_RESULT: waitForIdleTimeoutMs=0', + 'INSTRUMENTATION_RESULT: timeoutMs=8000', + 'INSTRUMENTATION_RESULT: maxDepth=128', + 'INSTRUMENTATION_RESULT: maxNodes=5000', + 'INSTRUMENTATION_RESULT: rootPresent=true', + 'INSTRUMENTATION_RESULT: captureMode=interactive-windows', + 'INSTRUMENTATION_RESULT: windowCount=1', + 'INSTRUMENTATION_RESULT: nodeCount=1', + 'INSTRUMENTATION_RESULT: truncated=false', + 'INSTRUMENTATION_RESULT: elapsedMs=12', + 'INSTRUMENTATION_RESULT: pixelDensity=2.625', + ...displayKeys, + 'INSTRUMENTATION_CODE: 0', + ].join('\n'); + return async (args) => { + if (args.includes('--show-versioncode')) { + return { + exitCode: 0, + stdout: 'package:com.callstack.agentdevice.snapshothelper versionCode:13004', + stderr: '', + }; + } + if (args.includes('instrument')) { + return { exitCode: 0, stdout, stderr: '' }; + } + throw new Error(`unexpected helper adb args: ${args.join(' ')}`); + }; +} + +// #3182: the published viewport comes from the display the helper measured, not from the tree, so a +// capture with content and a capture of an empty screen answer the same question. +test('snapshotAndroid publishes the display the helper measured (#3182)', async () => { + const capture = await snapshotAndroid(device, { + helperAdb: helperAdbServing({ width: 1080, height: 2400 }), + helperArtifact, + }); + + assert.deepEqual(capture.viewport, { width: 1080, height: 2400 }); + // The response publishes the fact once. The raw display pair stays on the helper transport and the + // backend metadata the response carries, so a consumer cannot read two copies of it. + assert.equal( + 'displayWidth' in (capture.androidSnapshot as Record), + false, + 'the backend metadata carries no second copy of the display read', + ); +}); + +test('the Android viewport survives the publication adapter into the daemon capture (#3182)', async () => { + const capture = await snapshotAndroid(device, { + helperAdb: helperAdbServing({ width: 1080, height: 2400 }), + helperArtifact, + }); + + assert.deepEqual(androidSnapshotPublicationInput(capture).viewport, { + width: 1080, + height: 2400, + }); +}); + +test('a helper with no display read leaves the Android viewport absent, never zero (#3182)', async () => { + const capture = await snapshotAndroid(device, { + helperAdb: helperAdbServing(), + helperArtifact, + }); + + assert.equal(capture.viewport, undefined); + assert.equal('viewport' in capture, false, 'absent is not the same as an empty box'); +}); + +// The consequence the display read buys: the box is a property of the device, so the capture that +// reports nothing about the tree can still report the screen its (absent) bounds belonged to. +test('a presentation-failed Android capture still publishes the display it read (#3182)', async () => { + const capture = await snapshotAndroid(device, { + helperAdb: helperAdbServing({ width: 1080, height: 2400 }), + helperArtifact, + androidPresentation: { deadlineAtMs: 100, now: () => 100 }, + }); + + assert.deepEqual(capture.nodes, [], 'the projection was discarded'); + assert.deepEqual(capture.viewport, { width: 1080, height: 2400 }); +}); + +test('androidSnapshotViewportFromHelperMetadata refuses an unusable display read (#3182)', () => { + assert.deepEqual( + androidSnapshotViewportFromHelperMetadata({ + outputFormat: 'uiautomator-xml', + displayWidth: 1080, + displayHeight: 2400, + }), + { width: 1080, height: 2400 }, + ); + for (const unusable of [ + {}, + { displayWidth: 0, displayHeight: 2400 }, + { displayWidth: 1080, displayHeight: 0 }, + { displayWidth: -1, displayHeight: 2400 }, + { displayWidth: Number.NaN, displayHeight: 2400 }, + { displayWidth: Number.POSITIVE_INFINITY, displayHeight: 2400 }, + ]) { + assert.equal( + androidSnapshotViewportFromHelperMetadata({ outputFormat: 'uiautomator-xml', ...unusable }), + undefined, + JSON.stringify(unusable), + ); + } +}); + +// The consumer this issue came from takes the extent of all rects as the screen. A display read +// answers that question without the tree, so a capture whose nodes carry no bounds at all still +// names the box those bounds would be measured in. +test('an Android capture with geometry-free nodes still publishes the display it read (#3182)', async () => { + const capture = await snapshotAndroid(device, { + helperAdb: helperAdbServing( + { width: 1080, height: 2400 }, + '', + ), + helperArtifact, + }); + + assert.deepEqual(capture.viewport, { width: 1080, height: 2400 }); + assert.equal( + capture.nodes.every((node) => node.rect === undefined), + true, + 'the tree really carries no geometry', + ); +}); diff --git a/packages/platform-android/src/__tests__/snapshot-helper-capture.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-capture.test.ts index db20cde242..635f28ff43 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-capture.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-capture.test.ts @@ -2,7 +2,10 @@ import assert from 'node:assert/strict'; import { beforeEach, test } from 'vitest'; import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; import { AppError } from '@agent-device/kernel/errors'; -import { captureAndroidSnapshotWithHelper } from '../snapshot-helper-capture.ts'; +import { + captureAndroidSnapshotWithHelper, + parseAndroidSnapshotHelperOutput, +} from '../snapshot-helper-capture.ts'; import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts'; import type { AndroidAdbExecutor } from '../snapshot-helper-types.ts'; import { @@ -159,7 +162,7 @@ test('a helper failure reported under a zero am exit status keeps its own reason ); }); -function helperOutput(xml: string): string { +function helperOutput(xml: string, resultLines: readonly string[] = []): string { return [ 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', 'INSTRUMENTATION_STATUS: helperApiVersion=1', @@ -172,6 +175,34 @@ function helperOutput(xml: string): string { 'INSTRUMENTATION_RESULT: helperApiVersion=1', 'INSTRUMENTATION_RESULT: ok=true', 'INSTRUMENTATION_RESULT: outputFormat=uiautomator-xml', + ...resultLines, 'INSTRUMENTATION_CODE: 0', ].join('\n'); } + +// The helper publishes the display's extent beside its density (#3182), and the transport reader +// keeps an omitted pair absent rather than carrying a zero the host would have to second-guess. +test('parseAndroidSnapshotHelperOutput carries the helper display extent beside its density (#3182)', () => { + const parsed = parseAndroidSnapshotHelperOutput( + helperOutput('', [ + 'INSTRUMENTATION_RESULT: pixelDensity=2.625', + 'INSTRUMENTATION_RESULT: displayWidth=1080', + 'INSTRUMENTATION_RESULT: displayHeight=2400', + ]), + ); + + assert.equal(parsed.metadata.pixelDensity, 2.625); + assert.equal(parsed.metadata.displayWidth, 1080); + assert.equal(parsed.metadata.displayHeight, 2400); +}); + +test('parseAndroidSnapshotHelperOutput leaves the display extent absent when the helper omits it (#3182)', () => { + const parsed = parseAndroidSnapshotHelperOutput( + helperOutput('', [ + 'INSTRUMENTATION_RESULT: pixelDensity=2.625', + ]), + ); + + assert.equal(parsed.metadata.displayWidth, undefined); + assert.equal(parsed.metadata.displayHeight, undefined); +}); diff --git a/packages/platform-android/src/__tests__/snapshot-helper-session-protocol.test.ts b/packages/platform-android/src/__tests__/snapshot-helper-session-protocol.test.ts index ce47d3199f..4dbcfb7cd9 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper-session-protocol.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper-session-protocol.test.ts @@ -37,10 +37,38 @@ test('parses the session envelope and snapshot metadata', () => { rootPresent: undefined, truncated: undefined, elapsedMs: undefined, + displayWidth: undefined, + displayHeight: undefined, }, }); }); +// The session transport reads the display pair exactly like the one-shot transport does (#3182), +// and a header that never arrived stays an absence rather than becoming a zero. +test('parses the session display extent beside its density and absent headers stay absent (#3182)', () => { + const xml = ''; + const withDisplay = sessionResponse({ + requestId: 'snapshot-1', + xml, + metadata: { pixelDensity: '2.625', displayWidth: '1080', displayHeight: '2400' }, + }); + + const parsed = parseAndroidSnapshotHelperSessionSnapshotResponse(withDisplay, 'snapshot-1'); + + assert.equal(parsed.metadata.pixelDensity, 2.625); + assert.equal(parsed.metadata.displayWidth, 1080); + assert.equal(parsed.metadata.displayHeight, 2400); + + const withoutDisplay = sessionResponse({ + requestId: 'snapshot-1', + xml, + metadata: { pixelDensity: '2.625' }, + }); + const sparse = parseAndroidSnapshotHelperSessionSnapshotResponse(withoutDisplay, 'snapshot-1'); + assert.equal(sparse.metadata.displayWidth, undefined); + assert.equal(sparse.metadata.displayHeight, undefined); +}); + test('rejects stale and truncated session snapshot responses', () => { const response = sessionResponse({ requestId: 'snapshot-old', diff --git a/packages/platform-android/src/__tests__/snapshot-helper.test.ts b/packages/platform-android/src/__tests__/snapshot-helper.test.ts index aa4d479d2f..802a5a36aa 100644 --- a/packages/platform-android/src/__tests__/snapshot-helper.test.ts +++ b/packages/platform-android/src/__tests__/snapshot-helper.test.ts @@ -82,6 +82,8 @@ test('parseAndroidSnapshotHelperOutput reconstructs XML chunks and metadata', () truncated: false, elapsedMs: 42, pixelDensity: undefined, + displayWidth: undefined, + displayHeight: undefined, transport: 'instrumentation', }); }); diff --git a/packages/platform-android/src/snapshot-capture.ts b/packages/platform-android/src/snapshot-capture.ts index 2edae82c77..141d83768e 100644 --- a/packages/platform-android/src/snapshot-capture.ts +++ b/packages/platform-android/src/snapshot-capture.ts @@ -9,7 +9,10 @@ import type { RawSnapshotNode, SnapshotBackend, SnapshotQualityVerdict, + SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; +import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; +import type { AndroidSnapshotHelperMetadata } from './snapshot-helper-types.ts'; import type { AndroidSnapshotAnalysis } from './ui-hierarchy.ts'; const androidCaptureEvidence = Symbol('androidSnapshotCaptureEvidence'); @@ -25,6 +28,12 @@ type AndroidSnapshotCaptureData = Readonly<{ analysis: AndroidSnapshotAnalysis; androidSnapshot: AndroidSnapshotBackendMetadata; quality?: SnapshotQualityVerdict; + /** + * The screen the captured bounds are measured in (#3182): the helper's own display read, so a + * capture of an empty screen still reports it. Derived, never stored, so this carrier cannot drift + * from the `displayWidth`/`displayHeight` pair it came from. + */ + viewport?: SnapshotViewportSize; }>; /** Opaque acquisition envelope: exact Android facts cannot be detached from the captured nodes. */ @@ -45,6 +54,23 @@ export function createAndroidSnapshotCapture( return data as AndroidSnapshotCapture; } +/** + * The one place Android's published viewport is built: from the display pair the helper transport + * carries (#3182), through the shared guard, so a zero the platform answered with never becomes a + * viewport a reader trusts. The pair lives only on the helper's own metadata; the backend metadata + * the response publishes keeps the converted fact instead of a second copy of the raw read. + */ +export function androidSnapshotViewportFromHelperMetadata( + metadata: AndroidSnapshotHelperMetadata, +): SnapshotViewportSize | undefined { + return snapshotViewportSizeFrom({ + x: 0, + y: 0, + width: metadata.displayWidth ?? Number.NaN, + height: metadata.displayHeight ?? Number.NaN, + }); +} + /** The sole adapter from Android acquisition into daemon snapshot publication. */ export function androidSnapshotPublicationInput( capture: AndroidSnapshotCapture, diff --git a/packages/platform-android/src/snapshot-helper-capture.ts b/packages/platform-android/src/snapshot-helper-capture.ts index 5ad2399e75..b4ed920a5d 100644 --- a/packages/platform-android/src/snapshot-helper-capture.ts +++ b/packages/platform-android/src/snapshot-helper-capture.ts @@ -491,6 +491,8 @@ function readHelperMetadata(finalResult: Record): AndroidSnapsho truncated: readOptionalBoolean(finalResult.truncated), elapsedMs: readOptionalNumber(finalResult.elapsedMs), pixelDensity: readOptionalNumber(finalResult.pixelDensity), + displayWidth: readOptionalNumber(finalResult.displayWidth), + displayHeight: readOptionalNumber(finalResult.displayHeight), }; } diff --git a/packages/platform-android/src/snapshot-helper-session-protocol.ts b/packages/platform-android/src/snapshot-helper-session-protocol.ts index f0e36ed205..8fff3e894b 100644 --- a/packages/platform-android/src/snapshot-helper-session-protocol.ts +++ b/packages/platform-android/src/snapshot-helper-session-protocol.ts @@ -304,5 +304,7 @@ function readSessionMetadata(headers: Record): AndroidSnapshotHe truncated: readInstrumentationResultBoolean(headers.truncated), elapsedMs: readInstrumentationResultNumber(headers.elapsedMs), pixelDensity: readInstrumentationResultNumber(headers.pixelDensity), + displayWidth: readInstrumentationResultNumber(headers.displayWidth), + displayHeight: readInstrumentationResultNumber(headers.displayHeight), }; } diff --git a/packages/platform-android/src/snapshot-helper-types.ts b/packages/platform-android/src/snapshot-helper-types.ts index 9431f3f21d..e392c7c2ab 100644 --- a/packages/platform-android/src/snapshot-helper-types.ts +++ b/packages/platform-android/src/snapshot-helper-types.ts @@ -97,6 +97,13 @@ export type AndroidSnapshotHelperMetadata = { elapsedMs?: number; /** Physical pixels per dp of the captured display, as the helper's own `DisplayMetrics` say. */ pixelDensity?: number; + /** + * The captured display's pixel extent, from the same `DisplayMetrics` read as `pixelDensity` + * (#3182). Absent when the helper's display read answered with nothing usable — the absence a host + * must read as unknown, never as a screen of zero size. + */ + displayWidth?: number; + displayHeight?: number; transport?: AndroidSnapshotHelperTransport; sessionReused?: boolean; }; diff --git a/packages/platform-android/src/snapshot-types.ts b/packages/platform-android/src/snapshot-types.ts index 8b93b638bc..4f086a5b5c 100644 --- a/packages/platform-android/src/snapshot-types.ts +++ b/packages/platform-android/src/snapshot-types.ts @@ -1,9 +1,23 @@ import type { AndroidSnapshotCaptureMode, AndroidSnapshotHelperInstallReason, + AndroidSnapshotHelperMetadata, AndroidSnapshotHelperTransport, } from './snapshot-helper-types.ts'; +/** + * One Android capture as the host holds it: the tree, the backend metadata the response publishes, + * and the raw helper transport metadata behind it. The helper pair (display extent, density) rides + * as raw transport facts rather than as a pre-derived viewport, so the single place that answers the + * viewport question is `snapshotAndroid`, through the shared guard. The backend metadata stays free + * of a second copy of the display read: the response publishes that fact once, as `viewport` (#3182). + */ +export type AndroidUiHierarchyCapture = { + xml: string; + metadata: AndroidSnapshotBackendMetadata; + helperMetadata: AndroidSnapshotHelperMetadata; +}; + export type AndroidSnapshotBackendMetadata = { backend: 'android-helper'; /** diff --git a/packages/platform-android/src/snapshot.ts b/packages/platform-android/src/snapshot.ts index cc94da3b31..d2593771a6 100644 --- a/packages/platform-android/src/snapshot.ts +++ b/packages/platform-android/src/snapshot.ts @@ -13,6 +13,7 @@ import { type HiddenContentHint, type RawSnapshotNode, type SnapshotOptions, + type SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; import { deriveMobileSnapshotHiddenContentHints } from '@agent-device/capture-kit/mobile-snapshot-semantics'; import { findProjectRoot, readVersion } from '@agent-device/host-kit/version'; @@ -52,7 +53,10 @@ import { } from './snapshot-helper-retirement.ts'; import { requireAndroidAdbHost } from './adb-host.ts'; import { parseAndroidSnapshotHelperManifest } from './snapshot-helper-artifact.ts'; -import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; +import type { + AndroidSnapshotBackendMetadata, + AndroidUiHierarchyCapture, +} from './snapshot-types.ts'; import { classifyAndroidHelperContent, type AndroidHelperContentRecoveryDecision, @@ -73,7 +77,11 @@ import { type AndroidSnapshotPresentationOptions, } from './snapshot-presentation.ts'; import { readAndroidSiblingOrder } from './ui-hierarchy-node.ts'; -import { createAndroidSnapshotCapture, type AndroidSnapshotCapture } from './snapshot-capture.ts'; +import { + androidSnapshotViewportFromHelperMetadata, + createAndroidSnapshotCapture, + type AndroidSnapshotCapture, +} from './snapshot-capture.ts'; const HELPER_INSTALL_TIMEOUT_MS = 30_000; /** @@ -117,6 +125,11 @@ export async function snapshotAndroid( ): Promise { const adb = resolveAndroidAdbProvider(device, options.helperAdb).exec; const capture = await captureAndroidUiHierarchy(device, options, adb); + // The one place Android answers the viewport question (#3182): the helper's own display read, + // through the shared guard, so both the healthy and the presentation-failed capture below publish + // the same box the tree was measured against, and a display the helper could not address stays + // absent rather than becoming a zero. + const viewport = androidSnapshotViewportFromHelperMetadata(capture.helperMetadata); const xml = capture.xml; const tree = parseUiHierarchyTree(xml); const androidSnapshot = withOcclusionScanDisclosure(capture.metadata, tree); @@ -147,6 +160,7 @@ export async function snapshotAndroid( ...androidSnapshotTruncationFields(truncated), androidSnapshot, quality: { state: 'healthy', backend: 'android-helper' } as const, + ...(viewport ? { viewport } : {}), }; return createAndroidSnapshotCapture(result, { clickability: buildAndroidSnapshotClickabilityEvidence(built), @@ -157,6 +171,7 @@ export async function snapshotAndroid( return attachAndroidPresentationFailureEvidence({ failure: error, androidSnapshot, + ...(viewport ? { viewport } : {}), }); } } @@ -164,6 +179,7 @@ export async function snapshotAndroid( function attachAndroidPresentationFailureEvidence(params: { failure: AndroidSnapshotPresentationFailure; androidSnapshot: AndroidSnapshotBackendMetadata; + viewport?: SnapshotViewportSize; }): AndroidSnapshotCapture { return createAndroidSnapshotCapture( { @@ -186,6 +202,7 @@ function attachAndroidPresentationFailureEvidence(params: { reason: params.failure.message, reasonCode: params.failure.qualityReasonCode, }, + ...(params.viewport ? { viewport: params.viewport } : {}), }, { clickability: { @@ -259,7 +276,7 @@ async function captureAndroidUiHierarchy( device: DeviceInfo, options: AndroidSnapshotOptions, adb: AndroidAdbExecutor, -): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { +): Promise { const adbProvider = resolveAndroidAdbProvider(device, options.helperAdb); const helper = await withDiagnosticTimer( 'android_snapshot_helper_artifact_resolution', @@ -285,7 +302,7 @@ async function captureAndroidUiHierarchyWithHelper( options: AndroidSnapshotOptions, adb: AndroidAdbExecutor, artifact: AndroidSnapshotHelperArtifact, -): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { +): Promise { const helperDeviceKey = getAndroidSnapshotHelperSessionDeviceKey(device); const releaseHelperSession = releasesHelperSessionAfterCapture(options, helperDeviceKey); try { @@ -313,7 +330,7 @@ async function captureAndroidHelperContentWithinWindow(params: { adbProvider: AndroidAdbProvider; artifact: AndroidSnapshotHelperArtifact; helperDeviceKey: string; -}): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { +}): Promise { const { options } = params; const recaptureDeadlineMs = resolveContentRecaptureDeadlineMs(options); const rejectContentUnavailable = async ( @@ -470,9 +487,10 @@ function formatAndroidHelperCaptureResult( capture: AndroidSnapshotHelperOutput, artifact: AndroidSnapshotHelperArtifact, installReason: AndroidSnapshotHelperInstallResult['reason'], -): { xml: string; metadata: AndroidSnapshotBackendMetadata } { +): AndroidUiHierarchyCapture { return { xml: capture.xml, + helperMetadata: capture.metadata, metadata: { backend: 'android-helper', pixelDensity: capture.metadata.pixelDensity, @@ -497,7 +515,7 @@ function formatAndroidHelperCaptureResult( } type AndroidHelperContentAttempt = - | { outcome: 'captured'; capture: { xml: string; metadata: AndroidSnapshotBackendMetadata } } + | { outcome: 'captured'; capture: AndroidUiHierarchyCapture } | { outcome: 'unusable'; decision: AndroidHelperContentRecoveryDecision }; async function captureAndroidHelperContentAttempt(params: { @@ -510,7 +528,7 @@ async function captureAndroidHelperContentAttempt(params: { previousContentReason: AndroidContentRecoveryReason | undefined; }): Promise { const { options, adb, adbProvider, artifact, helperDeviceKey, attempt } = params; - let helperCapture: { xml: string; metadata: AndroidSnapshotBackendMetadata }; + let helperCapture: AndroidUiHierarchyCapture; try { const install = await installAndroidSnapshotHelper( options, @@ -564,6 +582,7 @@ async function captureAndroidHelperContentAttempt(params: { outcome: 'captured', capture: { xml: helperCapture.xml, + helperMetadata: helperCapture.helperMetadata, metadata: systemSurfaceOnly ? { ...helperCapture.metadata, systemSurfaceOnly: true } : helperCapture.metadata, @@ -586,7 +605,7 @@ async function rejectAndroidHelperContentUnavailable(params: { signal?: AbortSignal; /** A transient capture does not own the helper, so its content verdict leaves the helper alone. */ retireHelper: boolean; -}): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { +}): Promise { emitDiagnostic({ level: 'error', phase: 'android_snapshot_helper_content_invalid', @@ -628,7 +647,7 @@ async function rejectAndroidHelperCaptureFailure(params: { helperDeviceKey: string; artifact: AndroidSnapshotHelperArtifact; adb: AndroidAdbExecutor; -}): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { +}): Promise { const failureReason = formatAndroidSnapshotHelperFailureReason(params.error); emitDiagnostic({ level: 'error', diff --git a/packages/platform-apple/src/__tests__/interactor-snapshot-viewport.test.ts b/packages/platform-apple/src/__tests__/interactor-snapshot-viewport.test.ts new file mode 100644 index 0000000000..d01d84257c --- /dev/null +++ b/packages/platform-apple/src/__tests__/interactor-snapshot-viewport.test.ts @@ -0,0 +1,48 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { IOS_SIMULATOR, MACOS_DEVICE } from './device-fixtures.ts'; +import { createAppleInteractor } from '../interactor.ts'; +import { runnerResultFor } from './recording-runner-provider.ts'; + +// #3182: an Apple capture publishes the box its rects are measured in — the app-window evidence the +// presentation fold already resolves — without the runner growing a second wire key for it. + +test('an Apple runner capture publishes the app-window box its rects are measured in (#3182)', async () => { + const interactor = createAppleInteractor( + IOS_SIMULATOR, + {}, + { + hasLiveSession: () => true, + runCommand: async () => runnerResultFor({ command: 'snapshot' }), + }, + ); + + const result = await interactor.snapshot(); + if ('stage' in result) throw new Error('Apple runner snapshot must be presented'); + + assert.deepEqual(result.viewport, { width: 390, height: 844 }); +}); + +// The desktop runner's nodes are absolute in window space (#2891), which no single box describes — +// a published viewport would claim a screen the coordinates do not answer to. +test('a macOS app capture publishes no viewport (#3182)', async () => { + const nodes = [ + { + index: 0, + type: 'Application', + label: 'System Settings', + rect: { x: 3200, y: -180, width: 1440, height: 900 }, + }, + ]; + const interactor = createAppleInteractor( + MACOS_DEVICE, + {}, + { hasLiveSession: () => true, runCommand: async () => ({ nodes }) }, + ); + + const result = await interactor.snapshot({ interactiveOnly: true }); + if ('stage' in result) throw new Error('Apple runner snapshot must be presented'); + + assert.equal(result.nodes?.length, 1); + assert.equal('viewport' in result, false); +}); diff --git a/packages/platform-apple/src/interactor.ts b/packages/platform-apple/src/interactor.ts index fac6673e02..2a4aa0e8dc 100644 --- a/packages/platform-apple/src/interactor.ts +++ b/packages/platform-apple/src/interactor.ts @@ -33,9 +33,10 @@ import type { import { captureMacOsSurfaceSnapshot } from './os/macos/surface-snapshot.ts'; import { presentAppleRunnerSnapshot, + type AppleRunnerSnapshotPresentation, readAppleSnapshotResult, + type AppleRunnerSnapshotResult, } from './runner/snapshot-presentation.ts'; -import type { AppleRunnerSnapshotResult } from './runner/snapshot-presentation.ts'; import { iosSystemSurfaceDisclosure } from '@agent-device/contracts/ios-system-surface'; import { iosTargetActivationDisclosure } from '@agent-device/contracts/ios-target-activation'; @@ -247,11 +248,13 @@ async function captureAppleRunnerSnapshot( ); assertReportedRunnerSnapshotNodes(device, options, result); const warnings = runnerSnapshotWarnings(result); + const presentation = presentRunnerSnapshotForDevice(device, options, result); return { - nodes: presentRunnerSnapshotForDevice(device, options, result), + nodes: presentation.nodes, truncated: result.truncated ?? false, backend: 'xctest' as const, producer: 'apple-runner' as const, + ...(presentation.viewport ? { viewport: presentation.viewport } : {}), ...(result.quality ? { quality: result.quality } : {}), ...(result.systemSurface ? { systemSurface: result.systemSurface } : {}), ...(result.keyboard ? { keyboard: result.keyboard } : {}), @@ -295,8 +298,10 @@ function presentRunnerSnapshotForDevice( device: DeviceInfo, options: SnapshotOptions | undefined, result: AppleRunnerSnapshotResult, -) { - if (isMacOs(device)) return result.nodes ?? []; +): AppleRunnerSnapshotPresentation { + // The desktop runner's nodes are already presented and carry absolute window-space rects, which + // no single box describes (#3182); the macOS surface capture publishes its own space instead. + if (isMacOs(device)) return { nodes: result.nodes ?? [] }; return presentAppleRunnerSnapshot(device.id, options, result); } 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 96cebd1001..516a373a95 100644 --- a/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts +++ b/packages/platform-apple/src/runner/__tests__/snapshot-presentation.test.ts @@ -141,7 +141,7 @@ test('a sparse verdict still presents the nodes it did read', () => { ], truncated: true, quality: { state: 'sparse', backend: 'private-ax', reasonCode: 'sparse-tree' }, - }); + }).nodes; assert.deepEqual( nodes.map((node) => node.label), @@ -200,7 +200,7 @@ test('a healthy payload with valid viewport roots still presents', () => { }; const nodes = presentAppleRunnerSnapshot('device-1', undefined, { nodes: [screen, button], - }); + }).nodes; assert.deepEqual( nodes.map((node) => node.index), [0, 1], @@ -227,7 +227,7 @@ test('a runner payload with the hittable bit absent presents without declaring i }, ]; for (const interactiveOnly of [false, true]) { - const presented = presentAppleRunnerSnapshot('device-1', { interactiveOnly }, { nodes }); + const presented = presentAppleRunnerSnapshot('device-1', { interactiveOnly }, { nodes }).nodes; const button = presented.find((node) => node.label === 'Not Now'); assert.ok(button, `interactiveOnly=${interactiveOnly}: the undecided button is presented`); assert.equal('hittable' in button, false); @@ -284,3 +284,105 @@ test('a malformed keyboard payload is restated by the kernel reader, never forwa assert.deepEqual(read, { kind: 'unmeasurable', reason }, `payload ${JSON.stringify(payload)}`); } }); + +// #3182: the response publishes the box the fold already resolved for the presentation, never a +// second box and never a zero. The viewport the validator receives and the viewport the capture +// publishes must be the same evidence — if the fold validated against one box and the response +// published another, a consumer scaling rects against `viewport` would scale against a screen the +// numbers were never checked against. +test('the published viewport is the root box the presentation validated against (#3182)', () => { + const screen: RawSnapshotNode = { + index: 0, + type: 'Application', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }; + const button: RawSnapshotNode = { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Open', + rect: { x: 16, y: 400, width: 80, height: 32 }, + hittable: true, + }; + + const presented = presentAppleRunnerSnapshot('device-1', undefined, { + nodes: [screen, button], + }); + + assert.deepEqual(presented.viewport, { width: 390, height: 844 }); +}); + +test('the published viewport follows the quality payload the fold prefers (#3182)', () => { + // A scoped capture can carry an unscoped quality payload whose Application root is the screen; + // the fold resolves the viewport from those roots, and the response must publish that same box. + const scopedRoot: RawSnapshotNode = { + index: 0, + type: 'Application', + rect: { x: 0, y: 100, width: 390, height: 400 }, + }; + const qualityRoot: RawSnapshotNode = { + index: 0, + type: 'Application', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }; + + const presented = presentAppleRunnerSnapshot('device-1', undefined, { + nodes: [scopedRoot], + qualityPayload: { nodes: [qualityRoot], truncated: false, scope: null }, + }); + + assert.deepEqual(presented.viewport, { width: 390, height: 844 }); +}); + +test('a capture with no usable viewport box publishes no viewport, never a zero (#3182)', () => { + // `runnerFatal` skips presentation, so this exercises the publication seam alone: a zero-extent + // root is the failed Apple read, and the guard must answer with absence. + const fatalZero = presentAppleRunnerSnapshot('device-1', undefined, { + nodes: [{ index: 0, type: 'Application', rect: { x: 0, y: 0, width: 0, height: 0 } }], + runnerFatal: true, + }); + assert.equal('viewport' in fatalZero, false); + + const rootless = presentAppleRunnerSnapshot('device-1', undefined, { + nodes: [{ index: 0, type: 'Other' }], + runnerFatal: true, + }); + assert.equal('viewport' in rootless, false); + + // The guard itself, off the early return: raw is the projection that validates no box, so it is the + // one that reaches publication still holding an unchecked root. A zero-extent Application there + // must answer with absence rather than the failed read's dimensions. + const rawZero = presentAppleRunnerSnapshot( + 'device-1', + { raw: true }, + { + nodes: [{ index: 0, type: 'Application', rect: { x: 0, y: 0, width: 0, height: 0 } }], + }, + ); + assert.equal('viewport' in rawZero, false); +}); + +test('a raw projection publishes no viewport, because nothing validated its box (#3182)', () => { + // The engine validates the viewport only for the regular projection. Publishing the largest root + // under `raw` would hand the caller the same largest-rect guess #3182 exists to retire, labelled + // as a measured fact — and the regular path would refuse this very payload. + const presented = presentAppleRunnerSnapshot( + 'device-1', + { raw: true }, + { + nodes: [ + { index: 0, type: 'Other', rect: { x: 0, y: 0, width: 120, height: 44 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Open', + rect: { x: 16, y: 900, width: 80, height: 32 }, + hittable: true, + }, + ], + }, + ); + + assert.equal('viewport' in presented, false); +}); diff --git a/packages/platform-apple/src/runner/snapshot-presentation.ts b/packages/platform-apple/src/runner/snapshot-presentation.ts index e8d1c136f7..b3d7e15bb5 100644 --- a/packages/platform-apple/src/runner/snapshot-presentation.ts +++ b/packages/platform-apple/src/runner/snapshot-presentation.ts @@ -21,10 +21,12 @@ import { } from '@agent-device/capture-kit/ios-snapshot-planning'; import { AppError } from '@agent-device/kernel/errors'; import { readSnapshotKeyboardBandFact } from '@agent-device/kernel/record'; +import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; import type { RawSnapshotNode, SnapshotKeyboardBandFact, SnapshotQualityVerdict, + SnapshotViewportSize, IosTargetActivation, } from '@agent-device/kernel/snapshot'; import { @@ -84,14 +86,31 @@ function readSystemSurfaceProvenance(value: unknown): IosSystemSurfaceProvenance return host && { bundleId: host.bundleId, kind: host.kind }; } +/** + * What one Apple runner capture leaves behind after presentation: the presented tree, and the box + * its rects are measured in (#3182). The viewport is the evidence the regular fold validated against — + * the app's own box when the runner read one, otherwise the largest window root in the payload, which + * on tvOS is the screen every `XCUIApplication` rect is measured on — and it is published through the + * same guard every other producer passes, so an unusable box stays absent instead of reaching a + * reader as zero. A raw projection and a runner-declared failure publish no box: neither had a + * viewport checked against its tree, so the roots alone would be a guess. This is the host-side owner + * ADR 0004 gives the viewport question; the runner itself never grows a second wire key for a fact + * the host already derives from the payload's roots. + */ +export type AppleRunnerSnapshotPresentation = Readonly<{ + nodes: RawSnapshotNode[]; + viewport?: SnapshotViewportSize; +}>; + export function presentAppleRunnerSnapshot( deviceId: string, options: SnapshotOptions | undefined, result: AppleRunnerSnapshotResult, -): RawSnapshotNode[] { +): AppleRunnerSnapshotPresentation { const nodes = result.nodes ?? []; + const viewportEvidence = runnerViewportEvidence(nodes, result.qualityPayload?.nodes); if (result.runnerFatal === true || (nodes.length === 0 && result.qualityPayload === undefined)) { - return nodes; + return { nodes }; } const request = createIosSnapshotRequest({ @@ -101,8 +120,6 @@ export function presentAppleRunnerSnapshot( scope: options?.scope, customActions: options?.customActions, }); - const viewport = runnerViewportEvidence(nodes, result.qualityPayload?.nodes); - const input: IosSnapshotInput = { stage: 'presented', presentation: { @@ -119,7 +136,7 @@ export function presentAppleRunnerSnapshot( }, validation: { presentationKey: buildIosSnapshotPresentationKey(request), - viewport, + viewport: viewportEvidence, hittability: { kind: 'available' }, lineage: { targetId: deviceId }, residue: [], @@ -127,7 +144,14 @@ export function presentAppleRunnerSnapshot( }; try { - return presentIosRunnerSnapshot(input, request).nodes; + // The engine returns the box its regular projection validated this payload against and no box + // for raw — deriving it here would only repeat that rule (#3182). + const presentation = presentIosRunnerSnapshot(input, request); + const validatedViewport = snapshotViewportSizeFrom(presentation.validatedViewport); + return { + nodes: presentation.nodes, + ...(validatedViewport ? { viewport: validatedViewport } : {}), + }; } catch (error) { throwSnapshotPresentationError(error, result); } diff --git a/src/__tests__/client-snapshot-viewport.test.ts b/src/__tests__/client-snapshot-viewport.test.ts new file mode 100644 index 0000000000..a2ff0c75d2 --- /dev/null +++ b/src/__tests__/client-snapshot-viewport.test.ts @@ -0,0 +1,55 @@ +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 box a capture's rects are measured in reaches Node.js callers as one optional field on the +// snapshot result (#3182). These cases own the client half of that wire: the pair survives verbatim, +// and every payload the kernel guard refuses becomes the absence meaning "the producer measured no +// box" — never the zero the broken producer wrote. + +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 viewport the producer published (#3182)', async () => { + const client = clientAnswering({ truncated: false, viewport: { width: 1080, height: 2400 } }); + + assert.deepEqual((await client.capture.snapshot()).viewport, { width: 1080, height: 2400 }); +}); + +test('client capture.snapshot reports no viewport field for a capture that carried none (#3182)', async () => { + const client = clientAnswering({ truncated: false }); + + assert.equal('viewport' in (await client.capture.snapshot()), false); +}); + +test('client capture.snapshot restates an unusable viewport payload as absence (#3182)', async () => { + for (const viewport of [ + 'screen', + [], + {}, + { width: 1080 }, + { width: '1080', height: 2400 }, + // A zero is the claim a producer may never make; the client refuses to hand it on as a size. + { width: 0, height: 2400 }, + { width: 1080, height: 0 }, + { width: Number.NaN, height: 2400 }, + { + width: Number.MAX_VALUE, + height: Number.MAX_VALUE, + x: -Number.MAX_VALUE / 2, + y: -Number.MAX_VALUE / 2, + }, + // The published shape carries no origin, so a broken producer's failed-read box usually arrives + // as bare maximal extents; minting an origin for them must not mint a screen either. + { width: Number.MAX_VALUE, height: Number.MAX_VALUE }, + ]) { + const client = clientAnswering({ truncated: false, viewport }); + const result = await client.capture.snapshot(); + + assert.equal(result.viewport, undefined, `payload ${JSON.stringify(viewport)}`); + assert.equal('viewport' in result, false, `payload ${JSON.stringify(viewport)}`); + } +}); diff --git a/src/agent-device-client.ts b/src/agent-device-client.ts index 104d169b48..de3a214b24 100644 --- a/src/agent-device-client.ts +++ b/src/agent-device-client.ts @@ -71,6 +71,7 @@ import { type MetroSessionHints, } from './metro/metro-session-hints.ts'; import { isRecord, 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'; import { normalizeScreenshotCaptureResult } from './client/screenshot-result.ts'; @@ -529,6 +530,7 @@ function optionalSnapshotResponseFields( | 'unchanged' | 'visibility' | 'keyboard' + | 'viewport' | 'warnings' | 'snapshotQuality' | 'snapshotDiagnostics' @@ -539,9 +541,11 @@ function optionalSnapshotResponseFields( const visibility = readObject(data.visibility); const unchanged = readObject(data.unchanged); const keyboard = readSnapshotKeyboardBandFact(data.keyboard); + const viewport = readSnapshotViewportSize(data.viewport); const snapshotDiagnostics = readSnapshotDiagnosticsSummary(data.snapshotDiagnostics); return { ...(keyboard ? { keyboard } : {}), + ...(viewport ? { viewport } : {}), ...(visibility ? { visibility: visibility as CaptureSnapshotResult['visibility'] } : {}), ...readSerializedSnapshotCaptureAnnotations(data), ...(unchanged ? { unchanged: unchanged as CaptureSnapshotResult['unchanged'] } : {}), diff --git a/src/backend.ts b/src/backend.ts index 2533e8f4d1..53a542f4e8 100644 --- a/src/backend.ts +++ b/src/backend.ts @@ -28,6 +28,7 @@ import type { SnapshotNode, SnapshotOptions, SnapshotState, + SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; // The backend's public leaf platform (approach b): backends distinguish iOS from @@ -58,6 +59,12 @@ export type BackendSnapshotResult = { * the band from `nodes`. */ keyboard?: SnapshotKeyboardBandFact; + /** + * 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 + * no screen to measure is allowed to give. + */ + viewport?: SnapshotViewportSize; } & SnapshotCaptureAnnotations; export type BackendSnapshotOptions = SnapshotOptions & { diff --git a/src/commands/capture/runtime/snapshot.test.ts b/src/commands/capture/runtime/snapshot.test.ts index b5b908b7d9..0e1bd4d683 100644 --- a/src/commands/capture/runtime/snapshot.test.ts +++ b/src/commands/capture/runtime/snapshot.test.ts @@ -14,6 +14,11 @@ import { } from '../../../runtime.ts'; import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fixtures'; import type { PostGestureOutcome } from '@agent-device/kernel/snapshot'; +import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; +import { + attachSnapshotClickabilityEvidence, + readSnapshotClickabilityEvidence, +} from '@agent-device/contracts/capture'; import { formatPostGestureOutcomeWarning } from '@agent-device/capture-kit/post-gesture-stability'; test('runtime snapshot captures nodes and updates the session baseline', async () => { @@ -807,3 +812,62 @@ test('runtime snapshot warns when its tree was read on a surface still moving af assert.deepEqual(result.warnings, [formatPostGestureOutcomeWarning(outcome)]); }); + +// The box a capture's rects are measured in is public output (#3182) for the same reason the +// keyboard band is: `snapshot --json` is how a caller learns the surface to scale against, and a +// producer that measured no box has to leave the key off rather than claim a screen it never read. + +test('runtime snapshot publishes the viewport its producer measured', async () => { + const device = createSnapshotOnlyDevice({ + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + backend: 'android', + viewport: snapshotViewportSizeFrom({ x: 0, y: 0, width: 1080, height: 2400 }), + }); + + const result = await device.capture.snapshot({ session: 'default' }); + + assert.deepEqual(result.viewport, { width: 1080, height: 2400 }); +}); + +test('runtime snapshot leaves the viewport off when the producer measured no box', async () => { + const device = createSnapshotOnlyDevice({ + nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Window', label: 'Home' }], + backend: 'android', + }); + + const result = await device.capture.snapshot({ session: 'default' }); + + assert.equal('viewport' in result, false); +}); + +// The Android helper's clickability evidence is retained by object identity, and this command +// merges the backend's facts into a copy of the state it hands over (#3182). A copy that forgets +// the evidence is silent until Maestro's clickable-first ordering finds no exact evidence, falls +// back to document order, and taps an inert duplicate of a tapped id (Android smoke lane). +test('runtime snapshot carries backend clickability evidence through the response merge', async () => { + const clickability = { + kind: 'exact', + provider: 'android-helper', + clickableByNodeIndex: new Map([ + [0, false], + [1, true], + ]), + } as const; + const state = makeSnapshotState( + [ + { index: 0, depth: 0, type: 'TextView', label: 'Inert duplicate target' }, + { index: 1, depth: 0, type: 'Button', label: 'Clickable duplicate target' }, + ], + { backend: 'android', producer: 'android-uiautomator' }, + ); + attachSnapshotClickabilityEvidence(state, clickability); + const device = createSnapshotOnlyDevice({ + snapshot: state, + viewport: snapshotViewportSizeFrom({ x: 0, y: 0, width: 1080, height: 2400 }), + }); + + const result = await device.capture.snapshot({ session: 'default' }); + + assert.deepEqual(readSnapshotClickabilityEvidence(result), clickability); + assert.deepEqual(result.viewport, { width: 1080, height: 2400 }); +}); diff --git a/src/commands/capture/runtime/snapshot.ts b/src/commands/capture/runtime/snapshot.ts index c0d9cd9ca7..fd1f2becb2 100644 --- a/src/commands/capture/runtime/snapshot.ts +++ b/src/commands/capture/runtime/snapshot.ts @@ -14,6 +14,7 @@ import type { SnapshotNode, SnapshotState, SnapshotUnchanged, + SnapshotViewportSize, SnapshotVisibility, } from '@agent-device/kernel/snapshot'; import type { BackendSnapshotResult } from '../../../backend.ts'; @@ -56,6 +57,12 @@ export type SnapshotCommandResult = { * keyboard was refused without reconstructing the band from the tree. */ keyboard?: SnapshotKeyboardBandFact; + /** + * The box this capture's rects are measured in, as its producer measured it (#3182) — the same + * space and orientation as the nodes beside it. Absent means the producer measured no box, never + * a zero. See `CaptureSnapshotResult.viewport` for what each producer measures. + */ + viewport?: SnapshotViewportSize; } & PublicSnapshotCaptureAnnotations; type SnapshotCapture = { @@ -99,6 +106,7 @@ export const snapshotCommand: RuntimeCommand< ? { snapshotDiagnostics: capture.result.snapshotDiagnostics } : {}), ...(capture.snapshot.keyboard ? { keyboard: capture.snapshot.keyboard } : {}), + ...(capture.snapshot.viewport ? { viewport: capture.snapshot.viewport } : {}), ...snapshotAppFields(capture), }); }; @@ -198,13 +206,26 @@ function normalizeBackendSnapshot( result: BackendSnapshotResult, runtime: AgentDeviceRuntime, ): SnapshotState { - if (result.snapshot) return result.snapshot; + // A backend may hand over state it built itself, and the response-level facts it publishes beside + // that state travel with this capture: returning the state alone would drop a producer's own + // keyboard band or viewport box (#3182) on the one path where the backend named them itself. + // The copy must carry the private clickability evidence with it: that fact is retained by object + // identity, so a spread that forgets it makes Maestro's clickable-first ordering fall back to + // divergence and tap the document-order (inert) duplicate of a tapped id. + if (result.snapshot) { + return copySnapshotClickabilityEvidence(result.snapshot, { + ...result.snapshot, + ...(result.keyboard ? { keyboard: result.keyboard } : {}), + ...(result.viewport ? { viewport: result.viewport } : {}), + }); + } return { nodes: result.nodes ?? [], truncated: result.truncated, backend: result.backend as SnapshotState['backend'], createdAt: now(runtime), ...(result.keyboard ? { keyboard: result.keyboard } : {}), + ...(result.viewport ? { viewport: result.viewport } : {}), }; } diff --git a/src/commands/output/result-serialization.ts b/src/commands/output/result-serialization.ts index 8df96ab13c..1429fda47d 100644 --- a/src/commands/output/result-serialization.ts +++ b/src/commands/output/result-serialization.ts @@ -78,6 +78,7 @@ export function serializeSnapshotResult(result: CaptureSnapshotResult): Record { const data = serializeSnapshotResult({ @@ -94,3 +95,26 @@ test('serializeSnapshotResult includes snapshot diagnostics', () => { snapshotDiagnostics, }); }); + +// #3182: the viewport rides the response once, beside the tree. An absent one stays absent rather +// than serializing as an empty box. +test('serializeSnapshotResult publishes the viewport beside the tree (#3182)', () => { + const data = serializeSnapshotResult({ + nodes: [], + truncated: false, + viewport: snapshotViewportSizeFrom({ x: 0, y: 0, width: 390, height: 844 }), + identifiers: { session: 'qa' }, + }); + + assert.deepEqual(data.viewport, { width: 390, height: 844 }); +}); + +test('serializeSnapshotResult omits an absent viewport instead of minting one (#3182)', () => { + const data = serializeSnapshotResult({ + nodes: [], + truncated: false, + identifiers: { session: 'qa' }, + }); + + assert.equal('viewport' in data, false); +}); diff --git a/src/core/__tests__/snapshot-state.test.ts b/src/core/__tests__/snapshot-state.test.ts index 859388bc01..5daa6ae3b5 100644 --- a/src/core/__tests__/snapshot-state.test.ts +++ b/src/core/__tests__/snapshot-state.test.ts @@ -2,6 +2,7 @@ import { expect, test } from 'vitest'; import { buildSnapshotState } from '@agent-device/capture-kit/snapshot-state'; import { resolveActionableTouchResolution } from '@agent-device/selectors/interaction-targeting'; import { createSnapshotVisibility } from '@agent-device/contracts/snapshot'; +import { snapshotViewportSizeFrom } from '@agent-device/kernel/rect'; import { attachSnapshotOcclusionContextEvidence } from '@agent-device/contracts/capture'; import { buildUiHierarchySnapshot, @@ -647,3 +648,29 @@ test('buildSnapshotState leaves an unmeasured keyboard unclaimed instead of abse expect('keyboard' in state).toBe(false); }); + +// #3182: the stored tree keeps the box the producer measured, so a consumer of the session snapshot — +// the find path, a later diff — reads the same box the response published instead of inferring one +// from the largest rect still on screen. A capture with no box leaves it absent rather than minting one. +test('buildSnapshotState carries the producer viewport into the stored state (#3182)', () => { + const state = buildSnapshotState( + { + nodes: [{ index: 0, type: 'Application' }], + backend: 'android', + producer: 'android-uiautomator', + viewport: snapshotViewportSizeFrom({ x: 0, y: 0, width: 1080, height: 2400 }), + }, + undefined, + ); + + expect(state.viewport).toEqual({ width: 1080, height: 2400 }); +}); + +test('buildSnapshotState leaves an unmeasured viewport absent (#3182)', () => { + const state = buildSnapshotState( + { nodes: [], backend: 'android', producer: 'android-uiautomator' }, + undefined, + ); + + expect('viewport' in state).toBe(false); +}); diff --git a/src/daemon/__tests__/response-views.test.ts b/src/daemon/__tests__/response-views.test.ts index 46856d4137..fc10d942ca 100644 --- a/src/daemon/__tests__/response-views.test.ts +++ b/src/daemon/__tests__/response-views.test.ts @@ -56,6 +56,21 @@ test('digest carries the foreground repair of the tree it collapsed', () => { expect(digest.targetActivation).toEqual(repair); }); +/** + * A digest drops the tree, so the box its rects were measured in (#3182) has to survive the + * collapse — otherwise an agent reading the digest has to infer the screen from nothing again. + */ +test('digest carries the viewport of the tree it collapsed (#3182)', () => { + const digest = snapshotView!( + { ...SNAPSHOT_DATA, viewport: { width: 390, height: 844 } }, + 'digest', + ); + expect(digest.viewport).toEqual({ width: 390, height: 844 }); + + const withoutViewport = snapshotView!(SNAPSHOT_DATA, 'digest'); + expect('viewport' in withoutViewport).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/src/daemon/response-views.ts b/src/daemon/response-views.ts index 785dc95520..2aebbe2ccf 100644 --- a/src/daemon/response-views.ts +++ b/src/daemon/response-views.ts @@ -34,6 +34,7 @@ function snapshotView(data: DaemonResponseData, level: ResponseLevel): DaemonRes // activation, and the occlusion/quality/visibility warnings a digest still has to surface). const carriedFields = [ 'visibility', + 'viewport', 'snapshotQuality', 'targetActivation', 'warnings', diff --git a/src/daemon/snapshot-capture.ts b/src/daemon/snapshot-capture.ts index d701fb9b90..268c2231c0 100644 --- a/src/daemon/snapshot-capture.ts +++ b/src/daemon/snapshot-capture.ts @@ -15,6 +15,7 @@ import { type SnapshotCaptureProvenance, type SnapshotKeyboardBandFact, type SnapshotState, + type SnapshotViewportSize, } from '@agent-device/kernel/snapshot'; import { resolveRefLabel } from '@agent-device/capture-kit/snapshot-node-lookup'; import { STALE_REF_HINT } from '@agent-device/selectors'; @@ -52,6 +53,8 @@ type SnapshotData = { quality?: unknown; /** The keyboard band the capture's producer measured (#2660), carried to the state guards read. */ keyboard?: SnapshotKeyboardBandFact; + /** The box the capture's rects are measured in (#3182), carried to the state the response reads. */ + viewport?: SnapshotViewportSize; } & Omit & SnapshotCaptureProvenance; diff --git a/test/integration/provider-scenarios/remote-proxy-parity.test.ts b/test/integration/provider-scenarios/remote-proxy-parity.test.ts index 1a8a66afc6..426f11eb2f 100644 --- a/test/integration/provider-scenarios/remote-proxy-parity.test.ts +++ b/test/integration/provider-scenarios/remote-proxy-parity.test.ts @@ -317,6 +317,7 @@ const PUBLISHED_SNAPSHOT_DATA_KEYS = [ 'refsGeneration', 'snapshotDiagnostics', 'truncated', + 'viewport', 'visibility', 'warnings', ] as const; diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index d59051db73..f4cdbdb705 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -346,6 +346,8 @@ The complete domain-client method map is: `client.devices.list()` returns `AgentDeviceDevice` entries. Their optional `model` and `osVersion` fields describe the hardware and OS when discovery reports them; see [Device discovery](/docs/commands#device-discovery) for the sources. +`client.capture.snapshot()` carries an optional `viewport: { width, height }` beside `nodes`: the box those rects are measured in, in the same coordinate space and orientation, so a consumer scales and clips against the screen it was shown instead of inferring one from the largest rect on screen. It is absent when the producer measured no box and never reported as a zero; see [`snapshot`](/docs/commands) for what each producer answers with. + `client.observability.events({ cursor, limit })` reads the session event timeline as paged JSON entries. Use `nextCursor` from the previous page to continue from the daemon-owned `events.ndjson` file without replaying already uploaded/displayed events. Cursors are absolute and survive the file's size rotation; a cursor older than the retained window rejects with `COMMAND_FAILED`, `details.reason: "EVENT_LOG_CURSOR_EXPIRED"`, and `details.earliestCursor` to resume from. The event timeline keeps operational context such as command/status/timing, paths, session/device/app identifiers, refs/selectors, and coordinates. Typed text, clipboard writes, push/event payloads, raw unknown command arguments, and matching raw message fragments are replaced with length-only placeholders. diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 61d23a4eb6..826bc7fe84 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -411,6 +411,16 @@ agent-device get attrs @e1 last: footers, tab bars, items after a long list, even when on screen. The snapshot carries a warning that says so; navigate or scroll so fewer elements render and re-run, and use `screenshot` as visual truth for the rest. +- `viewport: { width, height }` names the box the node rects are measured in, in the same coordinate + space and orientation as the rects beside it, so a consumer scales and clips against the screen it + was shown instead of inferring one from the largest rect on screen. Which surface it names is the + producer's answer: the app window on iOS (so iPad Split View and a foldable panel do not inflate + it), the measured screen on Android and Apple TV. It is absent when the producer measured no box — + a macOS capture whose rects are absolute in window space, a web or Linux capture that reads a tree + without reading a screen, or `--raw` on an Apple target, whose projection validates no box. (An + Android capture publishes its viewport under `--raw` too: the display it reads is the same screen + the raw rects are measured on.) It is never reported as a zero, and it is the full size only; + content-safe gesture bounds are separate. - `--scope ` returns the subtree of the first node in document order whose label, value, or identifier contains the scope text (case-insensitive) and whose subtree still has content in the requested projection, re-rooted at depth 0; no match returns an empty snapshot rather than the