[airowe]

Summary

The CLI requires a named session on a leased remote connection, but a direct RPC, socket, or Node client caller could omit it and land on the shared default session. Against a real daemon, an unnamed leases.allocate allocated a proxy lease.

One intake helper now decides this for the socket and every HTTP builder. A request names no session when its session is missing or blank, or when meta.sessionExplicit is false with no flags.session. Admission refuses it with REMOTE_SESSION_REQUIRED when it would allocate or act through a provider lease. Lease RPC params now carry sessionExplicit, so namesSessionExplicitly moved to kernel/record to keep lease-scope's eager closure flat.

Breaking for callers: unnamed Node and MCP clients on a provider lease, and unnamed doctor and capabilities on a leased session, are refused. Six docs pages showed such clients and now name a session.

Closes #4171.

Validation

Tested fac7ed586. pnpm check:affected --run --head fac7ed586: all selected checks passed (11 passed, 7 skipped). A real-client test covers the reproduction. Not device-facing. Earlier 8c6183747 gate failures were outside this diff, cause unproven. Apple Smoke: capture-close hit the swiftc 30s timeout, also on main 7b3f63fea.

Unresolved:

• Needs maintainer sign-off on refusing meta.sessionExplicit: false.
• Released HTTP clients send no sessionExplicit on the lease RPC, so their unnamed lease calls still pass.
• The ledger uses a compatible-change ack, not a protocol bump.
• Latest review read the code at 071a06ed2 and ran nothing.

🤖 Generated with Claude Code

[vercel]

@airowe is attempting to deploy a commit to the Callstack Team on Vercel.

A member of the Team first needs to authorize it.

[cubic-dev-ai]

All reported issues were addressed across 21 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 9 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[thymikee]

Reviewed 8c61837. The refusal does not yet cover the public Node client, so I would not merge this as is.

The daemon decides "caller named no session" only from the raw session string at the socket and HTTP intake (request-admission.ts#L198). The Node client always sends the literal session default and records the omission as meta.sessionExplicit: false (client-normalizers.ts#L457). No intake marks that request, so if (!req.internal?.sessionOmitted) return; lets an unnamed client.leases.allocate({ leaseProvider: 'proxy', ... }) (lease-client.ts#L20) still allocate a lease. An unnamed Node-client command also still acts through a lease held by the session it routes to. This is the reproduction from the issue, and it still holds for MCP and plugin callers built on the client. The daemon already reads this fact through namesSessionExplicitly (contracts.ts#L268) for routing. The rule should be: a request is unnamed when its session is missing or blank, or when meta.sessionExplicit === false. Decide it once in one intake helper that sets internal.sessionOmitted, and keep admission reading only that marker. Keep the literal default string so cwd-scoped routing does not change. Please add a real-daemon test that sends a client-shaped request (session default, meta.sessionExplicit: false, provider lease meta) and expects REMOTE_SESSION_REQUIRED. This needs the maintainer sign-off the PR body asks for. The cost is limited to unnamed leased requests from pre-#4159 CLIs and Node/MCP callers, because the check returns early without a provider lease.

Not blocking, and fine to take or leave: parseSocketDaemonRequest (transport.ts#L136) repeats requestSessionFields from http-rpc-methods.ts, and toReleaseMaterializedPathsDaemonRequest is a third intake with its own ?? 'default' and no marker, so one shared helper next to DaemonRequest would carry the rule above; also COMMANDS_THAT_NEVER_READ_THE_SESSION (request-admission.ts#L178) is a hand-written command set in a request-policy module, which ADR 0008 says belongs as a daemon-facet trait exposed through daemon-command-registry.ts, pinned in the registry test.

Would one intake helper in the daemon core, called by the socket parser and every HTTP builder, be simpler? It would return the session and internal.sessionOmitted, admission would keep a single check on that marker, and the command set would become a registry trait. That is the same file count or fewer, it closes the Node client and MCP route, and it leaves no second copy of the predicate. Before that, a maintainer needs to decide that a leased request with meta.sessionExplicit: false is refused. We also need to confirm that the current CLI sends sessionExplicit: true (or flags.session) when the session comes from AGENT_DEVICE_SESSION or config. I did not verify that, and I did not run the tests. I judged the real-daemon and under-lock tests only against the pre-change code path. I also did not check in-repo Node-client callers (plugins, replay and test runners) that might start being refused, or whether replay sub-steps carry internal.sessionOmitted like batch steps.

The only failing check is Vercel, which asks for Callstack team authorization for this fork and is not a build failure. The other 13 checks, including unit and smoke, pass at this commit. There are no conflicts. Before merge, the maintainer needs to decide on the sessionExplicit: false rule, and the intake helper plus the client-shaped real-daemon test need to land.

[airowe]

c5987ee11 implements your proposed rule so you can judge the cost. It doesn't settle the sign-off on refusing meta.sessionExplicit: false; that's still yours.

What changed

• resolveRequestSessionAtIntake in the daemon core now decides this for the socket parser and every HTTP builder, including toReleaseMaterializedPathsDaemonRequest. Admission still reads only internal.sessionOmitted, and the literal default stays.
• The hand-written command set is now the ignoresSessionLease trait on the daemon facet, pinned in the registry test.
• I added the client-shaped real-daemon test you asked for.

One gap your request shape would have missed
Over HTTP, client.leases.allocate uses the agent_device.lease.* methods, not agent_device.command, and those params never carried sessionExplicit. The client-shaped test passed while a real createAgentDeviceClient still got a lease. A second test now uses the real client, and lease RPC params carry the fact. Released HTTP clients don't send it, so their unnamed lease RPC calls still pass. On that route the literal default can't be told apart from a chosen name.

Your open questions

• CLI session from env or config: sessionExplicit is true for a session from --session, AGENT_DEVICE_SESSION and config. a session named by flag, env or config never falls back onto the pointer now asserts it, on snapshot only.
• Replay sub-steps: from reading the code, replayInvokeOverDispatch re-attaches the originating request's whole private half, so the marker carries. The existing envelope test covers that merge generally, but no test asserts this marker.
• In-repo Node-client callers: the benchmark clients in scripts/ios-snapshot-benchmark already name a session. Four test sites used unnamed clients on a provider lease and now name one.

The cost is wider than old callers
The documented examples used unnamed clients: AWS Device Farm, BrowserStack and client-api.md on the client, TestMu and Revyl on the lease calls, and one Limrun release. I changed all six pages to name a session. Anyone who copied those examples would be refused, so this probably needs a release note. I didn't run those flows against the real providers.

The gate passed at c5987ee11 (11 passed, 7 skipped). It did not select provider integration. I ran that lane locally before the last test fix: 244 of 245 passed. The one failure was an unnamed client in cloud-webdriver-runtime.test.ts. After naming its session, that file passed 7/7. I didn't rerun the full lane, so CI is the check on that.

🤖 Generated with Claude Code

[cubic-dev-ai]

All reported issues were addressed across 27 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[thymikee]

The earlier findings from the first review are fixed at 0dc46ec, and I have no new code findings. I only read the code. I did not run tests or pnpm check:affected, and I did not run the eager-closure gate, so the reason for moving namesSessionExplicitly into kernel/record is unverified. I also did not confirm that every MCP tool schema exposes session, so an MCP caller that relies on AGENT_DEVICE_SESSION is likely, but not proven, to send sessionExplicit:true.

Not blocking, and you can take or leave these: namesSessionExplicitly now lives in packages/kernel/src/record.ts, which holds generic record readers, so you could keep it there or give it a small kernel subpath named for session naming (not back in kernel/contracts). The doc comment on sessionOmitted in daemon-request.ts says only intake sets it, but session-batch.ts also copies it onto batch steps, so please say both.

The iOS smoke failure looks unrelated. The smoke:capture-close step record stop timed out in xcrun swiftc while compiling the video-validator helper, and #4150 fixes exactly that. This diff touches no record or Swift code, and that route names --session on a local simulator with no lease provider, so the new admission check returns early. I did not check whether main fails the same way. The Vercel check needs Callstack team authorization for this fork and is not a build failure. There are no conflicts.

The two open P1 threads on the literal default session and the missing sessionExplicit on legacy shapes still apply. Both are disclosed compatibility choices, so they need the maintainer's call, not a code change. The other three threads are fixed at this commit and can be resolved: the client-shape test, the remote-proxy doc carriers, and flags.session counting.

Before merge, a maintainer needs to sign off on refusing meta.sessionExplicit:false on provider leases, and on leaving released lease-RPC clients unrefused under a compatible-change note.

[airowe]

071a06ed2 fixes the sessionOmitted doc comment: it now names intake and session-batch. The gate passed there (11 passed, 7 skipped).

namesSessionExplicitly stays in kernel/record, because the gate refuses the subpath. I tried a kernel/session-naming subpath locally and ran eager-closure-budgets. It failed: lease-scope, daemon-client, client and command-surface are among the entries that each grow by that one module. It also adds statements to those closures, so the split tolerance doesn't apply. Back in kernel/contracts, the gate failed at 7644201fb. Moving it would need an APPROVED_CLOSURE_ADDITIONS row, which is your call. Tell me if you'd approve one and I'll move it.

The answers to what you left unverified:

• Main: it fails the same way. Apple run 41457810383 at 7b3f63fea hit smoke:capture-close with an xcrun timeout on the same swiftc compile.
• MCP schemas: I haven't checked them either, so that claim is still unproven.

The two P1 threads: I resolved them with a reply declining the change, before your comment. They remain listed under Unresolved in the PR body, and the decision is still yours. Reopen them if you'd rather track it there.

🤖 Generated with Claude Code

[thymikee]

The code at 071a06e looks good. The change since the earlier review (0dc46ec, #4177 (comment)) only touches a daemon comment, and all 14 earlier threads are now resolved. I did not re-check those threads beyond that comment-only change, and I did not run the eager-closure-budgets check or any tests locally.

The Vercel failure is a git authorization link for the website deploy. The change touches no website code, so it is likely unrelated. The other 13 checks pass. Cubic has not reviewed this head yet.

Before merge, the Vercel team authorization needs to be done, and the maintainer decision on refusing sessionExplicit:false on provider leases is still open. That decision is unchanged and I did not re-open it here.

[thymikee]

e2e prep for this break: tester-army/e2e#1513 names the session on its lease clients (BrowserStack/TestMu AI allocate+heartbeat, Limrun allocate+install). Checked on a real daemon at 3707e62: an unnamed client is refused with REMOTE_SESSION_REQUIRED, and a named one can allocate and heartbeat. The same change also works on 0.21.35.