fix(session-meta): prune to the union of live watchers; widen the shared-setting guard to crates/*/tests

[Broccolito]

Two review findings from the #201/#205/#209/#211 run: one functional (two Biorouter windows silently stop being told a chat's row moved), one instrument (the guard that was supposed to catch the second class of defect stopped looking at a third of the workspace, and two live offenders were sitting in the part it stopped reading).

Why

Finding 1 — the row map was pruned against one window's chats. SessionMetaEvents::last is process-global; the ids on a GET /sessions/changes is one window's open-chat list. The route called retain_watched(&ids) on every 2 s tick of every poll, so the map was pruned against whichever caller's list arrived last. observe adopts a first-seen id silently by design — the right behaviour for a chat you just opened, and the reason this defect is invisible: B's prune drops A's chats, A's next observe re-adopts them and publishes nothing, then A's prune drops B's. Neither window is ever told a row moved. Tab tear-off is a shipped feature, so two windows is the ordinary case, and the endpoint's whole reason to exist — seeing biorouter session --resume <id> --provider … from another process — is what stops working.

Finding 2 — the guard could not see where the offenders were. model::tests::no_test_parks_a_shared_setting_in_the_process_environment (#209) consolidated three per-key instruments into one table. The two it replaced walked all of crates/**; the table walked crates/*/src/** and documented the omission as deliberate. So a change that reads as a strict improvement narrowed the coverage by 129 of the workspace's 762 .rs files. Two unrestored set_var("BIOROUTER_ALLOW_PROJECT_HOOKS", "1") writes sat in that blind spot while three instruments reported the key handled: the guard walked src only, the ledger row named one writer, and the audit's own writer recipe grepped crates/biorouter/src.

Root cause

Finding 1. A process-global map pruned against a per-caller list. The two halves disagree about scope, and nothing in the code said so — retain_watched took a &[String] and had no way to know whose it was. Its if last.len() <= watched.len() { return; } early-out made it look conservative while doing exactly the wrong thing the moment a second window existed.

Finding 2. The stated reason for skipping tests/ was half right and drew the wrong conclusion. A crate's top-level tests/ file compiles to its own binary, so a write there cannot race the lib tests most of these readers live in — true. But it races every other test in that binary, which is a smaller hazard, not a different one, and nothing else in this repository looks there. Both offending files write a real .biorouter/hooks.yaml into their own working directory, so each binary's reader was sensitive from its first agent onward, and neither file ever removed the variable.

What changed

Finding 1 — a watcher registry and a claim with a lifetime

• crates/biorouter/src/session_meta.rs:178 — watchers: Mutex<HashMap<u64, Vec<String>>>, the open-chat ids of every live watcher keyed by a monotonic token (next_watcher), so a guard can only ever retire its own claim.
• crates/biorouter/src/session_meta.rs:339 — watch(self: &Arc<Self>, ids) -> WatchGuard, #[must_use]. Registering only ever grows the union, so it prunes nothing; pruning belongs on the drop.
• crates/biorouter/src/session_meta.rs:398 / :403 — WatchGuard and its Drop: retire the claim, then re-prune.
• crates/biorouter/src/session_meta.rs:362 — retain_union, private and reachable only from that drop. Prunes to the union of every live watcher. retain_watched is gone — zero references anywhere in the repo.
• crates/biorouter-server/src/routes/session_meta.rs:143 — let _claim = events.watch(&ids);, taken before the park loop and released when the request answers. It must be a named binding: let _ = events.watch(&ids) drops on the spot and reinstates the defect in a shape that reads as a fix. The comment says so at the call site.
• crates/biorouter-server/src/routes/session_meta.rs:214 — a source scan pinning that line, because no behavioural test in the workspace can see it: the row map lives in biorouter, the claim is taken in biorouter-server, and no test drives the route, so deleting the line leaves every suite green. Both traps this repo has already measured apply and are handled — the scan reads the production slice only (this module's own tests name the shape they forbid) and strips comments (the call site's warning spells let _ = events.watch(&ids), so an unstripped scan finds two claims and fails on a correct tree).
• crates/biorouter/src/session_meta.rs:90 / :142 / :205 — the union alone is not sufficient, and this is the part that is easy to get wrong. A claim lives for exactly one parked GET, so the only window's ids are unclaimed for an instant at every request boundary, and pruning strictly to the live union there would evict the ids that window is about to ask about again — swallowed for good, because re-adoption is silent. RETENTION (5 min) plus a Tracked { row, seen } stamp refreshed by every read closes that gap. with_retention(Duration::ZERO) lets a test observe an eviction without sleeping through it.

Finding 2 — widen the walk, and state the decision instead of parking it

• crates/biorouter/src/model.rs:1462–:1463 — the walk now takes crates/*/src/** and crates/*/tests/**. examples/ and build.rs stay out (8 files, measured); neither is compiled into a test binary, so a write there cannot reach one.
• crates/biorouter/src/model.rs:1529–:1537 — two non-vacuity floors, one per scope (scanned_src > 400, scanned_tests > 90). A single total is satisfied by the src half alone, so it would pass on the very narrowing this widening undoes. Measured today: 633 src, 121 tests, 7 examples, 1 build.rs = 762.
• crates/biorouter/src/model.rs:1371 — a # What it walks section recording why the second half was missing, so the next consolidation does not re-derive the omission.
• crates/biorouter/tests/hooks_agent_loop_tests.rs:156 and crates/biorouter/tests/global_memory_consent_agent_loop.rs:162 — .with_project_hooks(true) on AgentConfig, the seam Three live readers of the process environment take explicit inputs #205 added for exactly this, replacing the set_var. HooksManager::new_with_managed reads the variable with a bare std::env::var that no with_config_overrides task-local can reach, which is why the remedy is an argument and not an override.
• docs/testing/process-global-state.md:30 — the guard's stated scope, and the 129/762 measurement.
• docs/testing/process-global-state.md:122 — the existing row now names all three writers and corrects its own "no .biorouter/hooks.yaml in-tree, so no reader is sensitive today", which was true only of the lib.
• docs/testing/process-global-state.md:124 — a new row for the crates/*/tests writers, attributing the src readers to Three live readers of the process environment take explicit inputs #205 and these two to this PR, and recording the measurement that the pre-widening guard stays green with one of them restored.
• docs/testing/process-global-state.md:183 — the writer recipe under Re-measuring now greps crates/, not crates/biorouter/src.

Did the widened walk flag anything else? No. It reads 121 more files and finds no new offender. crates/*/tests does hold 12 BIOROUTER_PATH_ROOT writer call sites across 11 files (11 on one line, one split across two), and they are not silenced: BIOROUTER_PATH_ROOT has no row in WATCHED, deliberately — the ledger grades it open — deferred against ~46 live Paths::config_dir() readers and 33 lock_env writers, which is a separate piece of work, not something this PR narrowed the walk to avoid. The walk was not scoped down at any point.

No contract changed. No route signature, no request/response type, no event type. Verified rather than assumed: cargo run -p biorouter-server --bin generate_schema rewrote ui/desktop/openapi.json and git status came back clean, so the TS client needed no regeneration.

Tests

cargo fmt --all -- --check — exit 0. ./scripts/clippy-lint.sh — exit 0 (✅ All baseline clippy checks passed!); no too_many_lines baseline entry was added.

Finding 1, fail-before. git show origin/main:… for both files, with the new regression ported onto the pre-fix API (each window's own retain_watched(&ids) standing in for the guard it does not have):

thread 'session_meta::tests::two_windows_with_different_open_chats_do_not_evict_each_other' panicked at crates/biorouter/src/session_meta.rs:626:9:
assertion `left == right` failed: window A must learn that s1 moved
  left: 0
 right: 1

test result: FAILED. 12 passed; 1 failed; 0 ignored; 0 measured; 3758 filtered out; finished in 0.04s

Pass-after — cargo test -p biorouter --lib -- session_meta (14 tests; three are new: the two-window case, a_row_rewritten_between_two_polls_of_one_window_is_still_reported for the retention gap, and a_chat_nobody_watches_stops_being_tracked rewritten so dropping a guard is what releases the ids):

test result: ok. 14 passed; 0 failed; 0 ignored; 0 measured; 3758 filtered out; finished in 0.04s

cargo test -p biorouter-server --lib -- routes::session_meta:

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 579 filtered out; finished in 0.00s

The call-site guard, proved against two poisoned probes rather than reasoned about. With let _ = events.watch(&ids) — the spelling that compiles, silences #[must_use], and restores the defect:

the claim must be bound to a NAMED local. `let _ = …` drops the guard immediately, which prunes the process-global row map against this one caller's ids again — the defect this endpoint had, wearing a fix. Found: let _ = events.watch(&ids);

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 579 filtered out; finished in 0.04s

With the line deleted outright:

assertion `left == right` failed: the poll claims its ids exactly once, for the life of the request. Found: []
  left: 0
 right: 1

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 579 filtered out; finished in 0.04s

Finding 2, fail-before. With one of the two set_var lines restored, cargo test -p biorouter --lib -- model::tests::no_test_parks:

these tests park a shared setting in the PROCESS environment, where production code reads it live. `env_lock` does not help: it serialises the callers that ASK for it, and these readers never do.
  biorouter/tests/hooks_agent_loop_tests.rs:137 — BIOROUTER_ALLOW_PROJECT_HOOKS="1": the reader is live and unguarded, so any value changes what a concurrent test observes. Instead, pass the value as an argument — this reader calls `std::env::var`, which no task-local override can reach

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3771 filtered out; finished in 4.11s

And the claim the ledger makes, measured rather than reasoned about — the pre-widening walk (tests/ counted but not read) against that same restored line:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3771 filtered out; finished in 3.69s

Pass-after, both files restored:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3771 filtered out; finished in 4.11s

The two integration binaries whose writers were removed — they exercise real project hooks from a real .biorouter/hooks.yaml, so they would fail outright if the seam did not carry the opt-in:

cargo test -p biorouter --test hooks_agent_loop_tests
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.73s

cargo test -p biorouter --test global_memory_consent_agent_loop
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.53s

Full suites:

cargo test -p biorouter --lib
test result: ok. 3770 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 29.80s

cargo test -p biorouter-server --lib
test result: ok. 581 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.33s

Follow-ups

• The claim's lifetime is still only pinned by inspection. The new scan proves the guard is bound to a named local, so it lives to the end of the function — but nothing checks that the function is still the whole poll. If the park loop were ever extracted into a helper that took the claim itself, the scan would stay green while the claim's scope shrank. A route-level test that drives a real parked poll would close it, and needs a SessionManager fixture this module does not have today.
• BIOROUTER_PATH_ROOT stays open. The widened walk now reads its 12 crates/*/tests writers but cannot judge them, because the key has no WATCHED row. Adding one means first deciding what a legitimate write looks like against ~46 live Paths::config_dir() readers; the ledger already tracks this as deferred.
• Retention is time-based, so retain_union can hold an id for up to 5 minutes after its last watcher. Bounded and cheap, but a poll-count- or generation-based release would be exact. Only worth it if the map is ever measured to matter.

🤖 Generated with Claude Code