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

crates/biorouter-server/src/routes/session_meta.rs

@@ -21,11 +21,18 @@
 //!
 //! # The poll does the reading
 //!
-//! There is no background watcher and no registry of watched ids. The request
-//! carries the ids its client has open, and while parked it re-reads exactly
-//! those rows on a short interval, hands them to
-//! [`SessionMetaEvents::observe`], and returns as soon as the revision moves.
-//! An idle app with no chats open therefore reads nothing at all.
+//! There is no background watcher. The request carries the ids its client has
+//! open, and while parked it re-reads exactly those rows on a short interval,
+//! hands them to [`SessionMetaEvents::observe`], and returns as soon as the
+//! revision moves. An idle app with no chats open therefore reads nothing at
+//! all.
+//!
+//! ⚠ **A poll does register its ids, and that registry is not bookkeeping.**
+//! The row map behind `observe` is process-global while an id list is one
+//! window's, so a poll that pruned that map against its OWN list would evict a
+//! second window's chats — and a re-adopted id is adopted SILENTLY, so the two
+//! windows would thrash and neither would ever be told a row moved. The claim
+//! taken below lives for exactly this request; see [`SessionMetaEvents::watch`].
 //!
 //! ⚠ **`since=0` is a baseline request, not a replay.** A client establishing
 //! itself gets the current revision and no changes; asking for a replay from
@@ -126,6 +133,15 @@ pub async fn session_changes(
             .unwrap_or(MAX_WAIT)
             .min(MAX_WAIT);
 
+    // Claimed for the life of this poll and released when it answers, so the
+    // row map is pruned against the union of every live watcher rather than
+    // against whichever list arrived last.
+    //
+    // ⚠ It must be a NAMED binding. `let _ = events.watch(&ids)` drops the
+    // guard on the spot and reinstates the defect in a shape that reads as a
+    // fix.
+    let _claim = events.watch(&ids);
+
     // Adopt this caller's ids before parking. A chat opened a moment ago has not
     // changed, and reporting its whole row as new would wake every window on
     // connect — the first observation of an id is silent by construction
@@ -142,7 +158,6 @@ pub async fn session_changes(
         {
             events.observe(rows);
         }
-        events.retain_watched(&ids);
 
         let delta = events.since(query.since);
         if !delta.changes.is_empty() || delta.truncated {
@@ -186,4 +201,53 @@ mod tests {
             .join(",");
         assert_eq!(parse_ids(Some(&many)).len(), MAX_IDS);
     }
+
+    /// The claim must be BOUND, and nothing that RUNS can see whether it is.
+    ///
+    /// ⚠ This is the one line carrying the two-window fix, and every
+    /// behavioural test in the workspace passes without it: the row map lives
+    /// in `biorouter`, the poll's claim is taken here, and a route test drives
+    /// neither. Deleting `let _claim = …` restores the defect, and so does
+    /// writing `let _ = …`, which drops the guard on the spot while reading as
+    /// a fix. A source scan is the only instrument that can see either.
+    #[test]
+    fn the_poll_binds_its_watch_claim_for_the_life_of_the_request() {
+        // Production only. This module names the shape it forbids, so a scan
+        // that read its own tests would report itself as its first offender.
+        let (production, _) = include_str!("session_meta.rs")
+            .split_once("#[cfg(test)]")
+            .expect("this route's tests sit at the end, behind one `#[cfg(test)]`");
+        assert!(
+            production.len() > 5000,
+            "the production slice is {} bytes, far too short to be this route — the slice is              wrong and a clean result would mean nothing",
+            production.len()
+        );
+
+        // Comments are stripped for the same reason, and it is not theoretical
+        // here: the call site carries a warning that SPELLS the forbidden
+        // `let _ = …` form, so an unstripped scan sees two claims and fails on
+        // a correct tree. Truncating early can only lose a match, and a lost
+        // match fails the count below rather than passing quietly.
+        let claims: Vec<&str> = production
+            .lines()
+            .filter_map(|line| line.split("//").next())
+            .filter(|code| code.contains(".watch("))
+            .map(str::trim)
+            .collect();
+        assert_eq!(
+            claims.len(),
+            1,
+            "the poll claims its ids exactly once, for the life of the request. Found: {claims:?}"
+        );
+
+        // Whitespace-insensitive, so `let _=` cannot slip past a spelling.
+        let squashed: String = claims[0].chars().filter(|c| !c.is_whitespace()).collect();
+        assert!(
+            squashed.starts_with("let") && !squashed.starts_with("let_="),
+            "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: {}",
+            claims[0]
+        );
+    }
 }

crates/biorouter/src/model.rs

@@ -1172,8 +1172,8 @@ mod tests {
     struct Watched {
         key: &'static str,
         verdict: Verdict,
-        /// How many files under `crates/*/src` must still name this key **in
-        /// code**.
+        /// How many files under `crates/*/src` or `crates/*/tests` must still
+        /// name this key **in code**.
         ///
         /// ⚠ A **mention** count, not a match count, and deliberately so: a
         /// `Presence` row is healthy at zero matches, so a floor on matches
@@ -1364,10 +1364,27 @@ mod tests {
     ///    watchable set is the whole config surface, not this table. This table
     ///    is a list of measured hazards, not a closed set. Green here is not a
     ///    proof of hermeticity.
-    /// 3. **`crates/*/tests/`, which is skipped on purpose.** Each file there
-    ///    compiles to its own binary, so a write cannot reach the lib test
-    ///    binary these readers live in. It can still reach the other tests in
-    ///    its own file; that is a smaller, separate hazard the audit records.
+    /// 3. **`crates/*/examples/` and `build.rs`.** Neither is compiled into a
+    ///    test binary, so a write there cannot reach one at all. Eight files,
+    ///    and the only `.rs` in the workspace this walk does not read.
+    ///
+    /// # What it walks
+    ///
+    /// `crates/*/src/**` **and** `crates/*/tests/**`.
+    ///
+    /// ⚠ **The second half was missing, and the omission read as deliberate.**
+    /// The two guards this table replaced walked all of `crates/**`; the table
+    /// walked `crates/*/src/**` and its own documentation explained why
+    /// `tests/` was skipped — so consolidating three instruments into one
+    /// narrowed the coverage by 129 of the workspace's 762 `.rs` files while
+    /// looking like a strict improvement. The reasoning was half right: 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. It races
+    /// every other test in that binary, which is a smaller hazard and not a
+    /// different one — and it is invisible from anywhere else, which is the
+    /// part that matters. Two unrestored writes of
+    /// `BIOROUTER_ALLOW_PROJECT_HOOKS` sat there while this guard, the audit's
+    /// ledger and the audit's recipe all reported the key as handled.
     #[test]
     fn no_test_parks_a_shared_setting_in_the_process_environment() {
         // CARGO_MANIFEST_DIR is <workspace>/crates/biorouter; go up twice.
@@ -1410,7 +1427,8 @@ mod tests {
             })
             .collect();
 
-        let mut scanned = 0usize;
+        let mut scanned_src = 0usize;
+        let mut scanned_tests = 0usize;
         let mut mentions = vec![0usize; WATCHED.len()];
         let mut offenders: Vec<String> = Vec::new();
 
@@ -1427,18 +1445,26 @@ mod tests {
             if path.extension().and_then(|e| e.to_str()) != Some("rs") {
                 continue;
             }
-            // In scope: `crates/<crate>/src/**`. A `tests/` directory nested
-            // INSIDE `src` is an ordinary module of that crate's lib and stays
-            // in scope; only a crate's top-level `tests/` is its own binary.
+            // In scope: `crates/<crate>/src/**` and `crates/<crate>/tests/**`.
+            // A `tests/` directory nested INSIDE `src` is an ordinary module of
+            // that crate's lib and is already covered by the first arm.
+            //
+            // Counted per scope rather than in one total, because one number
+            // cannot tell "the tests half was walked" from "the src half grew" —
+            // and narrowing back to `src` alone is exactly the regression the
+            // widening exists to prevent.
             let Ok(relative) = path.strip_prefix(&crates) else {
                 continue;
             };
             let mut parts = relative.components();
             let _crate_name = parts.next();
-            if parts.next().map(|c| c.as_os_str()) != Some(std::ffi::OsStr::new("src")) {
-                continue;
+            match parts.next().and_then(|c| c.as_os_str().to_str()) {
+                Some("src") => scanned_src += 1,
+                Some("tests") => scanned_tests += 1,
+                // `examples/` and `build.rs`: not compiled into a test binary,
+                // so a write there cannot reach one.
+                _ => continue,
             }
-            scanned += 1;
             let Ok(source) = std::fs::read_to_string(path) else {
                 continue;
             };
@@ -1496,11 +1522,19 @@ mod tests {
             }
         }
 
-        // A walk that reads nothing agrees with a walk that finds nothing.
+        // A walk that reads nothing agrees with a walk that finds nothing. One
+        // floor per scope: a single total is satisfied by the `src` half alone,
+        // so it would pass on the very narrowing this widening undid.
+        assert!(
+            scanned_src > 400,
+            "the audit only scanned {scanned_src} files under crates/*/src, which is too few \
+             to have walked the workspace"
+        );
         assert!(
-            scanned > 400,
-            "the audit only scanned {scanned} files under crates/*/src, which is too few to \
-             have walked the workspace"
+            scanned_tests > 90,
+            "the audit only scanned {scanned_tests} files under crates/*/tests, which is too \
+             few to have walked them. A write parked there is invisible to every other test in \
+             its own binary, and to every other instrument in this repository"
         );
 
         // PER-KEY non-vacuity. One global floor lets most rows rot silently