refactor(android): drop the snapshot helper manifest installArgs - #2618
Merged
Merged
Conversation
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.
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
Reviewed at 2a14a6d, and I found no problems. Smoke Tests was still running at review time, not failed. There are no conflicts. |
thymikee
added
the
ready-for-human
Member
Author
|
Smoke Tests has now failed on 2a14a6d in the live iOS simulator fixture run: |
|
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
…ll carried #2618 removed installArgs from AndroidSnapshotHelperManifest and left the annotated fixture naming it, so origin/main fails its own Typecheck job (CI run 34944609119). The reader ignores the field on older manifests; a fixture of a current manifest has no reason to carry it.
This was referenced Sep 15, 2026
Closed
Merged
fix(host-kit): a killed command settles on its exit, and background exec cannot take a timeout
#2599
Merged
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
#2619 copied the snapshot helper manifest fixture after #2618 deleted `installArgs` from AndroidSnapshotHelperManifest, so the new fixture failed typecheck on main: the field is parsed by no one and the leniency case is already covered in snapshot-helper.test.ts. Repair for the breakage on main (2cafab3), unrelated to the settings vocabulary refactor it blocks here.
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
…tion (#2625) * refactor(settings): derive the permission vocabulary from one declaration packages/contracts/src/settings.ts now declares the settings permission actions, modes, app-scoped targets and macOS targets once, and the two parsers, the settings help fragments and its invalid-args message, the public client permission types, and the CLI's membership sets are built from those collections. Accepted names, normalization, error strings, help ordering and daemon positionals are unchanged; no surface widened what it accepts. The macOS action list stays a literal because it is a support fact owned by platform-apple. Part of #2614. * fix(android): drop the manifest installArgs the type no longer declares #2619 copied the snapshot helper manifest fixture after #2618 deleted `installArgs` from AndroidSnapshotHelperManifest, so the new fixture failed typecheck on main: the field is parsed by no one and the leniency case is already covered in snapshot-helper.test.ts. Repair for the breakage on main (2cafab3), unrelated to the settings vocabulary refactor it blocks here.
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
…ll carried #2618 removed installArgs from AndroidSnapshotHelperManifest and left the annotated fixture naming it, so origin/main fails its own Typecheck job (CI run 34944609119). The reader ignores the field on older manifests; a fixture of a current manifest has no reason to carry it.
thymikee
added a commit
that referenced
this pull request
Sep 15, 2026
…2613) * feat(record): report whether the recorder stopped beside the export ADR 0024 keeps two facts independent: whether a playable export exists, and whether the recorder has stopped. Only the first was answered; a stop that discards its recorder's fate still looks identical to one that watched it exit. Add `StopObservation` to contracts and have every backend's finish state its observation: `confirmed` for an exit or an acknowledged stop, `unconfirmed` for an unreadable probe or a missing exit, `lost` for a proven identity mismatch or a dead owning session. Record the disposition of the recorder's own artifact path beside it — `pending`, `retirable`, or `retired` — where a backend has a native path distinct from the export. The observation and disposition ride the completion, the durable manifest, the CLI/Node/MCP stop result (both optional, required list unchanged), and the docs. An invalidated Apple recording reports `lost` rather than claiming a stop that session could no longer have signalled. Both modules stay off the eagerly evaluated recording façade, which lists runtime exports over its sources. No stop changes outcome here: every field is additive and nothing yet decides a retry, a retention, or a deletion from it. * fix(android): drop the retired installArgs key the helper fixture still carried #2618 removed installArgs from AndroidSnapshotHelperManifest and left the annotated fixture naming it, so origin/main fails its own Typecheck job (CI run 34944609119). The reader ignores the field on older manifests; a fixture of a current manifest has no reason to carry it. * fix(android): answer a completed recording's disposition from the device The completed marker was written with nativePathDisposition 'retirable' a step before the chunks were removed, while the live result and the daemon manifest said 'retired', so the same recording reported two dispositions and a replay after a daemon restart claimed a retirement that had already happened. The disposition is now decided in one place, by looking at the device: the stop answers with it after disposal, and a replay of a completed marker re-reads that one field instead of replaying a value frozen before the removal. A device that reports a removal it did not perform stays owed. The marker keeps its pre-disposal value because publishing it before cleanup is what keeps an earned completion across a crash. Mutations proven to fail: serving the marker value unchanged fails the crash-replay and both pid-reuse replays; hardcoding 'retired' after cleanup fails the kept-artifact case. * docs(record): state which recorder and path states a stop can report today The docs, help, changelog and MCP descriptions read as if every declared state had a producer: no backend reports an unconfirmed recorder, an identity mismatch, or a pending path, so a reader expecting an export labelled unconfirmed got an error instead. Each surface now names what a stop emits today — confirmed, lost for an invalidated Apple recording, and retirable or retired for the recorder's own path — and says the rest of the vocabulary arrives with the ADR 0024 steps that gain those probes. The producer inventory itself now lives on the two vocabulary types, where the next reader meets it before writing another copy. * chore(gates): publish the two recording fact modules from contracts Adds the ./recording-stop-observation and ./recording-native-path subpaths the daemon, Android, Apple, capture-kit, and MCP surfaces read, and regenerates the contracts export snapshot package-boundaries diffs against. * fix(android): read a path's absence only from a probe that answered * test(mutation): repoint the indirect-ownership pin at a chain that still exists #2499 took `scroll-edge-state` out of the interaction runtime's imports, which left the indirect-ownership example naming a test that no longer reaches that kernel, and the model gate only noticed when a recording change made it runnable again. `back-runtime.test.ts` reaches edge detection through the daemon without importing the kernel file, so the omission this assertion exists to catch is still asserted.
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
Follow-up to #2603 / #2364. The reporter confirmed on ColorOS that
0.21.3installs both helpers with plainadb install -r, which leaves the snapshot helper manifest'sinstallArgsas a constant["install", "-r"]. The field only ever existed to carry-t. Its consumer chain parsed the array, validated it against a four-flag allowlist, lowered it to semantic options, and lowered those back to flags at the provider. None of that carried information any more.Changes:
installArgsfrom the manifest type, the parser,scripts/package-android-helper.sh, and the README contract. The parser ignores the field on older manifests (test added).replace: true, the same path the IME helper already uses. ThereadAndroidSnapshotHelperInstallOptionsexport and its option type are gone.installcapability toreplace. The-t,-d,-goptions had no producer left; the type is internal to the platform package, not SDK-exposed.Other helpers: the IME helper manifest never had
installArgs, and the iOS runner manifest has no install-args analogue, so nothing to do there.Not in this PR: the reporter's second finding (first helper install on ColorOS blocks on a system confirmation dialog, and the adb timeout hint then blames a wedged server) is a separate hint change.
Validation
pnpm typecheckpasses.snapshot-helper,adb-transfer,adb-executor,touch-helper,touch-helper-session,ime-helper,request-router-android-snapshot-helper,android-snapshot-capture-failure-reason— 97 tests pass.pnpm check:production-exportsreports no hits in touched files.android/snapshot-helper/distmanifest (untracked) still carriesinstallArgs; the parser ignores it, so no rebuild is needed for source checkouts.