Skip to content

fix(daemon): key the implicit session by workspace and platform - #2587

Merged
thymikee merged 2 commits into
mainfrom
fix/2580-platform-scoped-implicit-sessions
Sep 14, 2026
Merged

thymikee merged 2 commits into
mainfrom
fix/2580-platform-scoped-implicit-sessions

Conversation

@thymikee

@thymikee thymikee commented Sep 14, 2026

Copy link
Copy Markdown
Member

Summary

An implicit session was addressed by cwd:<workspace>:default — one slot per checkout — so a repo driving both platforms could not open the second: after open --platform ios, boot --platform android from the same directory failed with INVALID_ARGS against a session it never asked for.

--platform now selects the implicit session, so one workspace holds both:

agent-device open --platform ios --device "iPhone 17 Pro"      # cwd:<hash>:ios
agent-device boot --platform android --device Pixel_9_Pro_XL   # cwd:<hash>:android
agent-device snapshot       # AMBIGUOUS_MATCH, naming both + --platform hint
agent-device session list   # still routes, so the ambiguity stays resolvable

A platform-naming request joins a session it genuinely agrees with, so a session opened without --platform stays reachable and keeps its artifact directory; a same-platform device or target mismatch keeps the existing refusal instead of forking a twin. Ambiguity is refused, never resolved by open order. Also fixes a settling interaction republishing the session under SessionState.name, which session list already showed as a phantom row.

12 files, all session-routing plus docs. References #2580, which was self-closed as premature but still reproduces on 0.21.1.

Validation

Rebased on origin/main at 1b6515b. Head 9ffcb69: pnpm check:affected --run, check:fallow --base origin/main, tsc --noEmit and oxlint pass; src/daemon is 2757 tests.

Live, built CLI, fresh checkout per case:

  • iOS opened without --platform, then open --platform android: session list shows :default and :android, bare close refuses with AMBIGUOUS_MATCH naming both, and close --platform ios / --platform android each close their own.
  • AGENT_DEVICE_PLATFORM=android AGENT_DEVICE_SESSION_LOCK=reject against an existing iOS session routes boot/open to cwd:<hash>:android and the locked snapshot returns an Android tree instead of running on iOS.
  • open --platform androidsnapshotpress @e2 --settle held session list to one address; reverting only the interaction fix reproduced the phantom default row.

Follow-up, not in scope: --session accepts address-shaped values like cwd:<hash>:ios, which can squat an implicit key (pre-existing).

@github-actions

github-actions Bot commented Sep 14, 2026

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

@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.53 MB 4.53 MB -889 B
Package (unpacked) 4.53 MB 4.53 MB -889 B
Package (download) 1.34 MB 1.34 MB -374 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.1 ms 29.7 ms +0.6 ms
CLI --help 79.5 ms 80.4 ms +0.9 ms

@thymikee

Copy link
Copy Markdown
Member Author

Two routing blockers at 9ba53df.

In session-routing.ts:213, the default-address return bypasses the ambiguity check. Open iOS without --platform, then Android with --platform android: a bare press or close now silently selects iOS instead of AMBIGUOUS_MATCH. Check all implicit candidates before preferring the default leaf, and cover default + android through the attaching-command path.

In session-routing.ts:157, candidate matching uses req.flags even when the platform came from meta.lockPlatform. With an existing iOS session and an Android lock, empty flags make iOS appear to agree. The later lock policy leaves the platform unset because a session is already bound, so the command runs on iOS. Include the effective platform in candidate matching and cover an existing opposite-platform session, not only an empty store.

Checks are green and there are no merge conflicts. The reported live runs do not cover these two combinations; both need regression proof before this is ready.

An implicit session was addressed by `cwd:<workspace>:default`, one slot per checkout, so a repo
driving both platforms could not open the second one: after `open --platform ios`,
`open --platform android` and `boot --platform android` from the same directory failed with
INVALID_ARGS against a session they never asked for. The workaround named a session by hand on
every command of both legs.

`--platform` now selects the implicit session, so `cwd:<workspace>:ios` and
`cwd:<workspace>:android` coexist. A platform-naming request joins a session it genuinely agrees
with, so a session opened without `--platform` stays reachable and keeps its own artifact
directory; a same-platform device or target mismatch still gets the existing refusal instead of
forking a twin. A request naming no platform joins the workspace's only implicit session and
refuses with AMBIGUOUS_MATCH when several exist, while `sessionKind: 'inventory'` commands keep
routing so `session list` can resolve the ambiguity.

Two adjacent repairs were load-bearing: a settling interaction published the live session under
`SessionState.name` rather than the address the store owns, which `session list` already reported
as a phantom second row; and `--session-lock`'s platform reaches `flags` only after the key is
chosen, so routing now reads `meta.lockPlatform` too.
Review found two routes #2580's routing still chose a platform by accident.

The platform-less branch returned the `default` leaf before consulting the workspace, so opening
iOS without `--platform` and then Android with it left a bare `press` or `close` on the iOS device
instead of refusing with AMBIGUOUS_MATCH. The `default` leaf is now one candidate among the
workspace's sessions rather than a short-circuit, and it is still where a lone session lands.

Candidate matching read `req.flags` only, so a request whose platform came from `--session-lock`
claimed no platform and the workspace's opposite-platform session appeared to agree with it. The
lock policy then leaves the platform unset because a session is already bound, so nothing
downstream corrected the choice. Candidate matching now sees the effective platform.
@thymikee
thymikee force-pushed the fix/2580-platform-scoped-implicit-sessions branch from 9ba53df to 9ffcb69 Compare September 14, 2026 12:52
@thymikee

Copy link
Copy Markdown
Member Author

Both fixed at 9ffcb69, rebased on origin/main at 1b6515b.

session-routing.ts default-leaf bypassresolveUnselectedSessionAddress no longer short-circuits on sessionStore.get(defaultAddress). The default leaf is now one candidate among the workspace's implicit sessions: a lone session still lands there, but a workspace holding default (iOS, opened without --platform) beside :android refuses with AMBIGUOUS_MATCH instead of silently selecting the iOS device. Covered through the attaching-command path (close) in the default+android shape.

session-routing.ts lockPlatform matching — candidate matching now reads the effective platform, not just req.flags, via withImplicitPlatform. With an existing iOS session and an Android lock on empty flags, the iOS session is seen to disagree on platform and the request resolves its own :android leaf rather than joining iOS where the lock policy would leave the platform unset. Covered with the opposite-platform session present.

Both also proven live on the built CLI:

  • iOS without --platform + open --platform androidsession list shows :default and :android, bare close refuses with AMBIGUOUS_MATCH, and close --platform ios/--platform android each close their own.
  • AGENT_DEVICE_PLATFORM=android AGENT_DEVICE_SESSION_LOCK=reject against an iOS session routes boot/open to cwd:<hash>:android and the locked snapshot returns an Android tree — not iOS.

@thymikee

Copy link
Copy Markdown
Member Author

Both routing fixes look correct at 9ffcb69. A bare command now refuses with AMBIGUOUS_MATCH when the workspace holds default next to :android, and candidate matching uses the lock platform, so an Android lock no longer joins the iOS session. The new tests fail on the old code, and your live runs cover both cases.

The failing Smoke Tests job looks unrelated. The iOS Settings replay stopped at its screenshot step after a runner restart, and open had already worked in the same session. The same job passed on main at 730b201. Please rerun it before merge. No conflicts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 14, 2026
@thymikee
thymikee merged commit 28accb8 into main Sep 14, 2026
19 of 20 checks passed
@thymikee
thymikee deleted the fix/2580-platform-scoped-implicit-sessions branch September 14, 2026 14:22
thymikee added a commit that referenced this pull request Sep 14, 2026
…tle-on-exit-2522

* origin/main:
  perf(scroll-until): answer every candidate from one visibility index (#2596)
  refactor(daemon): resolve device interactors through a composed capability (#2593)
  refactor(daemon): stop the client value-importing daemon internals (#2559) (#2594)
  refactor(shell-quote): one implementation, reached through the runner host port (#2595)
  docs(agents): delete the file-size tiers nobody enforces (#2597)
  fix(daemon): key the implicit session by workspace and platform (#2587)
  refactor(cli-schema): orient the schema layer above commands (#2543 step 3) (#2590)
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