Session ids: allocate the test store prefix instead of hashing it, and panic on a duplicate

[Broccolito]

What I was asked to confirm, and what I actually found

The brief said #273's 40-minute test (ubuntu-latest) hang was colliding test
session ids meeting the process-global subagent_handle::HANDLES, that a
mitigation (clear_for_tests()) had been lost, and that I should find out why.

Two of those three are wrong, and saying so is most of the value here.

• The mitigation was never lost. acae89ea (2026-08-27) made id_prefix
  per-store under #[cfg(test)], and it is an ancestor of Four serve / apps serve / CLI lifecycle defects: an ignored browser token, an orphaned daemon, a stale cancellation warning, and an ungated scheduled run #273's own parent
  38270a50 — so it was in effect when Four serve / apps serve / CLI lifecycle defects: an ignored browser token, an orphaned daemon, a stale cancellation warning, and an ungated scheduled run #273 hung. clear_for_tests() never
  appears in any commit (git log --all -S): its own commit message records
  that it was written and reverted, because it clears a global that
  concurrently-running tests read.
• This PR therefore does not claim to have found Four serve / apps serve / CLI lifecycle defects: an ignored browser token, an orphaned daemon, a stale cancellation warning, and an ungated scheduled run #273's mechanism. I could
  not reproduce the hang: the suite is green at 1, 2, 3, 4, 8 and 16 threads on
  this machine, before and after.

What I did establish about #273, from the cancelled attempt's log
(gh api .../actions/jobs/103490679332/logs):

Three of the four stalled tests carry serial_test::serial(workspace_services)
and the fourth waits on subagent_tool's QUEUE_DEPTH_TESTS, which
unverified_steering_stays_recoverable_through_a_successful_run holds while it
also holds the workspace_services key. Exactly one test was wedged; three
were queued behind it. Same shape as the 2026-08 hang ("one of those two was
merely queued behind the other").

And I found two holes in the surviving mitigation — one measured unsound,
one measured live. This PR fixes the first and documents the second with a
reproduction rather than half-closing it.

Hole 1, fixed: the prefix was probabilistic

id_prefix was DefaultHasher(session_dir) truncated to u32. Two stores
whose paths collide in 32 bits both mint <prefix>_1 — silently, with the
symptom arriving later as a turn that never returns.

measurement	result
brute force over TempDir-shaped paths	colliding pair after 11,213 candidates
200,000 distinct paths → distinct prefixes	199,995 (5 real collisions)
ideal-32-bit expectation for 200,000	4.66 → measured/expected 1.07
distinct stores one lib-test run allocates	752 (instrumented)
resulting collision rate	~1 in 15,000 runs

So it is unsound, but too rare to be what hung #273 — stated plainly rather
than inflated into a cause. (An earlier reading of 114,690 stores was this PR's
own 200k test contaminating the count; corrected by re-measuring with the new
tests skipped.)

It is now allocated from a counter, which cannot collide at all.

Hole 2, proven live, deliberately left open

#[cfg(test)] compiles only for this crate's own unit tests. Every integration
binary in the workspace — and every test in biorouter-server, biorouter-mcp,
biorouter-cli — links this crate built without it, prefixes ids with the
date, and mints <date>_1 from every store. CI runs those binaries.

Measured by building the new duplicate guard outside cfg(test):

tests/agent.rs                        FAILED. 13 passed; 4 failed
tests/conversation_writeback_stress   FAILED.  1 passed; 8 failed

session id 20260912_1 was minted twice in one process: first by the store at
/var/folders/.../.tmpV1N423/sessions, now by the store at .../.tmpWZzXjd/sessions

Both ways to close it are a maintainer's call, and the second was written,
measured and removed:

• give later stores a non-date prefix in non-test builds — changes user-visible
  ids on a real production path (biorouter-acp/src/server.rs:346 constructs its
  own manager);
• floor the numeric part process-wide — one extra bind on CLAIM_NEXT_SESSION_N
  (MAX(db_max+1, high_water+1, ?2)), production provably identical at one store
  per process. But it breaks every fixture that replays id reuse, because
  a_rewrite_basis_cannot_cross_a_wipe_that_recycled_the_session_id asserts
  second == first — the wipe must hand the same id back. Seven such fixtures
  across four modules, found by running them, not by reading them.

Filed as a follow-up with the full reproduction.

What keeps it from being lost again

MINTED_IDS records every minted id and the store that minted it, and panics
naming both stores. The failure mode is the point: this bug does not fail where
it is caused, so a panic at the mint is the difference between five minutes and a
week of bisection. A re-mint by the same store is a quiet return, which is
what lets the documented id-reuse seam keep working.

Two defects in the guard's own first draft, both measured and fixed: it panicked
while holding its mutex, so one genuine detection became seven failures with six
meaningless PoisonErrors; and a clearing hook written for the reuse seam turned
out to be dead code, deleted once the test passed without it.

What this deletes

A fix at the source should remove the papers over it rather than become the
sixth. The tree carried five hand-rolled defences against one collision.
Gone: subagent_tool's reserve_child_session_ids spacer bands (40/80/120/160,
four call sites) and workspace_extension's seeded_target band counter, whose
k-th caller created 15 + k sessions.

Measured on one lib-test run, isolating just this removal:

	create_session calls	wall
with the bands	4264	48.66 s
without	1167	32.41 s

The two serial_test keys (subagent_session_bus, agent_manager_pin) are
kept, with their now-false doc comments corrected — both were added for the
collision and both instruct the next author to join them. They still buy
ordering, and a scheduling change whose failure mode is a 40-minute CI hang needs
more than a handful of local runs behind it.

Tests that fail before and pass after

Each mechanism was reverted in isolation and re-run:

reverted	result
allocator → 32-bit hash	two_stores_the_old_hashed_prefix_confused_now_get_different_prefixes FAILED (both 38a7a4d7), the_store_prefix_is_exact_at_a_scale_where_a_32_bit_hash_is_not FAILED (199,995 ≠ 200,000)
duplicate guard → no-op	the_duplicate_guard_panics_and_names_both_stores FAILED
nothing (shipped state)	7 passed

The hash-collision fixture is deterministic, not probabilistic: it asserts the
pair still collides under the old scheme first, so if DefaultHasher ever
changes the test fails loudly telling you to re-brute-force a pair, rather than
passing vacuously.

Repeated-run evidence (post-rebase onto main)

BIOROUTER_DISABLE_KEYRING=true cargo test -p biorouter --lib, run from
crates/biorouter. A 34-minute stall could not hide in any of these.

five consecutive runs, default threads
  run 1: wall=42s stalls=0 | ok. 4020 passed; 0 failed
  run 2: wall=49s stalls=0 | ok. 4020 passed; 0 failed
  run 3: wall=42s stalls=0 | ok. 4020 passed; 0 failed
  run 4: wall=49s stalls=0 | ok. 4020 passed; 0 failed
  run 5: wall=48s stalls=0 | ok. 4020 passed; 0 failed

thread-count sweep (LOW counts were the stressor in the 2026-08 bisect)
  threads=1:  wall=230s stalls=0 | ok. 4020 passed; 0 failed
  threads=2:  wall=130s stalls=0 | ok. 4020 passed; 0 failed
  threads=3:  wall=72s  stalls=0 | ok. 4020 passed; 0 failed
  threads=4:  wall=63s  stalls=0 | ok. 4020 passed; 0 failed
  threads=8:  wall=41s  stalls=0 | ok. 4020 passed; 0 failed
  threads=16: wall=31s  stalls=0 | ok. 4020 passed; 0 failed

8 integration binaries that build their own stores, 3 runs each: 24/24 green
  agent 17 · conversation_writeback_stress 9 · subagent_delegation 8
  checkpoint_agent_loop 2 · session_store_dispatch_boundary 3
  chatrecall_code_execution 25 · turn_abort_tests 4 · soft_interrupt_agent_loop 7

Baseline on the pre-change tree, for comparison: green at threads 4/1/2/3 in
162 s / 240 s / 100 s / 108 s — i.e. the hang does not reproduce here either way.

cargo fmt --check clean. cargo clippy -p biorouter --all-targets clean.

⚠ ./scripts/clippy-lint.sh fails on biorouter-server/src/routes/reply.rs:1863
(result_large_err on authorize_steer, from #260). Pre-existing on main —
git diff origin/main for that file is empty — and CI's clippy step is
informational (cargo clippy --workspace --all-targets --locked, no
-D warnings), so it is not gating. A chip already exists for it.

Production behaviour

Unchanged, and structurally so: every line added is #[cfg(test)]. The
#[cfg(not(test))] id_prefix (today's date) and CLAIM_NEXT_SESSION_N are
byte-identical to main. one_store_still_numbers_from_one_without_gaps pins
that one store still numbers 1..n with no gaps.

🤖 Generated with Claude Code