Repository navigation
Conversation
…evice (#3674) A command that names a session this daemon does not hold used to fall through to resolveTargetDevice({}) — the session-attached command carries no device selectors, so the device was picked from the host inventory. With two devices booted the caller met AMBIGUOUS_MATCH telling it to select a device; with one it met an app-session error. Either way the answer depended on how many devices happened to be up and sent the caller after a device selector when what it lost was its session. Refuse at the seam instead: requireNamedSessionHoldsDevice throws SESSION_NOT_FOUND with typed reason named_session_not_found and dispatched=no, wired into every resolution that would otherwise choose a device (snapshot, selector runtime, recording, sessionless close). A request that never named a session stays sessionless by design, and one that names a device selector as well keeps its stated target. Provenance is traced, not inferred: namesSessionExplicitly reads meta.sessionExplicit or a non-blank flags.session, replacing the two copies of hasExplicitSessionFlag. It lives in @agent-device/kernel/ contracts, beside the meta field it reads, so every consumer already evaluates it; the guard module reaches the hot seams through a function-scoped import, keeping the eager-closure gate at parity (ADR-0019 loading shape). The refusal is pinned by a router regression test, unit tests, and a new daemon.route.named-session-refusal dispatch-disclosure row.
#3674) The batch runner stamped flags.session on every step so nested invokes route to the parent's resolved address. The wire `session` field already does that; the flag is a provenance carrier, and the new named-session refusal reads it. An unconditional stamp therefore made a sessionless batch look caller-addressed and met SESSION_NOT_FOUND at the daemon — an error no caller of the batch surface asked for, with advice ("omit --session") the surface does not even offer. The stamp now follows the same traced provenance as every other carrier: meta.sessionExplicit or a caller-set step flag. Also from the same review: the named-session disclosure row's driver now pins the producer's own code and typed reason, so a reverted guard cannot satisfy it with the device-selection error it replaced; commands.md states which pre-dispatch refusals answer session_or_device_selector_required even for a named-but-unheld session.
|
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…every seam Round-2 review found #3674's refusal reaching some callers as an untyped event. `get`/`is` answered a missing session with the generic "no active session" error before the provenance guard could run; `record start` let its app-scope INVALID_ARGS refusal win over a session that was gone altogether; `appstate` carried no typed details and refused requests that had also stated a concrete device. The refusal details now have one producer, `namedSessionNotFoundDetails` beside its sibling builders, that the guard's AppError and `appstate`'s platform-aware response both build from; each seam orders the guard before its own missing-session refusal; `appstate` preserves a request naming an identity selector, matching the resolution guard. The guard module's function-scoped import is confined to the missing-session branch it can fire on: that keeps the eager- closure gate at parity and removes a first-time transform cost from the held-session wait path, which under coverage instrumentation alone overran `wait-runtime.test.ts`'s fake-timer budget at the unconditional form (3/3 deterministic failures restored, 3/3 passes at this form). `namesSessionExplicitly` now accepts the generic flag-record shape the batch runner carries, fixing the TS2345 the batch call site raised.
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Reviewed 7c3657d. The change has a defect: Apple
This also changes what snapshot, wait, get, is, record and close answer on a device, but the PR body only cites fixture tests and a historical revert result at explicit-session-resolution.ts:50. No live run shows the issue's one-command repro, so nothing proves the real host listing is skipped. With two iOS simulators booted and Not blocking, take or leave: the guard at explicit-session-resolution.ts:59 exempts any Could the refusal live in one place in the router instead, after the locked binding resolves The cubic-dev-ai thread on the router test still applies: request-router-named-session.test.ts:112-126 asserts the code and reason but not The Coverage failure comes from this diff, not from a flake. I read the log and the head code and did not run the tests locally. I also did not check which clients send |
…selector states appstate's identity-selector exception over-preserved: a named missing session with an explicit Apple device fell through to the session-required refusal, a route with no sessionless answer. Keep the typed named-session refusal wherever the command cannot serve the stated target without a session, and let only non-Apple identity selectors keep their target. Also pins the appstate refusal's typed reason/session/dispatched details, adds the record seam's no-inventory assertion, and trims seam comments to their invariants.
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Thanks for the update. The two simple-case fixes look right, but the Apple carve-out in appstate still misses a route, so the change is not ready at 81b22ab. The check at packages/daemon/src/sessions/handlers/session-appstate.ts:68 only reads the --platform flag. Take The live repro in the PR body is still pending, and this change affects snapshot, wait, get, is, record, close and appstate on real hosts (explicit-session-resolution.ts:47). Please boot two iOS simulators and set The earlier question is still open and has no reply: could the router refuse once, after the binding resolves, when there is no held session, On the open threads: the P3 thread at #3701 (comment) is addressed, because request-router-named-session.test.ts:130 now asserts The code does not pass yet because of the appstate route. Smoke Tests were still running when I checked, so I cannot attribute any CI result yet. Their route overlaps this diff (session resolution in snapshot-session.ts and selector-runtime-backend.ts), so a smoke failure there should be treated as real. Before merge, make Apple appstate answer |
The appstate refusal reported dispatched: 'no', a value it writes itself, so the test would still pass if the handler resolved a device before answering it. The new assertion pins the behavior instead: resolving the target device never runs on the named-session path. Reordering the guard to resolve-then-refuse with the same code, message, reason and dispatched fields passes every other pin and fails only this one.
|
Both requirements are addressed at Apple appstate typed reason. Restored in Live repro (repo-built
Take-or-leave items. The single-predicate/bare-platform gap and the router-central guard with a |
This branch has not been deployed
1 participant
Summary
Closes #3674. Requests naming a missing daemon session receive
SESSION_NOT_FOUND,reason: named_session_not_found, anddispatched: nobefore unconstrained device selection. Sessionless and intentionally targeted paths remain available. Apple appstate keeps the typed refusal because it cannot answer a target without a session; non-Apple identity selectors retain their runtime path. Batch provenance follows typed caller fields.Validation
Current head:
c6be0ce23f6e5425703bacba72291d45d4b7db1a(one test-only commit over81b22ab3c).pnpm check:affected --runonc6be0ce23→ exit 0,all runnable checks passed, pre/post check headc6be0ce23with a clean tree. CI on this head is still in flight; the earlier green 19+skip lane belongs to81b22ab3cand is not claimed for this head.c6be0ce23(repo-built dist,AGENT_DEVICE_STATE_DIR=$(mktemp -d), five booted iOS sims):snapshot --session never-opened --jsonanswersSESSION_NOT_FOUND/named_session_not_found/dispatched: noin 390 ms cold (180 ms warm) with a request log holding no resolve phase, against a 2,388 ms host listing.snapshot --session never-opened --udid <A>resolves simulator A and answers the device's ownios_app_session_required— notAMBIGUOUS_MATCH;apps --session never-opened --udid <A>succeeds (832 ms). Sessionlesssnapshot --jsonstill answersAMBIGUOUS_MATCH.e8f24cdba,vitest run packages/daemon/src/server/__tests__/request-router-named-session.test.tsfailed three assertions; at headac9fb48f0the same command passed all five (historical, earlier head).not.toHaveBeenCalled()pin.