diff --git a/Cargo.lock b/Cargo.lock index 5057d3e1c..3e6d6afa6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1329,6 +1329,7 @@ dependencies = [ "serial_test", "sha2 0.10.9", "socket2 0.6.1", + "sqlx", "tempfile", "thiserror 1.0.69", "tikv-jemalloc-ctl", diff --git a/crates/biorouter-server/Cargo.toml b/crates/biorouter-server/Cargo.toml index 254853142..c5140f084 100644 --- a/crates/biorouter-server/Cargo.toml +++ b/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. diff --git a/crates/biorouter-server/src/routes/session.rs b/crates/biorouter-server/src/routes/session.rs index 1391a31e7..e72d92003 100644 --- a/crates/biorouter-server/src/routes/session.rs +++ b/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 diff --git a/crates/biorouter-server/tests/declassify_store_busy.rs b/crates/biorouter-server/tests/declassify_store_busy.rs new file mode 100644 index 000000000..30355010d --- /dev/null +++ b/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] = + ::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) -> 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, + 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)); +} diff --git a/crates/biorouter/src/privacy/declassify.rs b/crates/biorouter/src/privacy/declassify.rs index e5f8282e6..c32ddb80c 100644 --- a/crates/biorouter/src/privacy/declassify.rs +++ b/crates/biorouter/src/privacy/declassify.rs @@ -364,6 +364,98 @@ pub enum DeclassifyOutcome { SessionNotFound, } +/// What a person is told when a declassification gave up waiting for the chat +/// store. Shared by both doors: the route answers it as a 503's body, and the +/// CLI prints it as the head of the error chain [`declassify`] returns. +/// +/// ⚠ **Every clause is a claim this module can stand behind, and no more.** +/// "Changed nothing" holds because the only statements that can fail this way +/// run inside the writing transaction (or are its commit), and a +/// `sqlx::Transaction` that is dropped without committing rolls back — so no +/// ledger row and no lowered tier survive it. It says the chat was not marked +/// public *by this request*, rather than "the chat is still private", on +/// purpose: another request may land while this one is being answered, and a +/// sentence about the row's present state would be a read this code never made. +/// +/// ⚠ **Short on purpose.** The desktop shows it in a toast whose message is +/// clamped to three lines, and a first draft that opened with the cause lost +/// "Try again in a moment" behind the ellipsis — measured in the running app. +/// So it leads with what happened to the chat, then the remedy. +/// +/// It invites the retry, unlike the proof-of-user refusals beside it in +/// `routes/session.rs`. Those are refusals a retry cannot change; this is a +/// wait that ran out, and the same call a moment later is the remedy. Both +/// doors sit behind a person — the user-action header, or a terminal — so no +/// model is being taught to loop on it. +/// +/// ⚠ **The retry is the person's, not this module's.** SQLite's busy handler is +/// already a bounded retry loop: every statement here polls the write lock for +/// the pool's full five-second `busy_timeout` before this error exists. A +/// second in-process attempt was measured (2026-09-13, two external writers on +/// the same `sessions.db`) and it mostly bought a longer spinner: 7 of 30 +/// declassifications gave up, an immediate retry rescued 4 of the 7 and the +/// other 3 failed again, at up to 9.5 s each — under the load that causes this +/// error the busy periods are correlated, not independent. +pub const DECLASSIFY_STORE_BUSY: &str = + "Nothing was changed and this chat was not marked public, because other writes kept the chat \ + store busy. Try again in a moment."; + +/// The error context [`declassify`] attaches when it failed only because the +/// store stayed busy. +/// +/// A type rather than a string, so a door asks [`is_store_busy`] — a downcast — +/// and a reworded sentence cannot quietly turn a 503 back into an empty 500. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct StoreBusy; + +impl std::fmt::Display for StoreBusy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(DECLASSIFY_STORE_BUSY) + } +} + +/// Did this declassification fail only because the chat store stayed busy? +/// +/// `true` exactly for an error [`declassify`] returned with [`StoreBusy`] +/// attached. It is the one distinction a door should draw from an `Err`: this +/// one says "try again", and every other failure is a genuine fault and says so. +pub fn is_store_busy(error: &anyhow::Error) -> bool { + error.downcast_ref::().is_some() +} + +/// Is this the store's busy wait running out, as opposed to any other fault? +/// +/// Two shapes, both meaning "other work held the write lock, or every pooled +/// connection, for longer than this process waits", and both cleared by the +/// same call a moment later: +/// +/// * SQLite answered `SQLITE_BUSY` once the pool's `busy_timeout` ran out — +/// extended code 5 — or one of its two other waited-out forms, +/// `SQLITE_BUSY_RECOVERY` (261) and `SQLITE_BUSY_TIMEOUT` (773). sqlx reports +/// `sqlite3_extended_errcode`, so they arrive as exactly those strings. The +/// daemon log of the report this was written for reads `(code: 5) database +/// is locked`. +/// * sqlx gave up acquiring a pooled connection (`PoolTimedOut`). +/// +/// ⚠ **`SQLITE_BUSY_SNAPSHOT` (517) is deliberately NOT one of them.** It is the +/// instant refusal of a transaction that READ before it wrote — no busy handler +/// is consulted for it — and the write-first statement at the top of +/// [`declassify_in_one_transaction`] exists so that it cannot happen here. If it +/// ever does, the lock ordering has regressed, and answering it as "busy, try +/// again" would dress that regression up as load. It stays a genuine failure: a +/// 500 and an `ERROR` in the log. +fn waited_out_the_store(error: &anyhow::Error) -> bool { + error + .chain() + .any(|cause| match cause.downcast_ref::() { + Some(sqlx::Error::PoolTimedOut) => true, + Some(sqlx::Error::Database(db)) => { + matches!(db.code().as_deref(), Some("5") | Some("261") | Some("773")) + } + _ => false, + }) +} + /// The ONLY writer in the tree permitted to lower `privacy_tier`. /// /// Every other write goes through the session update builder, whose emission is @@ -437,13 +529,48 @@ pub enum DeclassifyOutcome { /// honest and the direction is fail-safe — private is the protected state — but /// the user can watch their action undo itself. Preventing it would mean /// refusing to declassify a busy session, which §12.4 does not ask for. -/// ⚠ **`_ok` is borrowed rather than consumed**, and that is what keeps the two +/// ⚠ **`ok` is borrowed rather than consumed**, and that is what keeps the two /// doors at ONE construction site each. Both call this twice for a chat on the /// strong control — once to probe, once to write — and /// [`the_proof_of_user_is_constructed_in_exactly_two_places`] counts /// constructions per file, not calls. One human action, one proof, however many /// times the writer is asked. +/// +/// # Errors +/// +/// An `Err` means this call changed nothing: every write is inside one +/// transaction, and a transaction that does not reach its commit rolls back on +/// drop. One kind of `Err` is not a fault at all — the store stayed busy for +/// longer than the pool waits — and it carries [`StoreBusy`], so a door can tell +/// a person to try again rather than answer an empty 500 (which is what the +/// route did until 2026-09-13: two of thirty declassifications under a +/// saturating external writer came back bodyless at 5.4 s, and the desktop +/// showed `[object Object]`). Ask [`is_store_busy`]; everything else is genuine. pub async fn declassify( + sm: &SessionManager, + session_id: &str, + confirmation: Option<&str>, + authorization: Option<&SystemAuthorization>, + ok: &UserConfirmation, +) -> Result { + declassify_in_one_transaction(sm, session_id, confirmation, authorization, ok) + .await + .map_err(|error| { + if waited_out_the_store(&error) { + error.context(StoreBusy) + } else { + error + } + }) +} + +/// [`declassify`]'s body: one transaction, and everything its doc comment says +/// about lock ordering, grading and the ledger lives here. +/// +/// Split out only so the error it returns can be named in ONE place. The `?`s +/// below are many and each can fail on a busy store; wrapping them one by one +/// is how a new statement would come to be the one that answers an empty 500. +async fn declassify_in_one_transaction( sm: &SessionManager, session_id: &str, confirmation: Option<&str>, @@ -570,9 +697,24 @@ pub async fn declassify( .execute(&mut *tx) .await?; + // ⚠ **`updated_at` is not touched, and that is a decision, not an omission.** + // `updated_at` orders History, the sidebar's keyset pages and `biorouter + // session list`, and buckets History's date groups — it answers "when was + // this chat last USED". A classification change is not use. This statement + // stamped it until 2026-09-13, and the measured result was 796 + // declassifications moving months-old chats (20260224_11, created + // 2026-02-24) into History's "Today". + // + // Nothing needed the stamp to learn of the change. The session-row feed + // (`GET /sessions/changes`, `SessionStorage::session_meta_rows`) compares + // `privacy_tier`/`privacy_reason` and deliberately never `updated_at`; and a + // second window had no live signal at all before — it learned only when some + // unrelated refresh re-read a list that this stamp had re-sorted. The + // desktop now announces the change itself (`utils/sessionRowSync.ts`), and + // every list surface re-reads the row in place. sqlx::query( "UPDATE sessions \ - SET privacy_tier = 'public', privacy_reason = ?2, updated_at = datetime('now') \ + SET privacy_tier = 'public', privacy_reason = ?2 \ WHERE id = ?1", ) .bind(session_id) @@ -1733,4 +1875,237 @@ mod tests { let row = sm.get_session(&id, false).await.unwrap(); assert_eq!(row.privacy_tier, SessionClassification::Public); } + + /// Take `sessions.db`'s write lock the way a real transaction does — a + /// statement that writes nothing but takes the lock at its prologue, inside a + /// real `Transaction` — on a SECOND store over the same file, so nothing in + /// process memory orders it against the declassification. Dropping the + /// returned transaction releases it. + async fn hold_the_write_lock( + other: &SessionManager, + ) -> sqlx::Transaction<'static, sqlx::Sqlite> { + let pool = other.storage().pool().await.unwrap(); + let mut tx = pool.begin().await.unwrap(); + sqlx::query("UPDATE sessions SET id = id WHERE 1 = 0") + .execute(&mut *tx) + .await + .unwrap(); + tx + } + + /// Item 8 of the 1.90.4 hold (2026-09-13). Measured under a saturating + /// external writer: two of thirty declassifications failed at 5.40 s and + /// 5.46 s with `(code: 5) database is locked`, the route answered a + /// bodyless 500, and the desktop toasted `[object Object]`. + /// + /// Three things are pinned, and the second is the one that matters most: + /// + /// 1. The error is NAMED — [`is_store_busy`] says so, and its head is the + /// sentence a person can act on. On `origin/main` the error is sqlx's + /// alone. + /// 2. It changed nothing. The chat is still private and the ledger holds no + /// row claiming a transition. A lock timeout must never be reported, or + /// recorded, as a declassification. + /// 3. It is transient: once the lock is released, the same call succeeds. + /// + /// The lock is held for real, past the pool's five-second `busy_timeout`, so + /// this test takes about that long. `turn:*` provenance, so neither a phrase + /// nor a password is in the way and the store is the only variable. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn a_store_that_stays_busy_is_named_and_changes_nothing() { + let temp = tempfile::TempDir::new().unwrap(); + let sm = SessionManager::new(temp.path().to_path_buf()); + let id = private_session_with_reason(&sm, "turn:versa_azure").await; + let ok = UserConfirmation::for_test(); + + let other = SessionManager::new(temp.path().to_path_buf()); + let lock = hold_the_write_lock(&other).await; + let started = std::time::Instant::now(); + let error = declassify(&sm, &id, None, None, &ok) + .await + .expect_err("a store write-locked past the busy timeout cannot be declassified"); + let waited = started.elapsed(); + + assert!( + is_store_busy(&error), + "a declassification that only lost the write lock is not named as one: {error:#}" + ); + assert_eq!( + error.to_string(), + DECLASSIFY_STORE_BUSY, + "the error's head is not the sentence a person is shown" + ); + assert!( + format!("{error:#}").contains("database is locked"), + "the cause was swallowed rather than kept under the sentence: {error:#}" + ); + // It really waited: an instant failure here would be a snapshot conflict + // (a lock-ordering regression), which must not be classified as busy. + assert!( + waited >= std::time::Duration::from_secs(4), + "gave up after {waited:?}, well inside the five-second busy timeout" + ); + + drop(lock); + let row = sm.get_session(&id, false).await.unwrap(); + assert_eq!( + row.privacy_tier, + SessionClassification::Private, + "a declassification that timed out lowered the tier anyway" + ); + assert_eq!(row.privacy_reason.as_deref(), Some("turn:versa_azure")); + assert!( + audit_rows(&sm, &id).await.is_empty(), + "the ledger claims a transition that did not happen" + ); + + assert_eq!( + declassify(&sm, &id, None, None, &ok).await.unwrap(), + DeclassifyOutcome::Declassified, + "the same call did not succeed once the store was free" + ); + } + + /// The other half of item 8: only a busy store is called one. A fault that is + /// not contention — here a trigger that aborts the ledger insert — must stay a + /// genuine failure, or a broken daemon would keep telling people to "try + /// again in a moment" forever. It changes nothing either. + #[tokio::test] + async fn a_fault_that_is_not_a_busy_store_is_not_called_one() { + let temp = tempfile::TempDir::new().unwrap(); + let sm = SessionManager::new(temp.path().to_path_buf()); + let id = private_session_with_reason(&sm, "turn:versa_azure").await; + { + let pool = sm.storage().pool().await.unwrap(); + sqlx::query(&format!( + "CREATE TRIGGER fail_this_ledger_insert BEFORE INSERT ON classification_audit \ + WHEN NEW.session_id = '{id}' BEGIN SELECT RAISE(ABORT, 'injected fault'); END" + )) + .execute(pool) + .await + .unwrap(); + } + + let error = declassify(&sm, &id, None, None, &UserConfirmation::for_test()) + .await + .expect_err("the trigger aborts the ledger insert"); + assert!( + !is_store_busy(&error), + "a genuine fault was named a busy store: {error:#}" + ); + assert!(format!("{error:#}").contains("injected fault")); + + let row = sm.get_session(&id, false).await.unwrap(); + assert_eq!(row.privacy_tier, SessionClassification::Private); + assert!(audit_rows(&sm, &id).await.is_empty()); + } + + /// `SQLITE_BUSY_SNAPSHOT` (517) is not a busy store, and this is measured on + /// a real WAL file rather than asserted of a hand-built error: a transaction + /// that READ, then lost the write to a commit made after its snapshot, is + /// refused instantly with no busy wait. That is the shape PR #294's + /// write-first statement removed from `declassify`, so if it ever reaches a + /// door again it must read as the regression it is. + #[tokio::test] + async fn a_snapshot_conflict_is_not_classified_as_a_busy_store() { + let temp = tempfile::TempDir::new().unwrap(); + let reader_store = SessionManager::new(temp.path().to_path_buf()); + let s = reader_store + .create_session(std::env::temp_dir(), "snap".to_string(), SessionType::User) + .await + .unwrap(); + let writer_store = SessionManager::new(temp.path().to_path_buf()); + + let pool = reader_store.storage().pool().await.unwrap(); + let mut tx = pool.begin().await.unwrap(); + // Pin a read snapshot. + let _: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM sessions") + .fetch_one(&mut *tx) + .await + .unwrap(); + // Another store commits after that snapshot. + writer_store + .add_message( + &s.id, + &crate::conversation::message::Message::user().with_text("later"), + ) + .await + .unwrap(); + // The upgrade is refused. + let started = std::time::Instant::now(); + let refused = sqlx::query("UPDATE sessions SET name = 'x' WHERE id = ?1") + .bind(&s.id) + .execute(&mut *tx) + .await + .expect_err("a stale snapshot cannot upgrade to a writer"); + assert!( + started.elapsed() < std::time::Duration::from_secs(2), + "the snapshot conflict waited, so this test is not measuring 517" + ); + let code = match &refused { + sqlx::Error::Database(db) => db.code().map(|c| c.into_owned()), + other => panic!("expected a database error, got {other:?}"), + }; + assert_eq!( + code.as_deref(), + Some("517"), + "not SQLITE_BUSY_SNAPSHOT: {refused}" + ); + + assert!( + !waited_out_the_store(&anyhow::Error::from(refused)), + "a snapshot conflict was classified as a busy store" + ); + } + + /// Item 11 of the 1.90.4 hold (2026-09-13): declassifying a chat must not + /// move it in History. + /// + /// The writer stamped `updated_at = datetime('now')`, so a chat created + /// months ago jumped into History's "Today" (measured across 796 + /// declassifications; 20260224_11 was one). `updated_at` orders every chat + /// list and buckets History's dates, and a classification change is not use + /// of the chat. Fails on `origin/main`. + #[tokio::test] + async fn declassifying_a_chat_does_not_move_it_in_history() { + let temp = tempfile::TempDir::new().unwrap(); + let sm = SessionManager::new(temp.path().to_path_buf()); + const LAST_USED: &str = "2026-02-24 11:01:03"; + let mut ids = vec![]; + for reason in ["turn:versa_azure", "backfill:ollama"] { + let id = private_session_with_reason(&sm, reason).await; + let pool = sm.storage().pool().await.unwrap(); + sqlx::query("UPDATE sessions SET updated_at = ?1 WHERE id = ?2") + .bind(LAST_USED) + .bind(&id) + .execute(pool) + .await + .unwrap(); + ids.push(id); + } + let mut before = vec![]; + for id in &ids { + before.push(sm.get_session(id, false).await.unwrap().updated_at); + } + + // Both graded paths — the single click, and the phrase plus password — + // go through the same statement. + for id in &ids { + assert_eq!( + declassify_for_test(&sm, id).await.unwrap(), + DeclassifyOutcome::Declassified + ); + } + + for (id, was) in ids.iter().zip(before) { + let row = sm.get_session(id, false).await.unwrap(); + assert_eq!(row.privacy_tier, SessionClassification::Public); + assert_eq!( + row.updated_at, was, + "declassifying {id} moved its last-used time from {was} to {}, which re-sorts \ + History and puts a months-old chat under Today", + row.updated_at + ); + } + } } diff --git a/docs/security/privacy-tiers.md b/docs/security/privacy-tiers.md index 102580c0f..58b549eb9 100644 --- a/docs/security/privacy-tiers.md +++ b/docs/security/privacy-tiers.md @@ -128,6 +128,25 @@ this section is the ledger. `biorouter session declassify ` in the CLI, which is the only surface that reaches a private chat no listing shows. + Two properties added on 2026-09-13 (the 1.90.4 hold, items 8 and 11). **A declassification does + not move the chat's `updated_at`**: a classification change is not use of the chat, and stamping + it put months-old chats under History's "Today". The desktop announces the change on its own + channel instead (`ui/desktop/src/utils/sessionRowSync.ts`), so a second window's History row, + sidebar row and session page re-read the row in place. ⚠ **That channel carries raises too, and + must.** Pushing only the lowering let a second window badge a chat PUBLIC after a turn had raised + it straight back — a failure the renderer could not produce before the push existed. A chat store + that sees its chat's tier change announces it (`ChatStreamRegistry.noteControllerTier`), and a + list answer that raced a row read shows the higher tier until a third read settles it. The tab + strip's live map follows its store down as well as up; it used to only rise, which kept a + declassified chat's open tab private until a reload. **A declassification that only waited out + the store's busy timeout answers `503` with `Retry-After` and a sentence** + (`privacy::declassify::DECLASSIFY_STORE_BUSY`), distinct from a genuine failure's `500` + (`routes::session::DECLASSIFY_FAILED`); both mean nothing landed, and neither retries inside + the call — SQLite's busy handler already waited five seconds, and a second + attempt under the load that causes this was measured to fail again three times in seven. The + desktop never reports an outcome it did not read: after any answer that is not a `200` it reads + the row, because an answer can be lost after the daemon wrote. + ⚠ **R18's "no cached grant" is honoured on Linux and NOT guaranteed on macOS (F-13).** R18 asks for a prompt "raised once per operation (no session, no cached grant)". The Linux prompter implements that literally — it declines `pkexec`'s `org.freedesktop.policykit.exec` *because* diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 850fa6bb4..2d4696113 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -4316,7 +4316,10 @@ "description": "Session not found" }, "500": { - "description": "Internal server error" + "description": "Internal server error. Nothing was changed (body = plain text)" + }, + "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": [ diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 419e1f4aa..e90c75c69 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -7584,9 +7584,13 @@ export type DeclassifySessionErrors = { */ 404: unknown; /** - * Internal server error + * Internal server error. Nothing was changed (body = plain text) */ 500: unknown; + /** + * 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) + */ + 503: unknown; }; export type DeclassifySessionResponses = { diff --git a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.test.ts b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.test.ts index f8698c699..77232f7e4 100644 --- a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.test.ts +++ b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.test.ts @@ -9,10 +9,12 @@ import { notifySessionListChanged } from '../../utils/sessionListCache'; const mocks = vi.hoisted(() => ({ listSidebarSessions: vi.fn(), + getSession: vi.fn(), })); vi.mock('../../api', () => ({ listSidebarSessions: mocks.listSidebarSessions, + getSession: mocks.getSession, })); // The proof the desktop sends. Since issue #56's QA sweep (2026-09-10) the @@ -197,3 +199,119 @@ describe('a deleted chat leaves Recents without a reload', () => { expect(result.current.sessions.map((session) => session.id)).toContain('session-19'); }); }); + +/** + * Item 11 of the 1.90.4 hold (2026-09-13). The daemon used to stamp + * `updated_at` on a declassification, so the next head refresh carried the chat + * — moved to the top, which was the bug. With the chat left where it belongs, a + * refresh re-reads only the HEAD of the keyset and never reaches a months-old + * row, so the row is re-marked by id from ANOTHER window's announcement. + */ +describe('a declassified chat is re-marked in Recents without a reload', () => { + it('patches a scrolled-in row where it sits, from a sibling window', async () => { + const firstPage = Array.from({ length: 10 }, (_, index) => ({ + ...makeSummary(index), + privacy_tier: 'private' as const, + })); + const secondPage = Array.from({ length: 10 }, (_, index) => ({ + ...makeSummary(index + 10), + privacy_tier: 'private' as const, + })); + mocks.listSidebarSessions + .mockResolvedValueOnce({ + data: { sessions: firstPage, has_more: true, next_cursor: 'cursor-page-2' }, + }) + .mockResolvedValueOnce({ + data: { sessions: secondPage, has_more: false, next_cursor: null }, + }); + + const { result } = renderHook(() => useSidebarSessions()); + await waitFor(() => expect(result.current.sessions).toHaveLength(10)); + act(() => result.current.loadMore()); + await waitFor(() => expect(result.current.sessions).toHaveLength(20)); + const order = result.current.sessions.map((session) => session.id); + mocks.listSidebarSessions.mockClear(); + mocks.getSession.mockResolvedValue({ + data: { id: 'session-17', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + }); + + // Another window declassified it. + const sibling = new BroadcastChannel('biorouter:session-row'); + try { + sibling.postMessage({ sessionId: 'session-17' }); + await waitFor(() => + expect(result.current.sessions.find((s) => s.id === 'session-17')?.privacy_tier).toBe( + 'public' + ) + ); + } finally { + sibling.close(); + } + + expect(result.current.sessions.map((session) => session.id)).toEqual(order); + expect( + result.current.sessions + .filter((s) => s.id !== 'session-17') + .every((s) => s.privacy_tier === 'private') + ).toBe(true); + // No head refresh was needed — and one could not have reached this row. + expect(mocks.listSidebarSessions).not.toHaveBeenCalled(); + }); + + /** + * Defect D4 of the 2026-09-13 repair round, the sidebar half. A head refresh + * issued before a turn raised a chat, answered after the raise was patched + * in, drew the row public again. + */ + it('a page that raced a raise does not draw the row public again', async () => { + const publicRow = { ...makeSummary(0), privacy_tier: 'public' as const }; + mocks.listSidebarSessions.mockResolvedValueOnce({ + data: { sessions: [publicRow], has_more: false, next_cursor: null }, + }); + const { result } = renderHook(() => useSidebarSessions()); + await waitFor(() => expect(result.current.sessions).toHaveLength(1)); + + let answerPage: ((value: unknown) => void) | undefined; + mocks.listSidebarSessions.mockReturnValueOnce( + new Promise((resolve) => { + answerPage = resolve; + }) + ); + // A membership nudge from anywhere schedules a head refresh. + act(() => notifySessionListChanged()); + await waitFor(() => expect(mocks.listSidebarSessions).toHaveBeenCalledTimes(2)); + + mocks.getSession.mockResolvedValue({ + data: { id: 'session-0', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + }); + const sibling = new BroadcastChannel('biorouter:session-row'); + try { + sibling.postMessage({ sessionId: 'session-0' }); + await waitFor(() => expect(result.current.sessions[0].privacy_tier).toBe('private')); + } finally { + sibling.close(); + } + expect(mocks.getSession).toHaveBeenCalledTimes(1); + + // The read that settles the disagreement is held open, so the row's state + // in the meantime is observable. + let answerThirdRead: ((value: unknown) => void) | undefined; + mocks.getSession.mockReturnValueOnce( + new Promise((resolve) => { + answerThirdRead = resolve; + }) + ); + await act(async () => { + answerPage!({ data: { sessions: [publicRow], has_more: false, next_cursor: null } }); + }); + + await waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(2)); + expect(result.current.sessions[0].privacy_tier).toBe('private'); + await act(async () => { + answerThirdRead!({ + data: { id: 'session-0', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + }); + }); + expect(result.current.sessions[0].privacy_tier).toBe('private'); + }); +}); diff --git a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts index a8c1e59c3..fb796640a 100644 --- a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts +++ b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts @@ -3,6 +3,11 @@ import { listSidebarSessions, type SessionSummary } from '../../api'; import { userActionHeaders } from '../../utils/userAction'; import { subscribeSessionNameChanges } from '../../utils/sessionNameSync'; import { subscribeSessionListChanges, subscribeSessionRemoved } from '../../utils/sessionListCache'; +import { + settleRowsReadDuringFetch, + subscribeSessionRowChanges, + type SessionRowFacts, +} from '../../utils/sessionRowSync'; export const SIDEBAR_SESSION_PAGE_SIZE = 10; @@ -42,12 +47,18 @@ export default function useSidebarSessions(): SidebarSessionsState { const hasMoreRef = useRef(true); const hasLoadedRef = useRef(false); const loadingRef = useRef(false); + // Row reads delivered while a page request was in flight, settled against + // that page's answer. See `settleRowsReadDuringFetch`: without it, a head + // refresh issued before a turn raised a chat and answered after the raise was + // patched in would draw the chat public again. + const rowsReadDuringLoadRef = useRef(new Map()); const loadPage = useCallback(async (reset: boolean) => { if (loadingRef.current || (!reset && !hasMoreRef.current)) return; const cursor = reset ? null : nextCursorRef.current; loadingRef.current = true; + rowsReadDuringLoadRef.current.clear(); setIsLoading(true); try { @@ -59,7 +70,10 @@ export default function useSidebarSessions(): SidebarSessionsState { throwOnError: true, }); const page = response.data; - const mergedSessions = appendSessionPage(sessionsRef.current, page.sessions); + const mergedSessions = appendSessionPage( + sessionsRef.current, + settleRowsReadDuringFetch(page.sessions, rowsReadDuringLoadRef.current, false) + ); const pageHasMore = reset && hasLoadedRef.current ? hasMoreRef.current || page.has_more : page.has_more; @@ -135,6 +149,28 @@ export default function useSidebarSessions(): SidebarSessionsState { setSessions(remaining); }); + // A chat's classification changed in place — a declassification, or a raise + // a chat store announced, here or in another window (`sessionRowSync`). + // Patched by id, for the same reason the removal above is: `loadPage(true)` + // re-reads only the HEAD of the keyset, and a declassified chat is usually + // months old and nowhere near it. Until 2026-09-13 the daemon stamped + // `updated_at` on a declassification, so the next head refresh happened to + // carry the chat (moved to the top, which was the bug); with the chat left + // where it belongs, only this reaches its row. + const unsubscribeRows = subscribeSessionRowChanges((facts) => { + const { sessionId, privacy_tier } = facts; + if (loadingRef.current) rowsReadDuringLoadRef.current.set(sessionId, facts); + let changed = false; + const next = sessionsRef.current.map((session) => { + if (session.id !== sessionId || session.privacy_tier === privacy_tier) return session; + changed = true; + return { ...session, privacy_tier }; + }); + if (!changed) return; + sessionsRef.current = next; + setSessions(next); + }); + window.addEventListener('session-created', scheduleRefresh); window.addEventListener('message-stream-finished', scheduleRefresh); @@ -142,6 +178,7 @@ export default function useSidebarSessions(): SidebarSessionsState { unsubscribeNames(); unsubscribeList(); unsubscribeRemoved(); + unsubscribeRows(); if (refreshTimer !== undefined) window.clearTimeout(refreshTimer); window.removeEventListener('session-created', scheduleRefresh); window.removeEventListener('message-stream-finished', scheduleRefresh); diff --git a/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx b/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx index cb91a7a83..781d54513 100644 --- a/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx +++ b/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx @@ -108,16 +108,22 @@ function renderLayout( * * # The merge is `max`, not "freshest wins" * - * The tier is a permanent ratchet server-side - * (`crates/biorouter/src/privacy/mod.rs`) — public → private, never back — so a - * `private` from ANY source is a fact that still holds, and a `public` is only - * a lower bound. {@link mergeSessionTiers} folds the two with `max` and - * `undefined` stays unmarked. The invariant, which + * The tier is a ratchet server-side (`crates/biorouter/src/privacy/mod.rs`) + * with one exit, the user's declassification — so a `public` is only a lower + * bound, and a `private` holds until a declassification that BOTH sources are + * told about (`sessionRowSync`, the change feed). {@link mergeSessionTiers} + * folds the two with `max` and `undefined` stays unmarked. The invariant, which * `ChatGroupsShell.privacy.test.tsx` pins: this map may render private-from- - * either-source or unmarked, and can never render public over a source that has - * seen private. There is no failure mode in which it over-marks — no source - * here invents a tier, they only report a row. `ChatTabStrip`'s `privacyTiers` - * prop doc states the same thing, and the two must not drift apart again. + * either-source or unmarked, and can never render public over a source that + * still holds private. There is no failure mode in which it over-marks — no + * source here invents a tier, they only report a row. `ChatTabStrip`'s + * `privacyTiers` prop doc states the same thing, and the two must not drift + * apart again. + * + * ⚠ `max` across sources is only right because each source follows its OWN + * row down. The live map once refused to (it "mirrored the ratchet"), so a + * declassified chat's tab stayed private for as long as it was open — defect D1 + * of 2026-09-13; see `ChatStreamRegistry.subscribeSessionTiers`. * * # The cache still has to be warmed here * diff --git a/ui/desktop/src/components/privacy/sessionTier.ts b/ui/desktop/src/components/privacy/sessionTier.ts index 557e6adb1..68832c579 100644 --- a/ui/desktop/src/components/privacy/sessionTier.ts +++ b/ui/desktop/src/components/privacy/sessionTier.ts @@ -8,18 +8,28 @@ import type { SessionClassification } from '../../api/types.gen'; * `SessionClassification` is a two-element lattice — `public < private` — and * the daemon reduces it with `max` over the life of a session * (`crates/biorouter/src/privacy/mod.rs`; CLAUDE.md calls it "a permanent - * ratchet"). It only ever rises. That single fact decides every question a - * caller could otherwise get wrong: + * ratchet"). It rises on its own and falls only when the USER declassifies the + * chat (§12.4, `privacy::declassify`). So: * - * - A reading of `private` can never become false. Whoever saw it saw a fact - * about the row that still holds, however old the reading is. * - A reading of `public` can become false at any moment, and says nothing * about now. It is a lower bound, not an answer. + * - A reading of `private` can become false only through a declassification, + * which every source here is told about (`sessionRowSync`, the change feed). * - * So two readings of the same chat are combined with `max`, never with - * "whichever is fresher". Freshness is not the ordering that matters here, and - * a merge that preferred the newer source would let a source which has not yet - * heard about a ratchet overwrite one that has. + * ⚠ **That second line used to say "can never become false", and the tab strip + * was built on it.** The live map in `ChatStreamRegistry` then refused to + * follow its own store down, so a chat declassified with its tab open kept a + * private tab icon until the renderer reloaded (defect D1, 2026-09-13). The + * rule that survives is narrower, and it is two rules: + * + * - WITHIN one source, the newest reading wins. Each source is re-read when the + * row moves, in either direction, and holds what it last read. + * - ACROSS sources, `max` — these functions. Two sources disagree only while + * one of them has not yet been re-read, and while that is so the chat is + * shown private. A merge that preferred "the fresher source" would let one + * that has not yet heard about a raise overwrite one that has, and that is + * the direction a badge must never be wrong in; the price of `max` is a + * lowering that shows up once the slower source has been re-read too. * * # The direction a mistake must fall in * diff --git a/ui/desktop/src/components/sessions/DeclassifySessionDialog.test.tsx b/ui/desktop/src/components/sessions/DeclassifySessionDialog.test.tsx index e58ad0328..b9c4638b5 100644 --- a/ui/desktop/src/components/sessions/DeclassifySessionDialog.test.tsx +++ b/ui/desktop/src/components/sessions/DeclassifySessionDialog.test.tsx @@ -1,12 +1,25 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { DeclassifySessionDialog } from './DeclassifySessionDialog'; +import { DeclassifySessionDialog, declassifyToastSubject } from './DeclassifySessionDialog'; import type { Session } from '../../api'; const mocks = vi.hoisted(() => ({ declassifySession: vi.fn(), - toastError: vi.fn(), + // Returns the id the real `toastError` returns: its dedupe key, so two + // identical failures on one chat share one id exactly as they share one toast. + // The toast layer itself is exercised in `DeclassifySessionDialog.toastLayer.test.tsx`. + toastError: vi.fn( + ({ title, msg, dedupeScope }: { title: string; msg: string; dedupeScope?: string }) => + `error[${dedupeScope}]:${title}:${msg}` + ), + toastSuccess: vi.fn(), + dismissToast: vi.fn(), + announceSessionRowChanged: vi.fn(), + readSessionRowFacts: vi.fn(), + // Row reads delivered in this window, for the listener the dialog module + // installs; `deliverRow` below plays one. + rowListeners: new Set<(facts: { sessionId: string; privacy_tier: string }) => void>(), })); vi.mock('../../api', () => ({ @@ -15,13 +28,69 @@ vi.mock('../../api', () => ({ vi.mock('../../toasts', () => ({ toastError: mocks.toastError, - toastSuccess: vi.fn(), + toastSuccess: mocks.toastSuccess, + toastService: { dismiss: mocks.dismissToast }, })); vi.mock('../../utils/userAction', () => ({ userActionHeaders: async () => ({ 'X-User-Action': 'test-key' }), })); +vi.mock('../../utils/sessionRowSync', () => ({ + announceSessionRowChanged: mocks.announceSessionRowChanged, + readSessionRowFacts: mocks.readSessionRowFacts, + subscribeSessionRowChanges: ( + listener: (facts: { sessionId: string; privacy_tier: string }) => void + ) => { + mocks.rowListeners.add(listener); + return () => mocks.rowListeners.delete(listener); + }, +})); + +function deliverRow(sessionId: string, privacy_tier: 'public' | 'private') { + for (const listener of [...mocks.rowListeners]) listener({ sessionId, privacy_tier }); +} + +/** The row as `readSessionRowFacts` would hand it back after a failure. */ +function rowReads(privacy_tier: 'public' | 'private' | null) { + mocks.readSessionRowFacts.mockResolvedValue( + privacy_tier === null + ? null + : { + sessionId: s.id, + privacy_tier, + privacy_reason: privacy_tier === 'public' ? 'declassified_by_user' : 'turn:versa_azure', + } + ); +} + +/** + * Answer the way the generated client (`api/client/client.gen.ts`) really does, + * for BOTH ways of calling it — which is what lets a test here fail on the code + * it replaced rather than on a mock that only fits the new call. + * + * With `throwOnError` the client throws the parsed BODY and the Response is + * gone; a plain-text body stays a string and an empty one becomes `{}` + * (`finalError || {}`). Without it the same body comes back as `error`, beside + * the Response. + */ +function answerLikeTheClient(status: number, body: string) { + return async (options: { throwOnError?: boolean }) => { + if (status === 200) { + const data = { sessionId: s.id, privacyTier: 'public' }; + return { data, request: {}, response: { status } }; + } + const error = body || {}; + if (options.throwOnError) throw error; + return { data: undefined, error, request: {}, response: { status } }; + }; +} + +/** The daemon's 503 body, `privacy::declassify::DECLASSIFY_STORE_BUSY`. */ +const STORE_BUSY = + 'Nothing was changed and this chat was not marked public, because other writes kept the chat ' + + 'store busy. Try again in a moment.'; + const s = { id: 'abc123def456', name: 'Cohort of 4,102 patients', @@ -36,7 +105,10 @@ const s = { beforeEach(() => { vi.clearAllMocks(); - mocks.declassifySession.mockResolvedValue({}); + mocks.declassifySession.mockImplementation(answerLikeTheClient(200, '')); + // Every failure re-reads the row before it says anything. Unless a test says + // otherwise, the write did not land. + rowReads('private'); }); afterEach(() => { @@ -159,10 +231,12 @@ describe('DeclassifySessionDialog', () => { path: { session_id: 'abc123def456' }, body: { confirmation: 'def456' }, headers: { 'X-User-Action': 'test-key' }, - throwOnError: true, }) ); await waitFor(() => expect(onDeclassified).toHaveBeenCalledWith('abc123def456')); + // Item 11: the change announces itself, so a second window's lists re-read + // the row instead of waiting for a re-sort that no longer happens. + expect(mocks.announceSessionRowChanged).toHaveBeenCalledWith('abc123def456'); }); it('the single-click path holds the request open for the undo window', async () => { @@ -210,7 +284,6 @@ describe('DeclassifySessionDialog', () => { path: { session_id: 'abc123def456' }, body: { confirmation: null }, headers: { 'X-User-Action': 'test-key' }, - throwOnError: true, }) ); await waitFor(() => expect(onDeclassified).toHaveBeenCalledWith('abc123def456')); @@ -289,8 +362,11 @@ describe('DeclassifySessionDialog', () => { // stale prop, so clicking again fails identically — an unrecoverable // dialog. The strong control is always an acceptable answer to a refusal, // and it is the only one that can recover from a stale grade. - mocks.declassifySession.mockRejectedValue( - new Error("The confirmation did not match the last six characters of this chat's id.") + mocks.declassifySession.mockImplementation( + answerLikeTheClient( + 400, + "The confirmation did not match the last six characters of this chat's id. Nothing was changed." + ) ); render( @@ -313,7 +389,11 @@ describe('DeclassifySessionDialog', () => { it('surfaces a refusal instead of claiming the chat is now public', async () => { const user = userEvent.setup(); const onDeclassified = vi.fn(); - mocks.declassifySession.mockRejectedValue('Nothing was changed.'); + const refusal = + 'This chat does not record an observed turn on a private model as the reason it is private, ' + + 'so marking it public needs your operating system to confirm it is you. That did not happen, ' + + 'and nothing was changed.'; + mocks.declassifySession.mockImplementation(answerLikeTheClient(403, refusal)); render( @@ -321,7 +401,345 @@ describe('DeclassifySessionDialog', () => { await user.type(screen.getByLabelText(/last 6 characters/i), 'def456'); await user.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => + expect(mocks.toastError).toHaveBeenCalledWith({ + title: 'Could not mark this chat public — “Cohort of 4,102 patients” (abc123def456)', + msg: refusal, + dedupeScope: 'declassify:abc123def456', + }) + ); + expect(onDeclassified).not.toHaveBeenCalled(); + expect(mocks.toastSuccess).not.toHaveBeenCalled(); + expect(mocks.announceSessionRowChanged).not.toHaveBeenCalled(); + }); + + it('a thrown request (the daemon unreachable) is a failure, not a success', async () => { + const user = userEvent.setup(); + const onDeclassified = vi.fn(); + mocks.declassifySession.mockRejectedValue(new TypeError('Failed to fetch')); + + render( + + ); + await user.type(screen.getByLabelText(/last 6 characters/i), 'def456'); + await user.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => expect(mocks.toastError).toHaveBeenCalled()); + expect(onDeclassified).not.toHaveBeenCalled(); + expect(mocks.toastSuccess).not.toHaveBeenCalled(); + }); + + // Item 8 of the 1.90.4 hold (2026-09-13). Reproduced in the running app by + // holding `sessions.db`'s write lock across one single-click declassification: + // the daemon answered a bodyless 500 after its five-second busy wait, the toast + // read "Could not mark this chat public / [object Object]", and the dialog + // swapped the single click for the typed phrase under "That request was + // refused. This chat's record has changed since this list was loaded" — none + // of which was true. + it('a busy store says so in the daemon’s words and keeps the single click', async () => { + mocks.declassifySession.mockImplementation(answerLikeTheClient(503, STORE_BUSY)); + const onDeclassified = vi.fn(); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => + expect(mocks.toastError).toHaveBeenCalledWith({ + title: 'The chat store was busy — “Cohort of 4,102 patients” (abc123def456)', + msg: STORE_BUSY, + dedupeScope: 'declassify:abc123def456', + }) + ); + // Not escalated: a busy store says nothing about this chat's grade, so the + // control it was offered is still the right one to try again with. + await waitFor(() => expect(screen.getByRole('button', { name: /Make public/ })).toBeEnabled()); + expect(screen.queryByRole('textbox')).toBeNull(); + expect(screen.queryByText(/record has changed/i)).toBeNull(); + // And never reported public. + expect(onDeclassified).not.toHaveBeenCalled(); + expect(mocks.toastSuccess).not.toHaveBeenCalled(); + expect(mocks.announceSessionRowChanged).not.toHaveBeenCalled(); + }); + + it('a failure with no body is described, never shown as [object Object]', async () => { + mocks.declassifySession.mockImplementation(answerLikeTheClient(500, '')); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => expect(mocks.toastError).toHaveBeenCalled()); + const { title, msg } = mocks.toastError.mock.calls[0][0] as { title: string; msg: string }; + expect(msg).not.toContain('[object Object]'); + // Stated as what the row read back, which is all this dialog knows. + expect(msg).toMatch(/still private/); + expect(title).toBe( + 'Could not mark this chat public — “Cohort of 4,102 patients” (abc123def456)' + ); + expect(screen.queryByRole('textbox')).toBeNull(); + }); +}); + +/** + * Defect D2 of the 2026-09-13 repair round. The daemon wrote, and its answer + * never reached the renderer: measured by failing the POST's RESPONSE in the + * running app (CDP `Fetch.failRequest` at the response stage) with the daemon's + * 200 already sent. The database read `public` with one ledger row; the toast + * read "Biorouter could not be reached, so this chat was not marked public.", + * the dialog stayed open, and both windows' rows stayed private. A missing + * answer is not a "no", so the row is asked before a word is said. + */ +describe('an answer that never arrived', () => { + const lostAnswer = () => + mocks.declassifySession.mockRejectedValue(new TypeError('Failed to fetch')); + + it('is a success when the row reads public — the write landed', async () => { + const user = userEvent.setup(); + const onDeclassified = vi.fn(); + const onClose = vi.fn(); + lostAnswer(); + rowReads('public'); + + render( + + ); + await user.type(screen.getByLabelText(/last 6 characters/i), 'def456'); + await user.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => expect(onDeclassified).toHaveBeenCalledWith('abc123def456')); + expect(mocks.readSessionRowFacts).toHaveBeenCalledWith('abc123def456'); + expect(mocks.toastSuccess).toHaveBeenCalled(); + expect(mocks.toastError).not.toHaveBeenCalled(); + // Every window's rows re-read it, as after any success. + expect(mocks.announceSessionRowChanged).toHaveBeenCalledWith('abc123def456'); + expect(onClose).toHaveBeenCalled(); + }); + + it('says the chat is still private when the row says so, and not more', async () => { + const onDeclassified = vi.fn(); + lostAnswer(); + rowReads('private'); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(mocks.toastError).toHaveBeenCalled()); + const { msg } = mocks.toastError.mock.calls[0][0] as { msg: string }; + expect(msg).toMatch(/still private/); expect(onDeclassified).not.toHaveBeenCalled(); + expect(mocks.announceSessionRowChanged).not.toHaveBeenCalled(); + // A lost answer says nothing about the grade. + expect(screen.queryByRole('textbox')).toBeNull(); + }); + + it('claims neither outcome when the row cannot be read either', async () => { + const onDeclassified = vi.fn(); + lostAnswer(); + rowReads(null); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => expect(mocks.toastError).toHaveBeenCalled()); + const { msg } = mocks.toastError.mock.calls[0][0] as { msg: string }; + expect(msg).toMatch(/could not be asked whether this chat is now public/); + expect(msg).not.toMatch(/not marked public/); + expect(msg).not.toMatch(/still private/); + expect(onDeclassified).not.toHaveBeenCalled(); + expect(mocks.toastSuccess).not.toHaveBeenCalled(); + }); + + it('keeps the daemon’s own sentence when it gave one and the row is unreadable', async () => { + mocks.declassifySession.mockImplementation(answerLikeTheClient(503, STORE_BUSY)); + rowReads(null); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => + expect(mocks.toastError).toHaveBeenCalledWith({ + title: 'The chat store was busy — “Cohort of 4,102 patients” (abc123def456)', + msg: STORE_BUSY, + dedupeScope: 'declassify:abc123def456', + }) + ); + }); +}); + +/** + * Defect D3a of the 2026-09-13 repair round. Error toasts do not expire, so a + * failure that a later attempt overturned stayed beside "Chat marked public": + * measured still on screen 60 s after the success toast had closed — after two + * busy answers, after a lost answer, and after the escalation's refusal. + */ +describe('a failure report is retracted by the next outcome', () => { + it('a later success dismisses the failure it overturned', async () => { + mocks.declassifySession + .mockImplementationOnce(answerLikeTheClient(503, STORE_BUSY)) + .mockImplementationOnce(answerLikeTheClient(200, '')); + const onDeclassified = vi.fn(); + + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(mocks.toastError).toHaveBeenCalledTimes(1)); + const failureId = mocks.toastError.mock.results[0].value; + expect(mocks.dismissToast).not.toHaveBeenCalled(); + + await waitFor(() => expect(screen.getByRole('button', { name: /Make public/ })).toBeEnabled()); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + + await waitFor(() => expect(onDeclassified).toHaveBeenCalled()); + expect(mocks.dismissToast).toHaveBeenCalledWith(failureId); + expect(mocks.toastSuccess).toHaveBeenCalled(); + }); + + it('reaches a failure raised before the dialog was closed and reopened', async () => { + mocks.declassifySession + .mockImplementationOnce(answerLikeTheClient(503, STORE_BUSY)) + .mockImplementationOnce(answerLikeTheClient(200, '')); + const session = { ...s, id: 'reopened0001', privacy_reason: 'turn:versa_azure' }; + + const first = render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(mocks.toastError).toHaveBeenCalledTimes(1)); + const failureId = mocks.toastError.mock.results[0].value; + // Both entry points unmount the dialog when it closes. + first.unmount(); + + const onDeclassified = vi.fn(); + render( + + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(onDeclassified).toHaveBeenCalled()); + expect(mocks.dismissToast).toHaveBeenCalledWith(failureId); + }); + + it('a declassification made elsewhere retracts the failure too', async () => { + // Fail here, close the dialog, and mark the chat public from another + // window: this window hears it as a row read, and its "still private" toast + // is then a report about a chat that is not. + mocks.declassifySession.mockImplementation(answerLikeTheClient(503, STORE_BUSY)); + const session = { ...s, id: 'elsewhere001', privacy_reason: 'turn:versa_azure' }; + const view = render(); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(mocks.toastError).toHaveBeenCalledTimes(1)); + const failureId = mocks.toastError.mock.results[0].value; + view.unmount(); + + deliverRow('elsewhere001', 'private'); + expect(mocks.dismissToast).not.toHaveBeenCalled(); + + deliverRow('elsewhere001', 'public'); + expect(mocks.dismissToast).toHaveBeenCalledWith(failureId); + }); + + it('a different failure replaces the earlier one; the same failure keeps its toast', async () => { + const refusal = + "The confirmation did not match the last six characters of this chat's id. Nothing was changed."; + mocks.declassifySession + .mockImplementationOnce(answerLikeTheClient(503, STORE_BUSY)) + .mockImplementationOnce(answerLikeTheClient(503, STORE_BUSY)) + .mockImplementationOnce(answerLikeTheClient(400, refusal)); + const session = { ...s, id: 'replaced0001', privacy_reason: 'turn:versa_azure' }; + + render(); + const press = async (times: number) => { + await waitFor(() => + expect(screen.getByRole('button', { name: /Make public/ })).toBeEnabled() + ); + fireEvent.click(screen.getByRole('button', { name: /Make public/ })); + await waitFor(() => expect(mocks.toastError).toHaveBeenCalledTimes(times)); + }; + + await press(1); + const busyId = mocks.toastError.mock.results[0].value; + await press(2); + // Deduplicated onto the same toast: dismissing it would take away the + // report of the attempt just made. + expect(mocks.dismissToast).not.toHaveBeenCalled(); + + await press(3); + expect(mocks.dismissToast).toHaveBeenCalledWith(busyId); + }); +}); + +describe('declassifyToastSubject', () => { + it('names a placeholder-named chat by its id, since dozens of rows share the name', () => { + expect(declassifyToastSubject('New Session', '20260809_21')).toBe('chat 20260809_21'); + expect(declassifyToastSubject('New chat', '20260809_21')).toBe('chat 20260809_21'); + expect(declassifyToastSubject(' ', '20260809_21')).toBe('chat 20260809_21'); + expect(declassifyToastSubject(undefined, '20260809_21')).toBe('chat 20260809_21'); + }); + + it('quotes any other name, cuts a long one short, and follows it with the id', () => { + expect(declassifyToastSubject(' Subagent delegation request ', 'x')).toBe( + '“Subagent delegation request” (x)' + ); + const long = declassifyToastSubject('a'.repeat(59) + '😀😀', 'x'); + expect(long).toBe(`“${'a'.repeat(59)}…” (x)`); + // Characters, not UTF-16 units: an emoji at the cut is kept whole or dropped. + expect(declassifyToastSubject('b'.repeat(58) + '😀😀😀', 'x')).toBe( + `“${'b'.repeat(58)}😀…” (x)` + ); + }); + + it('tells two chats with the same name apart', () => { + // Auto-generated names repeat: the seed data already holds 20260809_23 and + // 20260809_25, both "Subagent delegation request". Two failures at once must + // not raise two toasts that read the same. + const a = declassifyToastSubject('Subagent delegation request', '20260809_23'); + const b = declassifyToastSubject('Subagent delegation request', '20260809_25'); + expect(a).not.toBe(b); + expect(a).toContain('20260809_23'); + expect(b).toContain('20260809_25'); }); }); diff --git a/ui/desktop/src/components/sessions/DeclassifySessionDialog.toastLayer.test.tsx b/ui/desktop/src/components/sessions/DeclassifySessionDialog.toastLayer.test.tsx new file mode 100644 index 000000000..273ec3b3e --- /dev/null +++ b/ui/desktop/src/components/sessions/DeclassifySessionDialog.toastLayer.test.tsx @@ -0,0 +1,306 @@ +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { useEffect } from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { ToastContainer, toast, type ToastTransitionProps } from 'react-toastify'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { DeclassifySessionDialog } from './DeclassifySessionDialog'; +import type { Session } from '../../api'; + +/** + * Defect D3a, second round (2026-09-13), measured by an independent tester on + * the desktop and reproduced in the dev app before this file existed: + * + * 1. chat Y fails with the store write-locked → 503, "The chat store was busy"; + * 2. chat X fails the same way → still ONE toast on screen; + * 3. X is retried and succeeds → "Chat marked public", and eight seconds later + * nothing at all. The database read Y private and X public: Y's failure + * report was gone although nothing about Y had been decided. + * + * The dialog kept its outstanding reports by CHAT, and `toastError` deduplicated + * them by CONTENT. The busy sentence is identical for every chat, so both + * reports were one toast, and X's success dismissed it. + * + * ⚠ Why these run against the REAL toast layer — `toasts.tsx` and + * react-toastify's container — when `DeclassifySessionDialog.test.tsx` mocks + * `toastError`: the defect lived in the seam between the two keys, and a mock of + * either half encodes an assumption about the other. What is asserted here is + * what the person sees: which reports are on screen. + */ + +const mocks = vi.hoisted(() => ({ + declassifySession: vi.fn(), + announceSessionRowChanged: vi.fn(), + // What `readSessionRowFacts` reads back after a failure, per chat. + rowTiers: new Map(), + rowListeners: new Set<(facts: { sessionId: string; privacy_tier: string }) => void>(), +})); + +vi.mock('../../api', () => ({ + declassifySession: mocks.declassifySession, +})); + +vi.mock('../../utils/userAction', () => ({ + userActionHeaders: async () => ({ 'X-User-Action': 'test-key' }), +})); + +vi.mock('../../utils/sessionRowSync', () => ({ + announceSessionRowChanged: mocks.announceSessionRowChanged, + readSessionRowFacts: async (sessionId: string) => ({ + sessionId, + privacy_tier: mocks.rowTiers.get(sessionId) ?? 'private', + privacy_reason: 'turn:versa_azure', + }), + subscribeSessionRowChanges: ( + listener: (facts: { sessionId: string; privacy_tier: string }) => void + ) => { + mocks.rowListeners.add(listener); + return () => mocks.rowListeners.delete(listener); + }, +})); + +/** The daemon's 503 body, `privacy::declassify::DECLASSIFY_STORE_BUSY`. */ +const STORE_BUSY = + 'Nothing was changed and this chat was not marked public, because other writes kept the chat ' + + 'store busy. Try again in a moment.'; + +/** + * The daemon's 500 body, `routes::session::DECLASSIFY_FAILED`. Not a 400: that + * one escalates the dialog to the typed phrase, which is not what these test. + */ +const FAILED = + 'Nothing was changed and this chat was not marked public, because Biorouter hit an error. The ' + + 'daemon log records it.'; + +type Answer = { status: number; body?: string }; + +/** Answers each chat's requests in order, the way the generated client returns them. */ +const answers = new Map(); +function answer(sessionId: string, ...queue: Answer[]) { + answers.set(sessionId, [...(answers.get(sessionId) ?? []), ...queue]); +} + +/** + * react-toastify removes a dismissed toast when its exit ANIMATION ends, and + * jsdom runs no animations — so without this every dismissed toast would stay in + * the document and "is it on screen" could never be answered. Only the animation + * is replaced; dismissal, dedup and the container's store are the real ones. + */ +function NoAnimation({ children, isIn, done }: ToastTransitionProps) { + useEffect(() => { + if (!isIn) done(); + }, [isIn, done]); + return <>{children}; +} + +/** Chat Y: a placeholder name, as `20260809_21` has ("New Session"). */ +function chatY(suffix: string): Session { + return { + id: `20260809_21${suffix}`, + name: 'New Session', + working_dir: '/tmp', + created_at: '2026-08-09T12:00:00Z', + updated_at: '2026-08-09T12:00:00Z', + extension_data: {}, + message_count: 4, + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + } as unknown as Session; +} + +/** Chat X: a named chat, as `20260809_23` is. */ +function chatX(suffix: string): Session { + return { ...chatY(suffix), id: `20260809_23${suffix}`, name: 'Subagent delegation request' }; +} + +/** + * Open the dialog on `session`, the way both entry points mount it. `press` + * presses Make public in THIS dialog (the single click, so after the undo + * window) and waits for the answer: a failure hands the button back, a success + * calls `onClose`. `close` unmounts it, which is what both entry points do on + * close. One dialog is open at a time, as in the app. + */ +function openDialog(session: Session) { + const onClose = vi.fn(); + const view = render( + + + + ); + const makePublic = () => screen.getByRole('button', { name: /Make public/ }); + return { + press: async () => { + const closedBefore = onClose.mock.calls.length; + await waitFor(() => expect(makePublic()).toBeEnabled()); + fireEvent.click(makePublic()); + await waitFor(() => expect(mocks.declassifySession).toHaveBeenCalledTimes(1)); + await waitFor(() => { + if (onClose.mock.calls.length === closedBefore) expect(makePublic()).toBeEnabled(); + }); + mocks.declassifySession.mockClear(); + }, + close: view.unmount, + }; +} + +/** Fail (or succeed) once on `session` and close the dialog. */ +async function attemptOnce(session: Session) { + const dialog = openDialog(session); + await dialog.press(); + dialog.close(); +} + +/** Every report on screen, as the text a person reads. */ +function reports(): string[] { + return screen + .queryAllByRole('alert') + .map((alert) => (alert.textContent ?? '').replace(/\s+/g, ' ').trim()); +} +const busyReports = () => reports().filter((text) => text.includes('Try again in a moment')); + +beforeEach(() => { + vi.clearAllMocks(); + answers.clear(); + mocks.rowTiers.clear(); + mocks.declassifySession.mockImplementation(async ({ path }: { path: { session_id: string } }) => { + const next = answers.get(path.session_id)?.shift() ?? { status: 200 }; + if (next.status === 200) { + mocks.rowTiers.set(path.session_id, 'public'); + return { + data: { sessionId: path.session_id, privacyTier: 'public' }, + request: {}, + response: { status: 200 }, + }; + } + return { data: undefined, error: next.body, request: {}, response: { status: next.status } }; + }); + render( + + + + ); +}); + +afterEach(async () => { + await act(async () => { + toast.dismiss(); + }); + cleanup(); +}); + +describe('a failure report belongs to its chat', () => { + it('a success on one chat leaves another chat’s failure on screen', async () => { + const y = chatY('a'); + const x = chatX('a'); + answer(y.id, { status: 503, body: STORE_BUSY }); + answer(x.id, { status: 503, body: STORE_BUSY }, { status: 200 }); + + await attemptOnce(y); + const onX = openDialog(x); + await onX.press(); + await onX.press(); + onX.close(); + + await waitFor(() => + expect(reports().some((t) => t.startsWith('Chat marked public'))).toBe(true) + ); + // Nothing about Y was decided, so Y's report is still there — and it is the + // only failure left, because X's own success retracted X's. + expect(busyReports()).toHaveLength(1); + expect(busyReports()[0]).toContain(`chat ${y.id}`); + expect(reports().some((t) => t.includes('Subagent delegation request'))).toBe(false); + }); + + it('two chats failing at once are two reports, each naming its chat', async () => { + const y = chatY('b'); + const x = chatX('b'); + answer(y.id, { status: 503, body: STORE_BUSY }); + answer(x.id, { status: 503, body: STORE_BUSY }); + + await attemptOnce(y); + await attemptOnce(x); + + // The same sentence twice is only useful if each says which chat it is + // about. A placeholder name is shared by dozens of rows, so that chat is + // named by its id. + const busy = busyReports(); + expect(busy).toHaveLength(2); + expect(busy.filter((t) => t.includes(`chat ${y.id}`))).toHaveLength(1); + expect(busy.filter((t) => t.includes('“Subagent delegation request”'))).toHaveLength(1); + }); + + it('two chats with the same name still keep their own reports', async () => { + // Naming the chat in the title is for the person; it is not what keeps the + // reports apart. Two chats can share a name, and then only the chat's id in + // the dedup scope stops one's success taking down the other's report. + const first = { ...chatX('f'), id: '20260809_31f', name: 'Weekly cohort' }; + const second = { ...chatX('f'), id: '20260809_32f', name: 'Weekly cohort' }; + answer(first.id, { status: 503, body: STORE_BUSY }); + answer(second.id, { status: 503, body: STORE_BUSY }, { status: 200 }); + + await attemptOnce(first); + const onSecond = openDialog(second); + await onSecond.press(); + expect(busyReports()).toHaveLength(2); + await onSecond.press(); + onSecond.close(); + + await waitFor(() => + expect(reports().some((t) => t.startsWith('Chat marked public'))).toBe(true) + ); + expect(busyReports()).toHaveLength(1); + }); + + it('a same-chat retry replaces its own report rather than stacking', async () => { + const x = chatX('c'); + answer(x.id, { status: 503, body: STORE_BUSY }, { status: 503, body: STORE_BUSY }); + + const onX = openDialog(x); + await onX.press(); + await onX.press(); + onX.close(); + + expect(busyReports()).toHaveLength(1); + }); + + it('a different failure on one chat does not take down another chat’s report', async () => { + const y = chatY('d'); + const x = chatX('d'); + answer(y.id, { status: 503, body: STORE_BUSY }); + answer(x.id, { status: 503, body: STORE_BUSY }, { status: 500, body: FAILED }); + + await attemptOnce(y); + const onX = openDialog(x); + await onX.press(); + await onX.press(); + onX.close(); + + const busy = busyReports(); + expect(busy).toHaveLength(1); + expect(busy[0]).toContain(`chat ${y.id}`); + // X's busy report was replaced by its new failure, not left beside it. + const faults = reports().filter((t) => t.includes('Biorouter hit an error')); + expect(faults).toHaveLength(1); + expect(faults[0]).toContain('“Subagent delegation request”'); + }); + + it('a public row read about one chat retracts only that chat’s report', async () => { + const y = chatY('e'); + const x = chatX('e'); + answer(y.id, { status: 503, body: STORE_BUSY }); + answer(x.id, { status: 503, body: STORE_BUSY }); + + await attemptOnce(y); + await attemptOnce(x); + expect(busyReports()).toHaveLength(2); + + // X declassified from another window: this window hears it as a row read. + await act(async () => { + for (const listener of [...mocks.rowListeners]) { + listener({ sessionId: x.id, privacy_tier: 'public' }); + } + }); + + await waitFor(() => expect(busyReports()).toHaveLength(1)); + expect(busyReports()[0]).toContain(`chat ${y.id}`); + }); +}); diff --git a/ui/desktop/src/components/sessions/DeclassifySessionDialog.tsx b/ui/desktop/src/components/sessions/DeclassifySessionDialog.tsx index 6fe1478ec..bcb8e5b06 100644 --- a/ui/desktop/src/components/sessions/DeclassifySessionDialog.tsx +++ b/ui/desktop/src/components/sessions/DeclassifySessionDialog.tsx @@ -1,7 +1,13 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; import { declassifySession, type Session } from '../../api'; -import { toastError, toastSuccess } from '../../toasts'; +import { toastError, toastService, toastSuccess } from '../../toasts'; import { userActionHeaders } from '../../utils/userAction'; +import { + announceSessionRowChanged, + readSessionRowFacts, + subscribeSessionRowChanges, +} from '../../utils/sessionRowSync'; +import { isDefaultSessionName } from '../../utils/sessionNameSync'; import { DangerousConfirmDialog } from '../ui/DangerousConfirmDialog'; import { Dialog, @@ -83,6 +89,173 @@ export function confirmationPhrase(sessionId: string): string { return [...sessionId].slice(-6).join(''); } +/** + * What went wrong, in the words the person is shown — item 8 of the 1.90.4 + * hold (2026-09-13). + * + * The daemon answers every failure of `POST /sessions/{id}/declassify` with a + * plain-text sentence, and that sentence is the message. It did not always: a + * lock timeout was a bodyless 500, the generated client throws the parsed body + * rather than the Response, and `String({})` put **`[object Object]`** in the + * toast. Measured in the running app with the store's write lock held across + * one click. + * + * So the status is read off the Response, never inferred from the body, and a + * body that is not a sentence gets one written here. A 503 is the daemon saying + * the chat store stayed write-locked by other work past its wait: the one + * failure a retry clears, titled so it cannot be read as a refusal. + * + * # A missing answer is not a "no" — defect D2 (2026-09-13) + * + * ⚠ This used to say "no Response means the request never reached the daemon" + * and tell the person **"this chat was not marked public"**. A Response can be + * lost AFTER the daemon wrote: measured by failing the POST's response in the + * renderer with the daemon's 200 already sent — the database read `public` + * with one ledger row while the toast said the chat was not marked public, the + * dialog stayed open, and both windows' rows stayed private. + * + * So nothing here reports an outcome the dialog did not read. After any answer + * that is not a 200 the row is read again (`readSessionRowFacts`), and: + * + * - a row that reads PUBLIC is a success, however it got there (the lost + * answer, or another window's declassification), and is never described + * here — `send` takes the success path instead; + * - a row that reads PRIVATE is the failure, stated as the chat's state + * ("still private"), with the daemon's own sentence when it gave one; + * - a row that cannot be read leaves only the daemon's sentence, if there was + * one — the daemon said nothing changed, and it is the one that knows. With + * neither, the dialog says it could not find out, and claims nothing. + */ +export interface DeclassifyFailure { + status: number | undefined; + title: string; + message: string; +} + +export function describeDeclassifyFailure( + status: number | undefined, + body: unknown, + /** The row as read AFTER the failure; `null` when it could not be read. */ + rowTier: 'public' | 'private' | null +): DeclassifyFailure { + const sentence = typeof body === 'string' && body.trim().length > 0 ? body.trim() : undefined; + let message: string; + if (sentence !== undefined) { + message = sentence; + } else if (rowTier === 'private') { + message = + status === undefined + ? 'No answer came back from Biorouter, and this chat is still private. Try again.' + : `Biorouter answered ${status} without saying why, and this chat is still private.`; + } else { + // ⚠ Kept short on purpose: the toast clamps its message to three lines and + // cuts a clause past them silently. Both are shorter than a 142-character + // version of the first, which was measured filling all three lines. + message = + status === undefined + ? 'No answer came back from Biorouter, and it could not be asked whether this chat is ' + + 'now public. Reopen chat history to check.' + : `Biorouter answered ${status} without a reason, and could not be asked whether this ` + + 'chat is now public. Reopen chat history to check.'; + } + return { + status, + title: status === 503 ? 'The chat store was busy' : 'Could not mark this chat public', + message, + }; +} + +/** + * The failure toast each chat's dialog last raised, by session id — defect D3a + * (2026-09-13). + * + * Error toasts do not expire (`toasts.tsx`: "a failure that expires unread is a + * failure that was never reported"), so a failure a later attempt overturned + * stayed on screen beside "Chat marked public" — measured still there 60 s + * after the success toast had closed, after two busy answers, after a lost + * answer, and after the escalation's refusal. A report about one attempt is + * retracted by the next outcome of the same operation. + * + * Module-level, not component state, because the dialog is unmounted on close: + * fail, close, reopen and succeed must retract the first failure too. And the + * outcome that overturns a failure need not come from this dialog at all — the + * same chat declassified from another window, or from the CLI while a store + * here holds it, reaches this window as a row read (`sessionRowSync`), and a + * toast still saying "still private" beside a public chat is the same stale + * report. + * + * # One chat, one report — D3a's second round (2026-09-13) + * + * ⚠ Keyed by chat here, and until this round deduplicated by CONTENT in + * `toastError` — and the busy sentence is the same for every chat. Measured in + * the dev app: chat Y failed busy, chat X failed busy (still one toast on + * screen), X was retried and succeeded, and Y's report was gone with Y still + * private. X's retraction had dismissed the one toast both reports shared, and + * a different failure on X, or a row read showing X public, did the same. + * + * So each report is raised under its chat (`dedupeScope`), and names the chat + * it is about (`declassifyToastSubject`). Reference-counting the shared toast + * was the other way to keep Y's report alive, and it is wrong for what the + * person reads: one toast would stand for several chats while saying "this + * chat", so after X succeeded it would sit beside "Chat marked public" still + * saying "this chat was not marked public" — the stale report this map exists + * to retract, now about a chat it does not name. A retry on the SAME chat still + * lands on its own toast id, so it replaces its report rather than stacking. + */ +const outstandingFailureToasts = new Map(); + +/** + * Which chat a failure toast is about, for its title. Two chats that fail at + * once raise two toasts carrying the same daemon sentence, and those are only + * useful if each says which chat it means. + * + * A placeholder name ("New Session", "New chat", "Session 5" — + * `isDefaultSessionName`) is shared by dozens of rows, so that chat is named by + * its id, which the dialog shows under the name. Any other name is quoted, and + * cut short (by characters, so an emoji is never split) because a toast title + * is not clamped — and it is followed by the id too, because a real name is not + * unique either: auto-generated names repeat, and two chats both called + * "Subagent delegation request" failing at once would otherwise raise two + * toasts that read identically. + */ +export function declassifyToastSubject(name: string | null | undefined, sessionId: string): string { + const trimmed = (name ?? '').trim(); + if (isDefaultSessionName(trimmed)) return `chat ${sessionId}`; + const chars = [...trimmed]; + const quoted = + chars.length <= SUBJECT_MAX_CHARS + ? `“${trimmed}”` + : `“${chars + .slice(0, SUBJECT_MAX_CHARS - 1) + .join('') + .trimEnd()}…”`; + return `${quoted} (${sessionId})`; +} +const SUBJECT_MAX_CHARS = 60; + +let stopFollowingRows: (() => void) | null = null; + +/** + * Started the first time a failure is reported, and kept for the renderer's + * life: a toast that outlives its dialog is exactly the case it is for. + */ +function followRowsForOutstandingFailures(): void { + if (stopFollowingRows) return; + stopFollowingRows = subscribeSessionRowChanges(({ sessionId, privacy_tier }) => { + if (privacy_tier === 'public') retractFailureToast(sessionId); + }); +} + +function retractFailureToast(sessionId: string, keep?: string | number): void { + const previous = outstandingFailureToasts.get(sessionId); + if (previous === undefined) return; + outstandingFailureToasts.delete(sessionId); + // An identical failure on THIS chat is deduplicated onto the same toast id, + // so dismissing it would take away the toast reporting the attempt just made. + // Another chat's identical failure has its own id and is never reached here. + if (previous !== keep) toastService.dismiss(previous); +} + type Phase = 'confirm' | 'undo' | 'sending'; export interface DeclassifySessionDialogProps { @@ -155,44 +328,93 @@ export function DeclassifySessionDialog({ // than the window — the session list's own change subscription, a search // debounce — means the request is never sent at all. Pinned by "the undo // window is a deadline, not a countdown a re-render restarts". - const latest = useRef({ onClose, onDeclassified }); + // + // The chat's NAME rides here for the same reason: the daemon renames a chat + // after its early turns, and a `send` keyed on the name would restart the undo + // window when that rename reached this row. + const latest = useRef({ onClose, onDeclassified, sessionName: session.name }); useLayoutEffect(() => { - latest.current = { onClose, onDeclassified }; + latest.current = { onClose, onDeclassified, sessionName: session.name }; }); const send = useCallback( async (confirmation: string | null) => { setPhase('sending'); + let status: number | undefined; + let body: unknown; try { - await declassifySession({ + // NOT `throwOnError`: the generated client then throws the parsed BODY + // and drops the Response, and the status is what separates a refusal + // from a busy store. See `describeDeclassifyFailure`. + const result = await declassifySession({ path: { session_id: session.id }, body: { confirmation }, // DR-16's proof-of-user. Without it the daemon refuses, correctly: // the server secret alone is reachable from any developer-enabled // agent shell (§9.3 A1) and is not evidence of a human. headers: await userActionHeaders(), - throwOnError: true, }); + status = result.response?.status; + body = result.error; + } catch (error) { + // A throw here carries no Response, so no status: the answer — if the + // daemon sent one — did not arrive. + status = undefined; + body = error; + } + + // Success is a 200, or a row that READS public after anything else. A + // missing Response is not evidence the write did not land (D2), so the + // row is asked before a word is said; and a private chat is never + // reported public unless the row says so. + let failure: DeclassifyFailure | null = null; + if (status !== 200) { + const row = await readSessionRowFacts(session.id); + if (row?.privacy_tier !== 'public') { + failure = describeDeclassifyFailure(status, body, row?.privacy_tier ?? null); + } + } + + if (failure === null) { + retractFailureToast(session.id); toastSuccess({ title: 'Chat marked public', msg: 'It no longer carries a private marker. The change is recorded.', }); + // Every list surface in every window re-reads this row in place (item + // 11: the daemon no longer re-sorts it to say something happened). + announceSessionRowChanged(session.id); latest.current.onDeclassified?.(session.id); latest.current.onClose?.(); - } catch (error) { - toastError({ - title: 'Could not mark this chat public', - msg: error instanceof Error ? error.message : String(error), - }); - // A request that carried no confirmation was refused, so the weak - // control this dialog rendered was the wrong one — most likely because - // the cached row's `turn:*` provenance has since been displaced by an - // `mcp:*` one. Returning to the same control would re-render it from - // the same stale prop and fail identically, forever. Escalating is the - // only recovery, and it is never the wrong answer to a refusal. - if (confirmation === null) setEscalated(true); - setPhase('confirm'); + return; + } + + // Raised under this chat, and naming it: see `outstandingFailureToasts`. + const toastId = toastError({ + title: `${failure.title} — ${declassifyToastSubject(latest.current.sessionName, session.id)}`, + msg: failure.message, + dedupeScope: `declassify:${session.id}`, + }); + retractFailureToast(session.id, toastId); + if (toastId !== undefined) { + outstandingFailureToasts.set(session.id, toastId); + followRowsForOutstandingFailures(); } + // A request that carried no confirmation was answered "the confirmation + // did not match" (400), so the weak control this dialog rendered was the + // wrong one — most likely because the cached row's `turn:*` provenance + // has since been displaced by an `mcp:*` one. Returning to the same + // control would re-render it from the same stale prop and fail + // identically, forever, so it escalates. + // + // ⚠ **Only on that answer.** This used to escalate on ANY failure, and a + // lock timeout then swapped the single click for the typed phrase under a + // sentence claiming "this chat's record has changed since this list was + // loaded" — a claim about the chat that a busy store does not make. A + // busy store, a network failure or a daemon fault leaves the grade + // exactly as it was, so the same control is the right one to try again. + if (confirmation === null && failure.status === 400) setEscalated(true); + setPhase('confirm'); }, [session.id] ); diff --git a/ui/desktop/src/components/sessions/SessionHistoryView.browserSurface.test.tsx b/ui/desktop/src/components/sessions/SessionHistoryView.browserSurface.test.tsx index 5103af2b2..2ac6b134f 100644 --- a/ui/desktop/src/components/sessions/SessionHistoryView.browserSurface.test.tsx +++ b/ui/desktop/src/components/sessions/SessionHistoryView.browserSurface.test.tsx @@ -63,7 +63,13 @@ function renderPrivatePage() { beforeEach(() => { vi.clearAllMocks(); - mocks.declassifySession.mockResolvedValue({}); + // The shape the generated client resolves a 200 with. The dialog reads the + // status off the Response, so a bare `{}` — no Response at all — is a failed + // request, exactly as it is at runtime. + mocks.declassifySession.mockResolvedValue({ + data: { sessionId: 'x', privacyTier: 'public' }, + response: { status: 200 }, + }); }); afterEach(() => { diff --git a/ui/desktop/src/components/sessions/SessionHistoryView.test.tsx b/ui/desktop/src/components/sessions/SessionHistoryView.test.tsx index 78aec9abd..7687af3fa 100644 --- a/ui/desktop/src/components/sessions/SessionHistoryView.test.tsx +++ b/ui/desktop/src/components/sessions/SessionHistoryView.test.tsx @@ -5,9 +5,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import SessionHistoryView from './SessionHistoryView'; import type { Session } from '../../api'; -const mocks = vi.hoisted(() => ({ declassifySession: vi.fn() })); +const mocks = vi.hoisted(() => ({ declassifySession: vi.fn(), getSession: vi.fn() })); -vi.mock('../../api', () => ({ declassifySession: mocks.declassifySession })); +vi.mock('../../api', () => ({ + declassifySession: mocks.declassifySession, + getSession: mocks.getSession, +})); vi.mock('../../utils/userAction', () => ({ userActionHeaders: async () => ({ 'X-User-Action': 'test-key' }), @@ -60,7 +63,13 @@ function renderView(over: Partial = {}, showActionButtons = false) { beforeEach(() => { vi.clearAllMocks(); - mocks.declassifySession.mockResolvedValue({}); + // The shape the generated client resolves a 200 with. The dialog reads the + // status off the Response, so a bare `{}` — no Response at all — is a failed + // request, exactly as it is at runtime. + mocks.declassifySession.mockResolvedValue({ + data: { sessionId: 'x', privacyTier: 'public' }, + response: { status: 200 }, + }); }); describe('SessionHistoryView — the privacy marker', () => { @@ -118,6 +127,41 @@ describe('SessionHistoryView — declassification', () => { expect(screen.getByTestId('privacy-badge')).toHaveAttribute('data-privacy', 'public') ); }); + + // Item 11 of the 1.90.4 hold (2026-09-13): a declassification made in ANOTHER + // window reaches this page too. The page reads its `session` prop once, and + // the daemon no longer re-sorts the chat to say something happened, so the + // badge follows the row announcement — and states the daemon's read of it. + it('follows a declassification made in another window', async () => { + mocks.getSession.mockResolvedValue({ + data: { + id: '20260714_130000', + privacy_tier: 'public', + privacy_reason: 'declassified_by_user', + }, + }); + renderView({ + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + id: '20260714_130000', + }); + expect(screen.getByTestId('privacy-badge')).toHaveAttribute('data-privacy', 'private'); + + const otherWindow = new BroadcastChannel('biorouter:session-row'); + try { + // A different chat first: this page must not move for it. + otherWindow.postMessage({ sessionId: 'someone-else' }); + otherWindow.postMessage({ sessionId: '20260714_130000' }); + await waitFor(() => + expect(screen.getByTestId('privacy-badge')).toHaveAttribute('data-privacy', 'public') + ); + } finally { + otherWindow.close(); + } + expect(mocks.getSession).toHaveBeenCalledWith( + expect.objectContaining({ path: { session_id: '20260714_130000' } }) + ); + }); }); /** diff --git a/ui/desktop/src/components/sessions/SessionHistoryView.tsx b/ui/desktop/src/components/sessions/SessionHistoryView.tsx index 7de042438..665329ce4 100644 --- a/ui/desktop/src/components/sessions/SessionHistoryView.tsx +++ b/ui/desktop/src/components/sessions/SessionHistoryView.tsx @@ -39,6 +39,7 @@ import { Message, Session } from '../../api'; import { PrivacyBadge } from '../ui/PrivacyBadge'; import { DeclassifySessionDialog } from './DeclassifySessionDialog'; import { DECLASSIFY_NEEDS_HOST_SHORT, declassifyBrowserReason } from './declassifyOnBrowser'; +import { subscribeSessionRowChanges } from '../../utils/sessionRowSync'; import { useNavigation } from '../../hooks/useNavigation'; import { ReadableContent } from '../Layout/ReadableContent'; import { MODAL_SIZE } from '../ModalShell'; @@ -217,6 +218,17 @@ const SessionHistoryView: React.FC = ({ const artifactPanel = useArtifactPanel({ isMobile: useIsMobile(), allowWindowResize: false }); const { splitPaneRef, artifact: presentedArtifact, openArtifact } = artifactPanel; useEffect(() => setTier(session.privacy_tier), [session.privacy_tier]); + // …and follow a declassification made anywhere else — another window's + // History row, or this chat's own page open twice. The `session` prop is read + // once when the page opens, so without this the badge above a chat that is + // no longer private would stay private until the page was reopened. + useEffect( + () => + subscribeSessionRowChanges(({ sessionId, privacy_tier }) => { + if (sessionId === session.id) setTier(privacy_tier); + }), + [session.id] + ); const messages = session.conversation || []; const billedTokenEstimate = billedSessionTokenEstimate(session); diff --git a/ui/desktop/src/components/sessions/SessionListView.browserSurface.test.tsx b/ui/desktop/src/components/sessions/SessionListView.browserSurface.test.tsx index 8c095c352..b186a8dd0 100644 --- a/ui/desktop/src/components/sessions/SessionListView.browserSurface.test.tsx +++ b/ui/desktop/src/components/sessions/SessionListView.browserSurface.test.tsx @@ -83,7 +83,13 @@ function openRowMenu() { beforeEach(() => { vi.clearAllMocks(); clearSessionListCache(); - mocks.declassifySession.mockResolvedValue({}); + // The shape the generated client resolves a 200 with. The dialog reads the + // status off the Response, so a bare `{}` — no Response at all — is a failed + // request, exactly as it is at runtime. + mocks.declassifySession.mockResolvedValue({ + data: { sessionId: 'x', privacyTier: 'public' }, + response: { status: 200 }, + }); mocks.listSessions.mockResolvedValue({ data: { sessions: [ diff --git a/ui/desktop/src/components/sessions/SessionListView.declassify.test.tsx b/ui/desktop/src/components/sessions/SessionListView.declassify.test.tsx index 1696d5468..ba8121310 100644 --- a/ui/desktop/src/components/sessions/SessionListView.declassify.test.tsx +++ b/ui/desktop/src/components/sessions/SessionListView.declassify.test.tsx @@ -58,7 +58,13 @@ beforeEach(() => { vi.clearAllMocks(); clearSessionListCache(); mocks.listSessions.mockResolvedValue({ data: { sessions: [] } }); - mocks.declassifySession.mockResolvedValue({}); + // The shape the generated client resolves a 200 with. The dialog reads the + // status off the Response, so a bare `{}` — no Response at all — is a failed + // request, exactly as it is at runtime. + mocks.declassifySession.mockResolvedValue({ + data: { sessionId: 'x', privacyTier: 'public' }, + response: { status: 200 }, + }); }); function row(overrides: Partial & { id: string; name: string }): Session { diff --git a/ui/desktop/src/components/ui/dialog.test.tsx b/ui/desktop/src/components/ui/dialog.test.tsx index 5a6b8598f..a757aacd2 100644 --- a/ui/desktop/src/components/ui/dialog.test.tsx +++ b/ui/desktop/src/components/ui/dialog.test.tsx @@ -57,6 +57,44 @@ describe('DialogContent dismissal contract', () => { expect(onClose).toHaveBeenCalledTimes(1); }); + /** + * Defect D3b (2026-09-13). Toasts sit above every modal and are rendered + * outside any dialog's tree, so a press on one reached Radix as a press + * OUTSIDE the dialog: measured in the running app, one click on an error + * toast's × closed "Make this chat public?" underneath while the toast stayed + * on screen a minute later. The toast layer's own markup is reproduced here — + * `section.Toastify` > `.Toastify__toast-container` > the card — because the + * container is the hook the guard keys on. + */ + it('does not treat a press on a toast as a press on the backdrop', async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + render(); + await user.click(screen.getByRole('button', { name: 'Open dialog' })); + + const layer = document.createElement('section'); + layer.className = 'Toastify'; + layer.innerHTML = + '
' + + '
' + + '
'; + document.body.appendChild(layer); + try { + fireEvent.pointerDown(layer.querySelector('.Toastify__close-button')!); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + + // …while the backdrop itself still dismisses, so the guard is not simply + // "never dismiss on a press". + fireEvent.pointerDown(document.querySelector('[data-slot="dialog-overlay"]')!); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + expect(onClose).toHaveBeenCalledTimes(1); + } finally { + layer.remove(); + } + }); + it('blocks Escape, backdrop, and the close button when not dismissible', async () => { const user = userEvent.setup(); const onClose = vi.fn(); diff --git a/ui/desktop/src/components/ui/dialog.tsx b/ui/desktop/src/components/ui/dialog.tsx index a75d4794b..5b2b863ba 100644 --- a/ui/desktop/src/components/ui/dialog.tsx +++ b/ui/desktop/src/components/ui/dialog.tsx @@ -39,6 +39,16 @@ const DialogOverlay = React.forwardRef< )); DialogOverlay.displayName = DialogPrimitive.Overlay.displayName; +/** + * Whether a pointer target sits in react-toastify's layer. `.Toastify` is the + * container the library renders (`App.tsx`'s `ToastContainer`); the app's own + * card class replaces the library's per-toast class, so the container is the + * one stable hook (see `toastLayer.test.ts`). + */ +function isInsideToastLayer(target: EventTarget | null): boolean { + return target instanceof Element && target.closest('.Toastify') !== null; +} + function DialogContent({ className, children, @@ -102,6 +112,16 @@ function DialogContent({ onPointerDownOutside={(event) => { onPointerDownOutside?.(event); if (!dismissible) event.preventDefault(); + // A press on a TOAST is not a press on the backdrop. Toasts sit above + // every modal (`--z-toast` > `--z-modal`) and are rendered outside + // any dialog's tree, so to Radix a click on one is "outside" — and + // it closed the dialog underneath while the toast, whose × had never + // received the click, stayed on screen. Measured on 2026-09-13 + // (defect D3b): a busy-store error over "Make this chat public?", + // one click on the toast's ×, the dialog gone and the error still + // there a minute later. `main.css` makes the toast layer take the + // click at all; this stops the dialog treating it as a dismissal. + if (isInsideToastLayer(event.target)) event.preventDefault(); }} {...props} > diff --git a/ui/desktop/src/hooks/chatStreamStore.declassify.test.tsx b/ui/desktop/src/hooks/chatStreamStore.declassify.test.tsx new file mode 100644 index 000000000..0bf8987e2 --- /dev/null +++ b/ui/desktop/src/hooks/chatStreamStore.declassify.test.tsx @@ -0,0 +1,240 @@ +/** + * A chat's classification moves in BOTH directions, and every surface that + * draws it has to follow — defects D1 and D4 of the 2026-09-13 repair round. + * + * **D1 — a declassified chat's open tab kept its private icon.** Measured in + * the running app, in one window and across two: the database, the store and + * the sidebar all read `public`, and the tab strip's icon stayed + * `data-privacy="private"` past twelve minutes, until a reload. The registry's + * live map only ever rose ("mirroring the ratchet"), so the store's lowered + * reading was thrown away and the strip's `max` held the stale one. + * + * **D4 — another window drew a private chat PUBLIC.** Declassify in window A + * (the push lowers window B's History and sidebar rows, as designed), then send + * one turn on a private model in A: the database went back to `private` / + * `turn:versa_azure`, A's sidebar followed, and B's History row and sidebar row + * stayed public for the whole watch — the sidebar over two minutes. Nothing + * pushed the raise. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MessageEvent, Session, TokenState } from '../api'; + +const mocks = vi.hoisted(() => ({ + reply: vi.fn(), + observeSessionEvents: vi.fn(), + resumeAgent: vi.fn(), + getSession: vi.fn(async (_options?: unknown) => ({ data: null }) as unknown), + listSessions: vi.fn(async () => ({ data: { sessions: [] } })), + updateFromSession: vi.fn(async () => ({ data: {} })), +})); + +vi.mock('../api', async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { ...actual, ...mocks }; +}); + +// The long poll is a network side effect with its own tests +// (`sessionMetaSubscription.test.ts`); here it must not be the thing that makes +// a store re-read, or a test of the row channel would pass on the poll. +vi.mock('../utils/sessionMetaSubscription', () => ({ + subscribeToSessionMeta: () => () => {}, +})); + +// The real channel, with the announcement observed. The positive case below +// also listens on the channel as another window would; the negative cases ask +// the spy, because "nothing arrived on a BroadcastChannel" has no barrier that +// Node orders against a different sender. +const announce = vi.hoisted(() => ({ spy: undefined as unknown as ReturnType })); +vi.mock('../utils/sessionRowSync', async (importOriginal) => { + const actual = (await importOriginal()) as typeof import('../utils/sessionRowSync'); + announce.spy = vi.fn(actual.announceSessionRowChanged); + return { ...actual, announceSessionRowChanged: announce.spy }; +}); + +import { ChatStreamRegistry } from './chatStreamStore'; + +const tokenState: TokenState = { + accumulatedInputTokens: 0, + accumulatedOutputTokens: 0, + accumulatedTotalTokens: 0, + inputTokens: 0, + outputTokens: 0, + totalTokens: 0, +}; +const finishFrame = { type: 'Finish', reason: 'stop', token_state: tokenState } as MessageEvent; + +async function* streamOf(...frames: MessageEvent[]) { + for (const frame of frames) yield frame; +} + +function row(id: string, over: Partial = {}): Session { + return { + id, + name: 'A named chat', + working_dir: '/tmp', + conversation: [], + message_count: 0, + total_tokens: 0, + created_at: '', + updated_at: '', + extension_data: {}, + user_set_name: true, + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + provider_name: 'versa_azure', + model_config: { model_name: 'gpt-5.5-2026-04-24', toolshim: false, context_limit: 400000 }, + ...over, + } as Session; +} + +const publicRow = (id: string) => + row(id, { privacy_tier: 'public', privacy_reason: 'declassified_by_user' }); + +/** One animation frame: store notifications, and so the tier map, are batched to it (#22). */ +const aFrame = () => new Promise((resolve) => setTimeout(resolve, 60)); + +/** Stands in for ANOTHER window on the row channel (Node's BroadcastChannel under jsdom). */ +const ROW_CHANNEL = 'biorouter:session-row'; + +let seq = 0; +const sid = (label: string) => `declass-${label}-${++seq}`; + +describe('the tab strip follows a declassification (D1)', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getSession.mockResolvedValue({ data: null }); + }); + + it('lowers a tier its store re-read, instead of holding the old private', async () => { + const id = sid('lower'); + mocks.resumeAgent.mockResolvedValue({ data: { session: row(id) } }); + const registry = new ChatStreamRegistry(); + const controller = registry.getController(id); + await controller.loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'private' }); + + // What the change feed does ~2 s after a declassification: re-read the row. + mocks.getSession.mockResolvedValue({ data: publicRow(id) }); + await controller.refreshSessionBinding(); + await aFrame(); + + expect(controller.getSnapshot().session?.privacy_tier).toBe('public'); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'public' }); + }); + + it('a row read in this window makes a store holding another tier re-read at once', async () => { + // The change feed watches at most 64 ids and the registry keeps every store + // it ever made, so it cannot be what every store relies on. + const id = sid('nudge'); + mocks.resumeAgent.mockResolvedValue({ data: { session: row(id) } }); + const registry = new ChatStreamRegistry(); + const stop = registry.followSessionRows(); + const otherWindow = new BroadcastChannel(ROW_CHANNEL); + try { + await registry.getController(id).loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'private' }); + + mocks.getSession.mockResolvedValue({ data: publicRow(id) }); + // Another window declassified it and announced so. + otherWindow.postMessage({ sessionId: id }); + + await vi.waitFor(async () => { + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'public' }); + }); + } finally { + otherWindow.close(); + stop(); + } + }); +}); + +describe('a raise reaches every window’s list rows (D4)', () => { + let otherWindow: BroadcastChannel; + let heard: string[]; + + beforeEach(() => { + vi.clearAllMocks(); + announce.spy.mockClear(); + mocks.getSession.mockResolvedValue({ data: null }); + heard = []; + otherWindow = new BroadcastChannel(ROW_CHANNEL); + otherWindow.onmessage = (event: globalThis.MessageEvent) => { + heard.push((event.data as { sessionId: string }).sessionId); + }; + }); + + afterEach(() => { + otherWindow.close(); + }); + + it('announces a raise its store sees', async () => { + const id = sid('raise'); + mocks.resumeAgent.mockResolvedValue({ data: { session: publicRow(id) } }); + mocks.getSession.mockResolvedValue({ data: row(id) }); + mocks.reply.mockResolvedValue({ + stream: streamOf( + { + type: 'PrivacyProviderPinned', + provider: 'versa_azure', + model: 'gpt-5.5-2026-04-24', + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + } as unknown as MessageEvent, + finishFrame + ), + }); + + const registry = new ChatStreamRegistry(); + const controller = registry.getController(id); + await controller.loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'public' }); + + await controller.handleSubmit('hi'); + + await vi.waitFor(() => expect(heard).toContain(id)); + expect(announce.spy).toHaveBeenCalledWith(id); + }); + + it('announces nothing for a store’s first reading of a chat', async () => { + const id = sid('first'); + mocks.resumeAgent.mockResolvedValue({ data: { session: row(id) } }); + + const registry = new ChatStreamRegistry(); + await registry.getController(id).loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'private' }); + + await aFrame(); + expect(announce.spy).not.toHaveBeenCalledWith(id); + }); + + it('does not announce again a change this window was already handed', async () => { + // Window B, told of A's declassification: its lists read the row, and its + // own store then catches up. Announcing that would make every window read + // the row a second time for nothing. + const id = sid('echo'); + mocks.resumeAgent.mockResolvedValue({ data: { session: row(id) } }); + const registry = new ChatStreamRegistry(); + const stop = registry.followSessionRows(); + try { + await registry.getController(id).loadSession(); + await aFrame(); + + mocks.getSession.mockResolvedValue({ data: publicRow(id) }); + otherWindow.postMessage({ sessionId: id }); + await vi.waitFor(async () => { + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [id]: 'public' }); + }); + + await aFrame(); + expect(announce.spy).not.toHaveBeenCalledWith(id); + } finally { + stop(); + } + }); +}); diff --git a/ui/desktop/src/hooks/chatStreamStore.tsx b/ui/desktop/src/hooks/chatStreamStore.tsx index 8089146e2..afe38be6f 100644 --- a/ui/desktop/src/hooks/chatStreamStore.tsx +++ b/ui/desktop/src/hooks/chatStreamStore.tsx @@ -34,7 +34,11 @@ import { updateCachedSessionList, } from '../utils/sessionListCache'; import { subscribeToSessionMeta } from '../utils/sessionMetaSubscription'; -import { raiseTier } from '../components/privacy/sessionTier'; +import { + announceSessionRowChanged, + lastKnownSessionTier, + subscribeSessionRowChanges, +} from '../utils/sessionRowSync'; import { isReadOnlySubagentChat } from '../components/subagent/subagentReadOnly'; import { mergeStopRecord } from '../components/conversation/turnStoppedNotice'; import { isBrowserSurface } from '../utils/surface'; @@ -4552,7 +4556,7 @@ export class ChatStreamRegistry { */ followSessionRows(): () => void { if (this.stopSessionMeta) return () => {}; - this.stopSessionMeta = subscribeToSessionMeta({ + const stopMeta = subscribeToSessionMeta({ // Only chats this renderer holds a row for can go stale, and only those // are worth a read on the daemon's side. openSessionIds: () => @@ -4563,6 +4567,27 @@ export class ChatStreamRegistry { void this.controllers.get(sessionId)?.refreshSessionBinding(); }, }); + // A row read THIS window just made (`sessionRowSync`: a declassification + // here or in another window, or a raise another store announced) that + // disagrees with the store holding the same chat. Handed over as a nudge, + // not applied: the store re-reads through `refreshSessionBinding`, the one + // path that orders overlapping reads (`bindingGeneration`). + // + // The long poll above would get there too, within about two seconds — but + // only for the ids it watches, and it watches at most 64 (`MAX_IDS` in + // `routes/session_meta.rs`) while this registry keeps every store it ever + // made. A store past that cap would otherwise keep a declassified chat's + // tab private until the renderer reloaded (defect D1, 2026-09-13). + const stopRows = subscribeSessionRowChanges(({ sessionId, privacy_tier }) => { + const controller = this.controllers.get(sessionId); + if (!controller?.hasLoadedSession()) return; + if (controller.getSnapshot().session?.privacy_tier === privacy_tier) return; + void controller.refreshSessionBinding(); + }); + this.stopSessionMeta = () => { + stopMeta(); + stopRows(); + }; return () => { this.stopSessionMeta?.(); this.stopSessionMeta = null; @@ -4622,11 +4647,27 @@ export class ChatStreamRegistry { * reading to the strip, so the three surfaces cannot disagree — and it needs * no fetch, because the answer was already in the window. * - * ⚠ **This map only ever RISES**, mirroring the daemon's own ratchet - * (`privacy::raise`). A controller whose session momentarily goes null — a - * reload, a rebind — must not retract a `private` it has already reported, or - * the strip would fall back to a cached `public` and un-mark a private chat. - * {@link raiseTier} is the whole rule. + * ⚠ **This map follows its stores DOWN as well as up.** It used to only rise + * ("mirroring the daemon's ratchet"), and the ratchet has one exit: a + * declassification (issue #56 §12.4). So a chat declassified while its tab + * was open kept a private tab icon for as long as it was watched — measured + * past twelve minutes on 2026-09-13 (defect D1), with the store, the sidebar + * and the database all reading `public` — because the store's lowered + * reading was discarded here and `mergeSessionTiers` then took `max` against + * it. Each store's newest DEFINED reading is adopted; it is the daemon's row + * as that store last read it, and every store is followed by the change feed + * and by `sessionRowSync`. + * + * ⚠ A store that holds NO row says nothing, and does not retract what it said. + * The strip must not fall back to a cached `public` because a store is between + * rows. (No production path empties a loaded store today; the rule is kept so + * one cannot silently un-mark a chat.) + * + * ⚠ **A change it sees is announced** (`sessionRowSync`), so every OTHER + * window's History row and sidebar row re-read the chat. Without that, a + * lowering pushed by the declassify dialog and a raise pushed by nobody left + * another window badging a private chat PUBLIC (defect D4). See + * {@link noteControllerTier}. * * ⚠ **O(1) per notification.** `handleControllerActivity` runs on every * snapshot notification, which during a turn is once per animation frame per @@ -4653,14 +4694,30 @@ export class ChatStreamRegistry { } private noteControllerTier(controller: ChatStreamController): void { + const sessionId = controller.sessionId; const reported = controller.getSnapshot().session?.privacy_tier ?? undefined; - const current = this.sessionTiers[controller.sessionId]; - const raised = raiseTier(current, reported); - if (raised === current) return; - this.sessionTiers = { ...this.sessionTiers }; - if (raised) this.sessionTiers[controller.sessionId] = raised; - else delete this.sessionTiers[controller.sessionId]; + const current = this.sessionTiers[sessionId]; + if (reported === undefined || reported === current) return; + this.sessionTiers = { ...this.sessionTiers, [sessionId]: reported }; for (const listener of this.tierListeners) listener(); + + // Tell every window's list surfaces — this one's included — to re-read the + // row. Only a CHANGE against something this window already believed: + // + // - `current` is this store's previous reading, so a turn that raised the + // chat, a change-feed re-read that found it lowered, a CLI declassification + // the feed carried; + // - `lastKnownSessionTier` is the last row this window's list surfaces were + // handed, so a chat opened for the first time HERE whose row no longer + // matches what History was last told about it. + // + // A store's very first reading of a chat nobody here has an opinion about + // announces nothing; that is every chat load. And a change this window's + // list surfaces have already been handed is not announced again: someone + // announced it, every window has read it, and this store is catching up. + const listed = lastKnownSessionTier(sessionId); + const believed = current ?? listed; + if (believed !== undefined && listed !== reported) announceSessionRowChanged(sessionId); } private handleControllerActivity = (controller: ChatStreamController): void => { diff --git a/ui/desktop/src/styles/main.css b/ui/desktop/src/styles/main.css index b53e36fa5..18d579f41 100644 --- a/ui/desktop/src/styles/main.css +++ b/ui/desktop/src/styles/main.css @@ -1862,6 +1862,21 @@ background-color var(--motion-fast) var(--ease-out); } +/* A toast stays clickable while a modal dialog is open. + Radix's modal sets `pointer-events: none` on for as long as the dialog + is open and gives only its own layer `auto`. The toast layer is a child of + that says nothing, so it inherited `none`: a toast drawn ABOVE the + dialog (`--z-toast` > `--z-modal`) could not be clicked, and the press fell + through to the dialog's backdrop and closed the dialog instead. Measured on + 2026-09-13 (defect D3b): an error toast over "Make this chat public?", one + click on its ×, the dialog gone and the toast still there a minute later. + On each card, not on the container, so the gaps between toasts still reach + the backdrop. `dialog.tsx` keeps a press that lands here from counting as a + press outside the dialog. */ +.Toastify__toast-container > * { + pointer-events: auto; +} + .Toastify__toast-container .Toastify__close-button:hover { opacity: 1; color: var(--text-default); diff --git a/ui/desktop/src/styles/toastLayer.test.ts b/ui/desktop/src/styles/toastLayer.test.ts index 10e52dbe2..2c1fe304c 100644 --- a/ui/desktop/src/styles/toastLayer.test.ts +++ b/ui/desktop/src/styles/toastLayer.test.ts @@ -121,3 +121,17 @@ describe('a newer notification lands directly below the older one', () => { expect(APP).toContain('toastClassName={() => TOAST_SURFACE_CLASS_NAME}'); }); }); + +/** + * Defect D3b (2026-09-13). Radix's modal sets `pointer-events: none` on + * while a dialog is open, and the toast layer inherited it: a toast drawn above + * the dialog could not be clicked, and the press fell through to the backdrop + * and closed the dialog. jsdom applies no stylesheet, so this is asserted at the + * source, like everything else here; `dialog.test.tsx` covers the half that + * decides whether a press on a toast dismisses the dialog. + */ +describe('a toast stays clickable over a modal dialog', () => { + it('gives every toast card pointer events back', () => { + expect(CSS).toMatch(/\.Toastify__toast-container\s*>\s*\*\s*\{[^}]*pointer-events:\s*auto\s*;/); + }); +}); diff --git a/ui/desktop/src/toasts.test.tsx b/ui/desktop/src/toasts.test.tsx index 898c424e8..574e668a0 100644 --- a/ui/desktop/src/toasts.test.tsx +++ b/ui/desktop/src/toasts.test.tsx @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const mocks = vi.hoisted(() => ({ success: vi.fn(), + error: vi.fn(), })); // react-toastify is the only thing `toastService.success` actually reaches; stub @@ -9,7 +10,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('react-toastify', () => ({ toast: Object.assign(vi.fn(), { success: mocks.success, - error: vi.fn(), + error: mocks.error, info: vi.fn(), warning: vi.fn(), loading: vi.fn(), @@ -19,7 +20,7 @@ vi.mock('react-toastify', () => ({ }), })); -import { toastService } from './toasts'; +import { toastError, toastService } from './toasts'; describe('toastService.success', () => { beforeEach(() => { @@ -62,3 +63,25 @@ describe('toastService.success', () => { expect(mocks.success.mock.calls[0][1]).toMatchObject({ autoClose: false }); }); }); + +describe('toastError', () => { + beforeEach(() => vi.clearAllMocks()); + + // D3a's second round: two chats that failed with the same sentence shared one + // toast, so the first chat whose failure was overturned dismissed the other's + // report. A scope keeps two subjects apart without giving up dedup within one. + it('a dedupe scope separates identical failures about different subjects', () => { + const busy = { title: 'The chat store was busy', msg: 'Try again in a moment.' }; + toastError({ ...busy, dedupeScope: 'declassify:a' }); + toastError({ ...busy, dedupeScope: 'declassify:b' }); + toastError({ ...busy, dedupeScope: 'declassify:a' }); + toastError(busy); + + const ids = mocks.error.mock.calls.map((call) => call[1].toastId); + expect(ids[0]).not.toBe(ids[1]); + expect(ids[2]).toBe(ids[0]); + // Unscoped callers keep the content key they always had. + expect(ids[3]).toBe('error:The chat store was busy:Try again in a moment.'); + expect(ids[3]).not.toBe(ids[0]); + }); +}); diff --git a/ui/desktop/src/toasts.tsx b/ui/desktop/src/toasts.tsx index de32255df..2a3c4d532 100644 --- a/ui/desktop/src/toasts.tsx +++ b/ui/desktop/src/toasts.tsx @@ -301,6 +301,18 @@ type ToastErrorProps = { * replacing what the user was doing. */ debugFailure?: Omit; + /** + * Narrows the dedup key to one subject: content-identical failures about two + * DIFFERENT subjects are then two toasts, while the same failure about the + * same subject still coalesces. + * + * For a caller that retracts its own report by the returned id. Without a + * scope, two subjects that fail with the same sentence share one toast, and + * the first subject whose failure is overturned dismisses the other's report + * too — defect D3a's second round, where one chat's successful declassify + * took away another chat's "The chat store was busy". + */ + dedupeScope?: string; }; function ToastErrorContent({ @@ -369,7 +381,14 @@ function ToastErrorContent({ ); } -export function toastError({ title, msg, traceback, recoverHints, debugFailure }: ToastErrorProps) { +export function toastError({ + title, + msg, + traceback, + recoverHints, + debugFailure, + dedupeScope, +}: ToastErrorProps) { // An error toast carries actions whenever there is something to copy or a // recovery path to offer — and a toast with actions is not click-to-dismiss, // because the click that misses the button must not destroy the button. @@ -389,7 +408,7 @@ export function toastError({ title, msg, traceback, recoverHints, debugFailure } // never reported. autoClose: false, closeOnClick: !hasActions, - toastId: dedupeKey('error', title, msg), + toastId: dedupeKey(dedupeScope === undefined ? 'error' : `error[${dedupeScope}]`, title, msg), } ); } diff --git a/ui/desktop/src/utils/sessionListCache.test.ts b/ui/desktop/src/utils/sessionListCache.test.ts index a6e5c6f34..b143a7fca 100644 --- a/ui/desktop/src/utils/sessionListCache.test.ts +++ b/ui/desktop/src/utils/sessionListCache.test.ts @@ -8,15 +8,18 @@ import { subscribeSessionListChanges, } from './sessionListCache'; import { announceSessionName } from './sessionNameSync'; +import { announceSessionRowChanged } from './sessionRowSync'; const mocks = vi.hoisted(() => ({ listSessions: vi.fn(), updateSessionName: vi.fn(), + getSession: vi.fn(), })); vi.mock('../api', () => ({ listSessions: mocks.listSessions, updateSessionName: mocks.updateSessionName, + getSession: mocks.getSession, })); // The proof the desktop sends. Since issue #56's QA sweep (2026-09-10) a list @@ -250,4 +253,142 @@ describe('sessionListCache', () => { await vi.waitFor(() => expect(mocks.listSessions).toHaveBeenCalled()); unsub(); }); + + // Item 11 of the 1.90.4 hold (2026-09-13). A declassification no longer + // re-sorts the chat, so nothing about the LIST changes and no refetch would + // notice. History's badge and its row menu read this entry, so it is patched + // where it sits — from the daemon's read, not from the announcement. + it('re-marks a declassified chat in place, without refetching or reordering', async () => { + mocks.listSessions.mockResolvedValue({ + data: { + sessions: [ + { id: 'recent', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + { id: 'old', privacy_tier: 'private', privacy_reason: 'backfill:ollama' }, + ], + }, + }); + await refreshSessionList(); + mocks.listSessions.mockClear(); + mocks.getSession.mockResolvedValue({ + data: { id: 'old', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + }); + + announceSessionRowChanged('old'); + + await vi.waitFor(() => + expect(getCachedSessionList()).toEqual([ + { id: 'recent', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + { id: 'old', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + ]) + ); + expect(mocks.listSessions).not.toHaveBeenCalled(); + }); + + /** + * Defect D4 of the 2026-09-13 repair round, the list half. A list request + * issued BEFORE a turn raised a chat can answer AFTER the raise was read and + * patched in; adopting the answer drew the chat public again. Neither reading + * is known to be the later one, so the row shows the higher tier and is read + * a third time. + */ + it('a list answer that raced a raise does not draw the chat public again', async () => { + mocks.listSessions.mockResolvedValueOnce({ + data: { sessions: [{ id: 'raced', privacy_tier: 'public', privacy_reason: null }] }, + }); + await refreshSessionList(); + + let answerList: ((value: unknown) => void) | undefined; + mocks.listSessions.mockReturnValueOnce( + new Promise((resolve) => { + answerList = resolve; + }) + ); + const refresh = refreshSessionList(); + await vi.waitFor(() => expect(mocks.listSessions).toHaveBeenCalledTimes(2)); + + // The raise, announced by the chat's store and read while the list is out. + mocks.getSession.mockResolvedValue({ + data: { id: 'raced', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + }); + announceSessionRowChanged('raced'); + await vi.waitFor(() => + expect(getCachedSessionList()?.[0]).toMatchObject({ privacy_tier: 'private' }) + ); + expect(mocks.getSession).toHaveBeenCalledTimes(1); + + // The list answers with what it saw before the raise. The read that + // settles it is held open, so what the cache shows in the meantime is + // observable rather than overwritten a microtask later. + let answerThirdRead: ((value: unknown) => void) | undefined; + mocks.getSession.mockReturnValueOnce( + new Promise((resolve) => { + answerThirdRead = resolve; + }) + ); + answerList!({ + data: { sessions: [{ id: 'raced', privacy_tier: 'public', privacy_reason: null }] }, + }); + await refresh; + + // The disagreement is settled by a read issued after both… + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(2)); + // …and until it lands the chat is not drawn public. + expect(getCachedSessionList()?.[0]).toMatchObject({ + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + }); + answerThirdRead!({ + data: { id: 'raced', privacy_tier: 'private', privacy_reason: 'turn:versa_azure' }, + }); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(getCachedSessionList()?.[0]).toMatchObject({ privacy_tier: 'private' }); + }); + + it('a list answer that raced a declassification is settled by a third read', async () => { + mocks.listSessions.mockResolvedValueOnce({ + data: { sessions: [{ id: 'lowered', privacy_tier: 'private', privacy_reason: 'turn:x' }] }, + }); + await refreshSessionList(); + + let answerList: ((value: unknown) => void) | undefined; + mocks.listSessions.mockReturnValueOnce( + new Promise((resolve) => { + answerList = resolve; + }) + ); + const refresh = refreshSessionList(); + await vi.waitFor(() => expect(mocks.listSessions).toHaveBeenCalledTimes(2)); + + mocks.getSession.mockResolvedValue({ + data: { id: 'lowered', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + }); + announceSessionRowChanged('lowered'); + await vi.waitFor(() => + expect(getCachedSessionList()?.[0]).toMatchObject({ privacy_tier: 'public' }) + ); + + let answerThirdRead: ((value: unknown) => void) | undefined; + mocks.getSession.mockReturnValueOnce( + new Promise((resolve) => { + answerThirdRead = resolve; + }) + ); + answerList!({ + data: { sessions: [{ id: 'lowered', privacy_tier: 'private', privacy_reason: 'turn:x' }] }, + }); + await refresh; + // Private until the order is known — never public on a guess… + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(2)); + expect(getCachedSessionList()?.[0]).toMatchObject({ privacy_tier: 'private' }); + // …and public once a read issued after both says so. + answerThirdRead!({ + data: { id: 'lowered', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + }); + await vi.waitFor(() => + expect(getCachedSessionList()?.[0]).toMatchObject({ + privacy_tier: 'public', + privacy_reason: 'declassified_by_user', + }) + ); + }); }); diff --git a/ui/desktop/src/utils/sessionListCache.ts b/ui/desktop/src/utils/sessionListCache.ts index 44312fbb2..c65f48311 100644 --- a/ui/desktop/src/utils/sessionListCache.ts +++ b/ui/desktop/src/utils/sessionListCache.ts @@ -1,6 +1,11 @@ import { listSessions, type Session } from '../api'; import { userActionHeaders } from './userAction'; import { subscribeSessionNameChanges } from './sessionNameSync'; +import { + settleRowsReadDuringFetch, + subscribeSessionRowChanges, + type SessionRowFacts, +} from './sessionRowSync'; let cachedSessions: Session[] | null = null; let inFlightRequest: Promise | null = null; @@ -58,6 +63,44 @@ subscribeSessionNameChanges(({ sessionId, name, userSetName }) => { emitChange(); }); +/** + * Row reads (`sessionRowSync`) delivered WHILE a list request was in flight, + * settled against that request's answer by `settleRowsReadDuringFetch`. + * + * ⚠ This used to be "not re-applied — a list that snaps a row back to private + * is the safe miss". The snap-back runs in BOTH directions, and the other one + * is not safe: a list issued before a turn raised a chat, landing after the + * raise was patched in, draws the chat public again. Recorded and cleared + * exactly as {@link namesPublishedDuringFetch} is, so it cannot grow. + */ +const rowsReadDuringFetch = new Map(); + +// A row whose classification changed in place — a declassification, or a raise +// a chat store announced, in this window or another (`sessionRowSync`). +// History's badge, its row menu ("Make this chat public" is offered on private +// rows only), Home recents and the tab strip's cached tiers all read this cache, +// so the entry is patched where it sits. Nothing is re-sorted: the daemon no +// longer moves `updated_at` for a classification change, so the row's place in +// the list is still right. +subscribeSessionRowChanges((facts) => { + const { sessionId, privacy_tier, privacy_reason } = facts; + if (inFlightRequest) rowsReadDuringFetch.set(sessionId, facts); + if (!cachedSessions) return; + const idx = cachedSessions.findIndex((s) => s.id === sessionId); + if (idx === -1) return; + const current = cachedSessions[idx]; + if ( + current.privacy_tier === privacy_tier && + (current.privacy_reason ?? null) === privacy_reason + ) { + return; + } + const next = cachedSessions.slice(); + next[idx] = { ...current, privacy_tier, privacy_reason }; + cachedSessions = next; + emitChange(); +}); + // ── Cross-window "the set of sessions changed" signal ────────────────────── // A sibling to sessionNameSync's name channel, for list MEMBERSHIP: a session // created, diverged, deleted or imported. Every list surface — sidebar Recents, @@ -204,6 +247,7 @@ export async function refreshSessionList(includeSubagents?: boolean): Promise listSessions({ @@ -217,7 +261,11 @@ export async function refreshSessionList(includeSubagents?: boolean): Promise ({ + getSession: vi.fn(), +})); + +vi.mock('../api', () => ({ + getSession: mocks.getSession, +})); + +vi.mock('./userAction', () => ({ + userActionHeaders: async () => ({ 'X-User-Action': 'test-proof' }), +})); + +function row(id: string, privacy_tier: 'public' | 'private', privacy_reason: string | null) { + return { data: { id, privacy_tier, privacy_reason } }; +} + +describe('sessionRowSync', () => { + const unsubscribes: Array<() => void> = []; + let other: BroadcastChannel; + + beforeEach(() => { + vi.clearAllMocks(); + other = new BroadcastChannel(CHANNEL_NAME); + }); + + afterEach(() => { + // The listener set is process-global: a failed assertion must not leave a + // listener behind for the next test to receive. + while (unsubscribes.length > 0) unsubscribes.pop()!(); + other.close(); + }); + + function listen(): SessionRowFacts[] { + const seen: SessionRowFacts[] = []; + unsubscribes.push(subscribeSessionRowChanges((facts) => seen.push(facts))); + return seen; + } + + it('re-reads a row another window announced and hands the read to every subscriber', async () => { + mocks.getSession.mockResolvedValue(row('s1', 'public', 'declassified_by_user')); + const first = listen(); + const second = listen(); + + other.postMessage({ sessionId: 's1' }); + + const expected = { + sessionId: 's1', + privacy_tier: 'public', + privacy_reason: 'declassified_by_user', + }; + await vi.waitFor(() => expect(second).toEqual([expected])); + expect(first).toEqual([expected]); + // ONE read for the window, shared by both subscribers, carrying the proof a + // private chat's row needs. + expect(mocks.getSession).toHaveBeenCalledTimes(1); + expect(mocks.getSession).toHaveBeenCalledWith({ + path: { session_id: 's1' }, + query: { metadata_only: true }, + headers: { 'X-User-Action': 'test-proof' }, + throwOnError: true, + }); + }); + + it('delivers what the daemon read, never what the message claimed', async () => { + // A chat declassified and raised straight back by a turn: the message says + // nothing about the tier, and if it did it would be the stale half. + mocks.getSession.mockResolvedValue(row('s2', 'private', 'turn:versa_azure')); + const seen = listen(); + + other.postMessage({ sessionId: 's2', privacy_tier: 'public' }); + + await vi.waitFor(() => expect(seen).toHaveLength(1)); + expect(seen[0].privacy_tier).toBe('private'); + }); + + it('reads nothing for a malformed message', async () => { + mocks.getSession.mockImplementation(async ({ path }: { path: { session_id: string } }) => + row(path.session_id, 'public', null) + ); + const seen = listen(); + + other.postMessage({}); + other.postMessage(null); + other.postMessage({ sessionId: 7 }); + other.postMessage({ sessionId: '' }); + // Posted last on the same port, so its arrival proves the four above were + // already processed. + other.postMessage({ sessionId: 'barrier' }); + + await vi.waitFor(() => expect(seen.map((f) => f.sessionId)).toEqual(['barrier'])); + expect(mocks.getSession).toHaveBeenCalledTimes(1); + }); + + it('does not deliver a read that a newer read of the same chat overtook', async () => { + let finishOlder: ((value: unknown) => void) | undefined; + mocks.getSession + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishOlder = resolve; + }) + ) + .mockResolvedValueOnce(row('s3', 'public', 'declassified_by_user')); + const seen = listen(); + + announceSessionRowChanged('s3'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(1)); + announceSessionRowChanged('s3'); + await vi.waitFor(() => expect(seen).toHaveLength(1)); + + // The older answer lands last, holding the older fact. + finishOlder!(row('s3', 'private', 'turn:versa_azure')); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(seen).toEqual([ + { sessionId: 's3', privacy_tier: 'public', privacy_reason: 'declassified_by_user' }, + ]); + }); + + it('re-reads in the announcing window too, and a failed read changes nothing', async () => { + mocks.getSession + .mockRejectedValueOnce(new Error('403')) + .mockResolvedValueOnce(row('s4', 'public', 'declassified_by_user')); + const seen = listen(); + + announceSessionRowChanged('s4'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(1)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(seen).toEqual([]); + + announceSessionRowChanged('s4'); + await vi.waitFor(() => expect(seen).toHaveLength(1)); + }); + + it('remembers the tier it last delivered, and nothing it failed to read', async () => { + mocks.getSession + .mockRejectedValueOnce(new Error('403')) + .mockResolvedValueOnce(row('s5', 'private', 'turn:versa_azure')); + listen(); + + announceSessionRowChanged('s5'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalledTimes(1)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(lastKnownSessionTier('s5')).toBeUndefined(); + + announceSessionRowChanged('s5'); + await vi.waitFor(() => expect(lastKnownSessionTier('s5')).toBe('private')); + }); + + it('answers a failed or mismatched row read with null, never a throw', async () => { + mocks.getSession.mockRejectedValueOnce(new Error('network')); + await expect(readSessionRowFacts('s6')).resolves.toBeNull(); + + mocks.getSession.mockResolvedValueOnce(row('someone-else', 'public', null)); + await expect(readSessionRowFacts('s6')).resolves.toBeNull(); + + mocks.getSession.mockResolvedValueOnce(row('s6', 'public', 'declassified_by_user')); + await expect(readSessionRowFacts('s6')).resolves.toEqual({ + sessionId: 's6', + privacy_tier: 'public', + privacy_reason: 'declassified_by_user', + }); + }); +}); diff --git a/ui/desktop/src/utils/sessionRowSync.ts b/ui/desktop/src/utils/sessionRowSync.ts new file mode 100644 index 000000000..618df634d --- /dev/null +++ b/ui/desktop/src/utils/sessionRowSync.ts @@ -0,0 +1,266 @@ +import { getSession, type SessionClassification } from '../api'; +import { userActionHeaders } from './userAction'; + +/** + * "A chat's ROW changed in place" — for a change the list surfaces render but + * that moves nothing in the list: a chat's classification moving in EITHER + * direction. A declassification (issue #56 §12.4) announces itself from the + * dialog that made it; a raise is announced by whichever window's chat store + * first sees it (`ChatStreamRegistry.noteControllerTier`). + * + * # Why this exists — item 11 of the 1.90.4 hold (2026-09-13) + * + * `privacy::declassify` used to stamp `updated_at = datetime('now')`, which put + * a chat created months ago into History's "Today" (measured across 796 + * declassifications). The stamp is gone, and the obvious worry was that + * something learned about a declassification *because* `updated_at` moved. + * Measured before removing it, with History open in two windows: window A + * declassified `20260803_1550`, and window B went on badging it private for the + * full 30 seconds watched. Nothing crossed. B learned only when some unrelated + * refresh re-read a list — which the stamp had re-sorted, so the chat came back + * at the TOP. Without the stamp that incidental path is gone for the sidebar + * outright: `useSidebarSessions` re-reads only the head of its keyset, and a + * months-old row is never in the head. + * + * So the change announces itself, and every list surface re-reads the row in + * place: + * + * - `sessionListCache` (History's rows, Home recents, the tab strip's cached + * tiers) patches the entry it holds; + * - `useSidebarSessions` patches the row it holds, wherever it sits; + * - `SessionHistoryView` moves its page badge. + * + * An open chat's store is followed by `GET /sessions/changes`, which compares + * `privacy_tier` and `privacy_reason` (never `updated_at`), and its poll tells + * the chat's controller to re-read within about two seconds. The registry also + * hands a controller every row read made here, so a store that holds a stale + * tier re-reads at once rather than on the next poll. + * + * # Both directions, or the push is a new way to be wrong + * + * ⚠ **Pushing only the lowering was defect D4 of the 2026-09-13 repair round.** + * Before this channel existed a second window never learned of a + * declassification, so it kept a private badge — stale, but in the safe + * direction. Once the lowering was pushed and nothing pushed the raise, the + * sequence "declassify in A, then send one turn on a private model in A" left + * window B's History row AND its sidebar row badging the chat PUBLIC for as + * long as it was watched (90 s, and the sidebar over two minutes), while the + * database read `private` / `turn:versa_azure`. A channel that can lower a + * badge must also be able to raise it, so a store that observes its chat's tier + * change announces here too — see `ChatStreamRegistry.noteControllerTier`. + * + * {@link lastKnownSessionTier} is what keeps that from echoing: a window that + * has already delivered a read of the new tier (because someone announced it) + * does not announce the same change again when its own store catches up. + * + * # The announcement is a nudge, never a payload + * + * The same rule `sessionBindingSync`'s app-model nudge and `sessionMetaSubscription` + * state: a receiver is handed an id and goes to look. A declassified chat can be + * raised straight back by a turn a moment later, and a window that applied + * "public" from a message would show a public badge over a private chat — the + * one direction a privacy badge must never be wrong in. Re-reading the daemon + * ends on whichever WRITE landed last. + * + * ⚠ **One read per announcement per window, shared by every subscriber.** The + * row is read here and handed to the subscribers, rather than each subscriber + * fetching for itself, so a declassification costs one `GET` per window. + * + * ⚠ **Announce only what has landed.** Call it after the write resolved (or a + * read showed that it had), or after a store READ the new tier; a nudge that + * outran its write would re-read the value it was sent to replace. + */ + +export interface SessionRowFacts { + sessionId: string; + privacy_tier: SessionClassification; + privacy_reason: string | null; +} + +type Listener = (facts: SessionRowFacts) => void; + +const listeners = new Set(); + +/** + * The newest read issued for each chat. Two announcements for one chat can + * overlap, and the answer to the first may arrive second; only the newest read + * is delivered. Generations come from one counter that never resets, so a read + * issued after an entry was cleared can never share a number with an older one + * still in flight. + */ +const newestRead = new Map(); +let readCounter = 0; + +/** + * The tier of the last read this window DELIVERED for each chat. See + * {@link lastKnownSessionTier}. Holds one short string per chat the channel has + * carried, which is bounded by the chats whose classification moved. + */ +const delivered = new Map(); + +let channel: BroadcastChannel | null = null; + +function getChannel(): BroadcastChannel | null { + if (channel) return channel; + // Lazy, and absent-tolerant, so a test environment without BroadcastChannel + // still loads the module — as `sessionNameSync` and `sessionBindingSync` do. + if (typeof BroadcastChannel === 'undefined') return null; + channel = new BroadcastChannel('biorouter:session-row'); + channel.onmessage = (event: MessageEvent) => { + // Shape-checked, not trusted: this arrives from another window. + const sessionId = (event.data as { sessionId?: unknown } | null | undefined)?.sessionId; + if (typeof sessionId !== 'string' || sessionId.length === 0) return; + void reread(sessionId); + }; + return channel; +} + +/** + * Read one chat's classification from the daemon — the row, not the + * transcript, with the proof-of-user a private chat's row needs. + * + * `null` when the read failed or answered for another chat. Never throws: a + * caller that needs to know what happened to a write asks this, and "could not + * ask" is an answer it has to handle rather than an exception it can forget. + */ +export async function readSessionRowFacts(sessionId: string): Promise { + try { + const response = await getSession({ + path: { session_id: sessionId }, + // The row, not the transcript: this wants two strings. + query: { metadata_only: true }, + // A private chat's row is refused without the proof-of-user, and the + // chat this exists for may still be private by the time it is read. + headers: await userActionHeaders(), + throwOnError: true, + }); + const row = response.data; + if (!row || row.id !== sessionId || !row.privacy_tier) return null; + return { + sessionId, + privacy_tier: row.privacy_tier, + privacy_reason: row.privacy_reason ?? null, + }; + } catch { + return null; + } +} + +async function reread(sessionId: string): Promise { + if (listeners.size === 0) return; + const generation = ++readCounter; + newestRead.set(sessionId, generation); + try { + const facts = await readSessionRowFacts(sessionId); + // Silent on failure, as `refreshSessionBinding` is: a read that failed + // leaves every surface exactly as stale as it was, which is not worth a + // toast. + if (!facts || newestRead.get(sessionId) !== generation) return; + delivered.set(sessionId, facts.privacy_tier); + for (const listener of [...listeners]) listener(facts); + } finally { + if (newestRead.get(sessionId) === generation) newestRead.delete(sessionId); + } +} + +/** + * Announce that `sessionId`'s row changed in place. Re-reads it for this window + * and tells every other window to do the same. + */ +export function announceSessionRowChanged(sessionId: string): void { + void reread(sessionId); + getChannel()?.postMessage({ sessionId }); +} + +/** + * Re-read `sessionId`'s row for THIS window only, and deliver it to this + * window's subscribers. No other window is told. + * + * For a disagreement this window found by itself: a list answer that differs + * from a row read which landed while that list was in flight. Nobody else holds + * that pair of readings, so nobody else needs the third one that settles it. + */ +export function rereadSessionRowHere(sessionId: string): void { + void reread(sessionId); +} + +/** + * Reconcile a LIST answer with the row reads this window delivered while that + * list request was in flight, and return the rows to publish. + * + * # Why a list surface cannot just take either one + * + * Neither reading is known to be the later one. A list request issued before a + * turn raised a chat can land after the raise was read and patched in, and a + * surface that adopted the list would put the chat back to PUBLIC — a raise + * undone by an older photograph. Replaying the row read over the list is wrong + * the other way: that read may itself have been issued before the list, so it + * could write an older `public` over a newer `private`. + * + * So a disagreement is settled the only way that does not guess: + * + * 1. until it is settled the row shows the HIGHER tier of the two (a chat is + * never drawn public while either reading says private), and + * 2. the row is read a third time, now — after both — and that read patches + * the surface through the ordinary channel ({@link rereadSessionRowHere}). + * + * A row the two readings agree on, and a row no read touched, is published as + * the list answered. + * + * `withReason` compares `privacy_reason` as well, for a surface whose rows carry + * it (`Session`); `SessionSummary` carries only the tier. + * + * Consumes `readDuringFetch` (it is cleared). + */ +export function settleRowsReadDuringFetch< + T extends { + id: string; + privacy_tier?: SessionClassification | null; + privacy_reason?: string | null; + }, +>(rows: T[], readDuringFetch: Map, withReason: boolean): T[] { + if (readDuringFetch.size === 0) return rows; + const unsettled: string[] = []; + const settled = rows.map((row) => { + const read = readDuringFetch.get(row.id); + if (!read) return row; + const agrees = + row.privacy_tier === read.privacy_tier && + (!withReason || (row.privacy_reason ?? null) === read.privacy_reason); + if (agrees) return row; + unsettled.push(row.id); + if (read.privacy_tier !== 'private' || row.privacy_tier === 'private') return row; + return withReason + ? { ...row, privacy_tier: read.privacy_tier, privacy_reason: read.privacy_reason } + : { ...row, privacy_tier: read.privacy_tier }; + }); + readDuringFetch.clear(); + for (const sessionId of unsettled) rereadSessionRowHere(sessionId); + return settled; +} + +/** + * The tier of the last read of `sessionId` this window delivered to its + * subscribers, or `undefined` when the channel has carried nothing for it. + * + * What it is for: a chat store that sees its tier change asks this before + * announcing, and says nothing when the window has already delivered that tier + * — the change was announced by whoever made it, every window has read it, and + * a second announcement would only make every window read it again. + */ +export function lastKnownSessionTier(sessionId: string): SessionClassification | undefined { + return delivered.get(sessionId); +} + +/** + * Subscribe to freshly-read rows. Returns the unsubscribe. Subscribe from a + * MOUNT, or — for a module that owns a cache, as `sessionListCache` does its + * name channel — at module scope; never from a getter. + */ +export function subscribeSessionRowChanges(listener: Listener): () => void { + getChannel(); + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +}