fix(session): a deleted chat's side rows go with it, and its id is never minted again

crates/biorouter-cli/src/commands/term.rs

@@ -551,10 +551,16 @@ mod tests {
         assert_eq!(plan.commands, vec!["first incarnation".to_string()]);
 
         sm.clear_all_sessions().await.unwrap();
+        // The allocator no longer restarts once the table is empty — `N` comes
+        // from `session_id_high_water`, which a reset does not lower. The cut's
+        // refusal still has to hold when an id IS recycled, because a build
+        // without the mark sharing the file, or a restored backup, can do it;
+        // the seam reproduces exactly that state.
+        sm.forget_minted_session_ids_for_test().await.unwrap();
         let recycled = term_session(&sm).await;
         assert_eq!(
             recycled, id,
-            "the id allocator restarts once the table is empty"
+            "the fixture must reproduce the id reuse, or this test proves nothing"
         );
         sm.add_message(&id, &logged(2, "second incarnation"))
             .await

crates/biorouter/src/agents/session_skills.rs

@@ -459,6 +459,11 @@ mod tests {
 
         // What `/reset` History does: empty `sessions`, then create afresh.
         sm.clear_all_sessions().await.unwrap();
+        // A reset does not lower `session_id_high_water`, so this build alone
+        // would give the new chat a fresh id. The override guard still has to
+        // hold without that — a build without the mark sharing the file, or a
+        // restored backup, can hand the id back — so the reuse is reproduced.
+        sm.forget_minted_session_ids_for_test().await.unwrap();
         let recreated = sm
             .create_session(
                 temp.path().to_path_buf(),

crates/biorouter/src/agents/subagent_handler.rs

@@ -2433,7 +2433,8 @@ mod tests {
 
     /// ⚠ **Serialized, and it has to be.** The session bus is a process-global
     /// map keyed by session **id**, but ids are minted per *store* as
-    /// `<date>_<n>` (`INSERT … SELECT MAX(CAST(SUBSTR(id, 10) AS INTEGER))`), so
+    /// `<date>_<n>` (`session_manager.rs`'s `CLAIM_NEXT_SESSION_N`, whose
+    /// high-water mark is per store and so starts at 1 in each one), so
     /// two tests that each stand up their own `TempDir` `SessionManager` both
     /// get `<today>_1` and publish into the *same* ring. Anything asserting on
     /// the sequence then reads another test's frames interleaved with its own.

crates/biorouter/src/agents/workspace_extension.rs

@@ -6797,7 +6797,8 @@ pub(crate) mod tests {
     ///
     /// `serial(agent_manager_pin)`: the pin is a process-global map keyed by
     /// session **id**, and ids are minted per *store* as `<date>_<n>`
-    /// (`session_manager.rs`'s `SELECT MAX(CAST(SUBSTR(id, 10) AS INTEGER))`),
+    /// (`session_manager.rs`'s `CLAIM_NEXT_SESSION_N`, whose high-water mark is
+    /// per store and so starts at 1 in each one),
     /// so every test that stands up its own `TempDir` `SessionManager` and
     /// registers its FIRST session is fighting over the single key `<today>_1`.
     /// `subagent_handler`'s two real-subagent tests are the other claimants.

crates/biorouter/src/checkpoint/manager.rs

@@ -183,13 +183,18 @@ impl CheckpointManager {
         })
     }
 
-    /// Remove a session's shadow repo + checkpoint rows (called on session
-    /// delete).
+    /// Remove a session's shadow repo + checkpoint rows.
+    ///
+    /// Deleting a chat does not come through here: `SessionStorage::delete_session`
+    /// removes the rows in the chat's own transaction and the same
+    /// [`super::repository_dir`] after it commits, which is what reaches the CLI
+    /// and every other caller that never builds a `CheckpointManager`.
     pub async fn gc(&self, session_id: &str) -> Result<()> {
-        let dir = self.data_root.join("checkpoints").join(session_id);
-        // BR-57: removing a whole shadow git repo is blocking file I/O — keep
-        // it off the async runtime like the rest of the checkpoint file work.
-        let _ = tokio::task::spawn_blocking(move || std::fs::remove_dir_all(&dir)).await;
+        if let Some(dir) = super::repository_dir(&self.data_root, session_id) {
+            // BR-57: removing a whole shadow git repo is blocking file I/O — keep
+            // it off the async runtime like the rest of the checkpoint file work.
+            let _ = tokio::task::spawn_blocking(move || std::fs::remove_dir_all(&dir)).await;
+        }
         self.session_manager.delete_checkpoints(session_id).await
     }
 }

crates/biorouter/src/checkpoint/mod.rs

@@ -18,8 +18,27 @@ pub use manager::CheckpointManager;
 pub use store::{Caps, ShadowRepo};
 
 use serde::{Deserialize, Serialize};
+use std::path::{Component, Path, PathBuf};
 use std::str::FromStr;
 
+/// The directory holding one chat's shadow repository and nothing else:
+/// `<data_root>/checkpoints/<session_id>` (the repository itself is its `git/`).
+///
+/// `None` unless the id is exactly one plain path component, because the id
+/// comes from a free-text column and the result is handed to
+/// `remove_dir_all`: `.` would name every chat's repository at once, `..` the
+/// data directory, and `a/.` would name chat `a`'s. No id `create_session`
+/// mints is any of those, but a restored or hand-edited database can hold one.
+pub(crate) fn repository_dir(data_root: &Path, session_id: &str) -> Option<PathBuf> {
+    let mut components = Path::new(session_id).components();
+    match (components.next(), components.next()) {
+        (Some(Component::Normal(name)), None) if name == std::ffi::OsStr::new(session_id) => {
+            Some(data_root.join("checkpoints").join(session_id))
+        }
+        _ => None,
+    }
+}
+
 /// Which snapshot boundary produced a checkpoint.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]

crates/biorouter/src/privacy/declassify.rs

@@ -504,12 +504,18 @@ pub async fn declassify(
     // proof is the only construction site, and it is behind the user-action
     // header. Writing a fabricated username here would be worse than writing
     // none.
-    sqlx::query(
+    // `session_incarnation` names the ROW declassified, not just its id: the
+    // ledger outlives the chat, and the backfill's declassification guard must
+    // not read this entry as a later chat's under the same id
+    // (`SessionStorage::NOT_DECLASSIFIED_BY_USER`).
+    sqlx::query(&format!(
         "INSERT INTO classification_audit ( \
             session_id, from_classification, to_classification, reason, actor, actor_kind, \
-            app_version, provider_name_at_change, privacy_reason_before, message_count_at_change \
-         ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)",
-    )
+            app_version, provider_name_at_change, privacy_reason_before, message_count_at_change, \
+            session_incarnation \
+         ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, {})",
+        crate::session::session_manager::LEDGER_SESSION_INCARNATION
+    ))
     .bind(session_id)
     .bind(from.as_sql())
     .bind(SessionClassification::Public.as_sql())

crates/biorouter/src/privacy/grant.rs

@@ -201,6 +201,34 @@ fn extension_key(extension: &str) -> String {
     crate::config::extensions::name_to_key(extension)
 }
 
+/// Whether a stored grant belongs to the chat that holds its session id now:
+/// that chat already existed when the grant was recorded. A predicate over
+/// `cross_affiliation_grants g JOIN sessions s ON s.id = g.session_id`.
+///
+/// ⚠ **The id is not the chat.** `create_session` minted `<day>_<MAX(N)+1>`, so
+/// deleting the newest chat of the day handed its id to the next one, and until
+/// `delete_session` took a chat's grants with it that next chat read every flow
+/// accepted in the deleted one as accepted in itself — the one thing
+/// [`GRANT_SCOPE_COPY`] tells the user cannot happen. The delete is not the
+/// only way a grant outlives its chat: every earlier build left them behind, a
+/// terminal `biorouter` lagging the desktop app still does while it shares the
+/// database, and a restored backup can hold grants for an id that is live
+/// again. None of those can be recorded after the chat now holding the id was
+/// created, so this test needs no cooperation from any of them.
+///
+/// Both timestamps come from SQLite's clock (`datetime('now')` and
+/// `CURRENT_TIMESTAMP`, or an imported chat's RFC 3339 `created_at`, which
+/// `datetime()` reads the same way), at one-second resolution. A grant from the
+/// chat's first second still counts — a user cannot be shown a refusal and
+/// accept it inside the second the chat was created in, and a stale grant would
+/// need a user to accept, delete the chat and start another within one. A row
+/// either side cannot read fails closed: the flow is asked about again.
+///
+/// One definition, read by [`is_granted`] and by the startup sweep in
+/// `session_manager` that deletes what it rejects, so the two can never
+/// disagree about which grants are live.
+pub(crate) const GRANT_IS_THE_CHATS_OWN: &str = "datetime(g.granted_at) >= datetime(s.created_at)";
+
 /// How deep the parent walk goes before giving up.
 ///
 /// A subagent may itself spawn a subagent, so the chain is genuinely longer than
@@ -265,6 +293,10 @@ pub async fn record(
 /// The ancestor walk is what makes "a subagent inherits its parent's grants"
 /// true. It reads upward only: a child's own grants are invisible to its parent,
 /// and a child has no way to create one regardless.
+///
+/// Only a grant recorded while the chat holding its id existed is read — see
+/// [`GRANT_IS_THE_CHATS_OWN`]. A grant whose chat is gone is not read either,
+/// because the lookup joins the chat's row.
 pub async fn is_granted(
     sm: &SessionManager,
     session_id: &str,
@@ -294,17 +326,20 @@ async fn granted_inner(
     let key = model_key(model);
     let ext = extension_key(extension);
 
+    let lookup = format!(
+        "SELECT COUNT(*) FROM cross_affiliation_grants g \
+           JOIN sessions s ON s.id = g.session_id \
+          WHERE g.session_id = ?1 AND g.extension = ?2 AND g.model_affiliation = ?3 \
+            AND {GRANT_IS_THE_CHATS_OWN}"
+    );
     let mut current = session_id.to_string();
     for _ in 0..MAX_PARENT_DEPTH {
-        let found: i64 = sqlx::query_scalar(
-            "SELECT COUNT(*) FROM cross_affiliation_grants \
-              WHERE session_id = ?1 AND extension = ?2 AND model_affiliation = ?3",
-        )
-        .bind(&current)
-        .bind(&ext)
-        .bind(&key)
-        .fetch_one(pool)
-        .await?;
+        let found: i64 = sqlx::query_scalar(&lookup)
+            .bind(&current)
+            .bind(&ext)
+            .bind(&key)
+            .fetch_one(pool)
+            .await?;
         if found > 0 {
             return Ok(true);
         }
@@ -970,4 +1005,100 @@ mod tests {
              render can never appear"
         );
     }
+
+    /// A grant belongs to the chat it was given in, and "the chat" is not "the
+    /// id". `create_session` mints `<day>_<MAX(N)+1>`, so deleting the newest
+    /// chat of the day hands its id to the next one — "delete the chat I just
+    /// made and start again". Before a delete took the chat's grants with it,
+    /// that next chat silently inherited every cross-institutional flow the user
+    /// had accepted in the deleted one: a flow nobody accepted in the new chat,
+    /// which is exactly what [`GRANT_SCOPE_COPY`] promises cannot happen.
+    #[tokio::test]
+    async fn a_deleted_chats_grant_is_not_inherited_by_the_next_chat_to_get_its_id() {
+        let (_dir, sm, id) = session_manager_with_a_chat().await;
+        record_for_test(&sm, &id, "ucsfomopagent", bound_to("stanford"))
+            .await
+            .unwrap();
+        assert!(is_granted(&sm, &id, "ucsfomopagent", bound_to("stanford")).await);
+
+        sm.delete_session(&id).await.unwrap();
+        // `session_id_high_water` stops THIS build reissuing the id, so the
+        // reuse is arranged — as a build without the mark sharing the file, or a
+        // restored backup, really produces it. This refusal is what has to hold
+        // when that happens, and it must not be deleted on the grounds that ids
+        // are single-use now.
+        sm.forget_minted_session_ids_for_test().await.unwrap();
+        let next = sm
+            .create_session(PathBuf::from("."), "next".to_string(), SessionType::User)
+            .await
+            .unwrap()
+            .id;
+        assert_eq!(
+            next, id,
+            "the fixture must reproduce the id reuse, or this test proves nothing"
+        );
+
+        assert!(
+            !is_granted(&sm, &next, "ucsfomopagent", bound_to("stanford")).await,
+            "a new chat inherited a cross-institutional approval given in a chat the user deleted"
+        );
+    }
+
+    /// ...and a delete is not the only way a grant can outlive its chat. Every
+    /// build before this one left grants behind on delete, a terminal
+    /// `biorouter` that lags the desktop app still does while it shares this
+    /// database, and a restored backup can hold grants for ids that are live
+    /// again. So the reader refuses, on its own, a grant recorded before the chat
+    /// now holding the id existed — it cannot have been given in that chat — and
+    /// it does so at read time, not only when a startup sweep next runs.
+    #[tokio::test]
+    async fn a_grant_recorded_before_its_chat_existed_is_not_read() {
+        let (_dir, sm, id) = session_manager_with_a_chat().await;
+        record_for_test(&sm, &id, "ucsfomopagent", bound_to("stanford"))
+            .await
+            .unwrap();
+
+        // The delete an older build performs: the chat goes, its grants stay.
+        let pool = sm.storage().pool().await.unwrap();
+        sqlx::query("DELETE FROM sessions WHERE id = ?1")
+            .bind(&id)
+            .execute(pool)
+            .await
+            .unwrap();
+        // Backdated, because a person takes longer than one clock second to
+        // accept a warning, delete the chat and start another; a fixture that
+        // did all three inside one second would be testing a tie no user makes.
+        sqlx::query("UPDATE cross_affiliation_grants SET granted_at = datetime('now', '-1 hour')")
+            .execute(pool)
+            .await
+            .unwrap();
+
+        // ...and the id has to come back for the grant to be misread as the new
+        // chat's at all. See the sibling test above for why forcing it is the
+        // honest fixture.
+        sm.forget_minted_session_ids_for_test().await.unwrap();
+        let next = sm
+            .create_session(PathBuf::from("."), "next".to_string(), SessionType::User)
+            .await
+            .unwrap()
+            .id;
+        assert_eq!(
+            next, id,
+            "the fixture must reproduce the id reuse, or this test proves nothing"
+        );
+        assert!(
+            !is_granted(&sm, &next, "ucsfomopagent", bound_to("stanford")).await,
+            "a grant older than the chat holding its id was read as that chat's"
+        );
+
+        // ...and the new chat's own acceptance of the same flow is honoured: the
+        // upsert refreshes `granted_at`, so the row is this chat's again.
+        record_for_test(&sm, &next, "ucsfomopagent", bound_to("stanford"))
+            .await
+            .unwrap();
+        assert!(
+            is_granted(&sm, &next, "ucsfomopagent", bound_to("stanford")).await,
+            "re-accepting the flow in the new chat must work"
+        );
+    }
 }