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.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/daemon/src/server/__tests__/request-router-named-session.test.ts">
<violation number="1" location="packages/daemon/src/server/__tests__/request-router-named-session.test.ts:112">
P3: This test can pass even if `record start` lists host devices before returning the refusal. Assert `listings.count` is zero to pin the pre-resolution behavior for this seam.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if (response.ok) return; | ||
| expect(response.error.code).toBe('SESSION_NOT_FOUND'); | ||
| expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); | ||
| expect(response.error.details?.session).toBe(LOST_SESSION); |
There was a problem hiding this comment.
P3: This test can pass even if record start lists host devices before returning the refusal. Assert listings.count is zero to pin the pre-resolution behavior for this seam.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/daemon/src/server/__tests__/request-router-named-session.test.ts, line 112:
<comment>This test can pass even if `record start` lists host devices before returning the refusal. Assert `listings.count` is zero to pin the pre-resolution behavior for this seam.</comment>
<file context>
@@ -90,6 +96,39 @@ test('the refusal answers without listing the host devices at all', async () =>
+ if (response.ok) return;
+ expect(response.error.code).toBe('SESSION_NOT_FOUND');
+ expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound);
+ expect(response.error.details?.session).toBe(LOST_SESSION);
+ expect(listings.count).toBe(0);
+});
</file context>
| expect(response.error.details?.session).toBe(LOST_SESSION); | |
| expect(response.error.details?.session).toBe(LOST_SESSION); | |
| expect(listings.count).toBe(0); |
|
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.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts">
<violation number="1" location="packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts:244">
P3: This checks only the refusal's self-reported `dispatched` value, so the test would miss resolving a device before returning it. Also assert `mockResolveTargetDevice` was not called to pin the pre-selection refusal.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| expect(response.error.message).not.toMatch(/omit --session/i); | ||
| expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound); | ||
| expect(response.error.details?.session).toBe('sim'); | ||
| expect(response.error.details?.dispatched).toBe('no'); |
There was a problem hiding this comment.
P3: This checks only the refusal's self-reported dispatched value, so the test would miss resolving a device before returning it. Also assert mockResolveTargetDevice was not called to pin the pre-selection refusal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/daemon/src/sessions/handlers/__tests__/session-appstate-input.test.ts, line 244:
<comment>This checks only the refusal's self-reported `dispatched` value, so the test would miss resolving a device before returning it. Also assert `mockResolveTargetDevice` was not called to pin the pre-selection refusal.</comment>
<file context>
@@ -239,6 +239,9 @@ test('appstate with explicit missing session returns SESSION_NOT_FOUND', async (
expect(response.error.message).not.toMatch(/omit --session/i);
+ expect(response.error.details?.reason).toBe(PRE_DISPATCH_REFUSAL_REASONS.namedSessionNotFound);
+ expect(response.error.details?.session).toBe('sim');
+ expect(response.error.details?.dispatched).toBe('no');
}
});
</file context>
| expect(response.error.details?.dispatched).toBe('no'); | |
| expect(response.error.details?.dispatched).toBe('no'); | |
| expect(mockResolveTargetDevice).not.toHaveBeenCalled(); |
|
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 |
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. Held-session waits avoid the missing-session guard import.
Validation
Current head:
81b22ab3c3b301fb482eda3628686c7bb5567d2e.pnpm check:affected --runis queued after its clean-tree/SHA precheck. No full-gate pass is claimed.e8f24cdbafailed three assertions; earlier fixed headac9fb48f0passed all five. This is not current-head gate evidence.