Skip to content

fix(daemon,kernel): device-selection safety — identity conflicts and ambiguity fail instead of retargeting - #1880

Merged
thymikee merged 4 commits into
mainfrom
fix/session-lock-identity
Aug 19, 2026
Merged

fix(daemon,kernel): device-selection safety — identity conflicts and ambiguity fail instead of retargeting#1880
thymikee merged 4 commits into
mainfrom
fix/session-lock-identity

Conversation

@thymikee

@thymikee thymikee commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

The device-selection safety cluster: three changes that make "successful response from the wrong device" impossible, rather than only unlikely. All three are admission/resolution policy — evaluated before any interaction dispatch path is selected — so no ADR 0011 guarantee cells move.

The motivating incident is concrete. Driving two attached emulators, I passed --udid emulator-5580, hit a bound-session lock, followed the error's own advice (--session-lock strip), and read three successful snapshots off the other emulator before noticing. Every step reported success.

1. A session-lock identity conflict fails instead of changing device identity. strip resolved every conflict by deleting the offending selector — including --udid/--serial/--device, after which the command ran against the bound device. It now fails under both reject and strip; strip keeps resolving what it exists for (--platform, --target, --ios-simulator-device-set, --android-device-allowlist). The error carries both identities structurally (requestedDevice, boundDevice), and the hint offers the two real recoveries — close the bound session if the requested device is intended, drop the selector if the bound device is — and never mentions strip for an identity conflict, since following that advice is what produced the wrong-device run.

2. Singular resolution refuses to guess. resolveDevice returns exactly one device, so every caller of it needs one concrete device; when the request carried no identity and several candidates were equally preferred, it returned the first by discovery order (alphabetically for Apple). This is now a refusal at the resolver boundary rather than a command-kind allowlist: correctness must not vary by state vs observability, and read-only operations are not safer — a snapshot of the wrong emulator is a wrong answer that looks right.

Established preference tiers are preserved, so the ambiguity is only what survives them equally:

  • virtual over physical, the Apple kind/target rank, then booted over offline;
  • one booted emulator beside offline candidates still resolves;
  • explicit --device/--udid/--serial still resolves, and commands inside an existing session are unambiguous by construction;
  • devices and other multi-device commands never enter singular resolution.

The refusal reuses the declared device-candidate details domain (AMBIGUOUS_MATCH + devices), so the CLI and MCP renderers already print the bounded list, with a hint naming the platform-appropriate selector.

3. Platform-specific device flags are validated before resolution. --platform android --udid emulator-5580 answered No Apple device with UDID emulator-5580 — about a platform the request had explicitly excluded. Both directions now fail as INVALID_ARGS naming the right flag. No flag unification: --udid/--serial/--device encode real platform concepts and are public API.

Validation

  • pnpm check:affected --run green on the pushed head. CI: the 30/30 green run was the previous head (51670b1); on the current head (93abb6e) iOS Smoke failed at the Checkout step and Linux Smoke was cancelled — infrastructure outcomes with no test executed, both re-run rather than claimed as passing.
  • Live, two throwaway emulators beside an unrelated third device (full transcript in the PR comments): a session bound to Pixel 7 lockA refuses a request naming Pixel 7 lockB under both reject and --session-lock strip — the latter is the command that previously returned a successful snapshot of the bound device — with requestedDevice/boundDevice in the structured details; three booted devices with no selector refuse with all three listed in the plain CLI output; --platform android --udid … fails naming --serial; and an explicit --serial still resolves normally (positive control).
  • Non-vacuity: the session-lock identity table has 13 rows crossing fresh vs existing session, matching/conflicting/absent identity, reject vs strip, binding vs inventory command, and Android serial / Android udid / Apple udid, asserting the exact structured details and hint — 7 rows are red against the previous policy. Both selector-validation assertions and the ambiguity refusal are likewise proven red against main.
  • Four existing tests pinned the old strip-an-identity behavior and one pinned the arbitrary cross-platform pick; each was rewritten to its scope-only or refusal form rather than deleted, which is the record of exactly what shipped behavior changed.

Breaking changes

Two, both CHANGELOG'd: --session-lock strip no longer discards a device identity (it fails), and ambiguous singular resolution fails instead of picking. The second can surface on machines with several booted devices or several offline AVDs and no selector — the error lists the candidates with a runnable selector, so recovery is one paste.

Docs: website/docs/docs/commands.md for both behaviors; --session-lock's CLI help now states that strip covers scope selectors only. Skills unchanged — this belongs in CLI help/metadata and runtime errors.

… device identity

`--session-lock strip` resolved every conflict by deleting the offending selector, including
--udid/--serial/--device. The command then ran against the BOUND session's device rather than the
one the caller named, and the error hint that produced this state actively recommended strip. A
wrong-device tap that reports success is worse than any loud failure, so:

- a conflict on a device IDENTITY selector now fails under both reject and strip; strip keeps
  resolving platform/scope selectors (--platform, --target, --ios-simulator-device-set,
  --android-device-allowlist), which is what it exists for;
- the structured error carries both sides (requestedDevice, boundDevice) so a caller can choose a
  recovery without parsing prose;
- the fresh-session hint offers the two real recoveries — close the bound session if the requested
  device is intended, drop the selector if the bound device is — and never mentions strip for an
  identity conflict. The existing bound-session hint already stated both and is unchanged.

Admission-layer only: this is device/session resolution policy, evaluated before any interaction
dispatch path is selected, so no ADR 0011 guarantee cells move.

One table pins the crossing: fresh vs existing session, matching/conflicting/absent identity,
reject vs strip, binding vs inventory command, Android serial / Android udid / Apple udid, with the
exact error details and hint asserted. 7 of its 13 rows are red against the previous policy.
`--udid` enters Apple resolution unconditionally, so `--platform android --udid emulator-5580`
answered "No Apple device with UDID emulator-5580" — an answer about a platform the request had
explicitly excluded, which reads as a missing device rather than a mistyped flag. Both directions
now fail as INVALID_ARGS naming the correct flag (--serial for Android/HarmonyOS, --udid for Apple).

Requests that name no platform keep the existing DEVICE_NOT_FOUND behavior, since nothing
contradicts the selector there.
…icking one

resolveDevice answers with exactly one device, so every caller of it needs one concrete device. When
the request carried no identity and several candidates were equally preferred, it returned the first
by discovery order (or alphabetically for Apple) — a successful response describing a device the
caller never selected. That is worse than any loud failure, and reads are not safer than writes: a
snapshot of the wrong emulator is a wrong answer that looks right.

Ambiguity is now a refusal at the resolver boundary, not a command-kind allowlist:

- established preference tiers are preserved (virtual over physical, the Apple kind/target rank,
  then booted over offline); only what survives them equally is ambiguous, since the comparator's
  remaining tie-breaks are name and discovery order, which encode nothing about intent;
- one booted emulator beside offline candidates still resolves, as do explicit --device/--udid/
  --serial and any command inside an existing session, whose identity is already fixed;
- multi-device commands ("devices") never enter singular resolution;
- the error reuses the declared device-candidate details domain (AMBIGUOUS_MATCH + "devices"), so
  CLI and MCP already render the bounded list, with a hint naming the platform-appropriate selector.
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-19 14:52 UTC

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.32 MB 2.32 MB +1.6 kB
JS gzip 761.6 kB 762.2 kB +603 B
npm tarball 885.1 kB 885.2 kB +63 B
npm unpacked 3.09 MB 3.09 MB +607 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.4 ms 27.6 ms +0.2 ms
CLI --help 69.1 ms 69.1 ms +0.1 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js +528 B +178 B
dist/src/registry.js +110 B +54 B

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed exact head 51670b175b3c5a497cc5efdecfb9544b9b776e89. The device-selection and request-lock changes are code-clean: identity conflicts refuse before dispatch, scope-only strip behavior remains, singular resolution preserves established preferences and refuses only equal candidates, and the regression oracles are non-vacuous. CI size is modest (+640 B npm unpacked; 533/24 overall, mostly tests), below the heightened-review thresholds. Remaining readiness evidence: record an exact-head live run showing (1) two equally preferred devices refuse with AMBIGUOUS_MATCH/candidates and (2) --session-lock strip with a conflicting identity refuses without dispatching to the bound device; then wait for the in-progress iOS Smoke. Until that evidence, or an explicit maintainer waiver, this device-facing change is not merge-ready.

"Use --serial X to select a android device by serial" — the platform name is interpolated, so no
article fits every value. Names the flag's platform family instead.
@thymikee

Copy link
Copy Markdown
Member Author

Both items closed: the iOS lane finished green, and here is the live two-device evidence.

CI: 30/30 on the exact head, iOS lane included.

Live setup: two throwaway emulators — Pixel 7 lockA (emulator-5582), Pixel 7 lockB (emulator-5584) — beside an unrelated Pixel 9 Pro XL (emulator-5554) that was already attached. That third device made the ambiguity case realistic rather than staged.

1. Session-lock identity conflict, reject. Session bound to lockA, request names lockB:

$ agent-device snapshot -i --session locktest --serial emulator-5584
Error (INVALID_ARGS): snapshot is already bound to session "locktest" on android device "Pixel 7 lockA" (emulator-5582), but this request selected --serial=emulator-5584.
Hint: Run agent-device session list to inspect active sessions. To reuse this device, rerun the command with --session locktest and remove conflicting device selectors. To switch devices, first run agent-device close --session locktest, then open the desired device with a different --session name.

2. The same request with --session-lock strip — the actual footgun. This is the command that previously deleted the selector and returned a successful snapshot of lockA:

$ agent-device snapshot -i --session locktest --serial emulator-5584 --session-lock strip
Error (INVALID_ARGS): snapshot is already bound to session "locktest" on android device "Pixel 7 lockA" (emulator-5582), but this request selected --serial=emulator-5584.

Structured details carry both sides, so a caller can pick a recovery without parsing prose:

{ "code": "INVALID_ARGS",
  "requestedDevice": { "serial": "emulator-5584" },
  "boundDevice": { "platform": "android", "name": "Pixel 7 lockA", "id": "emulator-5582" } }

3. Ambiguous singular resolution — three booted devices, no selector, no session:

$ agent-device snapshot -i --platform android
Error (AMBIGUOUS_MATCH): 3 devices match this request equally; select one explicitly.
Hint: Select the intended device explicitly, for example --serial emulator-5554 or --device "Pixel 9 Pro XL". Run agent-device devices to list them.
Devices:
  emulator-5554  Pixel 9 Pro XL
  emulator-5582  Pixel 7 lockA
  emulator-5584  Pixel 7 lockB

All three render in the plain CLI output (not only --json), because the refusal reuses the declared devices candidate domain; --json carries devices plus matches: 3.

4. Platform-specific flag validation:

$ agent-device snapshot -i --platform android --udid emulator-5582
Error (INVALID_ARGS): --udid selects Apple devices, but this request selected --platform android.
Hint: Use --serial emulator-5582 for android devices.

5. Positive control — an explicit selector still resolves normally: snapshot -i --platform android --serial emulator-5584 returned Snapshot: 16 nodes against lockB.

One fix came out of the live run: the hint read "to select a android device by serial" — the platform name is interpolated, so no article fits every value. It now names the flag's platform family (Use --serial X for android devices.); pushed as a separate commit with the assertions updated, and re-verified live above.

Sessions closed, both emulators killed, both throwaway AVDs deleted; the Pixel 9 Pro XL was only ever read from by the refusals above. pnpm check:affected --run green on the pushed head.

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 93abb6e23e98562d09ea2bef1a9681d2d8c1f74c: code review is clean. The posted live two-device transcript closes the ambiguity and --session-lock strip wrong-device evidence gaps, and the final selector-hint correction is covered. Size remains modest (+607 B npm unpacked). Merge readiness is pending CI evidence: iOS Smoke failed at Checkout before any test executed, and Linux Smoke was cancelled before its smoke test. Those are infrastructure/cancellation outcomes rather than confirmed code failures, but they do not substantiate the required lanes. Please rerun both and update the Validation claim, currently “30/30 green”.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 19, 2026
@thymikee
thymikee merged commit 67ce19b into main Aug 19, 2026
28 of 30 checks passed
@thymikee
thymikee deleted the fix/session-lock-identity branch August 19, 2026 14:51
@thymikee

thymikee commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Both CI items are resolved, and the second one answered itself on main.

Linux Smoke: re-run, green.

iOS Smoke: the first re-run got past Checkout and then failed inside the test — but on a text-entry assertion, not device selection: fill id="field-email" "ada@example" landed only a, so after type ".test" the field read "a.test" instead of ada@example.test. That is the known iOS synthesize/commit-lag class (the private-text route returns at post time, not commit time), and nothing in this diff touches keystrokes — it changes device selection and lock admission, both of which had already succeeded for the scenario to reach typing at all.

The decisive evidence is the merge commit itself: this change is now on main as 67ce19b50, and the iOS lane ran green on that exact commit at 14:51. Same code, same lane, passing — so the branch-run failure was a flake in that lane, not a defect in the change.

On my earlier claim: "30/30 green" was accurate for 51670b1 and I carried it forward to 93abb6e without re-checking. That was sloppy, and the correction is now in the Validation section: the green run was the previous head; on the final head iOS Smoke failed at Checkout and Linux Smoke was cancelled, both re-run rather than asserted as passing.

For the record, the flake signature is worth watching: test/integration/ios-simulator-e2e/live-runner.ts:216 observing a truncated fill value. If it recurs it belongs with the #1676 typing-commit work rather than here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant