fix(android): return meaningful occluding system surfaces instead of failing the snapshot#1301
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Review at exact head 23aa637 — not ready yet.
The branch is based on current main, and the local emulator evidence is useful for the demonstrated zero-app-root shade/QS shape. Mixed OEM window layouts, IME ordering semantics, and the requested Limrun smoke remain residual device risk. iOS Smoke is still running, so I did not apply ready-for-human. |
…s; gate the carve-out on non-chrome content (PR #1301 review) P1a — selector/find/wait routes no longer lose the disclosure. SnapshotState gains systemSurfaceOnly, stamped at the one seam where snapshot state and capture annotations meet (captureSnapshotAttempt), so every consumer — including session-stored snapshots — inherits it. The disclosure message moves to a shared module (snapshot/system-surface-disclosure.ts) used by the capture-runtime warning and a daemon response helper (handlers/system-surface-disclosure.ts) that appends it to ok-response warnings and error-response hints. Applied on both found and not-found outcomes across the public daemon selector routes: read-only find (exists/wait/get_text/get_attrs), mutating find (matched, unmatched, and ambiguous), wait (text/selector/ref/stable; pure sleep is exempt), get, and is. P1b — the >=3 meaningful-node floor for the system-surface carve-out now counts only NON-CHROME nodes. The status/nav-bar marker ids from the settle-chrome classifier (#1198/#1251) move to a shared contracts/android-system-chrome.ts (core/snapshot-chrome.ts keeps byte-identical behavior via the shared resource-id predicate), and classifyAndroidHelperContent excludes chrome-classified resource-ids from activeSystemSurfaceMeaningfulNodeCount: an active nav bar (Back + Home + Recents) or status chrome (clock/battery/wifi) is missing-app-content residue, not a usable shade. Shade/QS fixtures are unaffected (tile/notification ids are not chrome markers). P2 — regressions prove the production wiring, not just the classifier: snapshotAndroid stamps androidSnapshot.systemSurfaceOnly for a helper-backed shade capture; the capture runtime renders the disclosure warning from the annotation; daemon regressions pin the disclosure on mutating find (found), read-only find exists, and wait timeout against a shade capture; classifier fixtures pin nav-bar and status-chrome windows as unusable. Live-verified on emulator-5556 with the shade expanded: snapshot -i returns systemSurfaceOnly true plus the warning; find exists returns found:true plus the disclosure warning; wait timeout carries the disclosure in its hint.
|
Exact-head re-review (
All current CI is green; no |
|
All three findings addressed in 19a4313, CI green (22/22): P1 — disclosure threading. P1 — chrome-blind floor. The carve-out no longer counts raw meaningful nodes: P2 — production wiring proof. Helper-backed Live evidence: the snapshot route was verified on the emulator in the previous round (settled shade 5/5 and QS returning 🤖 Addressed by Claude Code |
…failing the snapshot (#1253) The notification shade and quick settings legitimately own the whole screen: the helper faithfully captures the active system window, but the content classifier treated the missing application window as a helper failure. Add a carve-out: an active/focused non-application window carrying meaningful content is returned as the snapshot, flagged systemSurfaceOnly, with an agent-facing warning explaining how to reach app content. Sparse or inactive system windows keep failing with the structured retriable error.
…s; gate the carve-out on non-chrome content (PR #1301 review) P1a — selector/find/wait routes no longer lose the disclosure. SnapshotState gains systemSurfaceOnly, stamped at the one seam where snapshot state and capture annotations meet (captureSnapshotAttempt), so every consumer — including session-stored snapshots — inherits it. The disclosure message moves to a shared module (snapshot/system-surface-disclosure.ts) used by the capture-runtime warning and a daemon response helper (handlers/system-surface-disclosure.ts) that appends it to ok-response warnings and error-response hints. Applied on both found and not-found outcomes across the public daemon selector routes: read-only find (exists/wait/get_text/get_attrs), mutating find (matched, unmatched, and ambiguous), wait (text/selector/ref/stable; pure sleep is exempt), get, and is. P1b — the >=3 meaningful-node floor for the system-surface carve-out now counts only NON-CHROME nodes. The status/nav-bar marker ids from the settle-chrome classifier (#1198/#1251) move to a shared contracts/android-system-chrome.ts (core/snapshot-chrome.ts keeps byte-identical behavior via the shared resource-id predicate), and classifyAndroidHelperContent excludes chrome-classified resource-ids from activeSystemSurfaceMeaningfulNodeCount: an active nav bar (Back + Home + Recents) or status chrome (clock/battery/wifi) is missing-app-content residue, not a usable shade. Shade/QS fixtures are unaffected (tile/notification ids are not chrome markers). P2 — regressions prove the production wiring, not just the classifier: snapshotAndroid stamps androidSnapshot.systemSurfaceOnly for a helper-backed shade capture; the capture runtime renders the disclosure warning from the annotation; daemon regressions pin the disclosure on mutating find (found), read-only find exists, and wait timeout against a shade capture; classifier fixtures pin nav-bar and status-chrome windows as unusable. Live-verified on emulator-5556 with the shade expanded: snapshot -i returns systemSurfaceOnly true plus the warning; find exists returns found:true plus the disclosure warning; wait timeout carries the disclosure in its hint.
|
Limrun smoke (the pending validation item) — run against this branch rebased onto current main (picks up #1278's direct Limrun runtime; rebase was conflict-free, branch force-pushed):
🤖 Addressed by Claude Code |
thymikee
force-pushed
the
feat/android-helper-only-snapshots-1253
branch
from
19a4313 to
bbbac6e
Compare
|
Exact-head re-review (
The Limrun smoke is credible for helper/provider routing but does not exercise |
… pin disclosure composition (PR #1301 review) Sessionless find/wait never store the consumed capture on a session record, so the disclosure read from the session store returned nothing. The selector capture runtime now reports every consumed snapshot through a shared slot on the runtime params, and disclosure reads prefer it; the session-store read remains only as a fallback for pre-captured snapshots. Regressions pin the sessionless route end-to-end and that the disclosure appends after existing success warnings and failure hints instead of replacing them.
|
Both remaining blockers addressed in 8c98dc0, on top of the rebased head: P1 — sessionless disclosure. The selector capture runtime now reports every consumed snapshot into a shared P2 — composition pinned. Two new direct regressions on CI-relevant: unit suite green (the handful of full-suite failures are the known process-spawn contention flakes — runtime-hints, apple runner-client/session, client-metro, daemon-client — all pass in isolation, 199/199). 🤖 Addressed by Claude Code |
|
Exact-head re-review ( The consumed-snapshot slot is initialized only in Initialize/pass the shared slot on the wait route and add a sessionless wait success/timeout regression. CI is still running and no |
…(PR #1301 review) dispatchWaitViaRuntime builds its selector runtime directly rather than via createSelectorRuntime, so sessionless waits had no slot for the capture runtime to report the consumed snapshot into and lost the system-surface disclosure. Regressions pin sessionless wait success and timeout, both asserting no session record exists and the disclosure is present.
|
Sessionless-wait blocker addressed in 76a180c: Daemon suite 1354/1354; typecheck/lint/format/fallow clean. 🤖 Addressed by Claude Code |
thymikee
added
the
ready-for-human
|
Exact-head re-review ( |
|
1 participant
Closes #1253. Closes #1284 (decision record on the issue).
Scope note
Most of #1253 already landed on main between filing and now: the stock
uiautomator dumpfallback is gone from the snapshot path, artifact/install failures throw actionable errors, capture/parse failures propagate structured helper details, and content-poor output fails with a retriable hint — no production path shells out to stock UIAutomator, and every successful direct snapshot reportsbackend: "android-helper".This PR completes the issue's remaining half — "these should be treated as helper/product issues to verify and fix where possible" — for the one case live-probing proved was being misclassified.
The bug (live-reproduced on emulator)
With the notification shade or quick settings open over an app session, the helper faithfully captures the active systemui window (
helperWindowTypes: [3], dozens of meaningful nodes — the old stock dump returned 104–142 nodes on the same screens), butclassifyAndroidHelperContentsaw no application window and failed the snapshot ascontent-poor-app-window/system-window-only. The agent got an error and a screenshot hint on a screen that had a perfectly actionable accessibility tree (dismiss button, notification rows, QS toggles).Fix
A third carve-out in the content classifier, alongside the existing alert-surface and IME ones: when the capture has no application window but an active/focused non-application window carrying meaningful content (≥3 meaningful nodes), the capture is the truth and is returned:
classifyAndroidHelperContentRecovery→classifyAndroidHelperContent, returning{ outcome: 'ok' | 'system-surface-only' | 'unusable' }(decision unchanged for unusable).AndroidSnapshotBackendMetadatagainssystemSurfaceOnly?: boolean; the capture runtime emits an agent-facing warning: the snapshot shows the system surface; interact with it or dismiss it to reach app content.Live verification (emulator, Settings session)
content-poor-app-window)systemSurfaceOnly: true+ warningsystemSurfaceOnly: true+ warning — consistent across 5 consecutive capturesempty-helper-outputfailureVerification
system-surface-only; sparse active surface → unusable; content-rich but inactive window → unusable); snapshot + capture-runtime suites green; typecheck/lint/format/tooling/fallow clean (including the complexity gate — the meaningful-node counter was split into focused helpers).runtime-hints.test.tsprocess-spawn contention flake (passes in isolation).Pending from the issue's validation list
snapshotHelperArtifactprecedence + identity guard), but a short live Limrun session onsnapshot -i --jsonis still worth doing before closing the loop operationally.