Browser: a subagent's tab is read-only, and says so before you try (SD-8) - #260
Merged
Conversation
…ey (SD-11)
`POST /agent/cancel`, `POST /interrupt`, `POST /agent/continuation/abandon`
and `POST /agent/continuation/recover` began with an unconditional
`is_user_action` check. A daemon started with no user-action key — every
`biorouter serve` daemon (SD-7), and a hand-run `biorouterd` — can never pass
it, so the browser's Stop button, mid-turn steering and Stop-and-Send answered
403 on every chat, public ones included (measured 2026-09-11 from the page).
On such a daemon the four routes now gate through `authorize_agent_control`,
the same call `/agent/stop` makes there: the reach rule, then no subagent's
chat. The refusal protected nothing — `/agent/stop` already cancels the same
turn for the same caller, `workspace_close { scope: "turn" }` does it for a
model, and `/reply` already puts the caller's text in front of the chat.
- A keyless steer is recorded unstamped, never `UserDirect`: nothing on that
daemon can establish that a person typed it, and `/reply` records the same
caller's message the same way.
- A daemon that holds a key (the desktop app's) is unchanged: proof only.
- A subagent's chat stays refused on a keyless daemon, and
`refuse_subagent_unless_user` now says the daemon, not the caller, is what
cannot prove a person acted (SD-8).
`tests/turn_control_no_user_key.rs` is its own binary because the digest is
a process-global OnceLock. All seven of its tests fail on the previous code;
one of them pins the premise (keyless `/agent/cancel` admits exactly what
keyless `/agent/stop` admits). The session_reach ordering census gains rows
for the three routes and new over-read controls in reply.rs, since
`interrupt` is no longer ungated.
Records the ruling the previous commit implements: on a daemon that holds
no user-action key, /agent/cancel, /interrupt and the two continuation
routes admit exactly the callers /agent/stop already admits there, a
subagent's chat excepted; a keyless steer is unstamped; a daemon that
holds a key is unchanged.
The record answers privacy-tiers §3.1's two questions in writing, and its
Q2 table records one finding rather than a guard: POST
/active_work/{id}/cancel cancels a subagent by registry id with no gate on
any daemon (the residual session_reach.rs already records).
- programmatic-session-access.md: the three routes honour the capability
header on a keyless daemon; the "different instrument" row now says the
proof applies on a daemon that holds a key; the troubleshooting entry no
longer claims a capable caller never sees a "no user-action key" refusal
(a subagent's session produces one).
- browser-access.md: Stop, steering and Stop-and-Send work in an ordinary
browser chat; a subagent's tab is read-only and does not yet say so.
- CLAUDE.md: an SD-11 bullet in "Browser access".
Numbered SD-11 because open PRs #226 and #229 both claim SD-9.
One conflict, in docs/deployment/serve-decisions.md: main added SD-9 (the launch token works until the daemon stops, #226) where this branch added SD-11. Both kept, in numeric order. CLAUDE.md's "SD-1..SD-9" range now names SD-11 too. main changed none of reply.rs, agent.rs or session_reach.rs since this branch's base; its auth.rs change (#231, the throttle) leaves user_action_proof and its three verdicts untouched.
…er or Stop (SD-8) On a `biorouter serve` daemon the proof that a person acted can never be produced (SD-7), and a delegated subagent's chat is where that proof decides everything: the daemon refuses every write to it from an unproven caller — `/reply`, the four turn-control routes SD-11 admits elsewhere, `/agent/stop`, the extension routes. Those refusals are the design. What SD-8 asks is that the interface say so before the click, as the approval card already does. So in a browser, and only for a subagent's chat, the composer is not mounted at all and one line takes the header Stop's place. Not disabled — absent: Send and steer are not the only writes the composer holds, and a greyed-out Send beside a live Stop-and-Send, continuation banner and extension picker would still be a row of controls that fail on click. The tab decides this at mount, from the badge the daemon's own workspace frame put on it, and not from either read of the chat. In a browser those reads queue behind the page's open event streams — six connections per origin, one stream per observed tab — and with a subagent running the refused `/agent/resume` alone took 4.8 s while the session read was still pending five seconds later. All of that is the running window, which is exactly when the ordinary composer was offering a Stop that could only be refused. The transcript's 45-second nudge moves with the composer it names: with no composer to stop from, it keeps the reassurance and drops the half that would send the reader to a control that is not there. Nothing on the daemon changes, and the desktop is untouched: `isBrowserSurface()` answers false there, so every path above is the one it already had.
…of not loading
Measured against a real `biorouter serve`: opening a delegated subagent's tab
rendered "Could not load this chat" over the daemon's own refusal — including
the tab the daemon itself opens to show a subagent it has just spawned, while
that subagent was running. `refuse_subagent_unless_user` refuses a subagent's
chat to any caller that cannot prove a person acted, `/agent/resume` is one of
the routes behind it, and the renderer loaded every chat through resume.
Reading it was never refused: `GET /sessions/{id}` and `GET /sessions/{id}/events`
both answered 200 for the same chat in the same second. So on the browser
surface, and only for a subagent's chat, the transcript comes from the session
read — the read `useSubagentSession` already makes — and the observer feed
follows it while it runs.
Everything the resume path does next is left out on purpose, because each piece
is refused or worse here: no second resume, no rejoin (it re-POSTs `/reply`),
and `agentReady` stays false so nothing reads AGENT state —
`/agent/callable_tool_count` answers through `get_or_create_agent`, and on a
miss it would mint a bare placeholder agent under the child's session id.
The fallback is confined to that one case. The desktop never takes it, a chat
that is not a subagent's never takes it, and a chat this caller may not read at
all still reports the resume's own refusal exactly as before.
SD-11 recorded the tab as an open question: a composer, a steer and a Stop that all refuse, with SD-8 requiring them to say so first. Measuring it found the tab worse off than the record said — it did not open at all — so SD-8 now carries the whole case: which routes refuse a subagent's chat and why the tab can only be read; that the transcript comes from the session read and its event feed, which the daemon already admitted; that nothing reads AGENT state, because `/agent/callable_tool_count` would mint a placeholder agent; and why the tab decides from the daemon's own badge rather than from either read. The browser guide's row said "the tab does not yet say so before you try", which is what changed. The subagents guide told every reader to type into the tab and press Stop; it now scopes that to the desktop, where both work.
…aemon SD-11 admitted four turn-control routes to the reach gate on a daemon that holds no user-action key, on one argument: the same caller already stops that turn through `/agent/stop`, and already puts text in front of that chat's model through `/reply`. For `/agent/cancel` and the two continuation routes that holds — their keyless arm calls literally the function `/agent/stop` calls, and `/agent/stop` is strictly more destructive, since it cancels the turn *and* evicts the agent. For `POST /interrupt` it is false. `/reply` takes the BR-33 single-turn lock and answers 409 whenever a *different* turn is already running in that chat; `/interrupt` answers 409 when none is. The preconditions are disjoint, so in the exact state where a steer lands, the route said to dominate it is refused. What was admitted is therefore new: attacker-chosen text injected into a turn already in flight, without cancelling it, which the person watching sees as their own turn changing direction. Cancel-then-reply, the nearest thing a caller holding the daemon secret already has, kills the turn first and is visible. So `/interrupt` moves off `authorize_turn_control` onto its own `authorize_steer` and keeps the proof on both kinds of daemon, which is what `main` had. Its body returns to `main`'s with it: a steer is only ever `Proven`, so it is stamped `UserDirect` unconditionally again and `TurnControlAuthority` is gone. The keyless refusal is a 403 carrying the new `STEER_NO_KEY` and is deliberately NOT empty. `biorouter session attach` tells a daemon that wants the proof apart from one that cannot check it by whether a turn-control 403 has a body (`session_watch::key_verdict`, the CLI follow-up stacked on this branch): empty means "this daemon holds a key", and the terminal prompts for one. On a keyless daemon that prompt would ask a `biorouter serve` user for a credential that does not exist and then report it as the wrong key. The sentence keeps the empty 403 meaning exactly what that reading needs. The gate asks nothing about the chat, so the refusal is byte-for-byte identical for a public chat, a private one, a subagent's and an id that does not exist — which keeps a route no proof can satisfy from becoming a per-id oracle. It was not: before this change a public chat answered 409 and a private one 403. Fail-before, `--test turn_control_no_user_key`: 7 passed, 2 failed — `a_keyless_daemon_refuses_the_steer_it_admits_the_stop_for` measured **202** with the text on the running turn's queue, and `a_keyless_steer_refusal_says_the_same_thing_about_every_chat` measured the per-chat divergence. 9/9 after. The census follows: `/interrupt` comes off `session_reach`'s gated list, because it never reaches that gate on either daemon, and goes back to being one of the file's negative controls. `privacy_guard_wiring`'s two `what:` strings are description-only; no count moves.
`read_user_action_digest` has a 2-second bounded read of stdin, and a desktop
launcher slow enough to miss it leaves the *desktop* daemon keyless. That was
survivable while a keyless daemon simply refused every control that needed the
proof — Stop answered 403 and the user complained. With SD-11 admitting Stop,
Stop-and-Send and the continuation routes there, the same misconfiguration
degrades quietly to a weaker gate instead of visibly breaking, so it has to be
reported.
The read now returns `Result<[u8; 32], NoUserActionKey>` with four arms rather
than one undifferentiated `None`: `HandStarted` (stdin is a terminal),
`NoneOffered` (stdin closed empty — `biorouter serve`'s `Stdio::null()`, SD-7),
`TimedOut` (a writer held the pipe open and wrote nothing inside the bound) and
`Malformed` (not a 32-byte hex digest). The last two are launcher faults —
nothing this repo ships does either on purpose — so their warning says so and
says to restart, while the two expected ones deliberately do not, or the line
would cry wolf on every `serve` start. Every arm names both consequences: what
this daemon refuses, and what it now admits instead.
Classification is split out of the I/O into `classify_digest_line`, because four
arms that are never told apart are the defect this commit is fixing;
`keyless_report_tests` covers the mapping and asserts each warning names both
consequences.
⚠ The 2 s bound is UNCHANGED. Nothing measured says the desktop launcher misses
it, and what was missing was the report rather than the time.
On evidence: this one's fail-before is a measurement of the old code, not a red
test. The whole of what a keyless daemon said before this commit was
"no user-action key on stdin: this daemon will refuse every request that
raises a session's privacy capability, including one made by the person at
the keyboard"
which names neither turn control nor which of the four causes happened. A red
test would have to reference the enum this commit introduces, so it could only
fail to compile; the new tests are drift guards and are not offered as
fail-before evidence.
…refuses (SD-8) An adversarial review of #260 found the shipped SD-8 record asserting that "the daemon refuses every write" to a subagent's chat while `POST /agent/update_working_dir` was one it did not. That route repoints the named chat at a directory of the caller's choosing and restarts its agent there — its own comment says so — and it consulted exactly one gate, `session_reach`, which is the PRIVACY slice and is deliberately inert for a public session. A delegated child's chat is normally public, so on a `serve` daemon an unproven caller could repoint a running child. The two 409s the route already had are not this boundary and must not be mistaken for it: `try_update_working_dir_if_empty` refuses a chat that has messages, and the turn lock refuses one that is busy. A just-spawned or queued child is neither, which is exactly the window in which a subagent's tab is interesting. Gated with `refuse_subagent_unless_user`, AFTER the reach gate and BEFORE the turn lock, so the three refusals keep the order the rest of the file uses: a chat the caller may not reach is refused without disclosing that it is a subagent's, and a subagent's without disclosing whether it is busy. Not `authorize_agent_control` (a second `session_reach`) and not `read_update_session` (whose read failure is a 500, where this route documents a 404). Fail-before, with the test in place and the gate absent: the public arm answered 400 rather than 403 — and the first draft of that test earned the 400 by naming a directory that does not exist, which is why it now posts `std::env::temp_dir()` and reads the working directory back. The **public** arm is the load-bearing one; the private arm was already refused by `session_reach` for a reason that has nothing to do with subagents. The claim is corrected as well, because gating one route does not make "every write" true: the sentence becomes an enumeration, and SD-8 now names the eight other session-addressing writes that reach a child's row and ask nothing about it — `DELETE /sessions/{id}` first, since it cancels the child's in-flight turn before deleting and is therefore a Stop by another name. All predate this record; what SD-8 is about is the tab's own controls, and a claim about the API surface as a whole would be #47's to make.
…not the renderer's restraint #260 stopped the renderer calling this route for a subagent's chat, because it answers through `get_or_create_agent` and would mint a bare placeholder agent under the child's session id. The review's point is that routing a client around an ungated route leaves the route ungated: it had no gate of any kind — not `session_reach`, not the subagent refusal — so any caller holding the daemon secret could name any chat and have an agent materialised for it, with the route's own 424 reporting what it had found. Gated the way the tier-bearing session reads are (`GET /sessions/{id}`, `POST /agent/resume`): `session_reach` FIRST, before the agent is fetched. The ordering is the substance — a gate under the fetch would satisfy a status assertion and still create the agent, which is why the test asserts `peek_agent(..).is_none()` as well. This is the hazard `agent_add_extension` already spells out at its own gate. #237 does not cover it. `routes::session_reach` is already on `main` and `get_callable_tool_count` was not among its four call sites in this file, so there is no conflict to avoid. ⚠ The review's premise about the sibling is wrong and worth recording: `GET /agent/tools` is also ungated, deliberately — it is the unfiltered permission-editor surface, so a person can administer private tools a public model cannot see, and it is not on `session_reach`'s gated list. `callable_tool_count` is the opposite kind of route, the model-facing count after Gate E. The handler now takes `HeaderMap` and returns `ErrorResponse` rather than a bare `StatusCode`, so its 424s keep the `{"message": …}` shape every other refusal in the file has instead of changing envelope with the failure (`agent_not_initialized`). Fail-before: 424 where 403 is now required. The new 403 is pinned in the generated spec by `openapi_describes_the_agent_route_failures_clients_must_handle` — extended to carry a method, so a GET route can be asserted at all — and the contract is regenerated.
…r reload (SD-8) An adversarial review of #260 put its LOW finding on the PR's own headline claim, and reading the code confirms it. The decision had three sources and on a fresh page all three read `false`: - `tabAnnotations` is ordinary React state in `ChatGroupsContext`, written only from live daemon workspace frames. The tab LAYOUT is persisted per window to `localStorage` (`chatGroups/chatGroupsStorage.ts`); the annotations are not. So a reload restores the subagent's tab with no badge — and a tab opened from History or a link never had one, so this is not only a reload bug. - `session?.session_type` is undefined until the store's load lands, which on this path is after `/agent/resume` has been refused and `GET /sessions/{id}` has returned. The PR's own comment measures that at 4.8 s with the session read still pending five seconds later. - `subagent.isSubagent` is positive-only: it can never say "not a subagent". `false` mounted the composer, with its Send, steer and Stop, for the whole window — which is precisely the running window SD-8 exists for. A boolean cannot express this, so the prop is gone rather than supplemented. `subagentComposerKind` answers `subagent | other | unknown` and `composerSlotMode` maps `unknown` to `withheld` on the browser surface only: no composer until the answer is in. The desktop holds the key, so it returns `composer` for all three kinds and nothing changes there. Two arms keep `withheld` from becoming a lockout, and both are deliberate: an empty `sessionId` (the tab before a first message mints a session — withholding there would leave a browser unable to start a chat) and a load that failed (the tab is already saying it could not be read, and a failure is not evidence of a subagent). `loadedSessionId` is compared against `sessionId` rather than assumed, because a chat is keyed by TAB id and the session behind a tab is rebindable. The cost is close to invisible: on that surface the transcript does not paint until the same read lands either, so what is withheld sits under an empty conversation. `subagentTabReadOnly` is keyed off `composerSlotMode(...) !== 'composer'` rather than off subagent-ness, because the two differ for exactly this window. Fail-before, with the specs in place and the two sources at HEAD: 6 failed | 17 passed, including "withholds the composer rather than mounting one it may have to take back" — `expected <textarea …> to be null`. Nothing visual is asserted in jsdom: the decision is a pure function with its own unit tests, and BaseChat's wiring is pinned against its source because BaseChat cannot mount there.
…d rather than assumed
The review's fourth finding is that a refusal for a subagent's chat differs in
body from a refusal for a chat that does not exist, so the pair reads as an
existence oracle for subagent ids. The difference is real. The oracle is
dominated, and this commit measures that rather than arguing it — the test
passes BEFORE any change, which is the evidence that finding 4 needs no fix.
Four outcomes for one unproven caller on `/agent/resume`:
- an id that does not exist → 403, `SESSION_OUT_OF_REACH`
- a PRIVATE subagent → 403, `SESSION_OUT_OF_REACH`, byte-identical
- a PUBLIC subagent → 403, and it is told it is a subagent
- a PUBLIC ordinary chat → 200
So the bodies separate only where the same request is already answered 200 next
door. `session_reach` is deliberately inert for public chats, so that caller can
have `GET /sessions/{id}` — the whole row, `session_type` included — for the very
chat it was just told about; the refusal discloses strictly less than the route
beside it hands over outright. Where the 200 is unavailable, which is a private
subagent, the pair collapses into the one sentence `SESSION_OUT_OF_REACH` exists
to be.
Making the bodies uniform was considered and rejected: it would take the "says
why" sentence away from the keyed desktop client that needs it, to close a
channel that leaks nothing its neighbour does not.
⚠ With the privacy master switch OFF the whole pair separates again, for every
chat on the machine: `session_reach` returns Ok before its store read, so the
private row joins the public one. That is the switch's pre-existing blast radius
(DR-17) rather than this route's, and it is recorded here and in SD-8 rather than
closed — the switch is agent-writable by design, so an OFF answer is announced,
never prevented.
The fourth row is asserted in the same run on purpose: if an unproven caller were
ever refused an ordinary public chat, the subagent body would become the only
existence signal and this finding would turn into a real oracle.
…e (SD-8) The PR author's own follow-up, and both predate #260: neither is reachable from the composer, which is why removing the composer did not close them. 5.1 — an ELICITATION card lives inside the transcript and posts its answer through `/reply`, the same write the composer makes. `BioRouterMessage` renders the form only under `hasElicitation && submitElicitationResponse`, so withholding the callback withholds the control rather than leaving a Submit that 403s. 5.2 — ARTIFACT AUTO-REPAIR needs no click at all: a figure that fails to render is the trigger. Checking first, as the brief asked, the answer is that this was NOT already closed. The subagent tab IS BaseChat, the live-chat surface, so it passes `onRenderError` like every other chat — and `shouldAutoRepairArtifact(chatState, lastAgentActiveAt, now)` is SATISFIED here rather than blocking, because a subagent's chat in a browser really does have a turn running. That is the whole case SD-8 is about, so the liveness gate is the one condition auto-repair meets. Closed with the same instrument the read-only transcript surfaces already use: `ArtifactViewer` installs its `postMessage` listener only when handed the callback. Both are keyed off `subagentTabReadOnly`, which is the slot's decision — so they are withheld while the tab's kind is still unknown too, not only once it is known to be a child's. Reported and deliberately NOT fixed here: `SecretRequestCard` posts `/action-required/secrets`, which a keyless daemon refuses for want of the proof, but the card offers its fields with no warning — unlike `ToolCallConfirmation`, which already reads `isBrowserSurface()` and puts a sentence where Allow and Deny would be. That is a surface-wide SD-8 gap on every browser chat rather than a subagent-tab one, and closing it inside this PR would widen its scope past the finding. Fail-before: both source assertions in `BaseChat.subagentReadOnly.test.ts` failed against HEAD (BaseChat cannot be mounted in jsdom, so its obligations are pinned against its source, as its other suites do).
Bookkeeping for the previous commit's new `session_reach` call site, kept out of it because this branch is append-only and that commit is already pushed-shaped. `crates/biorouter/tests/privacy_guard_wiring.rs` counts occurrences per file and compares a per-(needle, file) aggregate against ONE tuple per row, so the `routes/agent.rs` row moves `c(4, 4, 0)` → `c(5, 5, 0)`. The existing row is EXTENDED rather than duplicated, and the raise carries its justification inline — a route that had no gate at all, not a second gate on an already-guarded one. The census's own rule is that a count is never raised without a written reason, and "the review found a hole" is that reason. `session_reach`'s module header carries the canonical gated list, and a newly gated route missing from it is how a future sweep "discovers" a hole that is not there. The row also records why `GET /agent/tools` is NOT on the list, because the review assumed it was: that one is the unfiltered permission-editor surface, so a person can administer private tools a public model cannot see. `crates/biorouter/tests/privacy_capability.rs` needed nothing — no `CallCapability` site was added or moved, and `the_sites_that_decide_how_far_a_caller_reaches_are_exactly_these` passes unchanged.
…nto claude/stoic-wozniak-efc89d # Conflicts: # crates/biorouter/tests/privacy_guard_wiring.rs # docs/deployment/browser-access.md # docs/deployment/serve-decisions.md
Broccolito
added a commit
that referenced
this pull request
Sep 12, 2026
Both sides appended a decision record to serve-decisions.md; keep both, in number order (SD-11 from #260, then this branch's SD-13).
Broccolito
added a commit
that referenced
this pull request
Sep 12, 2026
Eleven conflicts, and two of them were substantive rather than textual. `routes/agent.rs` — both sides gated `GET /agent/callable_tool_count`, this branch through a wrapper and main (8437e94, the SD-8 review of #260) inline. The resolution keeps ONE gate: main's body and its fuller reasoning, in this branch's wrapper shape, because the gate has to hand back the refusal the way `session_reach` wrote it. Taking main's `?` into this route's `ErrorResponse` compiled and passed 526 tests, then failed the two that matter — the refusal came back as `{"message": …}` where every other gated route answers the PLAIN-TEXT bytes `GET /sessions/{id}` returns. One boundary, one body; the envelope is part of the boundary, and `get_tools` beside it has the same shape for the same reason. `useSidebarSessions.ts` — auto-merged with no conflict and did not compile. Main's deleted-chat handler (#264) decremented `nextOffsetRef`, which this branch had renamed; git took both halves. The decrement is now gone rather than renamed: it existed because an offset counts rows and the server's list had lost one, and a cursor names the sort key of the last row a page returned, so the boundary lands correctly even when the deleted row is the one it names. The rest: - `CLAUDE.md`, `serve-decisions.md` — this branch appended SD-10, main SD-11; both kept, in number order, and the CLAUDE.md range is now SD-1..SD-11. - `programmatic-session-access.md` — main's turn-control row placed with the other lifecycle rows; and the `next_offset` this branch renamed, which this file still spelled the old way. - `session_reach.rs` — main's SD-11 wording supersedes ours on `/agent/cancel` (SD-11 changed that fact); main's row joins the gated-list table beside the other helper-gated one; the "last two rows" sentence now NAMES those rows, since both sides appended after them and it had already stopped being true. `get_session_extensions` is dropped as an over-read control — this branch gated it, so main's row for it would now fail — and main's `reply.rs` `routes(` control is kept. - `privacy_guard_wiring.rs` — the `agent.rs` row is the union at c(6, 6, 0), not a second row: both sides gated `callable_tool_count` (one call either way) and this branch also gated `GET /agent/tools`; main's own text says SD-11 added no call in that file. - `KnowledgeContext.tsx` — main's #249 had already absorbed this branch's proof header and error handling into `refreshBases`/`fetchKnowledgeSelection`, so main's side stands, with the one thing only our comment said (a filtered list is pruned against, so a missing base reads as "deleted") folded in. - `useKnowledgeBases.ts` — our proof header on the delete, without our `setPrimaryKbId(null)`: main removed that deliberately, the daemon repairs the pointer, and `primaryKbId` is no longer in scope. - `openapi.json`, `types.gen.ts` — regenerated from the merged routes. Verified on the merged tree: privacy_capability 4, privacy_guard_wiring 3, biorouter-server --lib routes:: 528, knowledge_routes 68, biorouter --lib session::session_manager 215, frontend lint:check + format:check clean, vitest 306 over BioRouterSidebar + knowledge. `cargo fmt --check` clean. The OpenAPI spec and TS client regenerate to no diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SD-11 closed with one item open: a delegated subagent's tab in a browser offered a composer, a steer and a Stop that all refuse, and SD-8 requires such controls to say so before the click. This does that — and the first thing measuring it found was that the record understated the problem.
What a browser actually did with a subagent's tab
It could not open one. Every route that writes a subagent's chat needs proof that a person acted, and
POST /agent/resumeis among them (refuse_subagent_unless_user) — while the renderer loads every chat through resume. So the tab rendered "Could not load this chat" over the daemon's own refusal, including the tab the daemon itself opens to show a subagent it has just spawned, while that subagent was running.Measured against a real
biorouter serve(Claude Code provider, a real delegated child):POST /agent/resumeGET /sessions/{id}GET /sessions/{id}/eventsReading it was never refused. Only the renderer's choice of route was.
What it does now
The chat loads read-only (
chatStreamStore.loadReadOnlySubagentChat). On the browser surface, and only for a subagent's chat, a refused resume falls back to the session read — the readuseSubagentSessionalready makes — and the observer feed follows the child while it runs. Everything else the resume path does is left out, because each piece is refused or worse here: no second resume, no rejoin (it re-POSTs/reply), andagentReadystays false so nothing reads agent state.The tab says why it has no controls. The composer is not mounted at all, with the reason in its place, and one line takes the header Stop's — the shape
ToolCallConfirmationalready uses for Allow/Deny. Absent rather than disabled, because Send and steer are not the only writes the composer holds: Stop-and-Send, the continuation banner's Take over/Abandon, and the extension picker are all refused for a subagent's chat too, so a greyed-out Send would still leave a row of controls that fail on click.The 45-second nudge moves with the composer it names. "Still working. You can stop the turn from the composer" keeps its reassurance and drops the pointer to a control that is not there.
Review round: five findings, all closed
An adversarial security review raised four findings, and the PR's own author filed a fifth. One commit each, append-only.
1 · MEDIUM — the shipped doc claimed "the daemon refuses every write";
POST /agent/update_working_dirwas one it did not (86ee2483)Gated, not reworded — and the doc corrected too, because gating one route does not make a universal claim true.
That route repoints the named chat at a directory of the caller's choosing and restarts its agent there, and it consulted exactly one gate:
session_reach, which is the privacy slice and is deliberately inert for a public session. A delegated child's chat is normally public, so on aservedaemon an unproven caller could repoint a running child. The two 409s the route already had are not this boundary and must not be mistaken for it —try_update_working_dir_if_emptyrefuses a chat with messages, the turn lock refuses one that is busy, and a just-spawned or queued child is neither.Now gated with
refuse_subagent_unless_user, after the reach gate and before the turn lock, keeping the refusal order the rest of the file uses: a chat you may not reach is refused without disclosing that it is a subagent's, and a subagent's without disclosing whether it is busy.Fail-before: the public arm answered 400, not 403 — and the test's first draft earned that 400 by naming a directory that does not exist, which is why it now posts
std::env::temp_dir()and reads the working directory back. The public arm is load-bearing; the private arm was already refused bysession_reachfor an unrelated reason.Swept for other writes to a subagent's chat the daemon does not refuse, and each is reported in SD-8 (all pre-existing, none introduced here):
DELETE /sessions/{id}— which cancels the child's in-flight turn before deleting, so it is a Stop by another name, and the loudest of them — plusPUT /sessions/{id}/name,PUT …/user_workflow_values,POST …/edit_message,POST …/diverge,POST /agent/call_toolandPOST /agent/read_resource. A claim about the API surface as a whole would be #47's to make, and #47 is open; SD-8 is about the tab's own controls and now says so as an enumeration.2 · MEDIUM —
GET /agent/callable_tool_counthad no gate, and this PR routed the client around it (8437e94b)Correct: routing a client around an ungated route leaves the route ungated. #237 does not cover it —
routes::session_reachis already onmain, andcallable_tool_countwas not among its four call sites in this file, so there was nothing to skip and no conflict to create.Gated the way the tier-bearing session reads are (
GET /sessions/{id},POST /agent/resume):session_reachfirst, before the agent is fetched. The ordering is the substance —get_agent_for_routecreates an agent for a session that has none, so a gate under the fetch would satisfy a status assertion and still mint an agent for a chat the caller may not address, whose 424 would then report what it had found. The test assertspeek_agent(..).is_none()for that reason. (Fail-before: 424 where 403 is now required.)⚠ The finding's premise about the sibling is wrong, and it is worth recording:
GET /agent/toolsis also ungated, deliberately — it is the unfiltered permission-editor surface, so a person can administer private tools a public model cannot see, and it is not onsession_reach's gated list.callable_tool_countis the opposite kind of route: the model-facing count after Gate E.Generated API contractregenerated; both new 403s are pinned in the spec byopenapi_describes_the_agent_route_failures_clients_must_handle(extended to carry a method, so a GET route can be asserted at all).3 · LOW, on this PR's own headline claim — the read-only decision did fail open on a browser reload (
c9adabd0)Confirmed by reading, and it is worse than a reload. The decision had three sources and on a fresh page all three read
false:tabAnnotationsis ordinary React state inChatGroupsContext, written only from live daemon workspace frames. The tab layout is persisted per window tolocalStorage; the annotations are not. So a reload restores the subagent's tab with no badge — and a tab opened from History or a link never had one, so this is not only a reload bug.session?.session_typeis undefined until the store's load lands, which on this path is after the refused resume. This PR's own comment measures that at 4.8 s, with the session read still pending five seconds later.subagent.isSubagentis positive-only: it can never say "not a subagent".falsemounted the composer, with Send, steer and Stop, for the whole window — precisely the running window SD-8 exists for.A boolean cannot express this, so the prop is gone rather than supplemented.
subagentComposerKindanswerssubagent | other | unknown, andcomposerSlotModemapsunknownto withheld on the browser surface only. Two arms keep that from becoming a lockout, both deliberate: an emptysessionId(the tab before a first message mints a session) and a load that failed (the tab already says it could not be read, and a failure is not evidence of a subagent).loadedSessionIdis compared againstsessionIdrather than assumed, because a chat is keyed by tab id and the session behind a tab is rebindable.The cost is close to invisible: on that surface the transcript does not paint until the same read lands either, so what is withheld sits under an empty conversation. The desktop returns
composerfor all three kinds and changes not at all.Fail-before: 6 failed | 17 passed with the specs in place and the two sources at
HEAD, including "withholds the composer rather than mounting one it may have to take back" —expected <textarea …> to be null.4 · LOW — the distinct 403 bodies: a real difference, a dominated oracle. Measured and recorded, not changed (
578e27d9)Four outcomes for one unproven caller on
/agent/resume:SESSION_OUT_OF_REACHSESSION_OUT_OF_REACH— byte-identicalSo the bodies separate only where the same request is already answered 200 next door:
session_reachis inert for public chats, so that caller can haveGET /sessions/{id}— the whole row,session_typeincluded — for the very chat it was just told about. The refusal discloses strictly less than its neighbour hands over outright. Where the 200 is unavailable (a private subagent) the pair collapses into the one sentenceSESSION_OUT_OF_REACHexists to be. The test passes before any change, which is the evidence that this needed no fix; the fourth row is asserted in the same run on purpose, because if an unproven caller were ever refused an ordinary public chat the subagent body would become the only existence signal and this would turn into a real oracle.Uniform bodies were considered and rejected: that takes the "says why" sentence away from the keyed desktop client that needs it, to close a channel that leaks nothing.
⚠ With privacy tiers OFF the whole pair separates again, for every chat on the machine —
session_reachreturnsOkbefore its store read. Pre-existing, recorded in SD-8, and not fixed here: it is the master switch's blast radius (DR-17), and the switch is agent-writable by design.5 · The author's own follow-up — two writes a read-only tab could still make (
5728d758)Both predate this change and neither is reachable from the composer, which is why removing the composer did not close them.
/reply, the same write the composer makes.BioRouterMessagerenders the form only when handed a submit callback, so withholding the callback withholds the control rather than leaving a Submit that 403s.BaseChat, the live-chat surface, so it passesonRenderErrorlike any other chat — andshouldAutoRepairArtifactis satisfied here rather than blocking, because a subagent's chat in a browser really does have a turn running. That is the whole case SD-8 is about, so the liveness gate is the one condition auto-repair meets. Closed with the same instrument the read-only transcript surfaces use: no callback, no listener.Both key off the slot's decision, so they are withheld while the tab's kind is still unknown too.
Reported and deliberately not fixed here:
SecretRequestCardposts/action-required/secrets, which a keyless daemon refuses, but the card offers its fields with no warning — unlikeToolCallConfirmation, which already readsisBrowserSurface(). That is a surface-wide SD-8 gap on every browser chat rather than a subagent-tab one, and closing it here would widen this PR's scope past the finding.Census bookkeeping
privacy_guard_wiring.rs's row forroutes/agent.rsmovedc(4, 4, 0)→c(5, 5, 0)for the newsession_reachcall. The existing row was extended, never duplicated, and the raise carries its justification inline: the new site is a route that had no gate at all, not a second gate on an already-guarded one.privacy_capability.rsneeded nothing — noCallCapabilitysite moved.Verification
cargo test -p biorouter-server --lib -- routes::agent routes::session_reach— green, and none of fix(privacy): one reach gate for every HTTP route that names a chat or a knowledge base (QA H2, M1, M2, F0) #237's reach-gate tests regressed.cargo test -p biorouter --test privacy_guard_wiring --test privacy_capability— green.cargo fmt --check— clean.npm run lint:check— exit 0.npm run test:run—Test Files 1 failed | 447 passed,Tests 5046 passed | 1 skipped, zero failing tests. The one file issrc/utils/artifactCdnAssets.browser.test.ts, failing inafterAllwithHook timed out in 30000msonbrowser.close()— the known pre-existing teardown hang, fixed on a separate branch.biorouter servewith real delegated subagents, idle and running: the tab shows its transcript, follows the child live, shows the reason in the composer's place, and shows "Stopping needs the desktop app" while it runs.Stacking — #240's new head is merged in (
138fb5fe)#240 changed while this review round was in progress:
/interruptwas removed from SD-11's keylessrelaxation, because the equivalence argument does not hold for that one route. Its new head
(
b3374801) is merged here rather than carried stale, base stillmain.Three conflicts, all resolved by keeping both sides' intent: the census row for
routes/agent.rs(its corrected
/interruptsentence plus this round's count justification — one row, extendedtwice, never duplicated),
browser-access.md's split Stopping/Steering rows (theirs, with their newSD-11 anchor, plus this PR's subagent row), and SD-11's "Not decided here" (theirs, minus the
subagent sentence SD-8 now carries, followed by this PR's "Decided since").
⚠ Two sentences of this PR's own were made wrong by that change and are corrected in the merge
commit: SD-8's enumeration said "the four turn-control routes SD-11 admits elsewhere", and
subagentReadOnly.tslistedPOST /interruptamong the SD-11 routes. After #240,/interruptdemands the proof on every daemon — so it is still refused for a subagent's chat, for its own
reason rather than SD-11's. Nothing about what is refused changed; only which record explains it.
SD numbering
This amends SD-8 in place and carries #240's SD-11. It claims no new number, and renumbers nothing.
Left open, deliberately
/agent/resumeper subagent tab: the fallback is on the failure path rather than a pre-check, so nothing about an ordinary chat's load changes.SecretRequestCard's missing browser sentence (finding 5), and the privacy-tiers-OFF oracle (finding 4) are all recorded in SD-8 rather than fixed here.session cancelandattachsteering demanding a key locally — is untouched and still recorded there.🤖 Generated with Claude Code