From d7c522f97d6f56e0755e2b0212d9b9cb8ddbd13b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 21 Jul 2026 19:47:33 +0200 Subject: [PATCH 1/9] test(settle): pin that an expanded quick-settings shade stays visible to --settle (#1319) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1319 asked whether the systemui run-condemnation rule leaves `--settle` blind to a fully expanded quick-settings shade, the way it blinded the replay divergence in #1318. It does not, and this pins the reason. The settle loop captures `interactiveOnly: true` (`stable-capture.ts`). That walk drops the structural systemui window spine (`legacy_window_root`, `notification_panel`, `qs_frame`, the quick-settings ComposeView chain) — and those are exactly the nodes that merge the shade into ONE contiguous run in the `--raw` / non-raw shape #1318 measured. Under settle's shape the same capture arrives as five runs, only `split_shade_status_bar` carries a marker, and the 29 quick-settings nodes survive into both diff sides. So settle and divergence do not read one tree differently, as #1318 framed it: they consume different capture shapes, and settle already gets the outcome that layer wants — shade content diffs, status-bar churn does not. Separately, the quiet-detection loop digests the UNFILTERED capture (`digestSnapshotNodes` in `stable-capture.ts`), so the shade would reset the quiet window even in the hypothetical where chrome stripping had emptied the diff. Both halves of the question come out clean. No product change. The behavior is correct but rested on an untested structural coincidence: retaining the systemui spine in the interactive walk would re-merge the runs and make `--settle` report a full-cover shade as bare removals with no added content and no hint. The test fails if that happens (verified by flipping the helper to `interactiveOnly: false`). Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock), both directions #1319 asked about and both halves the test asserts: - shade OPENING mid-settle: settled after 6917ms: +28 -25 (added: brightness seekbar, Wi-Fi/Bluetooth/Mobile data/Quick Share/ Modes/Wallet tiles) - shade CLOSING mid-settle: settled after 6919ms: +25 -28 (mirror image) - the shade's own status bar ("Tue, Jul 21, Wifi signal full., T-Mobile") is absent from both settle diffs while `snapshot -i` of the identical screen still lists it — the run rule filters the churn rather than sitting inert. The archived #1318 capture run through the real interactive-only walk reproduces the live tree node-for-node (35 nodes, 29 kept, 6 stripped). --- .../android-ui-hierarchy-fixtures.ts | 21 +++++ .../snapshot-chrome-android-statusbar.test.ts | 80 ++++++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts b/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts index 88fb1ca93..3819214ea 100644 --- a/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts +++ b/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts @@ -72,6 +72,27 @@ export function walkNonRawAndroidFixture(rawNodes: RawSnapshotNode[]): RawSnapsh return buildUiHierarchySnapshot(tree, undefined, { raw: false }).nodes; } +/** + * Runs a `RawSnapshotNode[]` fixture through the REAL interactive-only Android + * walk (`buildUiHierarchySnapshot(..., { interactiveOnly: true })`), the shape + * `--settle` and `wait stable` actually consume: their loop captures with + * `interactiveOnly: true` (`stable-capture.ts`), so the tree reaching + * `withoutSettleChrome` is this one, NOT `walkNonRawAndroidFixture`'s. + * + * The distinction is load-bearing for systemui run classification, not a + * detail: `shouldIncludeInteractiveAndroidNode` additionally drops the + * structural window spine (`legacy_window_root`, `notification_panel`, + * `qs_frame`, the quick-settings ComposeView chain), and those are precisely + * the nodes that splice a systemui surface into ONE contiguous run. Verified + * live (2026-07-21, emulator-5554 Pixel 9 Pro XL API 37): the same expanded + * shade is one 95-node run under the non-raw walk and five runs under this + * one. + */ +export function walkInteractiveOnlyAndroidFixture(rawNodes: RawSnapshotNode[]): RawSnapshotNode[] { + const tree = rawFixtureToAndroidTree(rawNodes); + return buildUiHierarchySnapshot(tree, undefined, { raw: false, interactiveOnly: true }).nodes; +} + /** * Real device capture (checkout-form fixture app, Gboard open, status bar * visible) archived at `~/.agent-device-bench/replay-runs/android-ime/raw-ime2.json` diff --git a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts index 9a5a568ba..43fb5c085 100644 --- a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts +++ b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts @@ -1,9 +1,11 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { attachRefs, type RawSnapshotNode, type SnapshotNode } from '../../kernel/snapshot.ts'; -import { collectSettleChromeRefs } from '../snapshot-chrome.ts'; +import { collectSettleChromeRefs, withoutSettleChrome } from '../snapshot-chrome.ts'; import { ANDROID_IME_CAPTURE_RAW_NODES, + ANDROID_QS_SHADE_CAPTURE_RAW_NODES, + walkInteractiveOnlyAndroidFixture, walkNonRawAndroidFixture, } from '../../__tests__/test-utils/android-ui-hierarchy-fixtures.ts'; @@ -235,3 +237,79 @@ test('Android nav-bar leaves are recognized as chrome once their navigation_bar* 'expected home_handle to be dropped by the real non-raw walk, not retained', ); }); + +/** + * #1319: does the run-condemnation rule make `--settle` blind to a fully + * expanded quick-settings shade? It does NOT, and this pins the reason, + * because the reason is structural rather than intentional and nothing else + * guards it. + * + * #1318 established that under the `--raw` / non-raw walk this exact capture + * is ONE `legacy_window_root` run whose four status-icon markers condemn all + * 95 nodes — which is why the divergence layer needed its own fallback. But + * the settle loop never sees that shape: it captures `interactiveOnly: true` + * (`stable-capture.ts`), and that walk also drops the structural window spine + * that merged the shade into a single run. The shade therefore reaches + * `withoutSettleChrome` as SEPARATE runs, and only the status-bar one carries + * a marker. + * + * So settle and divergence disagreeing about these nodes (#1318's framing) is + * not two layers reading one tree differently — they consume different capture + * shapes. Settle already gets the outcome that layer wants: shade content + * survives the diff, status-bar churn does not. + * + * Revert sensitivity runs through the interactive walk: if + * `shouldIncludeInteractiveAndroidNode` ever retains the systemui spine, the + * runs re-merge, every assertion in the first half of this test flips, and + * `--settle` silently starts reporting a full-cover shade as bare removals + * with no added content. + * + * Live end-to-end confirmation (2026-07-21, emulator-5554 Pixel 9 Pro XL API 37, + * deskclock), covering both halves this test asserts and both directions #1319 + * asked about: + * + * - shade OPENING mid-settle: `settled after 6917ms: +28 -25`, the added lines + * being the brightness seekbar and the Wi-Fi/Bluetooth/Mobile data/Quick + * Share/Modes/Wallet tiles; + * - shade CLOSING mid-settle: `settled after 6919ms: +25 -28`, the mirror image; + * - and in the same runs the shade's OWN status bar (one group labeled + * "Tue, Jul 21, Wifi signal full., T-Mobile") is absent from the settle diff + * while `snapshot -i` of the identical screen still lists it — so the run rule + * is filtering the churn, not sitting inert. + */ +test('Android expanded quick-settings shade stays visible to --settle while its status bar is still stripped (#1319)', () => { + const appBundleId = 'com.google.android.deskclock'; + const nodes = attachRefs(walkInteractiveOnlyAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); + const kept = withoutSettleChrome(nodes, appBundleId); + const keptIdentifiers = new Set(kept.map((node) => node.identifier)); + + // The shade's actionable content reaches both diff sides, so opening or + // closing it can never read as "nothing changed". + assert.equal(keptIdentifiers.has('com.android.systemui:id/expanded_qs_scroll_view'), true); + assert.equal(keptIdentifiers.has('com.android.systemui:id/slider'), true); + assert.equal( + kept.filter((node) => node.identifier?.startsWith('com.android.systemui:id/qs_tile_')).length > + 0, + true, + 'expected the quick-settings tiles to survive settle chrome stripping', + ); + + // ...and the run rule still does the job it was written for: the status-bar + // run (clock/date/carrier/wifi ticking every second) is the canonical churn + // settle exists to ignore, so sparing the shade must not spare that too. + const chromeRefs = collectSettleChromeRefs(nodes, appBundleId); + for (const identifier of [ + 'com.android.systemui:id/split_shade_status_bar', + 'com.android.systemui:id/clock', + 'com.android.systemui:id/wifi_signal', + ]) { + assert.equal(chromeRefs.has(refForIdentifier(nodes, identifier)), true, identifier); + assert.equal(keptIdentifiers.has(identifier), false, identifier); + } + + // The contrast that explains the whole finding: the SAME capture, walked the + // way the divergence layer receives it, is one run and loses everything. + // This is what #1318 measured; settle escapes it only via the extra drops. + const nonRawNodes = attachRefs(walkNonRawAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); + assert.equal(withoutSettleChrome(nonRawNodes, appBundleId).length, 0); +}); From 4f004edd202b59ed4ad788a67ece8c3244985419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 21 Jul 2026 20:37:23 +0200 Subject: [PATCH 2/9] =?UTF-8?q?docs(android):=20correct=20the=20chrome-cla?= =?UTF-8?q?ssifier=20TODO=20=E2=80=94=20window-type=20keying=20is=20ruled?= =?UTF-8?q?=20out=20on=20device?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the #1319 investigation, acting on review feedback that a paragraph justifying fragile behavior means the code is wrong. Two candidate fixes for the capture-shape-dependent chrome classification were tested, and BOTH are dead. Recording that here so the next person does not re-walk them: 1. The window-type approach this file's own TODO proposed. Measured against the live helper XML (emulator-5554, Pixel 9 Pro XL API 37): systemui reports `window-type=3` (TYPE_SYSTEM) both collapsed and expanded, `TYPE_STATUS_BAR` (2000) never appears, the helper stamps window metadata on window ROOTS only (1 of 169 nodes in an expanded-shade capture), and an expanded shade is ONE window hosting the status icons AND the quick-settings tiles. No window-level signal separates them. The TODO promised a fix the device data rules out. 2. Replacing run-condemnation with "condemn each marked node's subtree plus fully-condemned ancestors". Shape-independent as intended, and it keeps the tiles in both walks — but on the COLLAPSED status bar it leaks the unmarked chrome the walk re-parents next to the markers ("Battery 100 percent.", the notification-icon summary, neither carrying any resource-id). That is the ticking-clock regression #1319 explicitly warned against. So run-condemnation is not a lazy workaround: it reconstructs identity the walk already discarded when it drops the `status_bar*` wrappers and re-parents chrome leaves next to content. That upstream information loss is the actual root cause, and a provenance-preserving walk is the real fix — larger than this PR, and it would let #1318's divergence fallback be revisited. Also trims the #1319 test's doc comment from a long justification of the current behavior down to the finding, the defect, and what the test holds in place. --- src/contracts/android-system-chrome.ts | 25 ++++++++-- .../snapshot-chrome-android-statusbar.test.ts | 49 ++++++------------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index 224227ddb..d0c0bdbef 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -30,11 +30,26 @@ const ANDROID_SYSTEM_CHROME_MARKER_PREFIXES = [ * `--raw` keeps the wrapper markers, so the prefix check above stays * load-bearing there. * - * A more robust fix would thread the AOSP window-type constants - * (`TYPE_STATUS_BAR` = 2000, `TYPE_NAVIGATION_BAR` = 2019) already parsed in - * `readNodeAttributes` (ui-hierarchy.ts) through to the output `SnapshotNode` - * and key off that instead of resource-ids — deferred until the id-based - * approach here proves insufficient. + * This note previously proposed threading the AOSP window-type constants + * (`TYPE_STATUS_BAR` = 2000, `TYPE_NAVIGATION_BAR` = 2019) through to the + * output `SnapshotNode` and keying off those instead. Measured against the + * helper XML on a live device (2026-07-21, emulator-5554, Pixel 9 Pro XL API + * 37), that does not work and should not be attempted: + * + * - systemui reports `window-type=3` (`TYPE_SYSTEM`) both with the shade + * collapsed and fully expanded; `TYPE_STATUS_BAR` never appears at all; + * - the helper stamps window metadata on window ROOT nodes only (1 of 169 + * nodes in an expanded-shade capture), not per node; + * - an expanded shade is ONE window hosting the status icons AND the + * quick-settings tiles, so no window-level signal separates them. + * + * The real constraint is upstream: the walk drops the `status_bar*` wrappers — + * the only nodes that identify the region — and re-parents chrome leaves next + * to unmarked chrome siblings that carry no id (`"Battery 100 percent."`, the + * notification-icon summary). Everything here is reconstructing identity the + * walk already discarded, which is why classification depends on capture + * shape (#1318 raw vs #1319 interactive-only). See #1319 for the measured + * dead ends and the provenance-preserving alternative. */ const ANDROID_SYSTEM_CHROME_MARKER_LEAF_IDS: ReadonlySet = new Set( [ diff --git a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts index 43fb5c085..dc917b453 100644 --- a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts +++ b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts @@ -239,43 +239,22 @@ test('Android nav-bar leaves are recognized as chrome once their navigation_bar* }); /** - * #1319: does the run-condemnation rule make `--settle` blind to a fully - * expanded quick-settings shade? It does NOT, and this pins the reason, - * because the reason is structural rather than intentional and nothing else - * guards it. + * #1319: `--settle` is NOT blind to a fully expanded quick-settings shade. + * Live-verified on emulator-5554 (Pixel 9 Pro XL API 37, deskclock) in both + * directions — opening mid-settle diffs `+28 -25`, closing diffs `+25 -28` — + * while the shade's own status bar stays stripped from both. * - * #1318 established that under the `--raw` / non-raw walk this exact capture - * is ONE `legacy_window_root` run whose four status-icon markers condemn all - * 95 nodes — which is why the divergence layer needed its own fallback. But - * the settle loop never sees that shape: it captures `interactiveOnly: true` - * (`stable-capture.ts`), and that walk also drops the structural window spine - * that merged the shade into a single run. The shade therefore reaches - * `withoutSettleChrome` as SEPARATE runs, and only the status-bar one carries - * a marker. + * The classification is capture-shape-dependent, which is the real defect + * (#1319 has the measurements): the same shade is ONE condemned run under the + * `--raw`/non-raw walk #1318 measured, and separate runs under the + * `interactiveOnly: true` walk settle captures with, because that walk also + * drops the systemui spine holding the run together. Settle lands on the right + * side of that; the divergence layer needed its own fallback to. * - * So settle and divergence disagreeing about these nodes (#1318's framing) is - * not two layers reading one tree differently — they consume different capture - * shapes. Settle already gets the outcome that layer wants: shade content - * survives the diff, status-bar churn does not. - * - * Revert sensitivity runs through the interactive walk: if - * `shouldIncludeInteractiveAndroidNode` ever retains the systemui spine, the - * runs re-merge, every assertion in the first half of this test flips, and - * `--settle` silently starts reporting a full-cover shade as bare removals - * with no added content. - * - * Live end-to-end confirmation (2026-07-21, emulator-5554 Pixel 9 Pro XL API 37, - * deskclock), covering both halves this test asserts and both directions #1319 - * asked about: - * - * - shade OPENING mid-settle: `settled after 6917ms: +28 -25`, the added lines - * being the brightness seekbar and the Wi-Fi/Bluetooth/Mobile data/Quick - * Share/Modes/Wallet tiles; - * - shade CLOSING mid-settle: `settled after 6919ms: +25 -28`, the mirror image; - * - and in the same runs the shade's OWN status bar (one group labeled - * "Tue, Jul 21, Wifi signal full., T-Mobile") is absent from the settle diff - * while `snapshot -i` of the identical screen still lists it — so the run rule - * is filtering the churn, not sitting inert. + * Until the shape-dependence is fixed upstream, this is what holds the settle + * side in place: retaining the spine in the interactive walk re-merges the + * runs and makes `--settle` report a full-cover shade as bare removals with no + * added content. */ test('Android expanded quick-settings shade stays visible to --settle while its status bar is still stripped (#1319)', () => { const appBundleId = 'com.google.android.deskclock'; From 8615771c62817498589fb826492572c3c134b531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 21 Jul 2026 20:53:18 +0200 Subject: [PATCH 3/9] fix(android): stamp status-bar chrome during the walk instead of reconstructing it downstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chrome classification gave opposite answers about the same screen depending on capture shape: an expanded quick-settings shade was 100% chrome under the `--raw`/non-raw walk (#1318 — every tile condemned, needing a divergence-local fallback) and ~17% chrome under the interactive-only walk (#1319). Two layers were papering over one broken classifier. Root cause: `shouldIncludeStructuralAndroidNode` drops the `status_bar*` / `navigation_bar*` containers — the only nodes that identify the region — and re-parents their leaves next to real content. Everything downstream was reconstructing identity the walk had already discarded, and reconstruction is what depended on shape. Fix: record it while it still exists. `walkUiHierarchyNode` threads `ancestorSystemChrome` exactly like the `ancestorHittable` it already carries, and stamps `systemChrome` on the emitted node; `androidUiNodes` tracks the same subtree for the streaming content-recovery pass. Classification is then per node and intrinsic, so `--raw` and non-raw agree by construction. This deletes what was compensating for the loss: - the 18 hand-picked marker leaf ids and their justification comment (what enumerates them is "descendant of a status/nav-bar container" — now stamped); - `collectAndroidSystemChromeRunIndexes`, the run-condemnation rule that let one `clock` node condemn 95 unrelated ones; - the leaf-id/prefix split, replaced by one container predicate that matches `status_bar`/`navigation_bar` as an id SEGMENT so the shade's own `split_shade_status_bar` counts. Net −53 lines across 8 files, almost all of it classifier and prose. Two alternatives were measured and rejected before this one (both recorded in #1319 so they are not re-attempted): - keying off AOSP window types, which this file's own TODO proposed: impossible. On a live device systemui reports `window-type=3` (TYPE_SYSTEM) collapsed AND expanded, `TYPE_STATUS_BAR` never appears, metadata is stamped on window roots only (1 of 169 nodes), and an expanded shade is ONE window holding the status icons and the tiles. - condemning marked subtrees plus fully-condemned ancestors: leaks the unmarked chrome the walk re-parents beside the markers ("Battery 100 percent.", the notification-icon summary) — the ticking-clock regression #1319 warned about. Test fixtures that modelled the old mechanism now model the device instead: the synthetic status bar gets the `status_bar_launch_animation_container` root the real capture has, and the content-recovery XML nests its chrome leaves inside their container. Assertions were not weakened — settle.test.ts stamps via the production predicate, and the #1319 test now asserts both walks classify IDENTICALLY, which is the property that was missing. Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock): - shade opening mid-settle: settled after 7655ms: +28 -25, tiles present, zero status-bar chrome (unchanged from before — settle was already right) - ordinary action, no shade: +1 -1, no chrome leak - real captures: collapsed status bar 9/9 chrome; expanded shade keeps every tile under BOTH walks, which is the behavior that changed --- .../interaction/runtime/settle.test.ts | 197 ++++++++++-------- src/contracts/android-system-chrome.ts | 88 ++------ .../snapshot-chrome-android-statusbar.test.ts | 135 +++++------- src/core/snapshot-chrome.ts | 62 ++---- src/kernel/snapshot.ts | 2 + .../snapshot-content-recovery.test.ts | 87 ++++---- .../android/snapshot-content-recovery.ts | 3 +- src/platforms/android/ui-hierarchy.ts | 49 ++++- 8 files changed, 285 insertions(+), 338 deletions(-) diff --git a/src/commands/interaction/runtime/settle.test.ts b/src/commands/interaction/runtime/settle.test.ts index 6f9e113ac..1acb27fa1 100644 --- a/src/commands/interaction/runtime/settle.test.ts +++ b/src/commands/interaction/runtime/settle.test.ts @@ -11,6 +11,7 @@ import { import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts'; import { ref, selector } from './selector-read.ts'; import { buildSettleTailEntries, NEVER_SETTLED_HINT } from './settle.ts'; +import { isAndroidSystemChromeWindowResourceId } from '../../../contracts/android-system-chrome.ts'; // #1101 --settle: quiet-window settle loop composition on the interaction // commands. Budgets are injected (fake clock) — no real waiting. @@ -696,19 +697,37 @@ const ANDROID_APP_BUNDLE_ID = 'org.reactnavigation.playground'; const ANDROID_SYSTEM_UI_BUNDLE_ID = 'com.android.systemui'; const ANDROID_IME_BUNDLE_ID = 'com.google.android.inputmethod.latin'; +/** + * Stamps `systemChrome` the way the real Android walk does (`walkUiHierarchyNode`), + * for tests that build `SnapshotNode`s directly instead of walking a capture. The + * container predicate is the production one, so only the descent is re-stated here. + */ +function withAndroidSystemChrome(nodes: T[]): T[] { + const byIndex = new Map(nodes.map((node) => [node.index, node])); + const inChrome = (node: T | undefined): boolean => { + if (!node) return false; + if (isAndroidSystemChromeWindowResourceId(node.identifier)) return true; + return node.parentIndex !== undefined && inChrome(byIndex.get(node.parentIndex)); + }; + return nodes.map((node) => (inChrome(node) ? { ...node, systemChrome: true } : node)); +} + +type AndroidChromeStampable = { index: number; parentIndex?: number; identifier?: string }; + function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') { const root = startIndex; - return [ + return withAndroidSystemChrome([ { + // Outermost status-bar node in the real capture: the walk drops the one + // truly anonymous wrapper above it, so this is what a capture starts with. index: root, depth: 0, type: 'android.widget.FrameLayout', + identifier: 'com.android.systemui:id/status_bar_launch_animation_container', bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID, rect: { x: 0, y: 0, width: 1344, height: 159 }, }, { - // The marker every real status-bar capture carries; the window-run - // drops as a whole because this member matches the status_bar* prefix. index: root + 1, depth: 1, parentIndex: root, @@ -746,7 +765,7 @@ function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') { bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID, rect: { x: 1200, y: 40, width: 60, height: 40 }, }, - ]; + ]); } // Real systemui VolumeDialog window captured live alongside the status bar @@ -2104,50 +2123,52 @@ test('buildSettleTailEntries drops the keyboard container and its chrome descend }); test('buildSettleTailEntries drops Android IME chrome and status-bar chrome (#1198)', () => { - const settledNodes = makeSnapshotState([ - { - index: 0, - depth: 0, - type: 'android.widget.Button', - label: 'Send', - bundleId: 'org.reactnavigation.playground', - rect: { x: 10, y: 20, width: 100, height: 40 }, - hittable: true, - }, - { - // Status-bar marker: this systemui run drops whole. - index: 1, - depth: 0, - type: 'android.widget.FrameLayout', - identifier: 'com.android.systemui:id/status_bar', - bundleId: 'com.android.systemui', - }, - { - index: 2, - depth: 1, - parentIndex: 1, - type: 'android.widget.TextView', - identifier: 'com.android.systemui:id/clock', - label: '12:23', - bundleId: 'com.android.systemui', - }, - { - index: 3, - depth: 0, - type: 'android.widget.FrameLayout', - bundleId: 'com.google.android.inputmethod.latin', - hittable: true, - }, - { - index: 4, - depth: 1, - parentIndex: 3, - type: 'android.widget.FrameLayout', - label: 'Delete', - bundleId: 'com.google.android.inputmethod.latin', - hittable: true, - }, - ]).nodes; + const settledNodes = makeSnapshotState( + withAndroidSystemChrome([ + { + index: 0, + depth: 0, + type: 'android.widget.Button', + label: 'Send', + bundleId: 'org.reactnavigation.playground', + rect: { x: 10, y: 20, width: 100, height: 40 }, + hittable: true, + }, + { + // Status-bar marker: this systemui run drops whole. + index: 1, + depth: 0, + type: 'android.widget.FrameLayout', + identifier: 'com.android.systemui:id/status_bar', + bundleId: 'com.android.systemui', + }, + { + index: 2, + depth: 1, + parentIndex: 1, + type: 'android.widget.TextView', + identifier: 'com.android.systemui:id/clock', + label: '12:23', + bundleId: 'com.android.systemui', + }, + { + index: 3, + depth: 0, + type: 'android.widget.FrameLayout', + bundleId: 'com.google.android.inputmethod.latin', + hittable: true, + }, + { + index: 4, + depth: 1, + parentIndex: 3, + type: 'android.widget.FrameLayout', + label: 'Delete', + bundleId: 'com.google.android.inputmethod.latin', + hittable: true, + }, + ]), + ).nodes; const result = buildSettleTailEntries(settledNodes, new Set(), 'org.reactnavigation.playground'); @@ -2158,45 +2179,47 @@ test('buildSettleTailEntries keeps unknown-foreign packages and drops only marke // Keep-unknown-foreign default: a system dialog's buttons (package // `android`) stay tail candidates; only the marked status/nav-bar // window-run drops, and that does not depend on knowing the session's app. - const settledNodes = makeSnapshotState([ - { - index: 0, - depth: 0, - type: 'android.widget.Button', - label: 'Send', - bundleId: 'org.reactnavigation.playground', - rect: { x: 10, y: 20, width: 100, height: 40 }, - hittable: true, - }, - { - index: 1, - depth: 0, - type: 'android.widget.FrameLayout', - identifier: 'com.android.systemui:id/status_bar_container', - bundleId: 'com.android.systemui', - rect: { x: 0, y: 0, width: 1344, height: 159 }, - }, - { - index: 2, - depth: 1, - parentIndex: 1, - type: 'android.widget.TextView', - identifier: 'com.android.systemui:id/clock', - label: '12:23', - bundleId: 'com.android.systemui', - rect: { x: 40, y: 40, width: 80, height: 40 }, - }, - { - index: 3, - depth: 0, - type: 'android.widget.Button', - label: 'Just once', - identifier: 'android:id/button_once', - bundleId: 'android', - rect: { x: 829, y: 2734, width: 255, height: 162 }, - hittable: true, - }, - ]).nodes; + const settledNodes = makeSnapshotState( + withAndroidSystemChrome([ + { + index: 0, + depth: 0, + type: 'android.widget.Button', + label: 'Send', + bundleId: 'org.reactnavigation.playground', + rect: { x: 10, y: 20, width: 100, height: 40 }, + hittable: true, + }, + { + index: 1, + depth: 0, + type: 'android.widget.FrameLayout', + identifier: 'com.android.systemui:id/status_bar_container', + bundleId: 'com.android.systemui', + rect: { x: 0, y: 0, width: 1344, height: 159 }, + }, + { + index: 2, + depth: 1, + parentIndex: 1, + type: 'android.widget.TextView', + identifier: 'com.android.systemui:id/clock', + label: '12:23', + bundleId: 'com.android.systemui', + rect: { x: 40, y: 40, width: 80, height: 40 }, + }, + { + index: 3, + depth: 0, + type: 'android.widget.Button', + label: 'Just once', + identifier: 'android:id/button_once', + bundleId: 'android', + rect: { x: 829, y: 2734, width: 255, height: 162 }, + hittable: true, + }, + ]), + ).nodes; for (const appBundleId of [undefined, 'org.reactnavigation.playground']) { const result = buildSettleTailEntries(settledNodes, new Set(), appBundleId); diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index d0c0bdbef..87508a4d6 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -1,84 +1,28 @@ /** - * Android status-bar/navigation-bar chrome markers, shared between the settle-chrome + * Android status-bar/navigation-bar chrome identity, shared between the settle-chrome * classifier (`core/snapshot-chrome.ts`, #1198) and the helper content classifier * (`platforms/android/snapshot-content-recovery.ts`). SystemUI hosts BOTH persistent chrome * and actionable overlays (volume panel, media/output pickers, notification shade, quick - * settings), so chrome is never a package-level fact: only these status/nav-bar marker - * resource-ids classify as chrome; every other systemui surface is real content. + * settings), so chrome is never a package-level fact: only the status/nav-bar window + * subtree is chrome; every other systemui surface is real content. */ export const ANDROID_SYSTEM_CHROME_PACKAGE = 'com.android.systemui'; -const ANDROID_SYSTEM_CHROME_MARKER_PREFIXES = [ - 'com.android.systemui:id/status_bar', - 'com.android.systemui:id/navigation_bar', -] as const; - /** - * Surviving status-bar/nav-bar LEAF ids (#1251). The non-raw Android walk - * (`walkUiHierarchyNode` in `platforms/android/ui-hierarchy.ts`) drops - * unlabeled/unidentified structural nodes via `shouldIncludeStructuralAndroidNode`, - * re-parenting their children upward — and that silently swallows every - * `status_bar*`/`navigation_bar*` WRAPPER node, i.e. the only nodes the prefix - * check above matches. A non-raw capture is left with just their labeled/ - * identified LEAVES (clock, battery, wifi/mobile icons, nav buttons), whose - * own resource-ids carry no `status_bar`/`navigation_bar` prefix, so the run - * loses its marker and `collectAndroidSystemChromeRunIndexes` stops dropping - * it (verified against a real `--raw` vs. default capture pair of the same - * screen). Recognize those leaves directly, by EXACT id — not prefix, to stay - * tight: nothing here should ever swallow an actionable systemui overlay like - * the volume dialog or a media/output picker, which live under unrelated ids. - * `--raw` keeps the wrapper markers, so the prefix check above stays - * load-bearing there. - * - * This note previously proposed threading the AOSP window-type constants - * (`TYPE_STATUS_BAR` = 2000, `TYPE_NAVIGATION_BAR` = 2019) through to the - * output `SnapshotNode` and keying off those instead. Measured against the - * helper XML on a live device (2026-07-21, emulator-5554, Pixel 9 Pro XL API - * 37), that does not work and should not be attempted: + * Resource-ids of the status-bar / navigation-bar WINDOW containers. `status_bar` and + * `navigation_bar` are matched as an id segment rather than a prefix so the shade's own + * `split_shade_status_bar` container counts too — live-verified on Pixel 9 Pro XL API 37, + * where an expanded shade hosts the status icons under exactly that id. * - * - systemui reports `window-type=3` (`TYPE_SYSTEM`) both with the shade - * collapsed and fully expanded; `TYPE_STATUS_BAR` never appears at all; - * - the helper stamps window metadata on window ROOT nodes only (1 of 169 - * nodes in an expanded-shade capture), not per node; - * - an expanded shade is ONE window hosting the status icons AND the - * quick-settings tiles, so no window-level signal separates them. - * - * The real constraint is upstream: the walk drops the `status_bar*` wrappers — - * the only nodes that identify the region — and re-parents chrome leaves next - * to unmarked chrome siblings that carry no id (`"Battery 100 percent."`, the - * notification-icon summary). Everything here is reconstructing identity the - * walk already discarded, which is why classification depends on capture - * shape (#1318 raw vs #1319 interactive-only). See #1319 for the measured - * dead ends and the provenance-preserving alternative. + * Membership is decided ONCE, during the walk (`walkUiHierarchyNode`), which stamps + * `systemChrome` on every descendant while the container is still in the tree. Consumers + * read that flag; nothing downstream re-derives chrome identity from ids. */ -const ANDROID_SYSTEM_CHROME_MARKER_LEAF_IDS: ReadonlySet = new Set( - [ - // status bar - 'clock', - 'battery', - 'statusIcons', - 'notificationIcons', - 'notification_icon_area', - 'system_icons', - 'cutout_space_view', - 'mobile_signal', - 'mobile_combo', - 'mobile_group', - 'wifi_signal', - 'wifi_combo', - 'wifi_group', - 'start_side_notif_and_chip_container', - // nav bar - 'back', - 'home', - 'recent_apps', - 'home_handle', - ].map((leaf) => `${ANDROID_SYSTEM_CHROME_PACKAGE}:id/${leaf}`), -); - -/** True when the resource-id marks Android status-bar or navigation-bar chrome. */ -export function isAndroidSystemChromeResourceId(resourceId: string | null | undefined): boolean { +export function isAndroidSystemChromeWindowResourceId( + resourceId: string | null | undefined, +): boolean { const identifier = resourceId ?? ''; - if (ANDROID_SYSTEM_CHROME_MARKER_LEAF_IDS.has(identifier)) return true; - return ANDROID_SYSTEM_CHROME_MARKER_PREFIXES.some((prefix) => identifier.startsWith(prefix)); + if (!identifier.startsWith(`${ANDROID_SYSTEM_CHROME_PACKAGE}:id/`)) return false; + const leaf = identifier.slice(`${ANDROID_SYSTEM_CHROME_PACKAGE}:id/`.length); + return /(^|_)(status_bar|navigation_bar)/.test(leaf); } diff --git a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts index dc917b453..e3b8d53d5 100644 --- a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts +++ b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts @@ -10,21 +10,9 @@ import { } from '../../__tests__/test-utils/android-ui-hierarchy-fixtures.ts'; /** - * `ANDROID_IME_CAPTURE_RAW_NODES` (shared test util, see its own doc comment) - * is a real device `--raw` capture: `--raw` keeps every structural wrapper - * node (`status_bar_container`, `status_bar_contents`, ...) so the OLD - * prefix-only marker check finds them and drops the whole run. - * - * The default (non-raw) walk drops unlabeled/unidentified structural nodes - * (`shouldIncludeStructuralAndroidNode` in `platforms/android/ui-hierarchy.ts`), - * re-parenting their children upward — which silently removes every one of - * those marker-bearing wrappers AND several other anonymous structural nodes - * that have neither a hittable descendant nor meaningful text/id (a real - * `android.view.View` example is `com.android.systemui:id/home_handle`, see - * the nav-bar test below). `walkNonRawAndroidFixture` (shared test util) runs - * the fixture through the REAL `buildUiHierarchySnapshot({ raw: false })` - * walk instead of hand-simulating a subset of its drops, so every inclusion - * decision below is production's, not a guess at which nodes matter. + * The `walk*AndroidFixture` helpers run a real `--raw` device capture through + * production's own walk, so every inclusion decision below is production's + * rather than a hand-simulation of it. */ function refForIdentifier(nodes: SnapshotNode[], identifier: string): string { @@ -33,21 +21,7 @@ function refForIdentifier(nodes: SnapshotNode[], identifier: string): string { return node.ref; } -/** - * Status-bar leaf identifiers that SURVIVE the real non-raw walk for this - * fixture (verified against `buildUiHierarchySnapshot({ raw: false })` - * directly). Several ids the naive prefix-only simulation used to assert - * (`notification_icon_area`, `notificationIcons`, `cutout_space_view`, - * `system_icons`, `statusIcons`, `mobile_group`, `wifi_combo`, `wifi_group`, - * `start_side_notif_and_chip_container`, `battery`) are unlabeled structural - * wrappers with a generic resource id and no hittable descendant: production - * `shouldIncludeStructuralAndroidNode` drops every one of them too, same as - * the `status_bar*`/`navigation_bar*` wrappers. Their content isn't lost — - * `battery`'s labeled child ("Battery 100 percent.") re-parents upward and is - * covered by the "every systemui-owned node is chrome" assertion below — but - * the WRAPPER identifier itself is gone from the walked tree, so it can't be - * looked up by id. - */ +/** Status-bar leaves that survive the non-raw walk for this fixture; none carries a chrome id. */ const STATUS_BAR_LEAF_IDENTIFIERS = [ 'com.android.systemui:id/clock', 'com.android.systemui:id/mobile_combo', @@ -55,7 +29,7 @@ const STATUS_BAR_LEAF_IDENTIFIERS = [ 'com.android.systemui:id/wifi_signal', ]; -test('Android non-raw capture: status-bar leaves are recognized as chrome once their status_bar*/navigation_bar* marker wrapper is dropped by the walk (#1251)', () => { +test('Android non-raw capture: status-bar leaves stay chrome after the walk drops the container that identifies them (#1251)', () => { const walkedNodes = walkNonRawAndroidFixture(ANDROID_IME_CAPTURE_RAW_NODES); const nodes = attachRefs(walkedNodes); const chromeRefs = collectSettleChromeRefs(nodes, 'com.callstack.agentdevicelab'); @@ -107,15 +81,9 @@ test('Android non-raw capture: status-bar leaves are recognized as chrome once t }); test('Android actionable systemui overlay (volume dialog) still survives the chrome filter (#1251)', () => { - // Filter-logic unit test, NOT a live-capture-path claim (#1264 finding 2): this - // exercises `collectSettleChromeRefs` in isolation over a synthetic systemui - // run appended to an unrelated capture fixture. The leaf ids ARE real, - // live-verified volume-dialog ids (`volume_dialog_container`, - // `volume_new_ringer_active_icon_container`), but appending them to this - // fixture does not reproduce how a live capture reaches the filter — that is - // covered separately by the full-capture invariant test below. The point - // here is narrower: the status-bar/nav-bar leaf-id set (#1251) must NOT - // broaden to "any systemui id" and drop an actionable overlay it is handed. + // Real volume-dialog ids grafted onto an unrelated capture (#1264 finding 2: + // synthetic placement, real ids). Chrome must stay a status/nav-bar fact and + // never widen to "any systemui id", which would swallow this overlay. const rawWithVolumeDialog: RawSnapshotNode[] = [ ...ANDROID_IME_CAPTURE_RAW_NODES, { @@ -239,56 +207,51 @@ test('Android nav-bar leaves are recognized as chrome once their navigation_bar* }); /** - * #1319: `--settle` is NOT blind to a fully expanded quick-settings shade. - * Live-verified on emulator-5554 (Pixel 9 Pro XL API 37, deskclock) in both - * directions — opening mid-settle diffs `+28 -25`, closing diffs `+25 -28` — - * while the shade's own status bar stays stripped from both. - * - * The classification is capture-shape-dependent, which is the real defect - * (#1319 has the measurements): the same shade is ONE condemned run under the - * `--raw`/non-raw walk #1318 measured, and separate runs under the - * `interactiveOnly: true` walk settle captures with, because that walk also - * drops the systemui spine holding the run together. Settle lands on the right - * side of that; the divergence layer needed its own fallback to. + * A fully expanded quick-settings shade hosts the status-bar icons and the + * quick-settings tiles in ONE window. Chrome classification must split them the + * same way whatever shape the capture arrives in — that is what #1318 (raw: + * every tile condemned) and #1319 (interactive-only: tiles kept) each hit from + * one side. * - * Until the shape-dependence is fixed upstream, this is what holds the settle - * side in place: retaining the spine in the interactive walk re-merges the - * runs and makes `--settle` report a full-cover shade as bare removals with no - * added content. + * Live-verified on emulator-5554 (Pixel 9 Pro XL API 37, deskclock): raising + * the shade mid-`--settle` diffs `+28 -25`, closing it `+25 -28`, and the + * shade's own status bar is absent from both while `snapshot -i` of the same + * screen still lists it. */ -test('Android expanded quick-settings shade stays visible to --settle while its status bar is still stripped (#1319)', () => { +test('Android expanded quick-settings shade: tiles survive and the status bar drops, identically in every capture shape (#1318/#1319)', () => { const appBundleId = 'com.google.android.deskclock'; - const nodes = attachRefs(walkInteractiveOnlyAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); - const kept = withoutSettleChrome(nodes, appBundleId); - const keptIdentifiers = new Set(kept.map((node) => node.identifier)); - // The shade's actionable content reaches both diff sides, so opening or - // closing it can never read as "nothing changed". - assert.equal(keptIdentifiers.has('com.android.systemui:id/expanded_qs_scroll_view'), true); - assert.equal(keptIdentifiers.has('com.android.systemui:id/slider'), true); - assert.equal( - kept.filter((node) => node.identifier?.startsWith('com.android.systemui:id/qs_tile_')).length > - 0, - true, - 'expected the quick-settings tiles to survive settle chrome stripping', - ); + for (const [shape, walk] of [ + ['interactive-only (--settle, wait stable)', walkInteractiveOnlyAndroidFixture], + ['non-raw (snapshot, replay divergence)', walkNonRawAndroidFixture], + ] as const) { + const nodes = attachRefs(walk(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); + const kept = withoutSettleChrome(nodes, appBundleId); + const keptIdentifiers = new Set(kept.map((node) => node.identifier)); - // ...and the run rule still does the job it was written for: the status-bar - // run (clock/date/carrier/wifi ticking every second) is the canonical churn - // settle exists to ignore, so sparing the shade must not spare that too. - const chromeRefs = collectSettleChromeRefs(nodes, appBundleId); - for (const identifier of [ - 'com.android.systemui:id/split_shade_status_bar', - 'com.android.systemui:id/clock', - 'com.android.systemui:id/wifi_signal', - ]) { - assert.equal(chromeRefs.has(refForIdentifier(nodes, identifier)), true, identifier); - assert.equal(keptIdentifiers.has(identifier), false, identifier); - } + // The shade's actionable content survives, so opening or closing it can + // never read as "nothing changed". + assert.equal(keptIdentifiers.has('com.android.systemui:id/slider'), true, shape); + assert.equal( + kept.some((node) => node.identifier?.startsWith('com.android.systemui:id/qs_tile_')), + true, + `expected the quick-settings tiles to survive chrome stripping — ${shape}`, + ); - // The contrast that explains the whole finding: the SAME capture, walked the - // way the divergence layer receives it, is one run and loses everything. - // This is what #1318 measured; settle escapes it only via the extra drops. - const nonRawNodes = attachRefs(walkNonRawAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); - assert.equal(withoutSettleChrome(nonRawNodes, appBundleId).length, 0); + // ...while the status-bar subtree sharing that window still drops: clock, + // date and wifi tick constantly and are the canonical churn settle ignores. + const chromeRefs = collectSettleChromeRefs(nodes, appBundleId); + for (const identifier of [ + 'com.android.systemui:id/split_shade_status_bar', + 'com.android.systemui:id/clock', + 'com.android.systemui:id/wifi_signal', + ]) { + assert.equal( + chromeRefs.has(refForIdentifier(nodes, identifier)), + true, + `${identifier} ${shape}`, + ); + assert.equal(keptIdentifiers.has(identifier), false, `${identifier} ${shape}`); + } + } }); diff --git a/src/core/snapshot-chrome.ts b/src/core/snapshot-chrome.ts index ff0909ba3..8047cd6f2 100644 --- a/src/core/snapshot-chrome.ts +++ b/src/core/snapshot-chrome.ts @@ -1,7 +1,4 @@ -import { - ANDROID_SYSTEM_CHROME_PACKAGE, - isAndroidSystemChromeResourceId, -} from '../contracts/android-system-chrome.ts'; +import { ANDROID_SYSTEM_CHROME_PACKAGE } from '../contracts/android-system-chrome.ts'; import type { SnapshotNode } from '../kernel/snapshot.ts'; import { isAndroidInputMethodSnapshotNode } from '../snapshot/android-input-method-overlays.ts'; import { normalizeType } from '../utils/text-surface.ts'; @@ -204,16 +201,10 @@ function collectSubtreeIndexes( // SystemUI hosts BOTH persistent chrome and actionable overlays (volume // panel, media/output pickers), so chrome is never a package-level fact. -// Within `com.android.systemui`, only window-runs carrying a status-bar or -// navigation-bar marker resource-id drop; every other systemui surface is -// kept. Marker set live-verified on the emulator: the status-bar window -// carries `status_bar*` ids throughout while the VolumeDialog window carries -// only `volume_dialog*` ids (`input_method_nav*` bars are IME-owned and -// handled by the IME tier). The marker constants and the resource-id-level -// predicate live in `contracts/android-system-chrome.ts` so the Android -// helper content classifier reuses the same classification. -function hasAndroidSystemChromeMarker(node: SnapshotNode): boolean { - return isAndroidSystemChromeResourceId(node.identifier ?? ''); +// `systemChrome` is stamped per node during the Android walk, from the +// status-bar/nav-bar container it descends from — see `walkUiHierarchyNode`. +function isAndroidSystemChromeNode(node: SnapshotNode): boolean { + return node.bundleId === ANDROID_SYSTEM_CHROME_PACKAGE && node.systemChrome === true; } /** @@ -249,7 +240,7 @@ function collectAndroidSettleChrome( const systemChromeIndexes = appBundleId === ANDROID_SYSTEM_CHROME_PACKAGE ? new Set() - : collectAndroidSystemChromeRunIndexes(nodes, byIndex, imeIndexes); + : collectAndroidSystemChromeIndexes(nodes, imeIndexes); // The one surviving container line per IME run; the rest of the run and all // status/nav-bar chrome never spend diff/tail budget. const strippedIndexes = new Set( @@ -268,47 +259,20 @@ function collectAndroidSettleChrome( } /** - * Systemui window-runs (contiguous same-package parent chains) that contain a - * status/nav-bar marker anywhere in the run. The whole marked run drops — - * unmarked wrappers above `status_bar_container` churn with the bar itself — - * while unmarked runs (volume panel, media pickers) are kept whole. + * Systemui nodes belonging to the status-bar / navigation-bar window. Chrome + * identity is per node and intrinsic, so an actionable systemui surface sharing + * a window with the status bar (an expanded quick-settings shade hosts both, + * #1318/#1319) keeps its own nodes. */ -function collectAndroidSystemChromeRunIndexes( +function collectAndroidSystemChromeIndexes( nodes: SnapshotNode[], - byIndex: Map, imeIndexes: ReadonlySet, ): Set { - const systemUiIndexes = new Set( + return new Set( nodes - .filter( - (node) => node.bundleId === ANDROID_SYSTEM_CHROME_PACKAGE && !imeIndexes.has(node.index), - ) + .filter((node) => !imeIndexes.has(node.index) && isAndroidSystemChromeNode(node)) .map((node) => node.index), ); - if (systemUiIndexes.size === 0) return new Set(); - // Union-find-lite: each systemui node resolves to its run root (the nearest - // ancestor chain member whose parent is absent or not systemui). - const runRootByIndex = new Map(); - const resolveRunRoot = (index: number): number => { - const cached = runRootByIndex.get(index); - if (cached !== undefined) return cached; - const parentIndex = byIndex.get(index)?.parentIndex; - const root = - parentIndex !== undefined && systemUiIndexes.has(parentIndex) - ? resolveRunRoot(parentIndex) - : index; - runRootByIndex.set(index, root); - return root; - }; - const markedRunRoots = new Set( - [...systemUiIndexes] - .filter((index) => { - const node = byIndex.get(index); - return node !== undefined && hasAndroidSystemChromeMarker(node); - }) - .map((index) => resolveRunRoot(index)), - ); - return new Set([...systemUiIndexes].filter((index) => markedRunRoots.has(resolveRunRoot(index)))); } /** iOS keyboard-window chrome unioned with Android IME/system chrome. */ diff --git a/src/kernel/snapshot.ts b/src/kernel/snapshot.ts index ccad5d38c..eca497729 100644 --- a/src/kernel/snapshot.ts +++ b/src/kernel/snapshot.ts @@ -66,6 +66,8 @@ export type RawSnapshotNode = { hiddenContentBelow?: boolean; interactionBlocked?: 'covered'; presentationHints?: string[]; + /** Android: node is inside a status-bar/navigation-bar window (see `systemChrome` in ui-hierarchy.ts). */ + systemChrome?: boolean; }; export type HiddenContentHint = { diff --git a/src/platforms/android/__tests__/snapshot-content-recovery.test.ts b/src/platforms/android/__tests__/snapshot-content-recovery.test.ts index 781be22d4..e87b5566e 100644 --- a/src/platforms/android/__tests__/snapshot-content-recovery.test.ts +++ b/src/platforms/android/__tests__/snapshot-content-recovery.test.ts @@ -320,24 +320,29 @@ test('rejects an active navigation-bar window with three-button chrome', () => { windowActive: true, packageName: 'com.android.systemui', className: 'android.widget.FrameLayout', - }), - node({ - text: 'Back', - resourceId: 'com.android.systemui:id/back', - packageName: 'com.android.systemui', - className: 'android.widget.ImageView', - }), - node({ - text: 'Home', - resourceId: 'com.android.systemui:id/home', - packageName: 'com.android.systemui', - className: 'android.widget.ImageView', - }), - node({ - text: 'Overview', - resourceId: 'com.android.systemui:id/recent_apps', - packageName: 'com.android.systemui', - className: 'android.widget.ImageView', + // Chrome identity lives on the container, as on a device: the leaves + // below carry no marker of their own. + resourceId: 'com.android.systemui:id/navigation_bar_frame', + children: [ + node({ + text: 'Back', + resourceId: 'com.android.systemui:id/back', + packageName: 'com.android.systemui', + className: 'android.widget.ImageView', + }), + node({ + text: 'Home', + resourceId: 'com.android.systemui:id/home', + packageName: 'com.android.systemui', + className: 'android.widget.ImageView', + }), + node({ + text: 'Overview', + resourceId: 'com.android.systemui:id/recent_apps', + packageName: 'com.android.systemui', + className: 'android.widget.ImageView', + }), + ], }), ]), { @@ -361,24 +366,27 @@ test('rejects an active status-chrome window with clock, battery, and signal ico windowActive: true, packageName: 'com.android.systemui', className: 'android.widget.FrameLayout', - }), - node({ - text: '7:52', - resourceId: 'com.android.systemui:id/clock', - packageName: 'com.android.systemui', - className: 'android.widget.TextView', - }), - node({ - text: 'Battery 100 percent', - resourceId: 'com.android.systemui:id/battery', - packageName: 'com.android.systemui', - className: 'android.widget.LinearLayout', - }), - node({ - text: 'Wifi signal full', - resourceId: 'com.android.systemui:id/wifi_signal', - packageName: 'com.android.systemui', - className: 'android.widget.ImageView', + resourceId: 'com.android.systemui:id/status_bar_container', + children: [ + node({ + text: '7:52', + resourceId: 'com.android.systemui:id/clock', + packageName: 'com.android.systemui', + className: 'android.widget.TextView', + }), + node({ + text: 'Battery 100 percent', + resourceId: 'com.android.systemui:id/battery', + packageName: 'com.android.systemui', + className: 'android.widget.LinearLayout', + }), + node({ + text: 'Wifi signal full', + resourceId: 'com.android.systemui:id/wifi_signal', + packageName: 'com.android.systemui', + className: 'android.widget.ImageView', + }), + ], }), ]), { @@ -473,9 +481,14 @@ function node(options: { className: string; windowType?: number; windowActive?: boolean; + /** Nested children, so chrome leaves can sit inside their status/nav-bar container as on a device. */ + children?: string[]; }): string { const windowAttrs = (options.windowType === undefined ? '' : ` window-type="${options.windowType}"`) + (options.windowActive === undefined ? '' : ` window-active="${options.windowActive}"`); - return ``; + const open = `` + : `${open}>${options.children.join('')}`; } diff --git a/src/platforms/android/snapshot-content-recovery.ts b/src/platforms/android/snapshot-content-recovery.ts index c33ddbf0d..d945161d0 100644 --- a/src/platforms/android/snapshot-content-recovery.ts +++ b/src/platforms/android/snapshot-content-recovery.ts @@ -1,6 +1,5 @@ import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; import { isAndroidInputMethodOwnedNode } from '../../contracts/android-input-ownership.ts'; -import { isAndroidSystemChromeResourceId } from '../../contracts/android-system-chrome.ts'; import { classifyAndroidAlertIdentifier } from './alert-detection.ts'; import { androidUiNodes, type AndroidUiNodeMetadata } from './ui-hierarchy.ts'; @@ -299,7 +298,7 @@ function recordMeaningfulWindowOwnership( // Status/nav-bar chrome must not satisfy the system-surface floor: an active navigation bar // (Back + Home + Recents) or status chrome (clock, battery, signal icons) is missing-app-content // residue, not a usable shade/quick-settings surface. Only non-chrome nodes count. - if (isInActiveSystemSurfaceWindow(summary) && !isAndroidSystemChromeResourceId(node.resourceId)) { + if (isInActiveSystemSurfaceWindow(summary) && node.systemChrome !== true) { summary.activeSystemSurfaceMeaningfulNodeCount += 1; } } diff --git a/src/platforms/android/ui-hierarchy.ts b/src/platforms/android/ui-hierarchy.ts index a9d44eea2..4bddb37a2 100644 --- a/src/platforms/android/ui-hierarchy.ts +++ b/src/platforms/android/ui-hierarchy.ts @@ -2,6 +2,7 @@ import type { RawSnapshotNode, Rect, SnapshotOptions } from '../../kernel/snapsh import { parseBounds } from '../../utils/bounds.ts'; import { isScrollableType } from '../../utils/scrollable.ts'; import { intersectArea } from '../../utils/screenshot-geometry.ts'; +import { isAndroidSystemChromeWindowResourceId } from '../../contracts/android-system-chrome.ts'; export type AndroidSnapshotAnalysis = { rawNodeCount: number; @@ -32,14 +33,36 @@ export type AndroidUiNodeMetadata = { windowActive?: boolean; windowFocused?: boolean; windowRect?: Rect; + /** Inside a status-bar/navigation-bar container (tracked by `androidUiNodes`). */ + systemChrome?: boolean; }; +/** + * Streams `` metadata in document order, carrying status-bar/nav-bar + * provenance on each node. The container id is the only chrome signal in the + * tree — its clock/battery/wifi leaves carry no marker of their own — so the + * subtree is tracked here rather than re-derived from a list of leaf ids. + */ export function* androidUiNodes(xml: string): IterableIterator { - const nodeRegex = /]*>/g; - let match = nodeRegex.exec(xml); + const tokenRegex = /]*>|<\/node>/g; + let depth = 0; + let chromeDepth: number | undefined; + let match = tokenRegex.exec(xml); while (match) { - yield readAndroidUiNodeMetadata(match[0]); - match = nodeRegex.exec(xml); + const token = match[0]; + if (token.startsWith('')) depth += 1; + else if (chromeDepth === depth) chromeDepth = undefined; + } + match = tokenRegex.exec(xml); } } @@ -115,6 +138,16 @@ export function buildUiHierarchySnapshot( return state.truncated ? { ...snapshot, truncated: true } : snapshot; } +/** + * Chrome provenance is stamped HERE, while the tree still has the wrapper that + * carries it. `shouldIncludeAndroidNode` drops `status_bar*`/`navigation_bar*` + * wrappers and re-parents their children upward, so downstream classifiers see + * a clock/battery/wifi leaf sitting next to real content with nothing left to + * say which region it came from. Recording it on the way down (like + * `ancestorHittable`) means the answer is identical in every capture shape — + * `--raw` keeps the wrapper, a default capture drops it, both stamp the same + * descendants. + */ function walkUiHierarchyNode( state: AndroidSnapshotBuildState, node: AndroidNode, @@ -122,6 +155,7 @@ function walkUiHierarchyNode( parentIndex?: number, ancestorHittable: boolean = false, ancestorCollection: boolean = false, + ancestorSystemChrome: boolean = false, ): void { if (state.maxNodes !== undefined && state.nodes.length >= state.maxNodes) { state.truncated = true; @@ -138,8 +172,10 @@ function walkUiHierarchyNode( hasInteractiveDescendant(state, node), ancestorCollection, ); + const systemChrome = + ancestorSystemChrome || isAndroidSystemChromeWindowResourceId(node.identifier); const currentIndex = include - ? appendAndroidSnapshotNode(state, node, depth, parentIndex) + ? appendAndroidSnapshotNode(state, node, depth, parentIndex, systemChrome) : parentIndex; const nextAncestorHittable = ancestorHittable || Boolean(node.hittable); const nextAncestorCollection = ancestorCollection || isCollectionContainerType(node.type); @@ -151,6 +187,7 @@ function walkUiHierarchyNode( currentIndex, nextAncestorHittable, nextAncestorCollection, + systemChrome, ); if (state.truncated) return; } @@ -161,6 +198,7 @@ function appendAndroidSnapshotNode( node: AndroidNode, depth: number, parentIndex?: number, + systemChrome?: boolean, ): number { const currentIndex = state.nodes.length; state.sourceNodes.push(node); @@ -180,6 +218,7 @@ function appendAndroidSnapshotNode( parentIndex, ...(node.hiddenContentAbove ? { hiddenContentAbove: true } : {}), ...(node.hiddenContentBelow ? { hiddenContentBelow: true } : {}), + ...(systemChrome ? { systemChrome: true } : {}), }); return currentIndex; } From 972ffac00d0cb19970f53a1f4cc4c2678ce48c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 09:58:56 +0200 Subject: [PATCH 4/9] fix(android): keep systemChrome provenance out of published nodes; simplify androidUiNodes Review blockers on 8615771c6: 1. `systemChrome` reached CLI/SDK/MCP payloads: `serializeSnapshotResult` passed `result.nodes` straight through, so an internal classification became a de-facto public contract and per-node bytes. Added `publicSnapshotNodes` next to the field it strips, mirroring the existing `publicSnapshotCaptureAnnotations` projection, and applied it at that boundary (the single serializer both the CLI and capture output route through). The regression test asserts the published nodes deep-equal their input minus the field AND that the serialized JSON never contains the string; it fails if the projection is removed. 2. Fallow complexity on `androidUiNodes` (10 cyclomatic / 19 cognitive): extracted `createAndroidChromeSubtreeTracker`, which owns the depth and chrome-subtree state, leaving the generator a token loop. Not baselined. Note the tracker keeps open/close as distinct operations on purpose: a self-closing tag leaves at the depth it entered, a container descends and is left by its ``, so collapsing them silently breaks nesting. --- src/contracts/result-serialization.test.ts | 21 +++++++++ src/contracts/result-serialization.ts | 3 +- src/kernel/snapshot.ts | 21 ++++++++- src/platforms/android/ui-hierarchy.ts | 50 ++++++++++++++++------ 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/src/contracts/result-serialization.test.ts b/src/contracts/result-serialization.test.ts index 6044ecff1..e1a21ffaa 100644 --- a/src/contracts/result-serialization.test.ts +++ b/src/contracts/result-serialization.test.ts @@ -192,3 +192,24 @@ test('serializeSnapshotResult includes snapshot diagnostics', () => { snapshotDiagnostics, }); }); + +test('serializeSnapshotResult omits internal systemChrome provenance from published nodes', () => { + // `systemChrome` is Android status/nav-bar provenance stamped during the walk + // for the chrome classifiers. It must never reach CLI --json, SDK, or MCP + // payloads: it is a derived internal classification, and publishing it would + // both grow every node and imply a contract we do not want to keep. + const data = serializeSnapshotResult({ + nodes: [ + { index: 0, ref: 'e1', type: 'android.widget.TextView', label: '7:03', systemChrome: true }, + { index: 1, ref: 'e2', type: 'android.widget.Button', label: 'Start' }, + ], + truncated: false, + } as unknown as Parameters[0]); + + assert.deepEqual(data.nodes, [ + { index: 0, ref: 'e1', type: 'android.widget.TextView', label: '7:03' }, + { index: 1, ref: 'e2', type: 'android.widget.Button', label: 'Start' }, + ]); + const serialized = JSON.stringify(data); + assert.equal(serialized.includes('systemChrome'), false, serialized); +}); diff --git a/src/contracts/result-serialization.ts b/src/contracts/result-serialization.ts index df38a8943..574d27a70 100644 --- a/src/contracts/result-serialization.ts +++ b/src/contracts/result-serialization.ts @@ -15,6 +15,7 @@ import { type SnapshotCaptureAnnotations, } from '../snapshot-capture-annotations.ts'; import type { PublicPlatform } from '../kernel/device.ts'; +import { publicSnapshotNodes } from '../kernel/snapshot.ts'; import { successText, withSuccessText } from '../utils/success-text.ts'; export function buildAppIdentifiers(params: { @@ -174,7 +175,7 @@ export function serializeCloseResult( export function serializeSnapshotResult(result: CaptureSnapshotResult): Record { return { - nodes: result.nodes, + nodes: Array.isArray(result.nodes) ? publicSnapshotNodes(result.nodes) : result.nodes, truncated: result.truncated, ...(result.appName ? { appName: result.appName } : {}), ...(result.appBundleId ? { appBundleId: result.appBundleId } : {}), diff --git a/src/kernel/snapshot.ts b/src/kernel/snapshot.ts index eca497729..1f03ba47c 100644 --- a/src/kernel/snapshot.ts +++ b/src/kernel/snapshot.ts @@ -66,10 +66,29 @@ export type RawSnapshotNode = { hiddenContentBelow?: boolean; interactionBlocked?: 'covered'; presentationHints?: string[]; - /** Android: node is inside a status-bar/navigation-bar window (see `systemChrome` in ui-hierarchy.ts). */ + /** + * INTERNAL. Android status-bar/navigation-bar provenance, stamped by + * `walkUiHierarchyNode` for the chrome classifiers. Never part of the public + * CLI/MCP node contract — `publicSnapshotNodes` strips it at serialization. + */ systemChrome?: boolean; }; +/** + * Projects captured nodes onto the published contract by dropping internal-only + * fields. Mirrors `publicSnapshotCaptureAnnotations`, which does the same for + * capture annotations: internal derivations stay out of CLI/SDK/MCP payloads + * rather than every consumer having to know which properties are private. + */ +export function publicSnapshotNodes(nodes: T[]): T[] { + if (!nodes.some((node) => node.systemChrome !== undefined)) return nodes; + return nodes.map((node) => { + if (node.systemChrome === undefined) return node; + const { systemChrome: _systemChrome, ...published } = node; + return published as T; + }); +} + export type HiddenContentHint = { hiddenContentAbove?: true; hiddenContentBelow?: true; diff --git a/src/platforms/android/ui-hierarchy.ts b/src/platforms/android/ui-hierarchy.ts index 4bddb37a2..abbfc1fff 100644 --- a/src/platforms/android/ui-hierarchy.ts +++ b/src/platforms/android/ui-hierarchy.ts @@ -38,29 +38,51 @@ export type AndroidUiNodeMetadata = { }; /** - * Streams `` metadata in document order, carrying status-bar/nav-bar - * provenance on each node. The container id is the only chrome signal in the - * tree — its clock/battery/wifi leaves carry no marker of their own — so the - * subtree is tracked here rather than re-derived from a list of leaf ids. + * Depth-first membership of the status-bar/nav-bar subtree over a `` token + * stream. The container id is the only chrome signal in the tree — its + * clock/battery/wifi leaves carry no marker of their own — so membership is + * tracked while descending rather than re-derived from a list of leaf ids. */ -export function* androidUiNodes(xml: string): IterableIterator { - const tokenRegex = /]*>|<\/node>/g; +function createAndroidChromeSubtreeTracker() { let depth = 0; let chromeDepth: number | undefined; + const leave = (): void => { + if (chromeDepth !== undefined && depth <= chromeDepth) chromeDepth = undefined; + }; + return { + /** Enters an opening tag; returns whether that node is inside the chrome subtree. */ + open(resourceId: string | null | undefined, selfClosing: boolean): boolean { + if (chromeDepth === undefined && isAndroidSystemChromeWindowResourceId(resourceId)) { + chromeDepth = depth; + } + const inChrome = chromeDepth !== undefined; + // A self-closing tag never becomes an ancestor, so it leaves at the depth + // it entered; a container tag descends and is left by its ``. + if (selfClosing) leave(); + else depth += 1; + return inChrome; + }, + /** Handles ``, ending the chrome subtree once its container closes. */ + close(): void { + depth -= 1; + leave(); + }, + }; +} + +/** Streams `` metadata in document order, carrying status-bar/nav-bar provenance. */ +export function* androidUiNodes(xml: string): IterableIterator { + const tokenRegex = /]*>|<\/node>/g; + const chrome = createAndroidChromeSubtreeTracker(); let match = tokenRegex.exec(xml); while (match) { const token = match[0]; if (token.startsWith('')) depth += 1; - else if (chromeDepth === depth) chromeDepth = undefined; + const inChrome = chrome.open(metadata.resourceId, token.endsWith('/>')); + yield inChrome ? { ...metadata, systemChrome: true } : metadata; } match = tokenRegex.exec(xml); } From 897f9e46c5d214e4ed09f0c5d1a9ad4315fa88fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 13:45:52 +0200 Subject: [PATCH 5/9] refactor(android): let the chrome classifier read the tree, deleting the test-side stamping helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review: the remaining long comments read as workarounds. Three were, and each pointed at code worth changing rather than explaining. 1. `collectAndroidSystemChromeIndexes` required the walk-stamped `systemChrome` even when the tree still held the container that defines it. Hand-built trees (every settle unit test) therefore classified as "no chrome at all", which is why they needed `withAndroidSystemChrome` — a test helper re-implementing the production descent, with a comment explaining why it had to exist. The classifier now reads the container from whichever source still has it: the parent chain when the capture kept it, the stamped flag when the walk dropped it. The helper, its type and its comment are deleted; no test re-states production logic, and any caller holding an intact tree is now classified correctly regardless of how it was produced. 2. `createAndroidChromeSubtreeTracker` tracked depth with arithmetic subtle enough to need a comment about self-closing tags — and subtle enough that an earlier revision of it was wrong. Replaced with a stack of open elements: push on descent, pop on ``, nothing to explain. 3. `walkInteractiveOnlyAndroidFixture`'s 16-line comment still described the run-classification this branch deleted — stale, not merely long. Cut to what the helper does. Also trimmed the contract, kernel, classifier and serialization-test comments to the fact each states. Comment lines in the branch drop from 116 to 51. No behavior change beyond (1), which strictly widens correct classification. 4318/4318 unit tests, lint, typecheck, build and smoke pass. --- .../android-ui-hierarchy-fixtures.ts | 17 +- .../interaction/runtime/settle.test.ts | 195 ++++++++---------- src/contracts/android-system-chrome.ts | 21 +- src/contracts/result-serialization.test.ts | 5 +- src/core/snapshot-chrome.ts | 33 ++- src/kernel/snapshot.ts | 12 +- src/platforms/android/ui-hierarchy.ts | 29 +-- 7 files changed, 133 insertions(+), 179 deletions(-) diff --git a/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts b/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts index 3819214ea..0adb03ae0 100644 --- a/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts +++ b/src/__tests__/test-utils/android-ui-hierarchy-fixtures.ts @@ -73,20 +73,9 @@ export function walkNonRawAndroidFixture(rawNodes: RawSnapshotNode[]): RawSnapsh } /** - * Runs a `RawSnapshotNode[]` fixture through the REAL interactive-only Android - * walk (`buildUiHierarchySnapshot(..., { interactiveOnly: true })`), the shape - * `--settle` and `wait stable` actually consume: their loop captures with - * `interactiveOnly: true` (`stable-capture.ts`), so the tree reaching - * `withoutSettleChrome` is this one, NOT `walkNonRawAndroidFixture`'s. - * - * The distinction is load-bearing for systemui run classification, not a - * detail: `shouldIncludeInteractiveAndroidNode` additionally drops the - * structural window spine (`legacy_window_root`, `notification_panel`, - * `qs_frame`, the quick-settings ComposeView chain), and those are precisely - * the nodes that splice a systemui surface into ONE contiguous run. Verified - * live (2026-07-21, emulator-5554 Pixel 9 Pro XL API 37): the same expanded - * shade is one 95-node run under the non-raw walk and five runs under this - * one. + * Runs a fixture through the REAL interactive-only walk — the shape `--settle` + * and `wait stable` consume, since their loop captures with + * `interactiveOnly: true` (`stable-capture.ts`). */ export function walkInteractiveOnlyAndroidFixture(rawNodes: RawSnapshotNode[]): RawSnapshotNode[] { const tree = rawFixtureToAndroidTree(rawNodes); diff --git a/src/commands/interaction/runtime/settle.test.ts b/src/commands/interaction/runtime/settle.test.ts index 1acb27fa1..7ff2978d8 100644 --- a/src/commands/interaction/runtime/settle.test.ts +++ b/src/commands/interaction/runtime/settle.test.ts @@ -11,7 +11,6 @@ import { import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts'; import { ref, selector } from './selector-read.ts'; import { buildSettleTailEntries, NEVER_SETTLED_HINT } from './settle.ts'; -import { isAndroidSystemChromeWindowResourceId } from '../../../contracts/android-system-chrome.ts'; // #1101 --settle: quiet-window settle loop composition on the interaction // commands. Budgets are injected (fake clock) — no real waiting. @@ -697,29 +696,11 @@ const ANDROID_APP_BUNDLE_ID = 'org.reactnavigation.playground'; const ANDROID_SYSTEM_UI_BUNDLE_ID = 'com.android.systemui'; const ANDROID_IME_BUNDLE_ID = 'com.google.android.inputmethod.latin'; -/** - * Stamps `systemChrome` the way the real Android walk does (`walkUiHierarchyNode`), - * for tests that build `SnapshotNode`s directly instead of walking a capture. The - * container predicate is the production one, so only the descent is re-stated here. - */ -function withAndroidSystemChrome(nodes: T[]): T[] { - const byIndex = new Map(nodes.map((node) => [node.index, node])); - const inChrome = (node: T | undefined): boolean => { - if (!node) return false; - if (isAndroidSystemChromeWindowResourceId(node.identifier)) return true; - return node.parentIndex !== undefined && inChrome(byIndex.get(node.parentIndex)); - }; - return nodes.map((node) => (inChrome(node) ? { ...node, systemChrome: true } : node)); -} - -type AndroidChromeStampable = { index: number; parentIndex?: number; identifier?: string }; - function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') { const root = startIndex; - return withAndroidSystemChrome([ + return [ { - // Outermost status-bar node in the real capture: the walk drops the one - // truly anonymous wrapper above it, so this is what a capture starts with. + // The outermost status-bar node a real capture starts with. index: root, depth: 0, type: 'android.widget.FrameLayout', @@ -765,7 +746,7 @@ function androidStatusBarNodes(startIndex: number, clockLabel = '12:23') { bundleId: ANDROID_SYSTEM_UI_BUNDLE_ID, rect: { x: 1200, y: 40, width: 60, height: 40 }, }, - ]); + ]; } // Real systemui VolumeDialog window captured live alongside the status bar @@ -2123,52 +2104,50 @@ test('buildSettleTailEntries drops the keyboard container and its chrome descend }); test('buildSettleTailEntries drops Android IME chrome and status-bar chrome (#1198)', () => { - const settledNodes = makeSnapshotState( - withAndroidSystemChrome([ - { - index: 0, - depth: 0, - type: 'android.widget.Button', - label: 'Send', - bundleId: 'org.reactnavigation.playground', - rect: { x: 10, y: 20, width: 100, height: 40 }, - hittable: true, - }, - { - // Status-bar marker: this systemui run drops whole. - index: 1, - depth: 0, - type: 'android.widget.FrameLayout', - identifier: 'com.android.systemui:id/status_bar', - bundleId: 'com.android.systemui', - }, - { - index: 2, - depth: 1, - parentIndex: 1, - type: 'android.widget.TextView', - identifier: 'com.android.systemui:id/clock', - label: '12:23', - bundleId: 'com.android.systemui', - }, - { - index: 3, - depth: 0, - type: 'android.widget.FrameLayout', - bundleId: 'com.google.android.inputmethod.latin', - hittable: true, - }, - { - index: 4, - depth: 1, - parentIndex: 3, - type: 'android.widget.FrameLayout', - label: 'Delete', - bundleId: 'com.google.android.inputmethod.latin', - hittable: true, - }, - ]), - ).nodes; + const settledNodes = makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'android.widget.Button', + label: 'Send', + bundleId: 'org.reactnavigation.playground', + rect: { x: 10, y: 20, width: 100, height: 40 }, + hittable: true, + }, + { + // Status-bar marker: this systemui run drops whole. + index: 1, + depth: 0, + type: 'android.widget.FrameLayout', + identifier: 'com.android.systemui:id/status_bar', + bundleId: 'com.android.systemui', + }, + { + index: 2, + depth: 1, + parentIndex: 1, + type: 'android.widget.TextView', + identifier: 'com.android.systemui:id/clock', + label: '12:23', + bundleId: 'com.android.systemui', + }, + { + index: 3, + depth: 0, + type: 'android.widget.FrameLayout', + bundleId: 'com.google.android.inputmethod.latin', + hittable: true, + }, + { + index: 4, + depth: 1, + parentIndex: 3, + type: 'android.widget.FrameLayout', + label: 'Delete', + bundleId: 'com.google.android.inputmethod.latin', + hittable: true, + }, + ]).nodes; const result = buildSettleTailEntries(settledNodes, new Set(), 'org.reactnavigation.playground'); @@ -2179,47 +2158,45 @@ test('buildSettleTailEntries keeps unknown-foreign packages and drops only marke // Keep-unknown-foreign default: a system dialog's buttons (package // `android`) stay tail candidates; only the marked status/nav-bar // window-run drops, and that does not depend on knowing the session's app. - const settledNodes = makeSnapshotState( - withAndroidSystemChrome([ - { - index: 0, - depth: 0, - type: 'android.widget.Button', - label: 'Send', - bundleId: 'org.reactnavigation.playground', - rect: { x: 10, y: 20, width: 100, height: 40 }, - hittable: true, - }, - { - index: 1, - depth: 0, - type: 'android.widget.FrameLayout', - identifier: 'com.android.systemui:id/status_bar_container', - bundleId: 'com.android.systemui', - rect: { x: 0, y: 0, width: 1344, height: 159 }, - }, - { - index: 2, - depth: 1, - parentIndex: 1, - type: 'android.widget.TextView', - identifier: 'com.android.systemui:id/clock', - label: '12:23', - bundleId: 'com.android.systemui', - rect: { x: 40, y: 40, width: 80, height: 40 }, - }, - { - index: 3, - depth: 0, - type: 'android.widget.Button', - label: 'Just once', - identifier: 'android:id/button_once', - bundleId: 'android', - rect: { x: 829, y: 2734, width: 255, height: 162 }, - hittable: true, - }, - ]), - ).nodes; + const settledNodes = makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'android.widget.Button', + label: 'Send', + bundleId: 'org.reactnavigation.playground', + rect: { x: 10, y: 20, width: 100, height: 40 }, + hittable: true, + }, + { + index: 1, + depth: 0, + type: 'android.widget.FrameLayout', + identifier: 'com.android.systemui:id/status_bar_container', + bundleId: 'com.android.systemui', + rect: { x: 0, y: 0, width: 1344, height: 159 }, + }, + { + index: 2, + depth: 1, + parentIndex: 1, + type: 'android.widget.TextView', + identifier: 'com.android.systemui:id/clock', + label: '12:23', + bundleId: 'com.android.systemui', + rect: { x: 40, y: 40, width: 80, height: 40 }, + }, + { + index: 3, + depth: 0, + type: 'android.widget.Button', + label: 'Just once', + identifier: 'android:id/button_once', + bundleId: 'android', + rect: { x: 829, y: 2734, width: 255, height: 162 }, + hittable: true, + }, + ]).nodes; for (const appBundleId of [undefined, 'org.reactnavigation.playground']) { const result = buildSettleTailEntries(settledNodes, new Set(), appBundleId); diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index 87508a4d6..3ad400103 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -1,22 +1,15 @@ /** - * Android status-bar/navigation-bar chrome identity, shared between the settle-chrome - * classifier (`core/snapshot-chrome.ts`, #1198) and the helper content classifier - * (`platforms/android/snapshot-content-recovery.ts`). SystemUI hosts BOTH persistent chrome - * and actionable overlays (volume panel, media/output pickers, notification shade, quick - * settings), so chrome is never a package-level fact: only the status/nav-bar window - * subtree is chrome; every other systemui surface is real content. + * Android status-bar/navigation-bar chrome identity, shared by the settle-chrome + * classifier (`core/snapshot-chrome.ts`) and the helper content classifier + * (`platforms/android/snapshot-content-recovery.ts`). SystemUI also hosts actionable + * overlays — volume panel, media pickers, the shade itself — so chrome is never a + * package-level fact: only the status/nav-bar container's subtree is chrome. */ export const ANDROID_SYSTEM_CHROME_PACKAGE = 'com.android.systemui'; /** - * Resource-ids of the status-bar / navigation-bar WINDOW containers. `status_bar` and - * `navigation_bar` are matched as an id segment rather than a prefix so the shade's own - * `split_shade_status_bar` container counts too — live-verified on Pixel 9 Pro XL API 37, - * where an expanded shade hosts the status icons under exactly that id. - * - * Membership is decided ONCE, during the walk (`walkUiHierarchyNode`), which stamps - * `systemChrome` on every descendant while the container is still in the tree. Consumers - * read that flag; nothing downstream re-derives chrome identity from ids. + * Resource-ids of the status-bar / navigation-bar WINDOW containers. Matched as an id + * segment, not a prefix, so an expanded shade's `split_shade_status_bar` counts. */ export function isAndroidSystemChromeWindowResourceId( resourceId: string | null | undefined, diff --git a/src/contracts/result-serialization.test.ts b/src/contracts/result-serialization.test.ts index e1a21ffaa..889860c91 100644 --- a/src/contracts/result-serialization.test.ts +++ b/src/contracts/result-serialization.test.ts @@ -194,10 +194,7 @@ test('serializeSnapshotResult includes snapshot diagnostics', () => { }); test('serializeSnapshotResult omits internal systemChrome provenance from published nodes', () => { - // `systemChrome` is Android status/nav-bar provenance stamped during the walk - // for the chrome classifiers. It must never reach CLI --json, SDK, or MCP - // payloads: it is a derived internal classification, and publishing it would - // both grow every node and imply a contract we do not want to keep. + // Internal classifier provenance must not reach CLI --json, SDK or MCP payloads. const data = serializeSnapshotResult({ nodes: [ { index: 0, ref: 'e1', type: 'android.widget.TextView', label: '7:03', systemChrome: true }, diff --git a/src/core/snapshot-chrome.ts b/src/core/snapshot-chrome.ts index 8047cd6f2..ada0e33be 100644 --- a/src/core/snapshot-chrome.ts +++ b/src/core/snapshot-chrome.ts @@ -1,4 +1,7 @@ -import { ANDROID_SYSTEM_CHROME_PACKAGE } from '../contracts/android-system-chrome.ts'; +import { + ANDROID_SYSTEM_CHROME_PACKAGE, + isAndroidSystemChromeWindowResourceId, +} from '../contracts/android-system-chrome.ts'; import type { SnapshotNode } from '../kernel/snapshot.ts'; import { isAndroidInputMethodSnapshotNode } from '../snapshot/android-input-method-overlays.ts'; import { normalizeType } from '../utils/text-surface.ts'; @@ -199,12 +202,23 @@ function collectSubtreeIndexes( return indexes; } -// SystemUI hosts BOTH persistent chrome and actionable overlays (volume -// panel, media/output pickers), so chrome is never a package-level fact. -// `systemChrome` is stamped per node during the Android walk, from the -// status-bar/nav-bar container it descends from — see `walkUiHierarchyNode`. -function isAndroidSystemChromeNode(node: SnapshotNode): boolean { - return node.bundleId === ANDROID_SYSTEM_CHROME_PACKAGE && node.systemChrome === true; +// A node is chrome when it sits in the status-bar/nav-bar container's subtree, +// read from whichever source still has the container: the tree when it survived +// the walk, else the `systemChrome` the walk stamped before dropping it. +function isAndroidSystemChromeNode( + node: SnapshotNode, + byIndex: Map, +): boolean { + if (node.bundleId !== ANDROID_SYSTEM_CHROME_PACKAGE) return false; + if (node.systemChrome === true) return true; + const seen = new Set(); + let current: SnapshotNode | undefined = node; + while (current?.bundleId === ANDROID_SYSTEM_CHROME_PACKAGE && !seen.has(current.index)) { + if (isAndroidSystemChromeWindowResourceId(current.identifier)) return true; + seen.add(current.index); + current = current.parentIndex === undefined ? undefined : byIndex.get(current.parentIndex); + } + return false; } /** @@ -240,7 +254,7 @@ function collectAndroidSettleChrome( const systemChromeIndexes = appBundleId === ANDROID_SYSTEM_CHROME_PACKAGE ? new Set() - : collectAndroidSystemChromeIndexes(nodes, imeIndexes); + : collectAndroidSystemChromeIndexes(nodes, byIndex, imeIndexes); // The one surviving container line per IME run; the rest of the run and all // status/nav-bar chrome never spend diff/tail budget. const strippedIndexes = new Set( @@ -266,11 +280,12 @@ function collectAndroidSettleChrome( */ function collectAndroidSystemChromeIndexes( nodes: SnapshotNode[], + byIndex: Map, imeIndexes: ReadonlySet, ): Set { return new Set( nodes - .filter((node) => !imeIndexes.has(node.index) && isAndroidSystemChromeNode(node)) + .filter((node) => !imeIndexes.has(node.index) && isAndroidSystemChromeNode(node, byIndex)) .map((node) => node.index), ); } diff --git a/src/kernel/snapshot.ts b/src/kernel/snapshot.ts index 1f03ba47c..cacc7199f 100644 --- a/src/kernel/snapshot.ts +++ b/src/kernel/snapshot.ts @@ -66,19 +66,13 @@ export type RawSnapshotNode = { hiddenContentBelow?: boolean; interactionBlocked?: 'covered'; presentationHints?: string[]; - /** - * INTERNAL. Android status-bar/navigation-bar provenance, stamped by - * `walkUiHierarchyNode` for the chrome classifiers. Never part of the public - * CLI/MCP node contract — `publicSnapshotNodes` strips it at serialization. - */ + /** INTERNAL, stripped by `publicSnapshotNodes`: status/nav-bar provenance for the chrome classifiers. */ systemChrome?: boolean; }; /** - * Projects captured nodes onto the published contract by dropping internal-only - * fields. Mirrors `publicSnapshotCaptureAnnotations`, which does the same for - * capture annotations: internal derivations stay out of CLI/SDK/MCP payloads - * rather than every consumer having to know which properties are private. + * Projects captured nodes onto the published contract by dropping internal-only fields, + * mirroring `publicSnapshotCaptureAnnotations` for capture annotations. */ export function publicSnapshotNodes(nodes: T[]): T[] { if (!nodes.some((node) => node.systemChrome !== undefined)) return nodes; diff --git a/src/platforms/android/ui-hierarchy.ts b/src/platforms/android/ui-hierarchy.ts index abbfc1fff..a64d6d8e3 100644 --- a/src/platforms/android/ui-hierarchy.ts +++ b/src/platforms/android/ui-hierarchy.ts @@ -38,34 +38,23 @@ export type AndroidUiNodeMetadata = { }; /** - * Depth-first membership of the status-bar/nav-bar subtree over a `` token - * stream. The container id is the only chrome signal in the tree — its - * clock/battery/wifi leaves carry no marker of their own — so membership is - * tracked while descending rather than re-derived from a list of leaf ids. + * Membership of the status-bar/nav-bar subtree over a `` token stream: the + * container id is the only chrome signal in the tree, its clock/battery/wifi + * leaves carrying no marker of their own. */ function createAndroidChromeSubtreeTracker() { - let depth = 0; - let chromeDepth: number | undefined; - const leave = (): void => { - if (chromeDepth !== undefined && depth <= chromeDepth) chromeDepth = undefined; - }; + const openElements: boolean[] = []; + const inChromeNow = (): boolean => openElements[openElements.length - 1] === true; return { /** Enters an opening tag; returns whether that node is inside the chrome subtree. */ open(resourceId: string | null | undefined, selfClosing: boolean): boolean { - if (chromeDepth === undefined && isAndroidSystemChromeWindowResourceId(resourceId)) { - chromeDepth = depth; - } - const inChrome = chromeDepth !== undefined; - // A self-closing tag never becomes an ancestor, so it leaves at the depth - // it entered; a container tag descends and is left by its ``. - if (selfClosing) leave(); - else depth += 1; + const inChrome = inChromeNow() || isAndroidSystemChromeWindowResourceId(resourceId); + if (!selfClosing) openElements.push(inChrome); return inChrome; }, - /** Handles ``, ending the chrome subtree once its container closes. */ + /** Handles ``. */ close(): void { - depth -= 1; - leave(); + openElements.pop(); }, }; } From 4027538b50349b5ac350ac2728b8d65015e2eee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 14:38:35 +0200 Subject: [PATCH 6/9] fix(android): keep chrome provenance internal --- src/__tests__/client.test.ts | 27 +++++++++++++ src/client/client-normalizers.ts | 3 +- src/contracts/android-system-chrome.ts | 26 ++++++++++++- src/contracts/result-serialization.test.ts | 18 --------- src/contracts/result-serialization.ts | 3 +- .../snapshot-chrome-android-statusbar.test.ts | 19 ++++++++++ src/core/snapshot-chrome.ts | 3 +- .../session-replay-divergence.test.ts | 38 +++---------------- .../__tests__/snapshot-handler.test.ts | 34 +++++++++++++++++ .../handlers/session-replay-divergence.ts | 12 +----- src/daemon/snapshot-runtime.ts | 4 +- src/kernel/snapshot.ts | 15 -------- .../__tests__/command-tools-parity.test.ts | 29 ++++++++++++++ src/platforms/android/ui-hierarchy.ts | 11 ++++-- 14 files changed, 157 insertions(+), 85 deletions(-) diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index ce0395bb5..32f95148d 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -938,6 +938,33 @@ test('client capture.snapshot preserves visibility metadata from daemon response }); }); +test('client capture.snapshot drops Android-internal chrome provenance', async () => { + const setup = createTransport(async () => ({ + ok: true, + data: { + nodes: [ + { + ref: 'e1', + index: 0, + depth: 0, + type: 'android.widget.TextView', + label: '7:03', + systemChrome: true, + }, + ], + truncated: false, + }, + })); + const client = createAgentDeviceClient(setup.config, { transport: setup.transport }); + + const result = await client.capture.snapshot(); + + assert.deepEqual(result.nodes, [ + { ref: 'e1', index: 0, depth: 0, type: 'android.widget.TextView', label: '7:03' }, + ]); + assert.equal(JSON.stringify(result).includes('systemChrome'), false); +}); + test('client capture.snapshot preserves refsGeneration from daemon responses (ADR 0014)', async () => { const setup = createTransport(async () => ({ ok: true, diff --git a/src/client/client-normalizers.ts b/src/client/client-normalizers.ts index e63723656..35b57d1c0 100644 --- a/src/client/client-normalizers.ts +++ b/src/client/client-normalizers.ts @@ -2,6 +2,7 @@ import type { DaemonRequest, SessionRuntimeHints } from '../daemon/types.ts'; import { AppError, type NormalizedError } from '../kernel/errors.ts'; import type { SnapshotNode } from '../kernel/snapshot.ts'; import { buildAppIdentifiers, buildDeviceIdentifiers } from '../contracts/result-serialization.ts'; +import { publicAndroidSnapshotNodes } from '../contracts/android-system-chrome.ts'; import { isAppleOs, isApplePlatform, isPublicPlatform, type AppleOS } from '../kernel/device.ts'; import { leaseScopeFromOptions, leaseScopeToRequestMeta } from '../core/lease-scope.ts'; import type { @@ -266,7 +267,7 @@ function normalizeTargetShutdownError(value: unknown): NormalizedError | undefin export function readSnapshotNodes(value: unknown): SnapshotNode[] { // Snapshot nodes are produced by the daemon snapshot pipeline and treated as trusted here. - return Array.isArray(value) ? (value as SnapshotNode[]) : []; + return Array.isArray(value) ? publicAndroidSnapshotNodes(value as SnapshotNode[]) : []; } export function buildMeta(options: InternalRequestOptions): DaemonRequest['meta'] { diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index 3ad400103..d74ca7b21 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -1,3 +1,5 @@ +import type { RawSnapshotNode, SnapshotNode } from '../kernel/snapshot.ts'; + /** * Android status-bar/navigation-bar chrome identity, shared by the settle-chrome * classifier (`core/snapshot-chrome.ts`) and the helper content classifier @@ -17,5 +19,27 @@ export function isAndroidSystemChromeWindowResourceId( const identifier = resourceId ?? ''; if (!identifier.startsWith(`${ANDROID_SYSTEM_CHROME_PACKAGE}:id/`)) return false; const leaf = identifier.slice(`${ANDROID_SYSTEM_CHROME_PACKAGE}:id/`.length); - return /(^|_)(status_bar|navigation_bar)/.test(leaf); + return /(^|_)(status_bar|navigation_bar)(_|$)/.test(leaf); +} + +/** Android-internal provenance retained in daemon session snapshots only. */ +export type AndroidSystemChromeProvenance = { + systemChrome?: true; +}; + +export type AndroidSystemChromeRawSnapshotNode = RawSnapshotNode & AndroidSystemChromeProvenance; + +export function hasAndroidSystemChromeProvenance(node: RawSnapshotNode): boolean { + return (node as AndroidSystemChromeRawSnapshotNode).systemChrome === true; +} + +/** Drops Android-internal provenance before snapshot nodes cross a public boundary. */ +export function publicAndroidSnapshotNodes(nodes: SnapshotNode[]): SnapshotNode[] { + if (!nodes.some((node) => Object.hasOwn(node, 'systemChrome'))) return nodes; + return nodes.map((node) => { + if (!Object.hasOwn(node, 'systemChrome')) return node; + const { systemChrome: _systemChrome, ...published } = node as SnapshotNode & + AndroidSystemChromeProvenance; + return published; + }); } diff --git a/src/contracts/result-serialization.test.ts b/src/contracts/result-serialization.test.ts index 889860c91..6044ecff1 100644 --- a/src/contracts/result-serialization.test.ts +++ b/src/contracts/result-serialization.test.ts @@ -192,21 +192,3 @@ test('serializeSnapshotResult includes snapshot diagnostics', () => { snapshotDiagnostics, }); }); - -test('serializeSnapshotResult omits internal systemChrome provenance from published nodes', () => { - // Internal classifier provenance must not reach CLI --json, SDK or MCP payloads. - const data = serializeSnapshotResult({ - nodes: [ - { index: 0, ref: 'e1', type: 'android.widget.TextView', label: '7:03', systemChrome: true }, - { index: 1, ref: 'e2', type: 'android.widget.Button', label: 'Start' }, - ], - truncated: false, - } as unknown as Parameters[0]); - - assert.deepEqual(data.nodes, [ - { index: 0, ref: 'e1', type: 'android.widget.TextView', label: '7:03' }, - { index: 1, ref: 'e2', type: 'android.widget.Button', label: 'Start' }, - ]); - const serialized = JSON.stringify(data); - assert.equal(serialized.includes('systemChrome'), false, serialized); -}); diff --git a/src/contracts/result-serialization.ts b/src/contracts/result-serialization.ts index 574d27a70..df38a8943 100644 --- a/src/contracts/result-serialization.ts +++ b/src/contracts/result-serialization.ts @@ -15,7 +15,6 @@ import { type SnapshotCaptureAnnotations, } from '../snapshot-capture-annotations.ts'; import type { PublicPlatform } from '../kernel/device.ts'; -import { publicSnapshotNodes } from '../kernel/snapshot.ts'; import { successText, withSuccessText } from '../utils/success-text.ts'; export function buildAppIdentifiers(params: { @@ -175,7 +174,7 @@ export function serializeCloseResult( export function serializeSnapshotResult(result: CaptureSnapshotResult): Record { return { - nodes: Array.isArray(result.nodes) ? publicSnapshotNodes(result.nodes) : result.nodes, + nodes: result.nodes, truncated: result.truncated, ...(result.appName ? { appName: result.appName } : {}), ...(result.appBundleId ? { appBundleId: result.appBundleId } : {}), diff --git a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts index e3b8d53d5..0aa127e96 100644 --- a/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts +++ b/src/core/__tests__/snapshot-chrome-android-statusbar.test.ts @@ -8,6 +8,7 @@ import { walkInteractiveOnlyAndroidFixture, walkNonRawAndroidFixture, } from '../../__tests__/test-utils/android-ui-hierarchy-fixtures.ts'; +import { isAndroidSystemChromeWindowResourceId } from '../../contracts/android-system-chrome.ts'; /** * The `walk*AndroidFixture` helpers run a real `--raw` device capture through @@ -29,6 +30,24 @@ const STATUS_BAR_LEAF_IDENTIFIERS = [ 'com.android.systemui:id/wifi_signal', ]; +test('Android chrome container ids match complete status/nav-bar segments only', () => { + assert.equal( + isAndroidSystemChromeWindowResourceId( + 'com.android.systemui:id/status_bar_launch_animation_container', + ), + true, + ); + assert.equal( + isAndroidSystemChromeWindowResourceId('com.android.systemui:id/split_shade_status_bar'), + true, + ); + assert.equal( + isAndroidSystemChromeWindowResourceId('com.android.systemui:id/status_barometer'), + false, + ); + assert.equal(isAndroidSystemChromeWindowResourceId('com.example:id/status_bar'), false); +}); + test('Android non-raw capture: status-bar leaves stay chrome after the walk drops the container that identifies them (#1251)', () => { const walkedNodes = walkNonRawAndroidFixture(ANDROID_IME_CAPTURE_RAW_NODES); const nodes = attachRefs(walkedNodes); diff --git a/src/core/snapshot-chrome.ts b/src/core/snapshot-chrome.ts index ada0e33be..669b4314e 100644 --- a/src/core/snapshot-chrome.ts +++ b/src/core/snapshot-chrome.ts @@ -1,5 +1,6 @@ import { ANDROID_SYSTEM_CHROME_PACKAGE, + hasAndroidSystemChromeProvenance, isAndroidSystemChromeWindowResourceId, } from '../contracts/android-system-chrome.ts'; import type { SnapshotNode } from '../kernel/snapshot.ts'; @@ -210,7 +211,7 @@ function isAndroidSystemChromeNode( byIndex: Map, ): boolean { if (node.bundleId !== ANDROID_SYSTEM_CHROME_PACKAGE) return false; - if (node.systemChrome === true) return true; + if (hasAndroidSystemChromeProvenance(node)) return true; const seen = new Set(); let current: SnapshotNode | undefined = node; while (current?.bundleId === ANDROID_SYSTEM_CHROME_PACKAGE && !seen.has(current.index)) { diff --git a/src/daemon/handlers/__tests__/session-replay-divergence.test.ts b/src/daemon/handlers/__tests__/session-replay-divergence.test.ts index 3156c155a..27c4f4a74 100644 --- a/src/daemon/handlers/__tests__/session-replay-divergence.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-divergence.test.ts @@ -1060,16 +1060,10 @@ test('buildReplayFailureDivergence: divergence capture drops the action snapshot expect(context?.snapshotInteractiveOnly).toBe(true); }); -// Wave-3 leg E (real walked fixture): a FULL-COVER quick-settings shade. Unlike -// the partial-cover overlay above, the shade owns the whole screen — every node -// is systemui and the status-bar icons share the shade's own window, so the -// run-level chrome rule (`collectAndroidSystemChromeRunIndexes`) condemns the -// ENTIRE capture, tiles included. The chrome filter must stay a FILTER and never -// become a narrower scoping (`captureDivergenceObservation`): a plain `snapshot` -// of this exact surface shows the tiles (#1301 `systemSurfaceOnly` carve-out), so -// a divergence publishing zero refs would be strictly NARROWER than `snapshot` — -// the invariant that amendment forbids. Live-verified 2026-07-17 on emulator-5556: -// pre-fix this screen came back with 0 refs. +// Real walked FULL-COVER quick-settings shade: every node is systemui and the +// status-bar icons share the shade's window. The provenance classifier must +// exclude those icons without excluding the actionable tiles; reverting to the +// old run-level classifier makes this screen empty. test('buildReplayFailureDivergence: a full-cover quick-settings shade still publishes its hittable tiles (wave-3 leg E)', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-divergence-qsshade-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); @@ -1120,9 +1114,8 @@ test('buildReplayFailureDivergence: a full-cover quick-settings shade still publ expect(screen.refs.length).toBe(20); // SCREEN_REF_CAPTURE_LIMIT, from 23 hittable tiles expect(screen.truncated).toBe(true); - // Still a filter, not a blanket chrome dump: EVERY published ref is a node an - // agent can actually act on, so the non-hittable status residue in the SAME - // condemned run (battery/wifi/mobile icons) never rides along. + // Every published ref is actionable; non-hittable status residue in the same + // systemui window (battery/wifi/mobile icons) never rides along. const published = new Map( (sessionStore.get(sessionName)?.snapshot?.nodes ?? []).map((node) => [node.ref, node]), ); @@ -1130,22 +1123,3 @@ test('buildReplayFailureDivergence: a full-cover quick-settings shade still publ expect(screen.refs.some((ref) => ref.label === 'Battery charging, 100 percent.')).toBe(false); expect(screen.refs.some((ref) => ref.label === 'Wifi signal full.')).toBe(false); }); - -// The hittable gate on the chrome fallback, pinned against the OTHER real -// capture: an ordinary COLLAPSED status bar (with app content present) must not -// be promoted. Its clock is labeled but NOT hittable in the real capture, while -// the expanded shade's big clock IS — which is exactly why hittability, not -// label presence, is the gate. Without this the fallback would publish a bare -// "7:03" as the screen whenever an app tree yielded no meaningful targets. -test('buildReplayFailureDivergence: the chrome fallback never promotes a non-hittable collapsed status bar', async () => { - const collapsedClock = ANDROID_IME_CAPTURE_RAW_NODES.find( - (node) => node.identifier === 'com.android.systemui:id/clock', - ); - expect(collapsedClock?.label).toBe('7:03'); - expect(collapsedClock?.hittable).not.toBe(true); - - const shadeClock = ANDROID_QS_SHADE_CAPTURE_RAW_NODES.find( - (node) => node.identifier === 'com.android.systemui:id/clock', - ); - expect(shadeClock?.hittable).toBe(true); -}); diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index f028cc1d9..6ec70b89f 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -14,6 +14,7 @@ import { buildInteractionSurfaceSignature } from '../../interaction-outcome-poli import { buildSnapshotPresentationKey } from '../../../kernel/snapshot.ts'; import { snapshotCliOutput } from '../../../commands/capture/output.ts'; import type { CaptureSnapshotResult } from '../../../client/client-types.ts'; +import type { AndroidSystemChromeRawSnapshotNode } from '../../../contracts/android-system-chrome.ts'; vi.mock('../../../core/dispatch.ts', async (importOriginal) => { const actual = await importOriginal(); @@ -402,6 +403,39 @@ test('snapshot on iOS runs when the session tracks an app', async () => { ); }); +test('snapshot keeps Android chrome provenance in session state but not the daemon response', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'android-internal-chrome'; + sessionStore.set(sessionName, makeSession(sessionName, androidDevice)); + const internalNode: AndroidSystemChromeRawSnapshotNode = { + index: 0, + depth: 0, + type: 'android.widget.TextView', + label: '7:03', + systemChrome: true, + }; + mockDispatch.mockResolvedValue({ + nodes: [internalNode], + truncated: false, + backend: 'android', + }); + + const response = await handleSnapshotCommands({ + req: { token: 't', session: sessionName, command: 'snapshot', positionals: [], flags: {} }, + sessionName, + logPath: '/tmp/daemon.log', + sessionStore, + }); + + expect(response?.ok).toBe(true); + if (!response?.ok) return; + expect(response.data?.nodes).toEqual([ + { index: 0, ref: 'e1', depth: 0, type: 'android.widget.TextView', label: '7:03' }, + ]); + expect(JSON.stringify(response.data)).not.toContain('systemChrome'); + expect(sessionStore.get(sessionName)?.snapshot?.nodes[0]).toHaveProperty('systemChrome', true); +}); + test('snapshot re-activates a complete frame; diff preserves it (ADR 0014)', async () => { const sessionStore = makeSessionStore(); const sessionName = 'android-stale-refs-marker'; diff --git a/src/daemon/handlers/session-replay-divergence.ts b/src/daemon/handlers/session-replay-divergence.ts index 06f541fbe..8542f1a30 100644 --- a/src/daemon/handlers/session-replay-divergence.ts +++ b/src/daemon/handlers/session-replay-divergence.ts @@ -381,14 +381,6 @@ function selectDivergenceScreenRefNodes( const nonChrome = nodes.filter( (node) => node.ref && !chromeRefs.has(node.ref) && isMeaningfulDivergenceTarget(node), ); - // Chrome fallback: an expanded shade is a single systemui run, so its own status - // icons condemn its tiles with it. Excluding chrome is a FILTER, never a narrower - // scoping (`captureDivergenceObservation`), so when the chrome runs ARE the - // screen, publish what an agent can still act on rather than a divergence - // narrower than a plain `snapshot`. Hittable, not labelled: a status clock is not - // a target, so a chrome-only screen still publishes nothing. - const meaningful = - nonChrome.length > 0 ? nonChrome : nodes.filter((node) => node.ref && node.hittable === true); // Occlusion fallback (#1264): a `covered` node is normally dropped — an agent // cannot tap what an overlay hides. But when a system overlay MASS-COVERS the // app, EVERY app node is annotated `covered`; dropping them all would emit an @@ -397,8 +389,8 @@ function selectDivergenceScreenRefNodes( // `covered` nodes are excluded only while non-covered candidates remain; if // the entire meaningful surface is covered, they are surfaced rather than // returning empty. - const visible = meaningful.filter((node) => node.interactionBlocked !== 'covered'); - const pool = visible.length > 0 ? visible : meaningful; + const visible = nonChrome.filter((node) => node.interactionBlocked !== 'covered'); + const pool = visible.length > 0 ? visible : nonChrome; // Rank within the cap instead of slicing document order (#1264 cap burial): // `SCREEN_REF_CAPTURE_LIMIT` is a BYTE bound, NOT a "first 20 in tree order" // policy. A separate-window overlay enumerates AFTER the app window's nodes, diff --git a/src/daemon/snapshot-runtime.ts b/src/daemon/snapshot-runtime.ts index 11d8027a3..2c10a75bc 100644 --- a/src/daemon/snapshot-runtime.ts +++ b/src/daemon/snapshot-runtime.ts @@ -21,6 +21,7 @@ import { maybeBuildAndroidSnapshotTimeoutFailure } from './android-snapshot-time import { summarizeSnapshotDiagnostics } from '../snapshot-diagnostics.ts'; import { nextSnapshotGeneration } from './session-snapshot.ts'; import { activateCompleteRefFrame } from './ref-frame.ts'; +import { publicAndroidSnapshotNodes } from '../contracts/android-system-chrome.ts'; export async function dispatchSnapshotViaRuntime(params: { req: DaemonRequest; @@ -47,8 +48,9 @@ export async function dispatchSnapshotViaRuntime(params: { // capture above already stored the next session via setRecord, so the // store holds the generation these refs were minted from. const refsGeneration = params.sessionStore.get(sessionName)?.snapshotGeneration; + const publicResult = { ...result, nodes: publicAndroidSnapshotNodes(result.nodes) }; return { - data: refsGeneration === undefined ? result : { ...result, refsGeneration }, + data: refsGeneration === undefined ? publicResult : { ...publicResult, refsGeneration }, record: { kind: 'snapshot', nodes: result.nodes.length, diff --git a/src/kernel/snapshot.ts b/src/kernel/snapshot.ts index cacc7199f..ccad5d38c 100644 --- a/src/kernel/snapshot.ts +++ b/src/kernel/snapshot.ts @@ -66,23 +66,8 @@ export type RawSnapshotNode = { hiddenContentBelow?: boolean; interactionBlocked?: 'covered'; presentationHints?: string[]; - /** INTERNAL, stripped by `publicSnapshotNodes`: status/nav-bar provenance for the chrome classifiers. */ - systemChrome?: boolean; }; -/** - * Projects captured nodes onto the published contract by dropping internal-only fields, - * mirroring `publicSnapshotCaptureAnnotations` for capture annotations. - */ -export function publicSnapshotNodes(nodes: T[]): T[] { - if (!nodes.some((node) => node.systemChrome !== undefined)) return nodes; - return nodes.map((node) => { - if (node.systemChrome === undefined) return node; - const { systemChrome: _systemChrome, ...published } = node; - return published as T; - }); -} - export type HiddenContentHint = { hiddenContentAbove?: true; hiddenContentBelow?: true; diff --git a/src/mcp/__tests__/command-tools-parity.test.ts b/src/mcp/__tests__/command-tools-parity.test.ts index 4887d8964..18f793248 100644 --- a/src/mcp/__tests__/command-tools-parity.test.ts +++ b/src/mcp/__tests__/command-tools-parity.test.ts @@ -93,3 +93,32 @@ test('MCP applies config-backed command defaults with explicit-input precedence assert.ok(calls.every((request) => request.command === 'snapshot')); assert.ok(calls.every((request) => request.flags?.appsFilter === undefined)); }); + +test('MCP snapshot structured content omits Android-internal chrome provenance', async () => { + const transport: AgentDeviceDaemonTransport = async () => ({ + ok: true, + data: { + nodes: [ + { + ref: 'e1', + index: 0, + depth: 0, + type: 'android.widget.TextView', + label: '7:03', + systemChrome: true, + }, + ], + truncated: false, + }, + }); + const executor = createCommandToolExecutor({ + createClient: (config) => createAgentDeviceClient(config, { transport }), + }); + + const result = await executor.execute('snapshot', { mcpOutputFormat: 'json' }); + + assert.deepEqual(result.structuredContent?.nodes, [ + { ref: 'e1', index: 0, depth: 0, type: 'android.widget.TextView', label: '7:03' }, + ]); + assert.equal(JSON.stringify(result.structuredContent).includes('systemChrome'), false); +}); diff --git a/src/platforms/android/ui-hierarchy.ts b/src/platforms/android/ui-hierarchy.ts index a64d6d8e3..a9202f04b 100644 --- a/src/platforms/android/ui-hierarchy.ts +++ b/src/platforms/android/ui-hierarchy.ts @@ -2,7 +2,10 @@ import type { RawSnapshotNode, Rect, SnapshotOptions } from '../../kernel/snapsh import { parseBounds } from '../../utils/bounds.ts'; import { isScrollableType } from '../../utils/scrollable.ts'; import { intersectArea } from '../../utils/screenshot-geometry.ts'; -import { isAndroidSystemChromeWindowResourceId } from '../../contracts/android-system-chrome.ts'; +import { + isAndroidSystemChromeWindowResourceId, + type AndroidSystemChromeRawSnapshotNode, +} from '../../contracts/android-system-chrome.ts'; export type AndroidSnapshotAnalysis = { rawNodeCount: number; @@ -34,7 +37,7 @@ export type AndroidUiNodeMetadata = { windowFocused?: boolean; windowRect?: Rect; /** Inside a status-bar/navigation-bar container (tracked by `androidUiNodes`). */ - systemChrome?: boolean; + systemChrome?: true; }; /** @@ -101,14 +104,14 @@ export function parseUiHierarchy( } export type AndroidBuiltSnapshot = { - nodes: RawSnapshotNode[]; + nodes: AndroidSystemChromeRawSnapshotNode[]; sourceNodes: AndroidUiHierarchy[]; truncated?: boolean; analysis: AndroidSnapshotAnalysis; }; type AndroidSnapshotBuildState = { - nodes: RawSnapshotNode[]; + nodes: AndroidSystemChromeRawSnapshotNode[]; sourceNodes: AndroidUiHierarchy[]; maxNodes?: number; maxDepth: number; From 7cd3ab91fffada32eb6aa8b012b0caaf90b444e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 14:57:51 +0200 Subject: [PATCH 7/9] refactor(android): tighten chrome provenance seam --- src/__tests__/client.test.ts | 27 --------------- src/client/client-normalizers.ts | 3 +- src/contracts/android-system-chrome.ts | 19 +++++------ .../__tests__/snapshot-handler.test.ts | 34 ------------------- src/daemon/snapshot-runtime.ts | 8 +++-- .../__tests__/command-tools-parity.test.ts | 29 ---------------- .../android/snapshot-content-recovery.ts | 3 +- src/platforms/android/ui-hierarchy.ts | 17 +++++----- .../android-lifecycle.test.ts | 30 ++++++++++++++++ 9 files changed, 56 insertions(+), 114 deletions(-) diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index 32f95148d..ce0395bb5 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -938,33 +938,6 @@ test('client capture.snapshot preserves visibility metadata from daemon response }); }); -test('client capture.snapshot drops Android-internal chrome provenance', async () => { - const setup = createTransport(async () => ({ - ok: true, - data: { - nodes: [ - { - ref: 'e1', - index: 0, - depth: 0, - type: 'android.widget.TextView', - label: '7:03', - systemChrome: true, - }, - ], - truncated: false, - }, - })); - const client = createAgentDeviceClient(setup.config, { transport: setup.transport }); - - const result = await client.capture.snapshot(); - - assert.deepEqual(result.nodes, [ - { ref: 'e1', index: 0, depth: 0, type: 'android.widget.TextView', label: '7:03' }, - ]); - assert.equal(JSON.stringify(result).includes('systemChrome'), false); -}); - test('client capture.snapshot preserves refsGeneration from daemon responses (ADR 0014)', async () => { const setup = createTransport(async () => ({ ok: true, diff --git a/src/client/client-normalizers.ts b/src/client/client-normalizers.ts index 35b57d1c0..e63723656 100644 --- a/src/client/client-normalizers.ts +++ b/src/client/client-normalizers.ts @@ -2,7 +2,6 @@ import type { DaemonRequest, SessionRuntimeHints } from '../daemon/types.ts'; import { AppError, type NormalizedError } from '../kernel/errors.ts'; import type { SnapshotNode } from '../kernel/snapshot.ts'; import { buildAppIdentifiers, buildDeviceIdentifiers } from '../contracts/result-serialization.ts'; -import { publicAndroidSnapshotNodes } from '../contracts/android-system-chrome.ts'; import { isAppleOs, isApplePlatform, isPublicPlatform, type AppleOS } from '../kernel/device.ts'; import { leaseScopeFromOptions, leaseScopeToRequestMeta } from '../core/lease-scope.ts'; import type { @@ -267,7 +266,7 @@ function normalizeTargetShutdownError(value: unknown): NormalizedError | undefin export function readSnapshotNodes(value: unknown): SnapshotNode[] { // Snapshot nodes are produced by the daemon snapshot pipeline and treated as trusted here. - return Array.isArray(value) ? publicAndroidSnapshotNodes(value as SnapshotNode[]) : []; + return Array.isArray(value) ? (value as SnapshotNode[]) : []; } export function buildMeta(options: InternalRequestOptions): DaemonRequest['meta'] { diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index d74ca7b21..a2343017b 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -1,4 +1,4 @@ -import type { RawSnapshotNode, SnapshotNode } from '../kernel/snapshot.ts'; +import type { SnapshotNode } from '../kernel/snapshot.ts'; /** * Android status-bar/navigation-bar chrome identity, shared by the settle-chrome @@ -27,19 +27,16 @@ export type AndroidSystemChromeProvenance = { systemChrome?: true; }; -export type AndroidSystemChromeRawSnapshotNode = RawSnapshotNode & AndroidSystemChromeProvenance; - -export function hasAndroidSystemChromeProvenance(node: RawSnapshotNode): boolean { - return (node as AndroidSystemChromeRawSnapshotNode).systemChrome === true; +export function hasAndroidSystemChromeProvenance(value: object): value is { systemChrome: true } { + return 'systemChrome' in value && value.systemChrome === true; } -/** Drops Android-internal provenance before snapshot nodes cross a public boundary. */ -export function publicAndroidSnapshotNodes(nodes: SnapshotNode[]): SnapshotNode[] { - if (!nodes.some((node) => Object.hasOwn(node, 'systemChrome'))) return nodes; +/** Drops Android-internal provenance at the daemon's public snapshot seam. */ +export function stripAndroidSystemChromeProvenance(nodes: SnapshotNode[]): SnapshotNode[] { + if (!nodes.some(hasAndroidSystemChromeProvenance)) return nodes; return nodes.map((node) => { - if (!Object.hasOwn(node, 'systemChrome')) return node; - const { systemChrome: _systemChrome, ...published } = node as SnapshotNode & - AndroidSystemChromeProvenance; + if (!hasAndroidSystemChromeProvenance(node)) return node; + const { systemChrome: _systemChrome, ...published } = node; return published; }); } diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index 6ec70b89f..f028cc1d9 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -14,7 +14,6 @@ import { buildInteractionSurfaceSignature } from '../../interaction-outcome-poli import { buildSnapshotPresentationKey } from '../../../kernel/snapshot.ts'; import { snapshotCliOutput } from '../../../commands/capture/output.ts'; import type { CaptureSnapshotResult } from '../../../client/client-types.ts'; -import type { AndroidSystemChromeRawSnapshotNode } from '../../../contracts/android-system-chrome.ts'; vi.mock('../../../core/dispatch.ts', async (importOriginal) => { const actual = await importOriginal(); @@ -403,39 +402,6 @@ test('snapshot on iOS runs when the session tracks an app', async () => { ); }); -test('snapshot keeps Android chrome provenance in session state but not the daemon response', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'android-internal-chrome'; - sessionStore.set(sessionName, makeSession(sessionName, androidDevice)); - const internalNode: AndroidSystemChromeRawSnapshotNode = { - index: 0, - depth: 0, - type: 'android.widget.TextView', - label: '7:03', - systemChrome: true, - }; - mockDispatch.mockResolvedValue({ - nodes: [internalNode], - truncated: false, - backend: 'android', - }); - - const response = await handleSnapshotCommands({ - req: { token: 't', session: sessionName, command: 'snapshot', positionals: [], flags: {} }, - sessionName, - logPath: '/tmp/daemon.log', - sessionStore, - }); - - expect(response?.ok).toBe(true); - if (!response?.ok) return; - expect(response.data?.nodes).toEqual([ - { index: 0, ref: 'e1', depth: 0, type: 'android.widget.TextView', label: '7:03' }, - ]); - expect(JSON.stringify(response.data)).not.toContain('systemChrome'); - expect(sessionStore.get(sessionName)?.snapshot?.nodes[0]).toHaveProperty('systemChrome', true); -}); - test('snapshot re-activates a complete frame; diff preserves it (ADR 0014)', async () => { const sessionStore = makeSessionStore(); const sessionName = 'android-stale-refs-marker'; diff --git a/src/daemon/snapshot-runtime.ts b/src/daemon/snapshot-runtime.ts index 2c10a75bc..be399afeb 100644 --- a/src/daemon/snapshot-runtime.ts +++ b/src/daemon/snapshot-runtime.ts @@ -21,7 +21,7 @@ import { maybeBuildAndroidSnapshotTimeoutFailure } from './android-snapshot-time import { summarizeSnapshotDiagnostics } from '../snapshot-diagnostics.ts'; import { nextSnapshotGeneration } from './session-snapshot.ts'; import { activateCompleteRefFrame } from './ref-frame.ts'; -import { publicAndroidSnapshotNodes } from '../contracts/android-system-chrome.ts'; +import { stripAndroidSystemChromeProvenance } from '../contracts/android-system-chrome.ts'; export async function dispatchSnapshotViaRuntime(params: { req: DaemonRequest; @@ -48,7 +48,11 @@ export async function dispatchSnapshotViaRuntime(params: { // capture above already stored the next session via setRecord, so the // store holds the generation these refs were minted from. const refsGeneration = params.sessionStore.get(sessionName)?.snapshotGeneration; - const publicResult = { ...result, nodes: publicAndroidSnapshotNodes(result.nodes) }; + // ADR 0014: retain provenance in the immutable operational/ref-frame tree; + // project only the published copy so settle and replay keep the full evidence. + const publicNodes = stripAndroidSystemChromeProvenance(result.nodes); + const publicResult = + publicNodes === result.nodes ? result : { ...result, nodes: publicNodes }; return { data: refsGeneration === undefined ? publicResult : { ...publicResult, refsGeneration }, record: { diff --git a/src/mcp/__tests__/command-tools-parity.test.ts b/src/mcp/__tests__/command-tools-parity.test.ts index 18f793248..4887d8964 100644 --- a/src/mcp/__tests__/command-tools-parity.test.ts +++ b/src/mcp/__tests__/command-tools-parity.test.ts @@ -93,32 +93,3 @@ test('MCP applies config-backed command defaults with explicit-input precedence assert.ok(calls.every((request) => request.command === 'snapshot')); assert.ok(calls.every((request) => request.flags?.appsFilter === undefined)); }); - -test('MCP snapshot structured content omits Android-internal chrome provenance', async () => { - const transport: AgentDeviceDaemonTransport = async () => ({ - ok: true, - data: { - nodes: [ - { - ref: 'e1', - index: 0, - depth: 0, - type: 'android.widget.TextView', - label: '7:03', - systemChrome: true, - }, - ], - truncated: false, - }, - }); - const executor = createCommandToolExecutor({ - createClient: (config) => createAgentDeviceClient(config, { transport }), - }); - - const result = await executor.execute('snapshot', { mcpOutputFormat: 'json' }); - - assert.deepEqual(result.structuredContent?.nodes, [ - { ref: 'e1', index: 0, depth: 0, type: 'android.widget.TextView', label: '7:03' }, - ]); - assert.equal(JSON.stringify(result.structuredContent).includes('systemChrome'), false); -}); diff --git a/src/platforms/android/snapshot-content-recovery.ts b/src/platforms/android/snapshot-content-recovery.ts index d945161d0..8dd59dbc9 100644 --- a/src/platforms/android/snapshot-content-recovery.ts +++ b/src/platforms/android/snapshot-content-recovery.ts @@ -1,5 +1,6 @@ import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; import { isAndroidInputMethodOwnedNode } from '../../contracts/android-input-ownership.ts'; +import { hasAndroidSystemChromeProvenance } from '../../contracts/android-system-chrome.ts'; import { classifyAndroidAlertIdentifier } from './alert-detection.ts'; import { androidUiNodes, type AndroidUiNodeMetadata } from './ui-hierarchy.ts'; @@ -298,7 +299,7 @@ function recordMeaningfulWindowOwnership( // Status/nav-bar chrome must not satisfy the system-surface floor: an active navigation bar // (Back + Home + Recents) or status chrome (clock, battery, signal icons) is missing-app-content // residue, not a usable shade/quick-settings surface. Only non-chrome nodes count. - if (isInActiveSystemSurfaceWindow(summary) && node.systemChrome !== true) { + if (isInActiveSystemSurfaceWindow(summary) && !hasAndroidSystemChromeProvenance(node)) { summary.activeSystemSurfaceMeaningfulNodeCount += 1; } } diff --git a/src/platforms/android/ui-hierarchy.ts b/src/platforms/android/ui-hierarchy.ts index a9202f04b..7d5b448f5 100644 --- a/src/platforms/android/ui-hierarchy.ts +++ b/src/platforms/android/ui-hierarchy.ts @@ -3,15 +3,18 @@ import { parseBounds } from '../../utils/bounds.ts'; import { isScrollableType } from '../../utils/scrollable.ts'; import { intersectArea } from '../../utils/screenshot-geometry.ts'; import { + type AndroidSystemChromeProvenance, isAndroidSystemChromeWindowResourceId, - type AndroidSystemChromeRawSnapshotNode, } from '../../contracts/android-system-chrome.ts'; +type AndroidRawSnapshotNode = RawSnapshotNode & AndroidSystemChromeProvenance; + export type AndroidSnapshotAnalysis = { rawNodeCount: number; maxDepth: number; }; +/** Parsed node metadata plus status/navigation subtree provenance when present. */ export type AndroidUiNodeMetadata = { text: string | null; desc: string | null; @@ -36,9 +39,7 @@ export type AndroidUiNodeMetadata = { windowActive?: boolean; windowFocused?: boolean; windowRect?: Rect; - /** Inside a status-bar/navigation-bar container (tracked by `androidUiNodes`). */ - systemChrome?: true; -}; +} & AndroidSystemChromeProvenance; /** * Membership of the status-bar/nav-bar subtree over a `` token stream: the @@ -104,14 +105,14 @@ export function parseUiHierarchy( } export type AndroidBuiltSnapshot = { - nodes: AndroidSystemChromeRawSnapshotNode[]; + nodes: AndroidRawSnapshotNode[]; sourceNodes: AndroidUiHierarchy[]; truncated?: boolean; analysis: AndroidSnapshotAnalysis; }; type AndroidSnapshotBuildState = { - nodes: AndroidSystemChromeRawSnapshotNode[]; + nodes: AndroidRawSnapshotNode[]; sourceNodes: AndroidUiHierarchy[]; maxNodes?: number; maxDepth: number; @@ -211,8 +212,8 @@ function appendAndroidSnapshotNode( state: AndroidSnapshotBuildState, node: AndroidNode, depth: number, - parentIndex?: number, - systemChrome?: boolean, + parentIndex: number | undefined, + systemChrome: boolean, ): number { const currentIndex = state.nodes.length; state.sourceNodes.push(node); diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 209f5ab4c..c5bb512aa 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -16,6 +16,17 @@ import { createProviderScenarioTempPath, withProviderScenarioResource } from './ type AndroidSettingsWorld = Awaited>; +const ANDROID_SYSTEM_SURFACE_XML = ` + + + + + + + + +`; + test('Provider-backed integration Android Settings flow uses scripted ADB provider', async () => { await withProviderScenarioResource(createAndroidSettingsWorld, async (world) => { const client = world.daemon.client(); @@ -26,6 +37,25 @@ test('Provider-backed integration Android Settings flow uses scripted ADB provid }); }, 15_000); +test('Provider-backed Android snapshot keeps chrome provenance off the public response', async () => { + await withProviderScenarioResource( + async () => await createAndroidSettingsWorld({ snapshotXml: () => ANDROID_SYSTEM_SURFACE_XML }), + async (world) => { + const client = world.daemon.client(); + await client.apps.open({ app: 'settings', ...world.selection }); + + const response = await world.daemon.callCommand('snapshot', [], world.selection); + const snapshot = assertRpcOk<{ nodes: Array<{ label?: string }> }>(response); + + assert.equal( + snapshot.nodes.some((node) => node.label === 'Display brightness'), + true, + ); + assert.equal(JSON.stringify(snapshot).includes('systemChrome'), false); + }, + ); +}); + test('Provider-backed integration Android text provider handles Unicode without shell input text', async () => { await withProviderScenarioResource( async () => await createAndroidSettingsWorld({ nativeTextInjection: true }), From 67c091e20bb3f8410b5e7d1bd6d572a86557cb0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 15:28:25 +0200 Subject: [PATCH 8/9] fix(android): close chrome provenance response leaks --- src/contracts/android-system-chrome.ts | 13 +++++--- .../session-replay-divergence.test.ts | 22 ++++++++++--- .../handlers/session-replay-divergence.ts | 9 ++++-- src/daemon/selector-recording.ts | 31 +++++++++++++------ src/daemon/selector-runtime.ts | 6 ++-- .../android-lifecycle.test.ts | 14 +++++++-- .../integration/provider-scenarios/harness.ts | 9 +++++- 7 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/contracts/android-system-chrome.ts b/src/contracts/android-system-chrome.ts index a2343017b..b8ce37e23 100644 --- a/src/contracts/android-system-chrome.ts +++ b/src/contracts/android-system-chrome.ts @@ -31,12 +31,15 @@ export function hasAndroidSystemChromeProvenance(value: object): value is { syst return 'systemChrome' in value && value.systemChrome === true; } +/** Drops Android-internal provenance from a node embedded in a public response. */ +export function stripAndroidSystemChromeProvenanceFromNode(node: SnapshotNode): SnapshotNode { + if (!hasAndroidSystemChromeProvenance(node)) return node; + const { systemChrome: _systemChrome, ...published } = node; + return published; +} + /** Drops Android-internal provenance at the daemon's public snapshot seam. */ export function stripAndroidSystemChromeProvenance(nodes: SnapshotNode[]): SnapshotNode[] { if (!nodes.some(hasAndroidSystemChromeProvenance)) return nodes; - return nodes.map((node) => { - if (!hasAndroidSystemChromeProvenance(node)) return node; - const { systemChrome: _systemChrome, ...published } = node; - return published; - }); + return nodes.map(stripAndroidSystemChromeProvenanceFromNode); } diff --git a/src/daemon/handlers/__tests__/session-replay-divergence.test.ts b/src/daemon/handlers/__tests__/session-replay-divergence.test.ts index 27c4f4a74..825253edf 100644 --- a/src/daemon/handlers/__tests__/session-replay-divergence.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-divergence.test.ts @@ -21,6 +21,7 @@ vi.mock('../../../utils/timeouts.ts', async (importOriginal) => { }); import { dispatchCommand } from '../../../core/dispatch.ts'; +import type { RawSnapshotNode } from '../../../kernel/snapshot.ts'; import { makeAndroidSession, makeIosSession, @@ -1061,10 +1062,21 @@ test('buildReplayFailureDivergence: divergence capture drops the action snapshot }); // Real walked FULL-COVER quick-settings shade: every node is systemui and the -// status-bar icons share the shade's window. The provenance classifier must -// exclude those icons without excluding the actionable tiles; reverting to the -// old run-level classifier makes this screen empty. -test('buildReplayFailureDivergence: a full-cover quick-settings shade still publishes its hittable tiles (wave-3 leg E)', async () => { +// status-bar icons share the shade's window. The second scenario pessimistically +// marks the whole tree as chrome to model older/OEM layouts whose status-bar +// container also owns real controls; the last-resort hittable fallback must keep +// replay repair actionable without promoting non-hittable status residue. +test.each([ + { + name: 'classifies the shade precisely', + prepare: (nodes: RawSnapshotNode[]) => nodes, + }, + { + name: 'survives a whole-tree chrome false positive', + prepare: (nodes: RawSnapshotNode[]) => + nodes.map((node) => ({ ...node, systemChrome: true as const })), + }, +])('buildReplayFailureDivergence: a full-cover quick-settings shade $name', async ({ prepare }) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-divergence-qsshade-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); const sessionName = 'default'; @@ -1073,7 +1085,7 @@ test('buildReplayFailureDivergence: a full-cover quick-settings shade still publ makeAndroidSession(sessionName, { appBundleId: 'com.google.android.deskclock' }), ); - const walked = walkNonRawAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES); + const walked = prepare(walkNonRawAndroidFixture(ANDROID_QS_SHADE_CAPTURE_RAW_NODES)); mockDispatchCommand.mockResolvedValue({ nodes: walked, truncated: false, backend: 'android' }); const action = { diff --git a/src/daemon/handlers/session-replay-divergence.ts b/src/daemon/handlers/session-replay-divergence.ts index 8542f1a30..eb51475d1 100644 --- a/src/daemon/handlers/session-replay-divergence.ts +++ b/src/daemon/handlers/session-replay-divergence.ts @@ -381,6 +381,11 @@ function selectDivergenceScreenRefNodes( const nonChrome = nodes.filter( (node) => node.ref && !chromeRefs.has(node.ref) && isMeaningfulDivergenceTarget(node), ); + // Keep replay repair actionable when an older Android/OEM hierarchy puts a + // status/nav id on a container that also owns real controls. This activates + // only when the chrome classifier would otherwise erase the entire screen. + const meaningful = + nonChrome.length > 0 ? nonChrome : nodes.filter((node) => node.ref && node.hittable === true); // Occlusion fallback (#1264): a `covered` node is normally dropped — an agent // cannot tap what an overlay hides. But when a system overlay MASS-COVERS the // app, EVERY app node is annotated `covered`; dropping them all would emit an @@ -389,8 +394,8 @@ function selectDivergenceScreenRefNodes( // `covered` nodes are excluded only while non-covered candidates remain; if // the entire meaningful surface is covered, they are surfaced rather than // returning empty. - const visible = nonChrome.filter((node) => node.interactionBlocked !== 'covered'); - const pool = visible.length > 0 ? visible : nonChrome; + const visible = meaningful.filter((node) => node.interactionBlocked !== 'covered'); + const pool = visible.length > 0 ? visible : meaningful; // Rank within the cap instead of slicing document order (#1264 cap burial): // `SCREEN_REF_CAPTURE_LIMIT` is a BYTE bound, NOT a "first 20 in tree order" // policy. A separate-window overlay enumerates AFTER the app window's nodes, diff --git a/src/daemon/selector-recording.ts b/src/daemon/selector-recording.ts index 4abe008e9..628fecc0d 100644 --- a/src/daemon/selector-recording.ts +++ b/src/daemon/selector-recording.ts @@ -1,4 +1,6 @@ import type { DaemonRequest } from './types.ts'; +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { stripAndroidSystemChromeProvenanceFromNode } from '../contracts/android-system-chrome.ts'; import { SessionStore } from './session-store.ts'; import { isInteractiveObservation } from './session-action-recorder.ts'; import { computeTargetEvidence, type RecordedTargetCapture } from './session-target-evidence.ts'; @@ -20,7 +22,12 @@ export function buildFindRecordResult( }; } -export function toDaemonFindData(result: Record): Record { +type DaemonFindResult = + | { kind: 'found'; waitedMs?: number } + | { kind: 'text'; ref: string; text: string; node: SnapshotNode } + | { kind: 'attrs'; ref: string; node: SnapshotNode }; + +export function toDaemonFindData(result: DaemonFindResult): Record { if (result.kind === 'found') { return { found: true, @@ -28,9 +35,9 @@ export function toDaemonFindData(result: Record): Record): Record { - const target = getResolvedTarget(result); +type DaemonGetResult = { + target: { kind: 'ref'; ref: string } | { kind: 'selector'; selector: string }; + text?: string; + node: SnapshotNode; +}; + +export function toDaemonGetData(result: DaemonGetResult): Record { + const { target } = result; return { - ...(target?.kind === 'ref' ? { ref: normalizeDaemonRef(target.ref) } : {}), - ...(target?.kind === 'selector' ? { selector: target.selector } : {}), + ...(target.kind === 'ref' ? { ref: normalizeDaemonRef(target.ref) } : {}), + ...(target.kind === 'selector' ? { selector: target.selector } : {}), ...(typeof result.text === 'string' ? { text: result.text } : {}), - ...(result.node && typeof result.node === 'object' ? { node: result.node } : {}), + node: stripAndroidSystemChromeProvenanceFromNode(result.node), }; } diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 9ddea48b2..2c00a5c11 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -492,16 +492,16 @@ function buildDirectIosGetResult( property: 'text' | 'attrs', selector: string, result: DirectIosSelectorQueryResult, -): Record | null { +) { if (!result.found || !result.node) return null; const base = { target: { kind: 'selector' as const, selector }, node: result.node, selectorChain: [selector], }; - if (property === 'attrs') return { kind: 'attrs', ...base }; + if (property === 'attrs') return { kind: 'attrs' as const, ...base }; if (typeof result.text !== 'string') return null; - return { kind: 'text', ...base, text: result.text }; + return { kind: 'text' as const, ...base, text: result.text }; } function buildDirectIosIsResult( diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index c5bb512aa..15b148639 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -37,7 +37,7 @@ test('Provider-backed integration Android Settings flow uses scripted ADB provid }); }, 15_000); -test('Provider-backed Android snapshot keeps chrome provenance off the public response', async () => { +test('Provider-backed Android reads keep chrome provenance internal across public node payloads', async () => { await withProviderScenarioResource( async () => await createAndroidSettingsWorld({ snapshotXml: () => ANDROID_SYSTEM_SURFACE_XML }), async (world) => { @@ -45,13 +45,21 @@ test('Provider-backed Android snapshot keeps chrome provenance off the public re await client.apps.open({ app: 'settings', ...world.selection }); const response = await world.daemon.callCommand('snapshot', [], world.selection); - const snapshot = assertRpcOk<{ nodes: Array<{ label?: string }> }>(response); + const snapshot = assertRpcOk<{ nodes: Array<{ ref: string; label?: string }> }>(response); assert.equal( snapshot.nodes.some((node) => node.label === 'Display brightness'), true, ); - assert.equal(JSON.stringify(snapshot).includes('systemChrome'), false); + + const clock = snapshot.nodes.find((node) => node.label === '7:03'); + assert.ok(clock?.ref); + assertRpcOk( + await world.daemon.callCommand('get', ['attrs', `@${clock.ref}`], world.selection), + ); + assertRpcOk( + await world.daemon.callCommand('find', ['label', '7:03', 'get', 'attrs'], world.selection), + ); }, ); }); diff --git a/test/integration/provider-scenarios/harness.ts b/test/integration/provider-scenarios/harness.ts index 196b73a05..f10a7b441 100644 --- a/test/integration/provider-scenarios/harness.ts +++ b/test/integration/provider-scenarios/harness.ts @@ -1,3 +1,4 @@ +import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -167,7 +168,7 @@ function commandRequest( } function responseToRpcResult(response: DaemonResponse, id: string): ProviderScenarioRpcResult { - return { + const rpcResult = { statusCode: 200, json: response.ok ? { @@ -185,4 +186,10 @@ function responseToRpcResult(response: DaemonResponse, id: string): ProviderScen }, }, }; + assert.equal( + JSON.stringify(rpcResult).includes('"systemChrome"'), + false, + 'Public RPC responses must not expose Android-internal systemChrome provenance', + ); + return rpcResult; } From d90cb798a96d24abc6a04bd5deaa38983ac2b8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 15:39:16 +0200 Subject: [PATCH 9/9] test(provider): enforce chrome invariant across clients --- test/integration/provider-scenarios/harness.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/integration/provider-scenarios/harness.ts b/test/integration/provider-scenarios/harness.ts index f10a7b441..b8a2e5e18 100644 --- a/test/integration/provider-scenarios/harness.ts +++ b/test/integration/provider-scenarios/harness.ts @@ -55,7 +55,7 @@ export async function createProviderScenarioHarness( path.join(os.tmpdir(), 'agent-device-provider-scenario-session-'), ); const sessionStore = new SessionStore(sessionDir); - const handleRequest = createRequestHandler({ + const requestHandler = createRequestHandler({ logPath: path.join(os.tmpdir(), 'agent-device-provider-scenario-daemon.log'), token: PROVIDER_SCENARIO_TOKEN, sessionStore, @@ -63,6 +63,11 @@ export async function createProviderScenarioHarness( trackDownloadableArtifact, ...deps, }); + const handleRequest: typeof requestHandler = async (request) => { + const response = await requestHandler(request); + assertNoInternalChromeProvenance(response); + return response; + }; const transport: AgentDeviceDaemonTransport = async (req) => await handleRequest({ @@ -168,7 +173,7 @@ function commandRequest( } function responseToRpcResult(response: DaemonResponse, id: string): ProviderScenarioRpcResult { - const rpcResult = { + return { statusCode: 200, json: response.ok ? { @@ -186,10 +191,12 @@ function responseToRpcResult(response: DaemonResponse, id: string): ProviderScen }, }, }; +} + +function assertNoInternalChromeProvenance(response: DaemonResponse): void { assert.equal( - JSON.stringify(rpcResult).includes('"systemChrome"'), + JSON.stringify(response).includes('"systemChrome"'), false, - 'Public RPC responses must not expose Android-internal systemChrome provenance', + 'Public daemon responses must not expose Android-internal systemChrome provenance', ); - return rpcResult; }