test(privacy): the census row for the workspace_set_tools pre-flight's capability sample

[Broccolito]

What is broken

privacy_capability::the_sites_that_decide_how_far_a_caller_reaches_are_exactly_these is red on main (run 34649064272, d6b39693), so every PR that merges main inherits it. Two sites that decide how far a caller reaches were added while rust.yml still ran only --lib --bins; #239 (merged 20:49) made CI run tests/*.rs, and the census has been red since.

This PR is one half of the fix — agents/workspace_inspector.rs. The other half is #259 (fix/census-bridge-test-capability), which removes bridge.rs's extra site rather than describing it. main goes green only when both land, and the two are disjoint — #259 never touches the census file, so either merge order is safe.

Measured on the combination (this branch with #259 merged locally):

cargo test -p biorouter --test privacy_capability   # 4 passed, 0 failed

On this branch alone the census still fails, with exactly one delta and nothing else — bridge.rs CallCapability::public_enforced( found 2 against a row of 1 — which is #259's half. That failure is expected here.

I deliberately do not add a bridge.rs row: #259 takes that file back to one occurrence, so a row of 2 here would re-break main the moment it lands. ⚠ Note for anyone checking by hand: the census skips comment lines, so a plain git grep -c over-counts that file by one — bridge.rs:2490 is a doc comment naming the constructor, not a site.

The change

agents/workspace_inspector.rs, CallCapability::sample(: count: 1 → 2, with the row's prose rewritten to describe both sites. No production code. The census mechanism is untouched.

Extending the existing row rather than adding a second is forced by the test: found is a BTreeMap keyed by (needle, file) with += hits, want is one tuple per EXPECTED row with no aggregation, and the comparison is a flat assert_eq! — so two rows for one file fail even at count: 1 each. The file's convention agrees (agent.rs is one row at 5).

Should the second site exist? Yes

An inspector runs before the dispatch that would admit a capability, so there is none in scope to inherit on the ordinary agent loop — the same reason already written for its sibling in this file. The alternatives are worse: read the master toggle directly (the bug CallCapability exists to prevent), or hoist the privacy gates out of the pre-flight so the always-confirm card is raised without them. And it is not the gate: handle_set_tools re-runs the same pre-flight against the capability the call is finally admitted on, so a model swapped between inspection and dispatch can only make this sample stale in the fail-safe direction. Nothing is removed.

What I verified by reading the code (please still check me)

• A pinned pair wins in both inspectors and the provider mutex is never read — but by different mechanisms, which the row now names separately after review caught it: the mutation inspector binds let mut sampled = capability and memoises into it; the crossing inspector has no sampled binding at all and matches the Option once at its sample point, after its candidates.is_empty() early return.
• The mutation inspector's sample( runs only inside the is_set_tools_call arm, only when nothing was pinned: at most one sample per batch, zero for a batch with no workspace_set_tools call.
• The pre-flight has three outcomes, not two (also from review): a Deny with the handler's sentence, the always-confirm card, or no card-worthy reason and on to dispatch.
• The only caller that pins a pair is the coding-agent bridge (bridge.rs:815); tool_inspection.rs's non-capability entry point passes None, as do the agent loop and the approval relay.
• The pair is handed to set_tools_preflight_refusal → preflight_set_tools, which asks refuse_unless_writable (§7's write row), resolve_added_extensions (Gate F reach per added extension), preflight_extension_removals (manageability per removed one) and privacy::bind_allowed (a provider switch).
• handle_set_tools calls preflight_set_tools(caller_session_id, cap, &args) at dispatch — the re-run above.
• bridge.rs's occurrences are #[cfg(test)] helpers, never production deciders. With Census: the bridge's inert test grant takes the file's one capability pair (#228 follow-up) #259 in, the census counts one there (:2513), matching its unchanged row; a plain grep also reports the doc comment at :2490, which the census skips.

Previously tagged as inferred, now answered by #244's author and checkable in the tree: sampling here is intended, and threading the pair from further up would be a regression. The agent loop's authoritative sample lives inside dispatch_tool_call (agent.rs, "THE sample for the agent loop's tool calls"), which runs after inspection and after any approval; the loop's inspection entries pass no capability. Hoisting that read to before inspection would fix the pair before an always-confirm card that can park for minutes, so a model swapped while the card sat parked would be ignored — the fail-open direction. Keeping this inspector's sample separate and non-authoritative is what makes its staleness fail safe, which is why the row says it is not the gate. The bridge is the one case where fixing the pair earlier is right, for the opposite reason: its child's calls arrive from another process with nothing to inherit.

Verification

cargo test -p biorouter --test privacy_capability   # workspace_inspector entry now matches;
                                                    # only the bridge.rs delta remains (above)
cargo test -p biorouter --test privacy_guard_wiring # 3 passed
cargo fmt --all -- --check                          # clean

⚠ This may not turn the ubuntu check green even once both halves land: the job's earlier --lib --bins step has been failing intermittently on agent_drafter::bundle::tests::a_timed_out_esbuild_reaps_its_whole_process_group (a process-reaping race in biorouter-mcp; it passes on main), and when it does, the integration step never runs.

🤖 Generated with Claude Code

[Broccolito]

Review from the #228/#259 side, since the bridge half of this census is mine. I checked every claim in the workspace_inspector.rs row against the code at d6b39693 rather than reading it for plausibility. The row is accurate and the count is right — one wording correction below, and it is worth making because the census's whole value is that its prose can be trusted without re-deriving it.

Verified

Claim	Evidence at d6b39693
exactly two sites, both non-comment	workspace_inspector.rs:392 and :744 — I ran the census's own rule (skip lines whose trimmed start is //, count occurrences): 2
(1) crossing = first-crossing disclosure, sampled once per batch	:741-744, one match before the per-candidate loop
(1) samples only after a cheap name check	:735-737 if candidates.is_empty() { return Ok(Vec::new()); } precedes it
(2) mutation = set_tools pre-flight (F4), lazy, memoised, one model for two calls in a batch	:389-395 inside the is_set_tools_call arm, sampled = Some(cap)
neither inherits on the ordinary agent loop	both ToolInspector::inspect impls pass None (:469, :828)
only the coding-agent bridge pins a pair	inspect_with_capability forwards it verbatim (:480-490); the only non-None caller of inspect_tools_with_capability is bridge.rs:511, while tool_inspection.rs:103 passes None
a pinned pair is used as is, mutex never read	true of both — (1) match capability, (2) the memo seeded from it

Both sites are legitimate deciders on the ordinary path, and (2)'s "it is NOT the gate — handle_set_tools re-runs the same pre-flight on the admitted capability, so staleness is fail-safe" is the right thing to have written down.

One correction

⚠ Both funnel into inspect_with_pinned_capability, whose first act is let mut sampled = capability

That is true of (2) only. let mut sampled = capability is :367, in WorkspaceMutationInspector. WorkspaceCrossingInspector has no sampled binding at all — the single occurrence of that word in its body is the comment at :739 — and it consumes the pinned pair at :741-744:

let cap = match capability {
    Some(capability) => capability,
    None => crate::privacy::CallCapability::sample(&self.provider).await,
};

deliberately after the candidates.is_empty() early return, which is the same cheap name check the row credits it with two sentences earlier. So the sentence as written contradicts (1)'s own description, and a later reader grepping sampled in the crossing inspector will not find it.

The property you are asserting holds for both; only the mechanism differs. Something like:

⚠ Both take the pinned pair AS IS and never read the provider mutex when given one — (1) by match capability at the point of use, (2) by seeding its sampled memo from it (let mut sampled = capability). The only caller that pins one is …

On the dropped bridge.rs hunk

Right call, and thank you for turning it around so fast. For the record, your reasoning there was sound on what you could see: inert_grant_for_test() is pub(crate) and could not reach mod tests' private test_capability(). #259 makes that helper pub(super), so the fixture now calls it and the file collapses back to one occurrence — which is why that row stays true exactly as it was, untouched.

With the hunk gone the two PRs are independent in both directions: this one touches only the census file, #259 only bridge.rs. Either merge order is safe.

[Broccolito]

Review from the author of the change this row describes (#244, the workspace_set_tools pre-flight). Read the diff at cdef6bb7 against the code, not just the body. The row is correct.

Narrowing to workspace_inspector.rs and reverting the bridge.rs row was the right call: #228's half removes its extra occurrence rather than describing it, so a row of 2 here would have re-broken the census the moment that lands.

The tagged question: is sampling here intended, or should the pair have been threaded from further up?

Intended — and threading it from further up would be a regression rather than a cleanup.

The agent loop's authoritative tool-call sample lives inside dispatch_tool_call (agents/agent.rs:7425, "THE sample for the agent loop's tool calls"), which runs after inspection and after any approval; the loop's inspection entry (agent.rs:5566) passes no capability, and that is not an oversight. "Thread it from further up" means hoisting that read above inspection and carrying it into dispatch. An always-confirm card can park for minutes waiting for a person, so the call would then run on a pair sampled before the user answered: a model swapped while the card sat parked would be ignored. That is the fail-open direction, and it is exactly what :7425's own comment exists to prevent — "fixed before the call RUNS … a swap that lands while the call sits behind the dispatch semaphore cannot change what the call already got permission to do."

Keeping the inspector's sample separate and non-authoritative is what makes its staleness fail safe: handle_set_tools re-runs the same preflight_set_tools against the capability the call is finally admitted on (workspace_extension.rs:3581). The bridge is the one case where fixing the pair earlier is right, for the opposite reason — a bridged child's calls arrive from another process with no capability to inherit, so re-reading per callback would be the two-reads race with a process boundary through it. The row already says both halves; nothing in it needs to change on my account.

Two precision fixes — optional, neither blocks a merge

This file's value is that a later reader can grep what a row claims, which is the only reason these are worth raising:

1. "Both funnel into inspect_with_pinned_capability, whose first act is let mut sampled = capability" is true of the mutation inspector (workspace_inspector.rs:367) but not of the crossing one: WorkspaceCrossingInspector::inspect_with_pinned_capability (:718) opens with the candidate filter and takes the pinned pair at :742 via match capability { Some(capability) => capability, None => …sample(…) }. Someone grepping let mut sampled in the crossing inspector finds nothing. Suggested: "Each funnels into its own inspect_with_pinned_capability, and a pinned pair is used AS IS in both — (2) through let mut sampled = capability, (1) through the match capability at its sample point — so the provider mutex is never read when one is supplied."
2. "the answer is either a Deny … or the always-confirm card" misses a third outcome: no refusal plus a change set_tools_reason does not consider card-worthy produces no inspection result at all and the call proceeds. Suggested: "… is a Deny carrying the handler's own sentence, the always-confirm card when the change is one §5 asks about, or silence."

Re-checked and holding

The funnel through inspect_with_pinned_capability; the lazy memoised sample inside the is_set_tools_call arm (so a batch with no workspace_set_tools call reads the provider mutex zero times, and a batch with two still gates on one model); providers/coding_agent/bridge.rs:815 as the only caller pinning a pair, against agent.rs:5588, approval_relay.rs:243 and tool_inspection.rs's non-capability entry, all of which pass None; the four gates inside preflight_set_tools (refuse_unless_writable, resolve_added_extensions, preflight_extension_removals, privacy::bind_allowed); and the dispatch re-run.

Independent measurement of main

On a clean checkout of d6b39693, counting the way the census counts (non-comment occurrences): agents/workspace_inspector.rs has exactly 2 CallCapability::sample( (:392, :744) against a row of 1, and providers/coding_agent/bridge.rs exactly 2 CallCapability::public_enforced( (:1086, :2502) against a row of 1. Two deltas, this PR closes one, #228's branch closes the other — the PR body's "green needs both" is accurate.

🤖 Generated with Claude Code

[Broccolito]

Measured: #258 + #259 together make the census green. Neither is green alone, so this is the only state that can be verified before merging.

Scratch branch = origin/main + this PR + the other, merged cleanly (d6552b80; #258 at 77394ab8, #259 at 3a131e1c):

cargo test -p biorouter --test privacy_capability
test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Row state in the combined tree — one row per file, which the census requires since found is keyed (needle, file) with counts summed:

file	needle	count
agents/workspace_inspector.rs	sample(	2 (#258)
providers/coding_agent/bridge.rs	public_enforced(	1 (#259, row text untouched)

⚠ Expect each PR's own test (ubuntu-latest) to stay red until the other lands: #259 alone fails on workspace_inspector.rs, #258 alone on bridge.rs. On #259 that failure is already in CI, naming the file this PR does not touch. Either merge order is safe; main is red now and goes green once both are in.