fix(privacy): a busy store says so, and declassifying keeps History's order

crates/biorouter-server/Cargo.toml

@@ -95,6 +95,11 @@ env-lock = { workspace = true }
 wiremock = { workspace = true }
 zip = "8.6.0"
 serial_test = { workspace = true }
+# `tests/declassify_store_busy.rs` holds `sessions.db`'s write lock from a
+# connection outside the daemon's pool, which no public API of `biorouter` can
+# do. The same version and features `biorouter` already compiles, so this adds
+# no crate to the build.
+sqlx = { version = "0.8", features = ["runtime-tokio-rustls", "sqlite"] }
 # Issue #56 DR-20 / Task 55. This crate's routes raise the OS authentication
 # prompt — `POST /sessions/{id}/declassify` and `/config/upsert`'s master-switch
 # arm — so its TESTS would type a real password on every run without a stand-in.

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

@@ -15,7 +15,8 @@ use axum::{
 use biorouter::agents::ExtensionConfig;
 use biorouter::conversation::message::Message;
 use biorouter::privacy::declassify::{
-    authenticate_declassification, declassify, DeclassifyOutcome, UserConfirmation,
+    authenticate_declassification, declassify, is_store_busy, DeclassifyOutcome, UserConfirmation,
+    DECLASSIFY_STORE_BUSY,
 };
 use biorouter::privacy::SessionClassification;
 use biorouter::session::extension_data::ExtensionState;
@@ -1558,6 +1559,19 @@ const DECLASSIFY_SYSTEM_AUTH_REFUSED: &str =
      so marking it public needs your operating system to confirm it is you. That did not happen, \
      and nothing was changed.";
 
+/// What `POST /sessions/{id}/declassify` says when it failed for a reason that
+/// is not a busy store — which it answers with
+/// `biorouter::privacy::declassify::DECLASSIFY_STORE_BUSY` and a 503 instead.
+///
+/// It used to say nothing at all: every `Err` was a bodyless 500. The sentence
+/// claims only what an `Err` from the writer guarantees — this request changed
+/// nothing — and does not invite a retry, because a genuine fault is not cleared
+/// by waiting. The cause goes to the daemon log, not the body: a database error
+/// can name paths, and a person cannot act on it anyway.
+pub const DECLASSIFY_FAILED: &str =
+    "Nothing was changed and this chat was not marked public, because Biorouter hit an error. The \
+     daemon log records it.";
+
 #[derive(Debug, Default, Deserialize, ToSchema)]
 #[serde(rename_all = "camelCase")]
 pub struct DeclassifySessionRequest {
@@ -1611,7 +1625,12 @@ pub struct DeclassifySessionResponse {
                                       request carried no proof it came from them (body = plain \
                                       text)"),
         (status = 404, description = "Session not found"),
-        (status = 500, description = "Internal server error")
+        (status = 500, description = "Internal server error. Nothing was changed (body = plain \
+                                      text)"),
+        (status = 503, description = "The session store stayed busy with other writes for longer \
+                                      than the daemon waits. Nothing was changed, and the same \
+                                      call a moment later can succeed; `Retry-After` is set \
+                                      (body = plain text)")
     ),
     security(
         ("api_key" = [])
@@ -1706,9 +1725,33 @@ async fn declassify_session(
                 privacy_tier: SessionClassification::Public,
             }))
         }
+        // Item 8 of the 1.90.4 hold (2026-09-13). Both arms used to be one: a
+        // bodyless 500, which the desktop toasted as `[object Object]` and — on
+        // the single-click path — read as a stale grade and escalated to the
+        // typed phrase with a sentence claiming the chat's record had changed.
+        // Measured under a saturating external writer: 2 of 30 at 5.40 s and
+        // 5.46 s, daemon log `(code: 5) database is locked`.
+        //
+        // Either way nothing was written: `declassify` changes nothing on an
+        // `Err` (its transaction rolls back on drop), and a probe that answered
+        // before it never writes. So neither body claims more than that.
+        Err(e) if is_store_busy(&e) => {
+            // WARN, not ERROR: nothing is broken, other work held the lock.
+            tracing::warn!(
+                "Declassifying session {} gave up waiting for the session store: {:#}",
+                session_id,
+                e
+            );
+            Err((
+                StatusCode::SERVICE_UNAVAILABLE,
+                [(axum::http::header::RETRY_AFTER, "1")],
+                DECLASSIFY_STORE_BUSY,
+            )
+                .into_response())
+        }
         Err(e) => {
-            tracing::error!("Failed to declassify session {}: {}", session_id, e);
-            Err(StatusCode::INTERNAL_SERVER_ERROR.into_response())
+            tracing::error!("Failed to declassify session {}: {:#}", session_id, e);
+            Err((StatusCode::INTERNAL_SERVER_ERROR, DECLASSIFY_FAILED).into_response())
         }
     }
 }
@@ -3895,6 +3938,11 @@ mod declassify_tests {
             DECLASSIFY_NO_USER_KEY,
             DECLASSIFY_CONFIRMATION_MISMATCH,
             DECLASSIFY_SYSTEM_AUTH_REFUSED,
+            // Not refusals, but the same route's plain-text bodies, so they are
+            // held to the same two rules: distinct, and never wearing a marker
+            // that sends the renderer's toast somewhere that cannot help.
+            DECLASSIFY_STORE_BUSY,
+            DECLASSIFY_FAILED,
         ];
         for (i, one) in all.iter().enumerate() {
             for other in &all[i + 1..] {
@@ -3915,6 +3963,14 @@ mod declassify_tests {
         // have no model audience and do not need to.
         assert!(DECLASSIFY_NEEDS_USER.contains("Do not retry"));
         assert!(DECLASSIFY_NO_USER_KEY.contains("Do not retry"));
+        // A busy store is the one failure a retry DOES clear, and a genuine
+        // fault the one it does not — so exactly one of the two says so.
+        assert!(DECLASSIFY_STORE_BUSY.contains("Try again"));
+        assert!(!DECLASSIFY_FAILED.to_lowercase().contains("try again"));
+        // Neither may say the chat is public, and both must say nothing changed.
+        for body in [DECLASSIFY_STORE_BUSY, DECLASSIFY_FAILED] {
+            assert!(body.contains("not marked public"), "{body}");
+        }
     }
 
     /// SD-8. A daemon that holds no key may not answer a person with advice only

crates/biorouter-server/tests/declassify_store_busy.rs

@@ -0,0 +1,270 @@
+//! Item 8 of the 1.90.4 release hold, at `POST /sessions/{id}/declassify`: a
+//! declassification that loses the chat store's write lock must SAY so, and must
+//! say something different from a genuine failure.
+//!
+//! **Measured before this was written** (2026-09-13). Under a saturating
+//! external writer on the same `sessions.db`, 2 of 30 declassifications came
+//! back as a bodyless 500 at 5.40 s and 5.46 s, daemon log `(code: 5) database
+//! is locked` — SQLite's busy timeout running out. Both rolled back, correctly,
+//! and both succeeded on retry. Reproduced in the desktop app by holding the
+//! write lock across one click: the toast read `Could not mark this chat public
+//! [object Object]`, and the single-click dialog escalated to the typed phrase
+//! with *"This chat's record has changed since this list was loaded"* — a claim
+//! about the chat that nothing established.
+//!
+//! What is pinned here, over the real route and the real `check_token` layer:
+//!
+//! * a store held past the busy timeout answers **503** with `Retry-After` and
+//!   `DECLASSIFY_STORE_BUSY`, writes no ledger row, leaves the chat private, and
+//!   the same call succeeds once the store is free;
+//! * any other failure answers **500** with `DECLASSIFY_FAILED`, not the busy
+//!   sentence, and changes nothing either.
+//!
+//! ⚠ **Its own binary on purpose.** The busy test holds `sessions.db`'s write
+//! lock for more than five seconds. In the lib's test binary, where the route's
+//! other tests live, every test that touches the shared store in parallel would
+//! wait out the same timeout and fail — a flake manufactured by the test. Here
+//! the store is this binary's alone (`test_sandbox`), and `#[serial]` keeps the
+//! two tests off each other.
+
+// Redirects this binary's Biorouter data/config/state dirs at a throwaway root
+// before `main`, so the lock below can never be taken on the developer's real
+// `sessions.db`.
+#[path = "../src/test_sandbox.rs"]
+mod test_sandbox;
+
+use std::sync::Arc;
+use std::time::{Duration, Instant};
+
+use axum::body::Body;
+use axum::http::{HeaderMap, Request, StatusCode};
+use biorouter::conversation::message::Message;
+use biorouter::model::ModelConfig;
+use biorouter::privacy::declassify::DECLASSIFY_STORE_BUSY;
+use biorouter::privacy::SessionClassification;
+use biorouter::session::session_manager::{SessionManager, SessionType, DB_NAME, SESSIONS_FOLDER};
+use biorouter_server::routes::session::DECLASSIFY_FAILED;
+use biorouter_server::state::AppState;
+use serial_test::serial;
+use sqlx::{ConnectOptions, Connection};
+use tower::ServiceExt;
+
+const TEST_SECRET: &str = "declassify-store-busy-secret";
+const TEST_USER_ACTION_KEY: &str = "declassify-store-busy-user-action-key";
+
+/// The desktop's daemon holds a user-action key, and the refusal for a caller
+/// without one comes BEFORE the store is touched — so without this every
+/// request here would be a 403 and measure nothing about the store.
+fn install_user_action_key() {
+    let digest: [u8; 32] =
+        <sha2::Sha256 as sha2::Digest>::digest(TEST_USER_ACTION_KEY.as_bytes()).into();
+    biorouter_server::auth::install_user_action_digest(Some(digest));
+    let mut headers = HeaderMap::new();
+    headers.insert("X-User-Action", TEST_USER_ACTION_KEY.parse().unwrap());
+    assert!(
+        biorouter_server::auth::is_user_action(&headers),
+        "the user-action digest did not take, so every request below would stop at the proof \
+         check and never reach the store"
+    );
+}
+
+/// A private chat that merely ran a turn on a private model: §12.4's single
+/// click, so neither a phrase nor an operating-system prompt stands between the
+/// request and the store.
+async fn seed_turn_private(state: &Arc<AppState>) -> String {
+    let manager = state.session_manager();
+    let session = manager
+        .create_session(
+            std::env::temp_dir().join("declassify_store_busy"),
+            "Store busy fixture".to_string(),
+            SessionType::User,
+        )
+        .await
+        .unwrap();
+    manager
+        .add_message(&session.id, &Message::user().with_text("patient MRN 12345"))
+        .await
+        .unwrap();
+    manager
+        .update(&session.id)
+        .provider_name("versa_azure")
+        .model_config(ModelConfig::new("gpt-4o").unwrap())
+        .raise_privacy(SessionClassification::Private, "turn:versa_azure")
+        .apply()
+        .await
+        .unwrap();
+    session.id
+}
+
+/// The request the desktop sends — secret, user-action proof, no confirmation —
+/// through the same `check_token` layer `commands::agent::run` installs.
+async fn post_declassify(
+    state: Arc<AppState>,
+    session_id: &str,
+) -> (StatusCode, HeaderMap, String) {
+    let app = biorouter_server::routes::session::routes(state).layer(
+        axum::middleware::from_fn_with_state(
+            TEST_SECRET.to_string(),
+            biorouter_server::auth::check_token,
+        ),
+    );
+    let request = Request::builder()
+        .method("POST")
+        .uri(format!("/sessions/{session_id}/declassify"))
+        .header("content-type", "application/json")
+        .header("X-Secret-Key", TEST_SECRET)
+        .header("X-User-Action", TEST_USER_ACTION_KEY)
+        .body(Body::from(r#"{"confirmation":null}"#))
+        .unwrap();
+    let response = app.oneshot(request).await.unwrap();
+    let status = response.status();
+    let headers = response.headers().clone();
+    let bytes = axum::body::to_bytes(response.into_body(), usize::MAX)
+        .await
+        .unwrap();
+    (
+        status,
+        headers,
+        String::from_utf8_lossy(&bytes).into_owned(),
+    )
+}
+
+/// A connection of our own to THIS binary's `sessions.db`, outside the daemon's
+/// pool — the shape of the external writer the measurement used.
+async fn external_connection() -> sqlx::SqliteConnection {
+    let path = SessionManager::shared_store_root()
+        .join(SESSIONS_FOLDER)
+        .join(DB_NAME);
+    assert!(
+        path.is_file(),
+        "{} does not exist, so a lock taken on it would not be the daemon's store",
+        path.display()
+    );
+    sqlx::sqlite::SqliteConnectOptions::new()
+        .filename(&path)
+        .connect()
+        .await
+        .unwrap()
+}
+
+/// What the store actually holds for this chat, read around the daemon.
+async fn stored_state(session_id: &str) -> (String, i64) {
+    let mut conn = external_connection().await;
+    let tier: String = sqlx::query_scalar("SELECT privacy_tier FROM sessions WHERE id = ?1")
+        .bind(session_id)
+        .fetch_one(&mut conn)
+        .await
+        .unwrap();
+    let ledger: i64 =
+        sqlx::query_scalar("SELECT COUNT(*) FROM classification_audit WHERE session_id = ?1")
+            .bind(session_id)
+            .fetch_one(&mut conn)
+            .await
+            .unwrap();
+    conn.close().await.unwrap();
+    (tier, ledger)
+}
+
+/// ⚠ **Fails on `origin/main`**, where this request answers a bodyless 500.
+#[tokio::test(flavor = "multi_thread")]
+#[serial]
+async fn a_store_held_past_the_busy_timeout_answers_503_in_words_and_changes_nothing() {
+    install_user_action_key();
+    let state = AppState::new().await.unwrap();
+    let id = seed_turn_private(&state).await;
+
+    // Hold the write lock for longer than the pool's five-second busy timeout.
+    let mut holder = external_connection().await;
+    sqlx::query("BEGIN IMMEDIATE")
+        .execute(&mut holder)
+        .await
+        .unwrap();
+
+    let started = Instant::now();
+    let (status, headers, body) = post_declassify(state.clone(), &id).await;
+    let waited = started.elapsed();
+
+    sqlx::query("ROLLBACK").execute(&mut holder).await.unwrap();
+    holder.close().await.unwrap();
+
+    assert_eq!(
+        status,
+        StatusCode::SERVICE_UNAVAILABLE,
+        "a declassification that only waited out the store's lock must not read as a daemon \
+         fault (body: {body:?})"
+    );
+    assert_eq!(
+        body, DECLASSIFY_STORE_BUSY,
+        "the 503 does not carry the sentence a person can act on"
+    );
+    assert_eq!(
+        headers
+            .get(axum::http::header::RETRY_AFTER)
+            .and_then(|v| v.to_str().ok()),
+        Some("1")
+    );
+    assert!(
+        waited >= Duration::from_secs(4),
+        "answered after {waited:?}: it did not wait for the lock, so this did not measure a \
+         lock timeout"
+    );
+
+    // Nothing landed. The one outcome this must never have is a private chat
+    // lowered, or a ledger row claiming it was, by a request that failed.
+    assert_eq!(
+        stored_state(&id).await,
+        ("private".to_string(), 0),
+        "a declassification that answered 503 changed the store"
+    );
+
+    // And it is transient: the same request, with the store free, succeeds.
+    let (status, _, body) = post_declassify(state.clone(), &id).await;
+    assert_eq!(
+        status,
+        StatusCode::OK,
+        "retry after the lock cleared: {body}"
+    );
+    assert_eq!(stored_state(&id).await, ("public".to_string(), 1));
+}
+
+/// The busy sentence is for a busy store only. A fault that waiting cannot clear
+/// — a trigger aborting the ledger insert stands in for one — is a 500 in its own
+/// words, and must never tell the person to try again.
+///
+/// ⚠ **Fails on `origin/main`**, where it answers a bodyless 500.
+#[tokio::test(flavor = "multi_thread")]
+#[serial]
+async fn a_genuine_fault_answers_500_in_its_own_words_and_changes_nothing() {
+    install_user_action_key();
+    let state = AppState::new().await.unwrap();
+    let id = seed_turn_private(&state).await;
+
+    let trigger = format!(
+        "fail_ledger_insert_{}",
+        id.replace(|c: char| !c.is_alphanumeric(), "_")
+    );
+    let mut conn = external_connection().await;
+    sqlx::query(&format!(
+        "CREATE TRIGGER {trigger} BEFORE INSERT ON classification_audit \
+         WHEN NEW.session_id = '{id}' BEGIN SELECT RAISE(ABORT, 'injected fault'); END"
+    ))
+    .execute(&mut conn)
+    .await
+    .unwrap();
+
+    let (status, _, body) = post_declassify(state.clone(), &id).await;
+
+    sqlx::query(&format!("DROP TRIGGER {trigger}"))
+        .execute(&mut conn)
+        .await
+        .unwrap();
+    conn.close().await.unwrap();
+
+    assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR, "body: {body:?}");
+    assert_eq!(body, DECLASSIFY_FAILED);
+    assert_ne!(
+        body, DECLASSIFY_STORE_BUSY,
+        "a genuine fault was told to try again"
+    );
+    assert_eq!(stored_state(&id).await, ("private".to_string(), 0));
+}