fix(android): install the snapshot and IME helpers as plain, non-test-only APKs - #2603
Merged
Merged
Conversation
…-only APKs The helpers set android:testOnly="true" since #464, where it only existed to keep the `adb install -t` step from the debuggable era working. Nothing depends on it: instrumentation can only be started from shell via `am instrument`, and the session server binds loopback. What the flag does do is trigger OEM adb install interception: ColorOS classifies a test-only package as a "PC install attack" and fails the session with vendor code -99, which removed every snapshot-backed command on those devices (#2364). Drop the manifest attribute from both helpers, stop passing -t from the local install path and the release manifest's installArgs, and update the READMEs. The consumer still accepts -t in installArgs so older release manifests parse. Verified on a stock API 36 emulator: both helpers install with plain `adb install -r`, and open/snapshot/click/fill run end to end through the daemon. ColorOS acceptance still needs a device we do not have; the reporter offered to test. Fixes #2364
Size Report
Startup median (7 runs, lower is better):
|
The IME helper no longer passes allowTestPackages; the install assertion still required it and failed coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
Author
|
Reviewed at 8d08bf2. I found no problems in the diff, and your fresh-install run on API 36 covers the changed install path. The Smoke Tests failure is Two paths are still not checked live: an upgrade over an existing testOnly helper install, and a real ColorOS device. The reporter's confirmation on #2364 is the open item. |
thymikee
added
the
ready-for-human
Member
Author
|
Reviewed at 8d08bf2. Dropping Does any test fail if Smoke Tests is still running on this head, and there are no conflicts. The ColorOS confirmation you asked the reporter for is still open. |
|
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
The release manifest's installArgs only ever spelled `install -r` plus the `-t` that #2603 retired with the testOnly flag. Its consumer parsed the array, validated it against a four-flag allowlist, lowered it to semantic options, and lowered those back to flags at the provider — a round trip that carried nothing once the flag was gone. The snapshot helper now installs with a fixed `replace: true`, the same path the IME helper already used. The manifest type, parser, packaging script, and README contract drop the field; older manifests that still carry it parse unchanged and the field is ignored. The adb provider install capability narrows to `replace` — `-t`, `-d`, `-g` had no producer left. Refs #2364.
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
…fixture `2cafab3ad0` typed this fixture as `AndroidSnapshotHelperManifest` while carrying `installArgs`, a field the manifest no longer has: #2603 retired the `-t` flag the array used to spell, the helper now installs with a fixed `adb install -r`, and the parser ignores the key on older blobs — which is what `snapshot-helper.test.ts` pins. The literal therefore failed `pnpm typecheck` with TS2353 on every branch, reding `Typecheck & Package` on `origin/main` itself. Nothing read the key. The install path builds its adb arguments from the provider's own `replace` option, and the assertion this fixture serves is about the timeout hint derived from the adb failure.
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
* feat(contracts): name the keyboard band that owns a tap point A software keyboard is its own system surface: it is never a covering sibling of app content, so the same-window occlusion classifier cannot see it, and it leaves the app window rect intact, so the viewport rule passes a covered tab bar. Nothing in the vocabulary said when a tap point belongs to the keyboard rather than to the app behind it (#2589). Derive the band from the captured tree -- topmost keyboard node down to the viewport bottom, across the keyboard's own columns -- and report one decision with four outcomes, failing open whenever the keyboard cannot be measured. Ownership reads accessibility type, role, and Android input-method provenance, never a locale-dependent label. Derivation and outcomes are pinned by contracts/fixtures/tap-keyboard-occlusion-policy.json. * chore(gates): export the keyboard tap-occlusion contract and pin its surface * fix(contracts): dock the keyboard band to the bottom edge, not the midline Band eligibility asked whether the topmost keyboard rect sat in the lower half of the viewport — a height claim dressed as a docking claim. Measured on iPhone 17 Pro (iOS 26.2) with Safari raising the system keyboard, portrait reports the key plane at 583..816 of an 874 pt viewport and landscape at 75..402 of a 402 pt one. The landscape keyboard therefore starts above that orientation's midline at 201, so the old rule declined to measure it and the guard was silently off in landscape: the same silent misfire #2589 reports, in the second orientation. Eligibility is now the measured question. Keyboard geometry has to come within 80 pt of the viewport's bottom edge to own the band down to it. Portrait's 58 pt home-indicator strip passes with 22 pt of slack, landscape passes exactly on the edge, and a surface that stops further up — an iPad floating or split keyboard, which the height heuristic could also misread as a system band — leaves the band unmeasured so the tap fails open rather than refusing on geometry that says nothing about the bottom of the screen. Docking cannot be read off height, which is what the golden table now pins: the same keyboard is 233 pt of an 874 pt viewport in portrait and 327 pt of a 402 pt one in landscape. Six cases gained the docked bottom row a real tree reports beside its top row, and five new cases pin docking itself plus the boundary it accepts: a bottom-anchored app keypad wearing the keyboard types owns its band. `resolveVisibleKeyboardSurface` is module-private. The table derives its band through `expected.frame`, so nothing outside the module needed it, and the surface derivation assertions that only that file could see now live in the table. * fix(contracts): say which tree the keyboard verdict read, and how to re-check it A refusal after `keyboard dismiss` reads like a lie: the keyboard is gone, and the band that refused the tap came from the tree the command measured against. The off-screen guard can spend one runner round trip confirming its target before it refuses (#1542); the keyboard guard cannot spend anything, because there is nothing to spend it on. `keyboard status` and `keyboard get` are Android-only — Apple never had a live keyboard read, and its cells refuse both — and the double-check hook answers whether the target itself is on screen, which a covering keyboard leaves true. So the refusal stops implying a probe it does not have: the hint names the snapshot as the source of the verdict and `snapshot -i` as the re-measurement that clears it. Same words from the coordinate disclosure, which has the same stale-tree exposure and no probe either. * fix(contracts): refuse to measure keyboard geometry that arrived rotated Verified on iPhone 17 Pro with the landscape keyboard up: the screenshot shows the keyboard full width across the bottom 327 pt of an 874 x 402 viewport, and both the regular and the interactive projection report the key plane as 162 x 327 with the QWERTY columns running down the short axis and the dock's globe button at y 8. iOS hands over the landscape keyboard's rects in the keyboard's own rotated space, so a band derived from them would cover a 141 pt strip on the left edge from y 77 down — refusing a headline the keyboard is nowhere near while letting a tab-bar item at (264, 360) tap straight through into a key. A docked keyboard is wide in both orientations, 402 x 233 in measured portrait and 874 x 327 in measured landscape, so geometry that comes back taller than it is wide is refused measurement and the tap fails open, which is the same stance an unresolvable viewport gets. Landscape coverage is not lost, only iOS's rotated read of it: the table now pins the measured rotated tree as `undetermined` and pins a wide, docked landscape keyboard as measurable against the landscape viewport's own bottom edge. Recording the misfire rather than hiding it: `#2589`'s silent tap-into-a-key stays open on landscape iOS until the Apple capture reports keyboard rects in the app's orientation space. That is a capture-side fix, not a classifier rule, and gets its own issue. * refactor(contracts): measure the band's width from the span that sets its top The width rule took a second span over the surface rects while the band was already about to be built from the anchor span. Measured trees place the dock's buttons inside the key columns, and the landscape read the rule exists to catch — key plane 162 x 327 — fails on either span, so the extra Math.min/max pair only widened the distance between what is checked and what is returned. The module doc stopped calling the point rule "the center rule", which is a description of the version #2602 no longer ships, and one case name gets its apostrophe. * fix(contracts): refuse a band whose keys do not tile the width it claims A split iPad keyboard docks both clusters at the bottom edge, and its surface container still reports the whole screen width, so the docking and width rules passed and the band ran across the middle of the screen — where the app content between the clusters is visible and tappable. The container says where the platform thinks the keyboard is; only the keys say where its controls are, so the band now measures whether the keys form one unbroken run of columns before trusting any of it. A gap is not something the band can be built around: nothing in the tree says where either cluster ends, so the geometry stays unmeasured and the tap fails open, as it does for a rotated keyboard or a floating one. Measured, not assumed. iPhone 17 Pro (26.2) reports a key row adjoining across 395 pt, iPad Pro 11-inch (M4) across 743.5 pt, and Gboard on Pixel 7 across 1070 px, none with a gap above a point, so the allowance is for projection rounding rather than layout — and the reduced two-key keyboards the table had been modelling now carry the measured row, which is what makes the new rule bite on a modelled split cluster instead of on a reporting seam. The same pass measured a three-button navigation bar on Android (126 px at 2274..2400 inside Gboard's reported region, which runs to the physical display bottom) and a docked iPad keyboard stopping 3 pt short of its viewport bottom, both now table rows. The rule is blunt in both directions and the table pins both costs: a sparse layout such as the iOS emoji panel reports corner keys over a wide container, does not tile, and reads undetermined, because choosing which gaps are app content is exactly the invention that produced the split-keyboard bug; and a projection reporting the container with no keys has nothing left to check, so the band is the container's own claim — every captured iOS and Android keyboard so far reports its keys, so that shape is the harness's rather than a captured one. Cases that reach a band also owe their whole shape now: the frame the refusal and the disclosure quote, and the control rects, which is where plane exclusion shows up. Previously the frame was compared only on occluded verdicts and the control rects never, so a regression in plane exclusion or in a clear verdict's band passed silently. The measurement rules are each one named function now, and the two ancestry walks the module carried — one asking whether a node belongs to the keyboard, one asking which nodes have keyboard nodes below them — read the projection's parent links through one primitive. * test(android): drop the retired installArgs from the install-timeout fixture `2cafab3ad0` typed this fixture as `AndroidSnapshotHelperManifest` while carrying `installArgs`, a field the manifest no longer has: #2603 retired the `-t` flag the array used to spell, the helper now installs with a fixed `adb install -r`, and the parser ignores the key on older blobs — which is what `snapshot-helper.test.ts` pins. The literal therefore failed `pnpm typecheck` with TS2353 on every branch, reding `Typecheck & Package` on `origin/main` itself. Nothing read the key. The install path builds its adb arguments from the provider's own `replace` option, and the assertion this fixture serves is about the timeout hint derived from the adb failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2364.
Both Android helpers set
android:testOnly="true". That attribute was added in #464 purely to keep theadb install -tstep from thedebuggableera working; nothing depends on it. Instrumentation can only be started from shell viaam instrument, and the helper's session server binds loopback, so the flag protects nothing here. What it does do is trip OEM adb-install interception: ColorOS classifies a test-only package as a "PC install attack" and fails the session with vendor code-99, which removed every snapshot-backed command on those devices.Changes:
android:testOnlyfrom the snapshot and IME helper manifests.-tfrom the local install path (helper-package-install.ts) and from the release manifest'sinstallArgs.-tin the consumer allowlist so previously published release manifests still parse.Validation
pnpm build:android;aapt2 dump xmltreeconfirms neither APK carriestestOnly; release manifestinstallArgsis["install", "-r"].adb install -rsucceeds for both helpers without-t; one-shotam instrument -e mode viewportreturnsok=true.open→snapshot -i(installs both helpers without-t) →click text="Search settings"→fill @e2 wifi→ snapshot shows the field as"wifi".pnpm typecheck; unit tests forsnapshot-helper,snapshot,touch-helper,adb-transfer, andrequest-router-android-snapshot-helperpass.Not verified: ColorOS itself. We have no such device; the reporter offered to test and has been asked to build from this branch.