From b150a225d5e6256e9f04d7bbbfe12d2c7aca54a6 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 01:55:58 -0700 Subject: [PATCH 01/14] fix(server): a keyless daemon starts a new chat on its configured private model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit biorouter serve spawns biorouterd with no user-action key (SD-7), and the new-chat gate demanded that key's proof before binding a private default, so every POST /agent/start on a serve daemon configured with a private provider was refused 409 — the 2026-09-10 QA run's F1. The configured default is the person's choice, made out of band with biorouter configure (open question 24 put the raise at the write), so on a daemon that holds no key the new-chat bind no longer asks for a proof nobody can give. A daemon that holds a key (the desktop's) still refuses a proof-less first bind: the renderer sends the proof for free, and a model holding the recovered secret should not mint a private chat. The exemption covers one provider at one moment. On a keyless daemon, /agent/update_provider now measures every move onto a private model from Public (raise_baseline), so a new chat cannot be moved sideways to a private model nobody configured. --- crates/biorouter-server/src/routes/agent.rs | 123 +++++- .../tests/new_chat_no_user_key.rs | 364 ++++++++++++++++++ 2 files changed, 480 insertions(+), 7 deletions(-) create mode 100644 crates/biorouter-server/tests/new_chat_no_user_key.rs diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 8dc1604f8..629edf7e9 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -341,6 +341,59 @@ fn configured_new_session_provider() -> Result, Er } } +/// SD-9 (`docs/deployment/serve-decisions.md`): does binding the operator's +/// configured default provider to a brand-new chat need a person's proof? +/// +/// A new chat has no capability of its own yet, so a private default reads as a +/// raise from Public — the reading `update_agent_provider` gives any first bind. +/// What differs is who chose the model. `/agent/start` names no provider; it +/// binds `BIOROUTER_PROVIDER`, a key only a proven person may write over HTTP +/// (DR-16, open question 24) or the operator wrote out of band with `biorouter +/// configure`. The proof is therefore asked for only where it can be given: +/// +/// * `Proven` — the desktop renderer, which sends `X-User-Action` on every start +/// (`ui/desktop/src/sessions.ts`). Binds. +/// * `Unproven` — a daemon that holds a key, and a caller that did not present +/// it: a script, or the model holding the daemon secret AR-11 found +/// recoverable. Refused, as before. The person proves themselves here at no +/// cost, and without the refusal a model could mint a private-capability chat +/// with an extension set of its own choosing. +/// * `NoKeyInstalled` — `biorouter serve` (SD-7) or a hand-run `biorouterd`. +/// Nobody on this daemon can prove anything, so a refusal here refuses the +/// person too, on every new chat, always — the 2026-09-10 QA's F1. Binds. +/// +/// Only the configured default is exempt, and only at creation: `/agent/start` +/// cannot name any other provider, and on a keyless daemon +/// `update_agent_provider` refuses every private bind ([`raise_baseline`]), so a +/// new chat there never reaches a private model the operator did not choose. +fn new_chat_bind_needs_user(enforced: bool, tier: ProviderTier, proof: UserActionProof) -> bool { + enforced + && raise_needs_user_action(ProviderTier::Public, tier) + && match proof { + UserActionProof::Proven | UserActionProof::NoKeyInstalled => false, + UserActionProof::Unproven => true, + } +} + +/// The capability `update_agent_provider` measures a raise from — SD-9's other +/// half. +/// +/// On a daemon that holds a user-action key it is the chat's live capability, +/// as it always was. On one that holds none, no chat's private capability came +/// from anything a person proved over HTTP: the only private binding such a +/// daemon hands out through its routes is [`new_chat_bind_needs_user`]'s +/// creation-time bind to the configured default. There is no private floor for +/// a request to build on, so a bind to ANY private provider is measured from +/// Public, and refused, since no proof can arrive. Without this, SD-9 would let +/// a new chat on a private default be moved sideways, `Private -> Private`, to a +/// private model nobody configured. +fn raise_baseline(current: ProviderTier, proof: UserActionProof) -> ProviderTier { + match proof { + UserActionProof::NoKeyInstalled => ProviderTier::Public, + UserActionProof::Proven | UserActionProof::Unproven => current, + } +} + async fn bind_new_session_provider( state: &AppState, session: &Session, @@ -355,10 +408,12 @@ async fn bind_new_session_provider( message: format!("Failed to configure the selected provider for the new chat: {error}"), status: StatusCode::BAD_REQUEST, })?; - if biorouter::privacy::privacy_tiers_enabled() - && raise_needs_user_action(ProviderTier::Public, provider.tier()) - && !is_user_action(headers) - { + // DR-15's master opt-out, read inside the gate as every #56 surface does. + if new_chat_bind_needs_user( + biorouter::privacy::privacy_tiers_enabled(), + provider.tier(), + user_action_proof(headers), + ) { return Err(ErrorResponse { message: PrivacyRefusal::TierRaiseNeedsUser { requested: provider_name, @@ -619,7 +674,7 @@ pub struct RestartAgentResponse { (status = 200, description = "Agent started successfully", body = Session), (status = 400, description = "Bad request", body = ErrorResponse), (status = 401, description = "Unauthorized - invalid secret key"), - (status = 409, description = "The selected private provider requires user-action proof", body = ErrorResponse), + (status = 409, description = "The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-9). A daemon with no user-action key binds its configured provider without one.", body = ErrorResponse), (status = 500, description = "Internal server error", body = ErrorResponse) ) )] @@ -1297,7 +1352,9 @@ async fn get_callable_tool_count( a public model cannot be bound to a private chat \ (body = PrivacyBarrierBody). DR-16: the bind raises this \ chat's capability to Private and the request carried no \ - proof it came from the user (body = plain text)", + proof it came from the user; on a daemon with no \ + user-action key, any bind to a private model (SD-9) \ + (body = plain text)", body = PrivacyBarrierBody), (status = 424, description = "Agent not initialized"), (status = 500, description = "Internal server error") @@ -1370,6 +1427,10 @@ async fn update_agent_provider( // DR-16 rejected. Sideways and downward binds are untouched for every // caller, which is what keeps Gate A's path, the CLI, // `restore_provider_from_session` and every apps-runtime bind working. + // The one exception is this route on a daemon with no user-action key, + // where a move onto a private model is measured from Public however the + // chat is bound today — `raise_baseline`, SD-9. The predicate itself is + // unchanged, and none of the in-process binds above passes through here. // // An unbound session reads as Public — `Agent::provider` errors when nothing // is bound (and when Gate B' refuses what is), and the conservative reading @@ -1380,11 +1441,13 @@ async fn update_agent_provider( .await .map(|p| p.tier()) .unwrap_or(ProviderTier::Public); + // SD-9's other half — see `raise_baseline`. + let baseline = raise_baseline(current, user_action_proof(&headers)); // DR-15's master opt-out, read INSIDE the gate. A direct read, not a // `CallCapability`: a provider raise over HTTP is not a tool call and has no // admitted capability to inherit. if biorouter::privacy::privacy_tiers_enabled() - && raise_needs_user_action(current, new_provider.tier()) + && raise_needs_user_action(baseline, new_provider.tier()) && !is_user_action(&headers) { return Err(( @@ -3117,6 +3180,52 @@ mod new_session_provider_binding_tests { .await .unwrap(); } + + /// SD-9, every proof verdict against both tiers. The keyless arm cannot be + /// reached through a route in this binary — the installed digest is a + /// process-global `OnceLock` and the test above installs one — so the route + /// half lives in `tests/new_chat_no_user_key.rs`, a binary that never does. + #[test] + fn only_a_daemon_that_can_check_a_proof_asks_a_new_chat_for_one() { + use UserActionProof::{NoKeyInstalled, Proven, Unproven}; + assert!(!new_chat_bind_needs_user(true, ProviderTier::Private, Proven)); + assert!(new_chat_bind_needs_user(true, ProviderTier::Private, Unproven)); + assert!( + !new_chat_bind_needs_user(true, ProviderTier::Private, NoKeyInstalled), + "a keyless daemon refusing its own configured default refuses every person, always" + ); + for proof in [Proven, Unproven, NoKeyInstalled] { + // A public default raises nothing, for anyone. + assert!(!new_chat_bind_needs_user(true, ProviderTier::Public, proof)); + // DR-15's master opt-out turns the gate off, not the question. + assert!(!new_chat_bind_needs_user(false, ProviderTier::Private, proof)); + } + } + + /// SD-9's other half: a keyless daemon measures every move onto a private + /// model from Public, so its exemption for the configured default cannot be + /// carried sideways to a private model nobody configured. + #[test] + fn a_keyless_daemon_has_no_private_floor_for_a_switch_to_build_on() { + use UserActionProof::{NoKeyInstalled, Proven, Unproven}; + for current in [ProviderTier::Private, ProviderTier::Public] { + assert_eq!(raise_baseline(current, NoKeyInstalled), ProviderTier::Public); + // A daemon that can check a proof keeps measuring from the live binding. + assert_eq!(raise_baseline(current, Proven), current); + assert_eq!(raise_baseline(current, Unproven), current); + } + // The composition `update_agent_provider` asks. Sideways onto a private + // model is a raise only where no proof can be checked. + let sideways = |proof| { + raise_needs_user_action( + raise_baseline(ProviderTier::Private, proof), + ProviderTier::Private, + ) + }; + assert!(sideways(NoKeyInstalled)); + assert!(!sideways(Unproven)); + assert!(!sideways(Proven)); + } } #[cfg(test)] diff --git a/crates/biorouter-server/tests/new_chat_no_user_key.rs b/crates/biorouter-server/tests/new_chat_no_user_key.rs new file mode 100644 index 000000000..4eb7f8264 --- /dev/null +++ b/crates/biorouter-server/tests/new_chat_no_user_key.rs @@ -0,0 +1,364 @@ +//! SD-9: on a daemon that holds no proof-of-user key — `biorouter serve` (SD-7) +//! or a hand-run `biorouterd` — a new chat starts on the operator's configured +//! provider, private or not, and nothing on the HTTP surface can move it onto a +//! private model the operator did not configure. +//! +//! The 2026-09-10 QA run (finding F1) measured the first half failing: with +//! `versa_azure` configured, every `POST /agent/start` on a `serve` daemon was +//! refused 409, because the new-chat gate asked for a proof this daemon can +//! never check. The browser showed nothing at all. +//! +//! ⚠ **Its own test binary on purpose**, for the reason `approval_no_user_key.rs` +//! gives: the installed digest is a process-global `OnceLock`, the lib's tests +//! install one, and inside that binary the keyless state is unreachable once the +//! first of them wins. Nothing here installs a digest — which is exactly how +//! `biorouter serve` starts its daemon. + +// Redirects this binary's Biorouter data/config/state dirs at a throwaway root +// before `main`, so nothing here can open the developer's real `sessions.db` +// or write the developer's real `config.yaml`. +#[path = "../src/test_sandbox.rs"] +mod test_sandbox; + +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; + +use axum::body::Body; +use axum::http::{HeaderMap, Request, StatusCode}; +use axum::Router; +use biorouter::config::{with_config_overrides, Config}; +use biorouter::conversation::message::Message; +use biorouter::privacy::refusal::USER_ACTION_REFUSAL_MARKER; +use biorouter::privacy::SessionClassification; +use biorouter::providers::versa_azure::VERSA_AZURE_DEPLOYMENT; +use biorouter_server::auth::{user_action_proof, UserActionProof}; +use biorouter_server::state::AppState; +use serde_json::{json, Value}; +use serial_test::serial; +use tower::ServiceExt; +use wiremock::matchers::{body_string_contains, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// The posture the QA run used: `biorouter configure` chose Versa. The key is a +/// placeholder — these tests construct the provider and never send it a request. +fn versa_is_the_configured_default() -> HashMap { + HashMap::from([ + ("BIOROUTER_PROVIDER".to_string(), "versa_azure".to_string()), + ( + "BIOROUTER_MODEL".to_string(), + VERSA_AZURE_DEPLOYMENT.to_string(), + ), + ( + "VERSA_AZURE_API_KEY".to_string(), + "placeholder-never-sent".to_string(), + ), + ]) +} + +/// Every test here stands on this: the daemon under test holds no key. +fn assert_the_daemon_is_keyless() { + assert_eq!( + user_action_proof(&HeaderMap::new()), + UserActionProof::NoKeyInstalled, + "something in this binary installed a user-action digest, so these tests would be \ + measuring a desktop daemon rather than a `biorouter serve` one" + ); + assert!( + biorouter::privacy::privacy_tiers_enabled(), + "privacy tiers are off, so no gate below would fire either way" + ); +} + +async fn post_json(app: Router, uri: &str, body: Value) -> (StatusCode, String) { + let response = app + .oneshot( + Request::builder() + .uri(uri) + .method("POST") + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(), + ) + .await + .unwrap(); + let status = response.status(); + let bytes = tokio::time::timeout( + Duration::from_secs(120), + axum::body::to_bytes(response.into_body(), usize::MAX), + ) + .await + .expect("the response body did not finish within two minutes") + .unwrap(); + (status, String::from_utf8_lossy(&bytes).into_owned()) +} + +fn start_request(working_dir: &std::path::Path) -> Value { + // No extensions: the chat under test needs a model and nothing else, and + // the machine default set is not this test's subject. + json!({ "working_dir": working_dir, "extension_overrides": [] }) +} + +async fn discard(state: &Arc, session_id: &str) { + // The tests run serially, so every cached agent is this test's. + state.clear_cached_agents().await; + let _ = state.session_manager().delete_session(session_id).await; +} + +/// F1, the half the QA run measured: the configured private provider is the +/// person's choice, made at the terminal, so binding it to a brand-new chat is +/// not a switch and needs no proof — on the one kind of daemon where no proof +/// can exist. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_starts_a_new_chat_on_its_configured_private_model() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + + let (status, body) = with_config_overrides( + versa_is_the_configured_default(), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!( + status, + StatusCode::OK, + "a keyless daemon refused to start a chat on its own configured model: {body}" + ); + let id = serde_json::from_str::(&body).unwrap()["id"] + .as_str() + .expect("the started session carries an id") + .to_string(); + + let row = state.session_manager().get_session(&id, false).await.unwrap(); + assert_eq!(row.provider_name.as_deref(), Some("versa_azure")); + assert_eq!( + row.model_config.map(|config| config.model_name).as_deref(), + Some(VERSA_AZURE_DEPLOYMENT) + ); + // O5: the ratchet fires on the first turn, never on the bind. A chat that + // has touched nothing is not yet private — it is private-CAPABLE. + assert_eq!(row.privacy_tier, SessionClassification::Public); + + let agent = state.get_agent_for_route(id.clone()).await.unwrap(); + assert_eq!( + agent + .provider() + .await + .expect("the first turn must not fail with `Provider not set`") + .get_name(), + "versa_azure" + ); + + discard(&state, &id).await; +} + +/// The complement the exemption must not leak into: a request that asks a new +/// chat for a DIFFERENT private model than the one the operator configured is +/// still refused. `/agent/start` cannot name one, so the request that can is +/// `/agent/update_provider` on the chat it just made — `Private -> Private`, +/// which the raise predicate alone would call sideways and wave through. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_will_not_move_a_new_chat_to_a_private_model_nobody_configured() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + + let (status, body) = with_config_overrides( + versa_is_the_configured_default(), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + let id = serde_json::from_str::(&body).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + + // A loopback Ollama is Private (`self_hosted_tier`), and constructing one + // opens no connection, so port 1 is never dialled. + let (status, body) = with_config_overrides( + HashMap::from([( + "OLLAMA_HOST".to_string(), + "http://127.0.0.1:1".to_string(), + )]), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/update_provider", + json!({ "session_id": id, "provider": "ollama", "model": "stub-model" }), + ), + ) + .await; + assert_eq!( + status, + StatusCode::CONFLICT, + "a keyless daemon moved a new chat onto a private model the operator never configured: \ + {body}" + ); + assert!( + body.contains(USER_ACTION_REFUSAL_MARKER), + "refused, but not by the tier gate: {body}" + ); + + let row = state.session_manager().get_session(&id, false).await.unwrap(); + assert_eq!( + row.provider_name.as_deref(), + Some("versa_azure"), + "the refused switch rewrote the row anyway" + ); + let agent = state.get_agent_for_route(id.clone()).await.unwrap(); + assert_eq!(agent.provider().await.unwrap().get_name(), "versa_azure"); + + discard(&state, &id).await; +} + +/// SD-1 is untouched by SD-9: the configured default itself still cannot be +/// changed from a keyless daemon, which is what makes it the operator's choice. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_still_refuses_to_change_the_configured_default() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let (status, body) = post_json( + biorouter_server::routes::config_management::routes(state), + "/config/set_provider", + json!({ "provider": "versa_azure", "model": VERSA_AZURE_DEPLOYMENT }), + ) + .await; + assert_eq!(status, StatusCode::CONFLICT, "{body}"); +} + +/// "privacy_tier ratchets on the first turn as usual": the exemption changes who +/// may bind the configured model, and nothing about what a turn on it does. +/// +/// The configured default here is an Ollama endpoint on loopback, because a +/// Versa module re-pointed at a stub server is no longer Private +/// (`ucsf_gateway_tier` reads the resolved host) and a test must not send +/// traffic to the real gateway. ⚠ No model runs: the endpoint is a `wiremock` +/// stub that returns one canned completion. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { + assert_the_daemon_is_keyless(); + + let stub = MockServer::start().await; + let chunk = |delta: Value, finish: Value| { + json!({ + "id": "stub", "object": "chat.completion.chunk", "model": "stub-model", + "choices": [{ "index": 0, "delta": delta, "finish_reason": finish }] + }) + }; + let sse = format!( + "data: {}\n\ndata: {}\n\ndata: [DONE]\n\n", + chunk(json!({ "role": "assistant", "content": "ready" }), Value::Null), + chunk(json!({ "content": "" }), json!("stop")), + ); + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .and(body_string_contains("\"stream\":true")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_string(sse), + ) + .with_priority(1) + .mount(&stub) + .await; + // Anything that asks without streaming — the chat's auto-title — gets a + // plain completion rather than an event stream it cannot parse. + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "stub", "object": "chat.completion", "model": "stub-model", + "choices": [{ "index": 0, "finish_reason": "stop", + "message": { "role": "assistant", "content": "Stub title" } }], + "usage": { "prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2 } + }))) + .mount(&stub) + .await; + + // Written to this binary's sandboxed config.yaml rather than scoped to a + // task: the turn runs on a spawned task, which a task-local override would + // not reach. + let config = Config::global(); + config.set_param("BIOROUTER_PROVIDER", "ollama").unwrap(); + config.set_param("BIOROUTER_MODEL", "stub-model").unwrap(); + config.set_param("OLLAMA_HOST", stub.uri()).unwrap(); + + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + let (status, body) = post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + let id = serde_json::from_str::(&body).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + let before = state.session_manager().get_session(&id, false).await.unwrap(); + assert_eq!(before.provider_name.as_deref(), Some("ollama")); + assert_eq!(before.privacy_tier, SessionClassification::Public); + + let message = Message::user().with_text("Reply with the single word ready."); + let (status, stream) = post_json( + biorouter_server::routes::reply::routes(Arc::clone(&state)), + "/reply", + json!({ "user_message": message, "session_id": id }), + ) + .await; + assert_eq!(status, StatusCode::OK, "{stream}"); + assert!( + stream.contains("\"Finish\""), + "the turn did not finish: {stream}" + ); + assert!( + stream.contains("ready"), + "the turn did not run on the stub: {stream}" + ); + + let after = state.session_manager().get_session(&id, false).await.unwrap(); + assert_eq!( + after.privacy_tier, + SessionClassification::Private, + "a turn on the configured private model did not ratchet the chat" + ); + assert_eq!(after.privacy_reason.as_deref(), Some("turn:ollama")); + + // The chat's NEXT request, now that it is private. A keyless daemon reaches a + // private chat only for a caller whose stated capability covers it, so a + // browser tab that states nothing loses the chat it just started; the host's + // provider, which is what a tab on this host states (SD-9), keeps it. + let reach = |caller: Option<&str>| { + let mut request = Request::builder().uri(format!("/sessions/{id}")); + if let Some(provider) = caller { + request = request.header("X-Caller-Provider", provider); + } + let app = biorouter_server::routes::session::routes(Arc::clone(&state)); + async move { + app.oneshot(request.body(Body::empty()).unwrap()) + .await + .unwrap() + .status() + } + }; + assert_eq!(reach(None).await, StatusCode::FORBIDDEN); + assert_eq!(reach(Some("ollama")).await, StatusCode::OK); + + discard(&state, &id).await; + for key in ["BIOROUTER_PROVIDER", "BIOROUTER_MODEL", "OLLAMA_HOST"] { + let _ = config.delete(key); + } +} From 7b77f9dadcbdb94ef8d8b034f1bbcc9898930514 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 01:55:58 -0700 Subject: [PATCH 02/14] test(privacy): point AR-15's closure scan at update_agent_provider the_documented_closure_is_the_one_the_code_performs took the FIRST TierRaiseNeedsUser in routes/agent.rs. Since eb594ded that has been the new-chat gate, not update_agent_provider's, so deleting the proof check from update_provider left the scan green (measured). The scan now starts at the handler AR-15 is about and is bounded by the next route. --- .../tests/privacy_ar15_is_retired.rs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/biorouter-server/tests/privacy_ar15_is_retired.rs b/crates/biorouter-server/tests/privacy_ar15_is_retired.rs index 6862181b4..057683be3 100644 --- a/crates/biorouter-server/tests/privacy_ar15_is_retired.rs +++ b/crates/biorouter-server/tests/privacy_ar15_is_retired.rs @@ -345,9 +345,24 @@ fn the_documented_closure_is_the_one_the_code_performs() { // The docs now assert a specific gate. If the gate goes, the docs are // wrong in the *dangerous* direction — claiming a hole is closed when it is // open — and no other test in this tree ties the two together. - let refusal = AGENT_ROUTE - .find("PrivacyRefusal::TierRaiseNeedsUser") - .expect("routes/agent.rs no longer refuses an unproven tier raise at all"); + // + // ⚠ The scan starts at `update_agent_provider`, the handler AR-15 is about. + // It used to take the FIRST refusal in the file, which stopped being this + // handler's when `eb594ded` put the new-chat gate above it: from then on the + // scan read that gate, and deleting the proof from this one left it green. + // The new-chat gate is SD-9's, a different rule with its own tests in + // `routes/agent.rs`. + let handler = AGENT_ROUTE + .find("async fn update_agent_provider") + .expect("update_agent_provider moved; AR-15's gate is the one inside it"); + let body = cut_from(AGENT_ROUTE, handler); + // Bounded by the next route, so a refusal that left this handler cannot be + // found in the one after it. + let body = cut_to(body, body.find("\n#[utoipa::path(").unwrap_or(body.len())); + let refusal = handler + + body + .find("PrivacyRefusal::TierRaiseNeedsUser") + .expect("update_agent_provider no longer refuses an unproven tier raise at all"); let guard = cut_to(AGENT_ROUTE, refusal) .rfind(" if ") .expect("the tier-raise refusal is not inside an `if`"); From 51d863cb698a11119c22554f5d925aff8514313a Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 01:55:58 -0700 Subject: [PATCH 03/14] fix(desktop): say when a chat fails to start, and keep what was typed A failed POST /agent/start reached console.error and nothing else on the Home composer (the QA run's F1): the text vanished and nothing appeared. One pure notice, startChatFailureNotice, now words every failure for a person (the daemon's own text stays behind Copy error), and every surface that starts a chat reports through it: Home, a fresh tab, a workflow window, the launcher, both Ask Biorouter buttons and Workflows. A source scan keeps an eighth surface from picking its own way to fail. Also fixed on the way: a workflow window whose start failed re-ran its creation effect in a loop, and a failed workflow start replaced the Workflows list with a list-load error. A browser tab now states the host's configured model (X-Caller-Provider) where the desktop proves the person: measured, a chat started on a private host model 403s its next request once its first reply makes it private, and 200s with the host provider stated. --- ui/desktop/src/App.tsx | 10 ++ ui/desktop/src/components/BaseChat.tsx | 16 +- .../GroupedExtensionLoadingToast.tsx | 12 +- .../src/components/Hub.startFailure.test.tsx | 140 +++++++++++++++++ ui/desktop/src/components/Hub.tsx | 22 ++- .../components/workflows/WorkflowsView.tsx | 9 +- ui/desktop/src/toasts.tsx | 7 +- ui/desktop/src/utils/launcherMessage.ts | 7 +- ui/desktop/src/utils/startChatFailure.test.ts | 146 ++++++++++++++++++ ui/desktop/src/utils/startChatFailure.ts | 92 +++++++++++ .../src/utils/userAction.surface.test.ts | 75 +++++++++ ui/desktop/src/utils/userAction.ts | 63 ++++++++ 12 files changed, 580 insertions(+), 19 deletions(-) create mode 100644 ui/desktop/src/components/Hub.startFailure.test.tsx create mode 100644 ui/desktop/src/utils/startChatFailure.test.ts create mode 100644 ui/desktop/src/utils/startChatFailure.ts create mode 100644 ui/desktop/src/utils/userAction.surface.test.ts diff --git a/ui/desktop/src/App.tsx b/ui/desktop/src/App.tsx index 8c43452cb..237e2c913 100644 --- a/ui/desktop/src/App.tsx +++ b/ui/desktop/src/App.tsx @@ -58,6 +58,8 @@ import { View, ViewOptions } from './utils/navigationUtils'; import { useNavigation } from './hooks/useNavigation'; import { errorMessage } from './utils/conversionUtils'; +import { startChatFailureNotice } from './utils/startChatFailure'; +import { toastError } from './toasts'; import { getInitialWorkingDir } from './utils/workingDir'; import { deliverLauncherMessage } from './utils/launcherMessage'; import { ChatStreamProvider } from './hooks/chatStreamStore'; @@ -168,6 +170,14 @@ const PairRouteContent = ({ setChat }: { setChat: (chat: ChatType) => void }) => }); } catch (error) { console.error('Failed to create session:', error); + toastError(startChatFailureNotice(error, { kept: false })); + // Leave. Every input this effect keys on is still true after a + // failure, so staying would re-run it the moment `isCreatingSession` + // drops — a `POST /agent/start` loop for as long as the refusal + // holds. Home is the resting state for a layout with no tabs (#38). + // The cargo here is a workflow window's; a launcher message arrives + // with a session id and never reaches this branch. + navigate('/', { replace: true }); } finally { setIsCreatingSession(false); } diff --git a/ui/desktop/src/components/BaseChat.tsx b/ui/desktop/src/components/BaseChat.tsx index d3abed351..5519df2c9 100644 --- a/ui/desktop/src/components/BaseChat.tsx +++ b/ui/desktop/src/components/BaseChat.tsx @@ -72,7 +72,8 @@ import { useBoundAffiliation } from './privacy/useBoundAffiliation'; import { getSessionTitlePadding } from './Layout/TitlebarControls'; import { announceSessionName, renameSession } from '../utils/sessionNameSync'; import { toastError, toastWarning } from '../toasts'; -import { errorMessage, isConnectionError } from '../utils/conversionUtils'; +import { errorMessage } from '../utils/conversionUtils'; +import { startChatFailureNotice } from '../utils/startChatFailure'; import { Greeting } from './common/Greeting'; import { navigateWithViewTransition } from '../utils/navigationUtils'; import { unwrapGuardrailFrameInContent } from '../utils/guardrailFrame'; @@ -736,8 +737,9 @@ export function collectArtifactsFromMessages( * is unreachable the awaited createSession rejects *after* the text is already * gone — and the bare catch used to show nothing, so the message silently * vanished. Restore the typed text (via a `restore-chat-input` event the composer - * listens for) and surface a visible toast. Connection detection only picks the - * wording; the toast + restore fire on ANY rejection, so no silent path remains. + * listens for) and surface a visible toast. The words are + * `startChatFailureNotice`'s, shared with every other surface that starts a + * chat; the toast + restore fire on ANY rejection, so no silent path remains. * Exported so it can be unit-tested without Electron. */ export function handleCreateSessionError( @@ -754,13 +756,7 @@ export function handleCreateSessionError( }, }) ); - const connection = isConnectionError(err); - toastError({ - title: connection ? 'Backend disconnected' : 'Failed to start chat', - msg: connection - ? 'Biorouter could not reach its backend. Your message was kept - try again in a moment.' - : errorMessage(err), - }); + toastError(startChatFailureNotice(err, { kept: true })); } /** diff --git a/ui/desktop/src/components/GroupedExtensionLoadingToast.tsx b/ui/desktop/src/components/GroupedExtensionLoadingToast.tsx index ed2552cd9..4a6fc4328 100644 --- a/ui/desktop/src/components/GroupedExtensionLoadingToast.tsx +++ b/ui/desktop/src/components/GroupedExtensionLoadingToast.tsx @@ -4,6 +4,11 @@ import { Button } from './ui/button'; import { ModalShell } from './ModalShell'; import { NotificationContent, type NotificationStatus } from './alerts/NotificationSurface'; import { startNewSession } from '../sessions'; +// ⚠ `toasts.tsx` renders this component, so this import closes a cycle. It is +// the one `utils/extensionErrorUtils` already closes, and `toastError` is only +// read inside a click handler, long after both modules have evaluated. +import { toastError } from '../toasts'; +import { startChatFailureNotice } from '../utils/startChatFailure'; import { useNavigation } from '../hooks/useNavigation'; import { formatExtensionErrorMessage } from '../utils/extensionErrorUtils'; import { getInitialWorkingDir } from '../utils/workingDir'; @@ -232,7 +237,12 @@ export function GroupedExtensionLoadingToast({ onOpenChange={setReportOpen} extensions={extensions} onAskBiorouter={ - setView ? (hints) => startNewSession(getInitialWorkingDir(), hints, setView) : null + setView + ? (hints) => + void startNewSession(getInitialWorkingDir(), hints, setView).catch((error) => + toastError(startChatFailureNotice(error, { kept: false })) + ) + : null } /> )} diff --git a/ui/desktop/src/components/Hub.startFailure.test.tsx b/ui/desktop/src/components/Hub.startFailure.test.tsx new file mode 100644 index 000000000..3f9715245 --- /dev/null +++ b/ui/desktop/src/components/Hub.startFailure.test.tsx @@ -0,0 +1,140 @@ +import React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; + +/** + * F1 of the 2026-09-10 QA run, on the surface where it was measured: type into + * the Home composer, press Enter, and have `POST /agent/start` fail. The + * composer had already wiped itself, and the failure went to `console.error` + * and nowhere else — no toast, no message, the text gone. + * + * The real Hub and the real ChatInput are rendered, because the property lives + * in the handshake between them: ChatInput restores its box only when the + * submit resolves `false`, and Hub decides what it resolves. Only the heavy or + * context-hungry children are stubbed, as in `ChatInput.workingDir.test.tsx`. + */ + +const { mockCreateSession, mockToastError } = vi.hoisted(() => ({ + mockCreateSession: vi.fn(), + mockToastError: vi.fn(), +})); + +vi.mock('../sessions', () => ({ createSession: mockCreateSession })); +vi.mock('../toasts', async (importOriginal) => ({ + ...(await importOriginal()), + toastError: mockToastError, +})); +vi.mock('./sessions/SessionsInsights', () => ({ SessionInsights: () => null })); +vi.mock('./ConfigContext', () => ({ + useConfig: () => ({ + extensionsList: [], + getProviders: vi.fn(async () => []), + read: vi.fn(async () => null), + }), +})); +vi.mock('./ModelAndProviderContext', () => ({ + useModelAndProvider: () => ({ + getCurrentModelAndProvider: vi.fn(async () => ({ model: null, provider: null })), + currentModel: null, + currentProvider: null, + currentModelSupportsVision: false, + currentModelSupportedInputMimeTypes: null, + }), +})); +vi.mock('../hooks/useDiverge', () => ({ + useDiverge: () => ({ diverge: vi.fn() }), +})); +vi.mock('./settings/models/bottom_bar/ModelsBottomBar', () => ({ default: () => null })); +vi.mock('./bottom_menu/BottomMenuExtensionSelection', () => ({ + BottomMenuExtensionSelection: () => null, +})); +vi.mock('./bottom_menu/BottomMenuSkillSelection', () => ({ + BottomMenuSkillSelection: () => null, +})); +vi.mock('./bottom_menu/BottomMenuKnowledgeSelection', () => ({ + BottomMenuKnowledgeSelection: () => null, +})); +vi.mock('./bottom_menu/BottomMenuReasoningEffort', () => ({ + BottomMenuReasoningEffort: () => null, +})); +vi.mock('./bottom_menu/CostTracker', () => ({ CostTracker: () => null })); +vi.mock('./MessageQueue', () => ({ default: () => null })); +vi.mock('./MentionPopover', () => { + const MentionPopoverMock = React.forwardRef(() => null); + MentionPopoverMock.displayName = 'MentionPopoverMock'; + return { default: MentionPopoverMock }; +}); +vi.mock('../api', () => ({ + getSession: vi.fn(async () => ({ data: null })), + llamacppStatus: vi.fn(async () => ({ data: {} })), + updateWorkingDir: vi.fn(async () => ({ data: {} })), +})); + +import Hub from './Hub'; + +/** What `POST /agent/start` answered on the QA run's `biorouter serve` daemon. */ +const SERVE_DAEMON_REFUSAL = { + message: + "Switching this chat to a private model is the user's decision, not yours. The request to " + + "switch it to 'versa_azure' did not come from the model picker, so the chat is unchanged and " + + 'still on its current model. Do not retry; the same call will be refused again.', +}; + +beforeEach(() => { + vi.clearAllMocks(); + Object.assign(window, { + appConfig: { + get: (key: string) => (key === 'BIOROUTER_WORKING_DIR' ? '/default/workdir' : undefined), + }, + electron: { + directoryChooser: vi.fn(async () => ({ canceled: true, filePaths: [] })), + addRecentDir: vi.fn(), + logInfo: vi.fn(), + getPathForFile: vi.fn(() => ''), + on: vi.fn(), + off: vi.fn(), + }, + }); +}); + +describe('Hub: a chat that fails to start', () => { + it('says so in words for a person, and the composer keeps what was typed', async () => { + mockCreateSession.mockRejectedValueOnce(SERVE_DAEMON_REFUSAL); + const setView = vi.fn(); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + render(); + + const composer = screen.getByPlaceholderText('Ask Biorouter anything…') as HTMLTextAreaElement; + fireEvent.change(composer, { target: { value: 'Reply with the single word ready.' } }); + fireEvent.keyDown(composer, { key: 'Enter' }); + + await waitFor(() => expect(mockCreateSession).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(mockToastError).toHaveBeenCalledTimes(1)); + const notice = mockToastError.mock.calls[0][0] as { title: string; msg: string }; + expect(notice.title).toBe('Failed to start chat'); + expect(notice.msg).not.toContain('Do not retry'); + expect(notice.msg).toContain('Your message was kept.'); + + await waitFor(() => expect(composer.value).toBe('Reply with the single word ready.')); + expect(setView).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); + + it('opens the chat when the start succeeds, with nothing to report', async () => { + mockCreateSession.mockResolvedValueOnce({ id: 'session-1' }); + const setView = vi.fn(); + render(); + + const composer = screen.getByPlaceholderText('Ask Biorouter anything…') as HTMLTextAreaElement; + fireEvent.change(composer, { target: { value: 'hello' } }); + fireEvent.keyDown(composer, { key: 'Enter' }); + + await waitFor(() => + expect(setView).toHaveBeenCalledWith( + 'pair', + expect.objectContaining({ resumeSessionId: 'session-1', initialMessage: 'hello' }) + ) + ); + expect(mockToastError).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/desktop/src/components/Hub.tsx b/ui/desktop/src/components/Hub.tsx index 8b4228478..9e4b2f506 100644 --- a/ui/desktop/src/components/Hub.tsx +++ b/ui/desktop/src/components/Hub.tsx @@ -29,6 +29,8 @@ import { getInitialWorkingDir } from '../utils/workingDir'; import { createSession } from '../sessions'; import LoadingBioRouter from './LoadingBioRouter'; import type { UserAttachment } from '../types/message'; +import { toastError } from '../toasts'; +import { startChatFailureNotice } from '../utils/startChatFailure'; export default function Hub({ setView, @@ -39,7 +41,16 @@ export default function Hub({ const [workingDir, setWorkingDir] = useState(getInitialWorkingDir()); const [isCreatingSession, setIsCreatingSession] = useState(false); - const handleSubmit = async (e: React.FormEvent) => { + /** + * Resolves FALSE when no chat was started, which is ChatInput's signal to put + * the box back exactly as the user left it — text, reference chips and pasted + * images — since it wiped itself before this awaited anything. + * + * ⚠ The failure used to reach `console.error` and nothing else: the text + * vanished and the Home screen sat unchanged (the 2026-09-10 QA run, F1). It + * is now a toast in words for a person — see `startChatFailureNotice`. + */ + const handleSubmit = async (e: React.FormEvent): Promise => { const customEvent = e as unknown as CustomEvent; const combinedTextFromInput = customEvent.detail?.value || ''; const attachments = (customEvent.detail?.attachments ?? []) as UserAttachment[]; @@ -49,6 +60,7 @@ export default function Hub({ const extensionConfigs = getExtensionConfigsWithOverrides(extensionsList); clearExtensionOverrides(); setIsCreatingSession(true); + e.preventDefault(); try { const session = await createSession(workingDir, { @@ -61,13 +73,17 @@ export default function Hub({ initialMessage: combinedTextFromInput, initialAttachments: attachments, }); + return true; } catch (error) { console.error('Failed to create session:', error); setIsCreatingSession(false); + toastError(startChatFailureNotice(error, { kept: true })); + return false; } - - e.preventDefault(); } + // A second send while the first is still creating the chat: refused, so + // the composer keeps it. + return false; }; return ( diff --git a/ui/desktop/src/components/workflows/WorkflowsView.tsx b/ui/desktop/src/components/workflows/WorkflowsView.tsx index fdea8dc64..032bee99a 100644 --- a/ui/desktop/src/components/workflows/WorkflowsView.tsx +++ b/ui/desktop/src/components/workflows/WorkflowsView.tsx @@ -46,6 +46,7 @@ import { import { SearchView } from '../conversation/SearchView'; import cronstrue from 'cronstrue'; import { getInitialWorkingDir } from '../../utils/workingDir'; +import { startChatFailureNotice } from '../../utils/startChatFailure'; import { DropdownMenu, DropdownMenuContent, @@ -147,9 +148,11 @@ export default function WorkflowsView() { resumeSessionId: session.id, }); } catch (error) { - console.error('Failed to load workflow:', error); - const errorMsg = error instanceof Error ? error.message : 'Failed to load workflow'; - setError(errorMsg); + // A toast, not `setError`: that state is the LIST's load error, and + // setting it replaced a list that had loaded fine with "Couldn't load + // workflows", whose Try again reloads the list rather than the chat. + console.error('Failed to start workflow chat:', error); + toastError(startChatFailureNotice(error, { kept: false })); } }; diff --git a/ui/desktop/src/toasts.tsx b/ui/desktop/src/toasts.tsx index 2671b6340..de32255df 100644 --- a/ui/desktop/src/toasts.tsx +++ b/ui/desktop/src/toasts.tsx @@ -8,6 +8,7 @@ import { ExtensionLoadingStatus, } from './components/GroupedExtensionLoadingToast'; import { getInitialWorkingDir } from './utils/workingDir'; +import { startChatFailureNotice } from './utils/startChatFailure'; import { launchDependencyDebugSession } from './utils/launchDependencyDebug'; import type { DependencyFailure } from './utils/dependencyDebugPrompt'; @@ -340,7 +341,11 @@ function ToastErrorContent({ diff --git a/ui/desktop/src/utils/launcherMessage.ts b/ui/desktop/src/utils/launcherMessage.ts index de2924669..832e3f5b5 100644 --- a/ui/desktop/src/utils/launcherMessage.ts +++ b/ui/desktop/src/utils/launcherMessage.ts @@ -1,5 +1,7 @@ import type { NavigateFunction } from 'react-router-dom'; import { createSession } from '../sessions'; +import { toastError } from '../toasts'; +import { startChatFailureNotice } from './startChatFailure'; import { getInitialWorkingDir } from './workingDir'; import type { PairRouteState } from '../components/Pair'; @@ -31,7 +33,9 @@ import type { PairRouteState } from '../components/Pair'; * The message itself still travels as location.state, which is where the IN * effect reads cargo from once the param has opened the gate. On failure we * navigate nowhere: the marker keeps the window parked on the empty pane (the - * pre-#38 resting state) rather than silently discarding the launch intent. + * pre-#38 resting state) rather than silently discarding the launch intent — + * and the failure is said out loud, in `startChatFailureNotice`'s words, since + * a parked pane explains nothing by itself. */ export async function deliverLauncherMessage( navigate: NavigateFunction, @@ -46,5 +50,6 @@ export async function deliverLauncherMessage( }); } catch (error) { console.error('Failed to create session for launcher message:', error); + toastError(startChatFailureNotice(error, { kept: false })); } } diff --git a/ui/desktop/src/utils/startChatFailure.test.ts b/ui/desktop/src/utils/startChatFailure.test.ts new file mode 100644 index 000000000..2e4d56bff --- /dev/null +++ b/ui/desktop/src/utils/startChatFailure.test.ts @@ -0,0 +1,146 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative, sep } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { + BACKEND_DISCONNECTED_TITLE, + START_CHAT_FAILED_TITLE, + isStartRefusedForWantOfProof, + startChatFailureNotice, +} from './startChatFailure'; + +/** + * The body the 2026-09-10 QA run captured from `POST /agent/start` on a + * `biorouter serve` daemon with `versa_azure` configured (finding F1), verbatim. + * Under `throwOnError` the generated client throws the parsed body, so this + * object IS what a caller's `catch` receives. + */ +const QA_REFUSAL = { + message: + "Switching this chat to a private model is the user's decision, not yours. The request to " + + "switch it to 'versa_azure' did not come from the model picker, so the chat is unchanged and " + + 'still on its current model. Do not retry; the same call will be refused again. If this task ' + + 'genuinely needs a private model, stop and ask the user to switch this chat to a private ' + + 'model first, in the desktop app under Settings > Models, or with the model chip in the ' + + 'composer.', +}; + +describe('startChatFailureNotice', () => { + it('words the privacy refusal for a person and keeps the daemon text behind Copy error', () => { + const notice = startChatFailureNotice(QA_REFUSAL, { kept: true }); + expect(notice.title).toBe(START_CHAT_FAILED_TITLE); + // Every instruction in the refusal is addressed to a model, and the last + // one names a control the browser surface deliberately disables (SD-1). + for (const forAModel of [ + 'Do not retry', + 'not yours', + 'model picker', + 'stop and ask the user', + 'model chip', + ]) { + expect(notice.msg).not.toContain(forAModel); + } + expect(notice.msg).toContain('Your message was kept.'); + expect(notice.traceback).toBe(QA_REFUSAL.message); + }); + + it('only claims the message was kept when the caller put it back', () => { + expect(startChatFailureNotice(QA_REFUSAL, { kept: false }).msg).not.toContain('kept'); + }); + + it('shows a failure already written for a person as it came', () => { + // The A/B leg of the QA run: the same sandbox with a public provider and no key. + const body = { + message: + 'Failed to configure the selected provider for the new chat: Configuration value not ' + + 'found: OPENAI_API_KEY', + }; + expect(startChatFailureNotice(body, { kept: false })).toEqual({ + title: START_CHAT_FAILED_TITLE, + msg: body.message, + }); + expect(startChatFailureNotice(body, { kept: true }).msg).toBe( + `${body.message} Your message was kept.` + ); + }); + + it('says the backend is unreachable when the request never got an answer', () => { + const notice = startChatFailureNotice(new TypeError('Failed to fetch'), { kept: true }); + expect(notice.title).toBe(BACKEND_DISCONNECTED_TITLE); + expect(notice.msg).toContain('could not reach its backend'); + expect(notice.msg).toContain('Your message was kept'); + }); + + it('recognizes the refusal by its marker, in the shapes the daemon sends, and nothing else', () => { + expect(isStartRefusedForWantOfProof(QA_REFUSAL)).toBe(true); + expect(isStartRefusedForWantOfProof(QA_REFUSAL.message)).toBe(true); + // A thrown Error that happens to carry the words is a bug, not a policy. + expect(isStartRefusedForWantOfProof(new Error(QA_REFUSAL.message))).toBe(false); + expect(isStartRefusedForWantOfProof({ message: 'Failed to create session: disk full' })).toBe( + false + ); + expect(isStartRefusedForWantOfProof(null)).toBe(false); + expect(isStartRefusedForWantOfProof(undefined)).toBe(false); + }); +}); + +/** + * Every surface that starts a chat reports a failure through the notice. + * + * Seven surfaces started a chat when this was written, and before it six of + * them handled a failure four different ways: a console line (the Home composer + * the QA run hit, the launcher), an unhandled rejection (both "Ask Biorouter" + * buttons), a retry loop (a window opened for a workflow), and a list-load + * error over a list that had loaded (Workflows). An eighth surface must not get + * to pick a fifth. + */ +describe('every surface that starts a chat', () => { + const SRC = join(__dirname, '..'); + const STARTS_A_CHAT = /\b(?:createSession|startNewSession|startAgent)\(/; + // `sessions.ts` DEFINES the first two and wraps the third; it reports nothing + // because it has no one to report to. + const DEFINITIONS = new Set(['sessions.ts']); + + const sourceFiles = (dir: string): string[] => + readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) { + // The generated client is not a surface. + return name === 'api' || name === 'node_modules' ? [] : sourceFiles(path); + } + return /\.tsx?$/.test(name) && !/\.test\.tsx?$/.test(name) ? [path] : []; + }); + + // Prose about a call is not a call: `navigationUtils.ts` names + // `startNewSession()` in a comment and starts nothing. + const withoutComments = (source: string) => + source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|\s)\/\/.*$/gm, '$1'); + + const surfaces = sourceFiles(SRC) + .map((path) => ({ + rel: relative(SRC, path).split(sep).join('/'), + source: withoutComments(readFileSync(path, 'utf8')), + })) + .filter(({ rel, source }) => !DEFINITIONS.has(rel) && STARTS_A_CHAT.test(source)); + + it('finds the surfaces it is guarding, so the scan is not vacuous', () => { + const found = surfaces.map(({ rel }) => rel); + for (const known of [ + 'App.tsx', + 'toasts.tsx', + 'utils/launcherMessage.ts', + 'components/Hub.tsx', + 'components/BaseChat.tsx', + 'components/GroupedExtensionLoadingToast.tsx', + 'components/workflows/WorkflowsView.tsx', + ]) { + expect(found).toContain(known); + } + }); + + it.each(surfaces.map(({ rel, source }) => [rel, source] as const))( + '%s reports a failed start in the shared words', + (_rel, source) => { + expect(source).toMatch(/startChatFailureNotice\(|handleCreateSessionError\(/); + } + ); +}); diff --git a/ui/desktop/src/utils/startChatFailure.ts b/ui/desktop/src/utils/startChatFailure.ts new file mode 100644 index 000000000..2fdcb3aff --- /dev/null +++ b/ui/desktop/src/utils/startChatFailure.ts @@ -0,0 +1,92 @@ +import { errorMessage, isConnectionError } from './conversionUtils'; +import { USER_ACTION_REFUSAL_MARKER } from './userAction'; + +/** + * What a person is told when `POST /agent/start` fails — on every surface that + * starts a chat: the Home composer, a fresh tab's composer, a window opened for + * a workflow, a launcher message, the "Ask Biorouter" buttons. + * + * The 2026-09-10 QA run (finding F1) found the Home composer swallowing this + * failure whole: the typed text vanished, nothing appeared, and the only trace + * was a console line. The daemon's refusal had been correct, and was written + * for a model — "Do not retry", "ask the user to switch this chat" — so showing + * it verbatim would have been the second half of the same bug. Hence one pure + * mapping, shared by every caller, that picks words for a person and keeps the + * daemon's own text in the toast's copyable details. + * + * Pure (no toast, no DOM) so the words are tested without rendering anything, + * and so `toasts.tsx`, which itself starts chats, can use it without an import + * cycle. Each caller hands the result to `toastError`. + */ +export type StartChatFailureNotice = { + title: string; + msg: string; + /** The daemon's own words, behind "Copy error", when `msg` replaced them. */ + traceback?: string; +}; + +export const START_CHAT_FAILED_TITLE = 'Failed to start chat'; +export const BACKEND_DISCONNECTED_TITLE = 'Backend disconnected'; + +/** + * The daemon refused to bind its private default because the request carried + * no proof it came from a person, on a backend that holds a user-action key + * (SD-9). The `serve` daemon holds none and binds its default, so this reaches + * a person only on a desktop app pointed at a backend started elsewhere — the + * case `NO_USER_PROOF_TOAST_MSG` in `ModelAndProviderContext` words the same way. + * + * ⚠ The route answers with an `ErrorResponse`, so under `throwOnError` the + * thrown value is the parsed `{ message }` object — not the plain string + * `isUserActionRefusal` tests for, which is `/agent/update_provider`'s shape. + * Both are accepted, keyed on the marker. A real `Error` carrying the same words + * is not a policy refusal, whatever it reads. + */ +export const isStartRefusedForWantOfProof = (error: unknown): boolean => { + if (error instanceof Error) return false; + const text = + typeof error === 'string' + ? error + : typeof error === 'object' && + error !== null && + 'message' in error && + typeof error.message === 'string' + ? error.message + : null; + return text !== null && text.includes(USER_ACTION_REFUSAL_MARKER); +}; + +/** + * @param kept whether the caller has put the message back where the person can + * see it (the composer). Say so only when it is true. + */ +export function startChatFailureNotice( + error: unknown, + { kept }: { kept: boolean } +): StartChatFailureNotice { + // `errorMessage` answers its default, not the text, for a bare string. + const daemonText = typeof error === 'string' ? error : errorMessage(error); + if (isConnectionError(error)) { + return { + title: BACKEND_DISCONNECTED_TITLE, + msg: kept + ? 'Biorouter could not reach its backend. Your message was kept - try again in a moment.' + : 'Biorouter could not reach its backend. Try again in a moment.', + traceback: daemonText, + }; + } + const keptSentence = kept ? ' Your message was kept.' : ''; + if (isStartRefusedForWantOfProof(error)) { + return { + title: START_CHAT_FAILED_TITLE, + msg: + 'Biorouter is connected to a backend started outside the app, which could not confirm ' + + 'the request came from you, so it did not start a chat on its private model. To use a ' + + `private model, start the chat in the Biorouter app.${keptSentence}`, + traceback: daemonText, + }; + } + // Every other refusal on this route is already written for a person — + // "Failed to configure the selected provider for the new chat: …" — so it is + // shown as it came, rather than replaced by something vaguer. + return { title: START_CHAT_FAILED_TITLE, msg: `${daemonText}${keptSentence}` }; +} diff --git a/ui/desktop/src/utils/userAction.surface.test.ts b/ui/desktop/src/utils/userAction.surface.test.ts new file mode 100644 index 000000000..70851962e --- /dev/null +++ b/ui/desktop/src/utils/userAction.surface.test.ts @@ -0,0 +1,75 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockReadConfig } = vi.hoisted(() => ({ mockReadConfig: vi.fn() })); +vi.mock('../api', () => ({ readConfig: mockReadConfig })); + +import { + CALLER_PROVIDER_HEADER, + resetHostProviderForTests, + userActionHeaders, +} from './userAction'; +import { BROWSER_SURFACE_MARKER } from './surface'; + +/** + * SD-9: what each surface says on the requests the daemon's reach gate reads. + * + * The browser half is the one that was missing. On a `biorouter serve` daemon a + * chat started on the host's private model is private from its first reply, and + * the daemon reaches a private chat only for a caller whose stated capability + * covers it — so a tab that stated nothing lost its own chat after one answer. + */ +const onBrowser = () => { + document.documentElement.dataset.biorouterSurface = BROWSER_SURFACE_MARKER; +}; + +beforeEach(() => { + vi.clearAllMocks(); + resetHostProviderForTests(); + delete document.documentElement.dataset.biorouterSurface; + Object.assign(window, { + electron: { getUserActionKey: vi.fn(async () => 'desktop-user-action-key') }, + }); +}); + +afterEach(() => { + delete document.documentElement.dataset.biorouterSurface; +}); + +describe('userActionHeaders', () => { + it('proves the person on the desktop, and states no capability', async () => { + expect(await userActionHeaders()).toEqual({ 'X-User-Action': 'desktop-user-action-key' }); + expect(mockReadConfig).not.toHaveBeenCalled(); + }); + + it("states the host's configured model in a browser, and claims no proof", async () => { + onBrowser(); + mockReadConfig.mockResolvedValue({ data: 'versa_azure' }); + + expect(await userActionHeaders()).toEqual({ [CALLER_PROVIDER_HEADER]: 'versa_azure' }); + expect(mockReadConfig).toHaveBeenCalledWith({ + body: { key: 'BIOROUTER_PROVIDER', is_secret: false }, + }); + // Read once per page: the host's model cannot change under a running tab. + await userActionHeaders(); + expect(mockReadConfig).toHaveBeenCalledTimes(1); + }); + + it('says nothing when the host model cannot be read, and asks again next time', async () => { + onBrowser(); + mockReadConfig.mockRejectedValueOnce(new TypeError('Failed to fetch')); + expect(await userActionHeaders()).toEqual({}); + + mockReadConfig.mockResolvedValueOnce({ data: 'versa_azure' }); + expect(await userActionHeaders()).toEqual({ [CALLER_PROVIDER_HEADER]: 'versa_azure' }); + }); + + it('uses the header name the daemon reads', () => { + const gate = readFileSync( + join(__dirname, '../../../../crates/biorouter-server/src/routes/session_reach.rs'), + 'utf8' + ); + expect(gate).toContain(`pub const CALLER_PROVIDER_HEADER: &str = "${CALLER_PROVIDER_HEADER}";`); + }); +}); diff --git a/ui/desktop/src/utils/userAction.ts b/ui/desktop/src/utils/userAction.ts index dd3c5da4c..2e6cb6c47 100644 --- a/ui/desktop/src/utils/userAction.ts +++ b/ui/desktop/src/utils/userAction.ts @@ -1,3 +1,6 @@ +import { readConfig } from '../api'; +import { isBrowserSurface } from './surface'; + /** * Issue #56 DR-16: the header that proves a request came from the person at the * keyboard rather than from the model. @@ -96,7 +99,67 @@ export const COPY_OF_PRIVATE_REFUSAL_MARKER = 'only the person at the keyboard m export const isPrivateCopyRefusal = (error: unknown): boolean => typeof error === 'string' && error.includes(COPY_OF_PRIVATE_REFUSAL_MARKER); +/** + * Mirrored from `CALLER_PROVIDER_HEADER` in + * `crates/biorouter-server/src/routes/session_reach.rs`. It carries the NAME of + * the provider the caller runs under; the daemon resolves the tier itself. + */ +export const CALLER_PROVIDER_HEADER = 'X-Caller-Provider'; + +/** The host's `BIOROUTER_PROVIDER`, once it has been read successfully. */ +let hostProvider: string | undefined; + +/** + * The provider the machine running `biorouter serve` was configured with. + * + * Only a successful read is cached. A failure answers `null` and is asked again + * next time, so a transient error cannot pin the page to the public side for as + * long as it stays open. + */ +async function hostConfiguredProvider(): Promise { + if (hostProvider) return hostProvider; + try { + const { data } = await readConfig({ body: { key: 'BIOROUTER_PROVIDER', is_secret: false } }); + if (typeof data === 'string' && data.trim()) { + hostProvider = data.trim(); + return hostProvider; + } + } catch { + // Say nothing, which the daemon reads as the public side — fail-safe. + } + return null; +} + +/** For tests: forget the cached host provider. */ +export const resetHostProviderForTests = (): void => { + hostProvider = undefined; +}; + +/** + * The headers that answer the daemon's "may this caller reach this chat?" on + * the requests that need an answer — one surface at a time. + * + * * **The desktop app proves the person**: `X-User-Action`, the key the + * Electron main process minted and handed the daemon's digest on stdin. + * * **A browser states its model** (SD-9). The `biorouter serve` daemon holds no + * key (SD-7), so there is no person to prove, and a browser session runs the + * model the host was configured with (SD-1). It says so the way `biorouter + * session` does from a terminal: `X-Caller-Provider` naming that provider. + * Without it, a chat started on a private host model became unreachable from + * the tab the moment its first reply made it private — measured: the next + * request answered 403, the same request stating the host's provider 200. + * + * ⚠ **Not authentication, and not a widening.** Anything holding the daemon + * secret can send that header already (`session_reach.rs` says as much); what + * this adds is that the one legitimate browser client says what is true of it. + * On a host configured with a public model it states a public one, and private + * chats stay out of reach exactly as before. + */ export const userActionHeaders = async (): Promise> => { + if (isBrowserSurface()) { + const provider = await hostConfiguredProvider(); + return provider ? { [CALLER_PROVIDER_HEADER]: provider } : {}; + } try { return { 'X-User-Action': await window.electron.getUserActionKey() }; } catch { From a80ca7116f2fa5e86128e01c9e2b6d0085b0c7df Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:06:48 -0700 Subject: [PATCH 04/14] fix(desktop): every start-failure notice offers Copy error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notice set a traceback only when it replaced the daemon's words, so the commonest failure on a serve host — the configured model has no credential there — showed no Copy error. It is always set now. --- ui/desktop/src/utils/startChatFailure.test.ts | 2 ++ ui/desktop/src/utils/startChatFailure.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ui/desktop/src/utils/startChatFailure.test.ts b/ui/desktop/src/utils/startChatFailure.test.ts index 2e4d56bff..d9936bbce 100644 --- a/ui/desktop/src/utils/startChatFailure.test.ts +++ b/ui/desktop/src/utils/startChatFailure.test.ts @@ -57,6 +57,8 @@ describe('startChatFailureNotice', () => { expect(startChatFailureNotice(body, { kept: false })).toEqual({ title: START_CHAT_FAILED_TITLE, msg: body.message, + // Always copyable: the troubleshooting guide sends people to "Copy error". + traceback: body.message, }); expect(startChatFailureNotice(body, { kept: true }).msg).toBe( `${body.message} Your message was kept.` diff --git a/ui/desktop/src/utils/startChatFailure.ts b/ui/desktop/src/utils/startChatFailure.ts index 2fdcb3aff..93fb26761 100644 --- a/ui/desktop/src/utils/startChatFailure.ts +++ b/ui/desktop/src/utils/startChatFailure.ts @@ -21,8 +21,8 @@ import { USER_ACTION_REFUSAL_MARKER } from './userAction'; export type StartChatFailureNotice = { title: string; msg: string; - /** The daemon's own words, behind "Copy error", when `msg` replaced them. */ - traceback?: string; + /** The daemon's own words, behind the toast's "Copy error". */ + traceback: string; }; export const START_CHAT_FAILED_TITLE = 'Failed to start chat'; @@ -87,6 +87,11 @@ export function startChatFailureNotice( } // Every other refusal on this route is already written for a person — // "Failed to configure the selected provider for the new chat: …" — so it is - // shown as it came, rather than replaced by something vaguer. - return { title: START_CHAT_FAILED_TITLE, msg: `${daemonText}${keptSentence}` }; + // shown as it came, rather than replaced by something vaguer. It is still the + // traceback too, which is what puts "Copy error" on the toast. + return { + title: START_CHAT_FAILED_TITLE, + msg: `${daemonText}${keptSentence}`, + traceback: daemonText, + }; } From fe977583726b6e0893b0470a0851cbc5ce783f7e Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:06:49 -0700 Subject: [PATCH 05/14] =?UTF-8?q?docs(serve):=20SD-9=20=E2=80=94=20a=20new?= =?UTF-8?q?=20chat=20starts=20on=20the=20operator's=20model=20without=20a?= =?UTF-8?q?=20proof?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the ruling behind the new-chat fix: what it exempts (the configured default, at creation, on a daemon with no user-action key), what it does not (any other private model; any daemon that holds a key), the two halves that keep it from opening more, and the consequence to accept, with the privacy checklist's two questions answered as an enumeration. browser-access.md no longer promises a private-provider serve host works while it refused every chat, and says what a browser chat on one does. programmatic-session-access.md names the browser as a sender of X-Caller-Provider and the one proof-less bind. SD-1 named a route that does not exist (/config/provider); it is /config/set_provider. The two 409 descriptions in the OpenAPI spec now say which daemon refuses. --- CLAUDE.md | 16 +++ docs/deployment/browser-access.md | 28 ++++- .../deployment/programmatic-session-access.md | 15 ++- docs/deployment/serve-decisions.md | 101 +++++++++++++++++- ui/desktop/openapi.json | 4 +- ui/desktop/src/api/types.gen.ts | 4 +- 6 files changed, 156 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 17e8c1799..e689d88c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1177,6 +1177,22 @@ replaced a standalone `biorouter-headless` binary and its Linux tarball, both de **agent**, so every surface that writes a capability key asks `isBrowserSurface()` (`ui/desktop/src/utils/surface.ts`) and explains *before* the user can reach the 409. Do not "fix" browser mode by weakening the refusal. +- **A new chat starts on the configured model without a proof, and nothing else does** (SD-9). + Until it, a `serve` daemon with a private provider configured refused EVERY `/agent/start` + (the 2026-09-10 QA's F1): the new-chat bind asked for a proof a keyless daemon cannot check. + Three pieces, each load-bearing — measured by removing it: on a keyless daemon + `new_chat_bind_needs_user` (`routes/agent.rs`) lets the configured default bind, while a keyed + daemon still refuses a proof-less private first bind; `raise_baseline` makes a keyless + daemon's `/agent/update_provider` measure every move onto a private model from Public, or the + exemption would carry sideways to a private model nobody configured; and the browser states the + host's model as `X-Caller-Provider` (`userActionHeaders()` on `isBrowserSurface()`), without + which a chat's first reply ratcheted it private and its next request 403'd. Tests: + `cargo test -p biorouter-server --test new_chat_no_user_key` (its own binary: the digest is a + process-global `OnceLock`). ⚠ **Still unreachable in a browser, and out of SD-9's scope:** + `/agent/cancel` and `/interrupt` require the proof unconditionally, so Stop and mid-turn + steering cannot work on a keyless daemon. ⚠ `privacy_ar15_is_retired.rs`'s closure scan took + the FIRST `TierRaiseNeedsUser` in `routes/agent.rs`, which from `eb594ded` was the new-chat gate + and not AR-15's — it now starts at `update_agent_provider`. - **A control that can never work here says so, before it is touched** (SD-8). The same `Stdio::null()` that closes SD-1 means NO approval carrying `requires_user_proof` can ever be granted on a `serve` daemon — for anyone, always. So `confirm_tool_action` answers a diff --git a/docs/deployment/browser-access.md b/docs/deployment/browser-access.md index 299acf6b2..259b8c568 100644 --- a/docs/deployment/browser-access.md +++ b/docs/deployment/browser-access.md @@ -22,7 +22,9 @@ first and then [Headless Linux deployment](headless-linux.md). ## Quickstart Choose the provider and model **before** you start serving — a browser session cannot change them -(see [The model is fixed before you start](#the-model-is-fixed-before-you-start)): +(see [The model is fixed before you start](#the-model-is-fixed-before-you-start)). Either kind +works: a commercial model, or a private one your institution hosts or that runs on the machine, +which is the choice to make for patient data. ```bash biorouter configure @@ -165,6 +167,21 @@ provider is chosen once, at the terminal, and the tier that choice implies holds session in that daemon. A run started against an institutional model is private for its whole life; one started against a commercial model is public for its whole life. Neither can drift. +What that means for a chat you start in the browser: + +- **It starts on the configured model**, private or not, with nothing asked of you — choosing it + at the terminal was the decision. +- **A private model makes it private with its first reply.** The tab you are in keeps working with + it: the browser tells the daemon which model it runs under, and a chat is open to anything + running under a model at least as private as the chat. +- **Nothing in the browser can move it onto a different private model.** The configured one is the + only private model a chat started here ever reaches. +- **A private chat started in the desktop application opens here only when the host's model is + private too.** On a host configured with a commercial model it stays out of reach, with a card + saying so; open it in the desktop application instead. + +The reasoning is recorded as [decision SD-9](serve-decisions.md#sd-9--a-new-chat-starts-on-the-operators-model-without-a-proof-and-nothing-else-does). + **The fix is to choose the provider before you start serving:** ```bash @@ -184,7 +201,7 @@ differs: | Area | In a browser | |---|---| -| Chat, sessions, history, extensions, skills, knowledge bases, workflows | Work as they do in the desktop application. | +| Chat, sessions, history, extensions, skills, knowledge bases, workflows | Work as they do in the desktop application, with two differences that follow from the fixed model: a new chat starts on the host's configured model, and a private chat opens only on a host whose configured model is private. See [The model is fixed before you start](#the-model-is-fixed-before-you-start). | | Workspace control, several conversations at once, live app agents | Work — these are WebSocket-backed daemon routes, reached on the same origin. | | Model and provider selection | **Not available.** See [The model is fixed before you start](#the-model-is-fixed-before-you-start). | | File and folder pickers | No native dialog. You type a path, and it is a path **on the machine running the daemon**, not on the machine holding the browser. | @@ -210,6 +227,13 @@ you ran, and finds it either next to that CLI or next to the application the CLI recorded). If the application has moved or been reinstalled since, run `biorouter setup-path` again from inside it — `\resources\bin\biorouter.exe setup-path`. +**A message does not start a chat.** The composer keeps what you typed, and a notice in the corner +says why. The commonest cause is on the host rather than in the browser — for example *Failed to +configure the selected provider for the new chat: Configuration value not found: +OPENAI_API_KEY* means the model chosen with `biorouter configure` has no credential on the serving +machine. Fix it there, restart `serve`, and open the new address it prints. **Copy error** on the +notice copies the daemon's own words, for a bug report. + **The tab says the link needs its access token.** The `?t=` part was dropped — from a copy-paste, a chat client shortening the link, or a bookmark saved after the redirect. Use the full address as printed. If the launch has since restarted, the token has changed; read the new one from the diff --git a/docs/deployment/programmatic-session-access.md b/docs/deployment/programmatic-session-access.md index 7d976e883..3ce60bb08 100644 --- a/docs/deployment/programmatic-session-access.md +++ b/docs/deployment/programmatic-session-access.md @@ -53,7 +53,10 @@ case with its own logic — it is that rule, stated on a request. The header is read by [`crates/biorouter-server/src/routes/session_reach.rs`](../../crates/biorouter-server/src/routes/session_reach.rs); `biorouter session watch`, `send` and `attach` already send it, which is why those commands reach a -private chat from a terminal that can never prove a human is present. +private chat from a terminal that can never prove a human is present. So does the browser interface +`biorouter serve` serves, naming the model the host was configured with +([SD-9](serve-decisions.md#sd-9--a-new-chat-starts-on-the-operators-model-without-a-proof-and-nothing-else-does)): +a browser, like a terminal, can never carry the proof, and runs the model its host chose. ## What the header is *not* @@ -67,9 +70,13 @@ which got the answer backwards in both directions: a terminal running an institu refused, while the desktop app was admitted for the same chat while running a public one. **It is not a way to raise or lower a tier.** Reaching a chat and *reclassifying* one are separate -decisions. Raising a session's classification, declassifying it, and binding a private model all -still require proof that the person at the keyboard acted (`X-User-Action`), and no header changes -that. A capability is a fact about a model; neither of those is a decision a model may make. +decisions. Raising a session's classification, declassifying it, and binding a private model to a +chat all still require proof that the person at the keyboard acted (`X-User-Action`), and no header +changes that. A capability is a fact about a model; neither of those is a decision a model may make. +The one bind that needs no proof is not a header's doing either: on a daemon with no user-action +key, a **new** chat starts on the model the operator configured, private or not, because choosing +it with `biorouter configure` was the decision +([SD-9](serve-decisions.md#sd-9--a-new-chat-starts-on-the-operators-model-without-a-proof-and-nothing-else-does)). **It is not a per-request opt-out.** There is no header that turns the gate off. The only machine-wide switch is the privacy master switch, which lives in its own record beside diff --git a/docs/deployment/serve-decisions.md b/docs/deployment/serve-decisions.md index f413d2bef..92205c5b5 100644 --- a/docs/deployment/serve-decisions.md +++ b/docs/deployment/serve-decisions.md @@ -17,7 +17,8 @@ This page records the decisions that replaced that arrangement. They were taken several of them only make sense as a set: the reason a browser session cannot switch models (SD-1) is also the reason it needs no proof-of-user mechanism, which is the reason the daemon can be spawned with a closed stdin (SD-7) — and the reason every control that needs that proof -must say so before the user reaches for it (SD-8). Read [the architecture](serve-architecture.md) +must say so before the user reaches for it (SD-8), and the reason the one model the operator +chose must not need that proof at all (SD-9). Read [the architecture](serve-architecture.md) for how the result is built, and [browser access](browser-access.md) for how to use it. Records are identified `SD-n` — *serve decision*. The numbering is stable; a superseded record @@ -27,7 +28,7 @@ keeps its number and says what replaced it. ## SD-1 — A browser session cannot change its model or provider, and that is the point -**Ruling.** `POST /config/provider` continues to refuse a request that carries no proof a human +**Ruling.** `POST /config/set_provider` continues to refuse a request that carries no proof a human made it. Browser-served Biorouter installs no such proof. A browser session therefore runs whatever provider and model the machine was already configured with, and the model picker is inert. @@ -46,6 +47,12 @@ anyone opens a tab — and the tier that choice implies holds for every session A run started against an institutional Bedrock model is private for its whole life; one started against a commercial model is public for its whole life. Neither can drift. +> ⚠ **The first half of that sentence was unreachable until SD-9.** A `serve` daemon holds no +> user-action key, and the new-chat bind asked for that key's proof before binding a private +> model — so with an institutional model configured, no chat could be started at all, and the +> interface showed nothing (the 2026-09-10 QA run, finding F1). See +> [SD-9](#sd-9--a-new-chat-starts-on-the-operators-model-without-a-proof-and-nothing-else-does). + **Displaced alternatives.** - *Mint a digest scoped to a loopback bind.* Rejected: it makes the guarantee depend on the @@ -227,6 +234,96 @@ can never half-believe a person is reachable. --- +## SD-9 — A new chat starts on the operator's model without a proof, and nothing else does + +**Ruling.** On a daemon that holds no user-action key — the one `biorouter serve` starts (SD-7), +or a `biorouterd` started by hand — `POST /agent/start` binds the operator's configured provider +to the new chat without asking for proof of a person, whether that provider is public or private. +Three things hold beside it: + +- On that daemon, `POST /agent/update_provider` refuses every move onto a private model, whatever + the chat runs on now. The configured model is the only private model a chat there can reach. +- A daemon that holds a key — the desktop application's — is unchanged. Its renderer sends the + proof on every start, and a start that lacks it is refused as before. +- The browser interface states the host's configured model on the requests that reach into a chat + (`X-Caller-Provider`), the way `biorouter session` already does from a terminal, so a chat its + first reply made private stays reachable from the tab that started it. + +**Why.** The configured model is the person's decision, made out of band. `/agent/start` names no +provider: it binds `BIOROUTER_PROVIDER`, which only a proven person may write over HTTP, or which +the operator wrote at the terminal with `biorouter configure`. Open question 24 of the privacy plan +already put the raise at that write — *a raise of every future session* — and SD-1 already says the +tier that choice implies *holds for every session in that daemon*. A new chat taking that tier is +the choice being honored, not a switch. DR-16 governs raising a chat that exists, and a chat that +did not exist a moment ago has nothing to raise. + +On a daemon with no key, asking for the proof can only refuse everyone. The 2026-09-10 QA run +measured it: with an institutional model configured, every new chat on a `serve` daemon was +refused 409, in a sentence written for a model that pointed at a model picker SD-1 disables, and +the interface showed nothing at all. A control nobody can pass is not a boundary; it is the +product not working. + +The other two halves close what the exemption would otherwise open. Without the rule on +`/agent/update_provider`, a chat bound to the private default could be moved to a different private +model — `Private → Private`, which DR-16's raise predicate calls sideways and allows — that nobody +configured. Without the capability statement, the tab that started a chat lost it after one reply: +the reply makes the chat private (the classification ratchets on the turn, never on the bind), and a +keyless daemon reaches a private chat only for a caller whose stated capability covers it. Measured: +the chat's next request answered 403 with nothing stated, and 200 with the host's provider stated. + +**Why not on every daemon.** On a daemon that holds a key the proof costs the person nothing — the +renderer attaches it to every start — and it still refuses a caller that cannot present it. That +includes a model holding the daemon secret, which AR-11 found recoverable and which could otherwise +mint a private-capability chat through `/agent/start` with an extension set of its own choosing. +Relaxing the gate there buys the person nothing and gives that model something. + +**Who can do this, and what else reaches the same place.** The two questions every privacy +control answers in writing ([privacy tiers §3.1](../security/privacy-tiers.md)): + +- *Who can initiate it.* On a keyless daemon, anything holding the daemon secret: the person in + the browser and, indistinguishably, a model running in a chat on that daemon that has recovered + the secret. Both get the configured model and nothing else. +- *What else reaches a chat running on the configured private model:* + + | Door | Proof asked | Changed here | + |---|---|---| + | `workspace_open { new: … }` — binds the machine default through `restore_provider_from_session` | None, on every daemon (privacy tiers, "Did not ship") | No | + | `POST /agent/restart` on a row that names no provider — `restore_provider_from_session` falls back to the configured default | None | No | + | An app session's creation bind (DR-21) | None, deliberately | No | + | `POST /agent/update_provider` onto a private model | The proof; on a keyless daemon, refused outright | Yes | + | `POST /config/set_provider`, and `/config/upsert` or `/config/remove` on a capability key | The proof (SD-1, open question 24) | No — the configured model stays the operator's to choose | + +**Displaced alternatives.** + +- *Keep the refusal, and explain it in the interface.* Rejected. SD-8's explanation is for a + control that can never work; this one is the product's core. A `serve` deployment whose only + model is institutional would be a chat application that cannot chat. +- *Exempt every new chat, whatever provider it asks for.* Rejected. The operator's choice is what + makes the bind legitimate, so a provider the request picked would be a switch. `/agent/start` + names none today, and a field that ever let it name one must not inherit this exemption. +- *Exempt the configured model on every daemon.* Rejected; see *Why not on every daemon*. +- *Let a keyless daemon treat its configured model as the capability of any request that states + none.* Rejected. An absent header resolving to Public is the fail-safe the reach gate is built on, + and a default that raised it would speak for every caller rather than for the client that says + what it runs. + +**Consequence to accept.** On a keyless daemon whose configured model is private, a model running +in a chat on that daemon — a public-model chat resumed from the shared session store — that has +recovered the daemon secret can start a private-capability chat through `/agent/start` with +extensions it chose, and can reach private chats by stating the host's provider. It could already +do the first through `workspace_open { new }`, and the second by spelling a provider name (the +header is not authentication, as `session_reach.rs` records); and the filesystem read-deny that +would stop it carrying anything back out did not ship. It is recorded rather than closed: on a +daemon that cannot tell a person from a model, closing it means refusing the person. + +And one visible change: a browser tab on a host configured with a private model now opens private +chats started in the desktop application on the same machine, which it was refused before. That is +the reach rule — *the caller's capability must be at least the chat's classification* — admitting +it, exactly as it admits `biorouter session` configured with the same model. On a host configured +with a public model nothing changes, and private chats stay out of the browser's reach. + +--- + ## Related documentation - [Architecture of the serving path](serve-architecture.md) — how the decisions above are built. diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 1a66a7468..0b269a025 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -689,7 +689,7 @@ "description": "Unauthorized - invalid secret key" }, "409": { - "description": "The selected private provider requires user-action proof", + "description": "The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-9). A daemon with no user-action key binds its configured provider without one.", "content": { "application/json": { "schema": { @@ -881,7 +881,7 @@ "description": "The session is out of reach, or the target is a subagent and the request lacks user-action proof" }, "409": { - "description": "Refused by a privacy boundary (issue #56). Gate A: a public model cannot be bound to a private chat (body = PrivacyBarrierBody). DR-16: the bind raises this chat's capability to Private and the request carried no proof it came from the user (body = plain text)", + "description": "Refused by a privacy boundary (issue #56). Gate A: a public model cannot be bound to a private chat (body = PrivacyBarrierBody). DR-16: the bind raises this chat's capability to Private and the request carried no proof it came from the user; on a daemon with no user-action key, any bind to a private model (SD-9) (body = plain text)", "content": { "application/json": { "schema": { diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 68b657939..82535f4d0 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -4719,7 +4719,7 @@ export type StartAgentErrors = { */ 401: unknown; /** - * The selected private provider requires user-action proof + * The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-9). A daemon with no user-action key binds its configured provider without one. */ 409: ErrorResponse; /** @@ -4872,7 +4872,7 @@ export type UpdateAgentProviderErrors = { */ 403: unknown; /** - * Refused by a privacy boundary (issue #56). Gate A: a public model cannot be bound to a private chat (body = PrivacyBarrierBody). DR-16: the bind raises this chat's capability to Private and the request carried no proof it came from the user (body = plain text) + * Refused by a privacy boundary (issue #56). Gate A: a public model cannot be bound to a private chat (body = PrivacyBarrierBody). DR-16: the bind raises this chat's capability to Private and the request carried no proof it came from the user; on a daemon with no user-action key, any bind to a private model (SD-9) (body = plain text) */ 409: PrivacyBarrierBody; /** From ddaa856c8b3c2d1781faf4e0855f309ca537063a Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:11:00 -0700 Subject: [PATCH 06/14] fix(server): the keyless-daemon warning says what SD-9 still allows It said the daemon refuses every request that raises a session's privacy capability. A new chat binding the configured provider is no longer one of them, so an operator reading it would conclude a private model cannot work. --- crates/biorouter-server/src/commands/agent.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/biorouter-server/src/commands/agent.rs b/crates/biorouter-server/src/commands/agent.rs index e9e784051..b08ae34db 100644 --- a/crates/biorouter-server/src/commands/agent.rs +++ b/crates/biorouter-server/src/commands/agent.rs @@ -124,8 +124,9 @@ pub async fn run() -> Result<()> { let user_action_digest = read_user_action_digest().await; if user_action_digest.is_none() { tracing::warn!( - "no user-action key on stdin: this daemon will refuse every request that raises a \ - session's privacy capability, including one made by the person at the keyboard" + "no user-action key on stdin: this daemon will refuse every request that raises an \ + existing chat's privacy capability, including one made by the person at the \ + keyboard; a new chat still starts on the configured provider (SD-9)" ); } // A tool whose approval can never be granted must not be offered. `serve` From 3b1336500ae0cf91462775396b4867bd912cdae1 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:28:22 -0700 Subject: [PATCH 07/14] style(desktop): prettier on the surface test --- ui/desktop/src/utils/userAction.surface.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ui/desktop/src/utils/userAction.surface.test.ts b/ui/desktop/src/utils/userAction.surface.test.ts index 70851962e..7254a5d84 100644 --- a/ui/desktop/src/utils/userAction.surface.test.ts +++ b/ui/desktop/src/utils/userAction.surface.test.ts @@ -5,11 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const { mockReadConfig } = vi.hoisted(() => ({ mockReadConfig: vi.fn() })); vi.mock('../api', () => ({ readConfig: mockReadConfig })); -import { - CALLER_PROVIDER_HEADER, - resetHostProviderForTests, - userActionHeaders, -} from './userAction'; +import { CALLER_PROVIDER_HEADER, resetHostProviderForTests, userActionHeaders } from './userAction'; import { BROWSER_SURFACE_MARKER } from './surface'; /** From 7ea48e73ac58ef822738d9edae94f9ae6fc17a79 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:29:47 -0700 Subject: [PATCH 08/14] style(server): rustfmt the SD-9 tests --- crates/biorouter-server/src/routes/agent.rs | 23 ++++++++++--- .../tests/new_chat_no_user_key.rs | 34 ++++++++++++++----- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 629edf7e9..36ace3292 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -3188,8 +3188,16 @@ mod new_session_provider_binding_tests { #[test] fn only_a_daemon_that_can_check_a_proof_asks_a_new_chat_for_one() { use UserActionProof::{NoKeyInstalled, Proven, Unproven}; - assert!(!new_chat_bind_needs_user(true, ProviderTier::Private, Proven)); - assert!(new_chat_bind_needs_user(true, ProviderTier::Private, Unproven)); + assert!(!new_chat_bind_needs_user( + true, + ProviderTier::Private, + Proven + )); + assert!(new_chat_bind_needs_user( + true, + ProviderTier::Private, + Unproven + )); assert!( !new_chat_bind_needs_user(true, ProviderTier::Private, NoKeyInstalled), "a keyless daemon refusing its own configured default refuses every person, always" @@ -3198,7 +3206,11 @@ mod new_session_provider_binding_tests { // A public default raises nothing, for anyone. assert!(!new_chat_bind_needs_user(true, ProviderTier::Public, proof)); // DR-15's master opt-out turns the gate off, not the question. - assert!(!new_chat_bind_needs_user(false, ProviderTier::Private, proof)); + assert!(!new_chat_bind_needs_user( + false, + ProviderTier::Private, + proof + )); } } @@ -3209,7 +3221,10 @@ mod new_session_provider_binding_tests { fn a_keyless_daemon_has_no_private_floor_for_a_switch_to_build_on() { use UserActionProof::{NoKeyInstalled, Proven, Unproven}; for current in [ProviderTier::Private, ProviderTier::Public] { - assert_eq!(raise_baseline(current, NoKeyInstalled), ProviderTier::Public); + assert_eq!( + raise_baseline(current, NoKeyInstalled), + ProviderTier::Public + ); // A daemon that can check a proof keeps measuring from the live binding. assert_eq!(raise_baseline(current, Proven), current); assert_eq!(raise_baseline(current, Unproven), current); diff --git a/crates/biorouter-server/tests/new_chat_no_user_key.rs b/crates/biorouter-server/tests/new_chat_no_user_key.rs index 4eb7f8264..ae168e9ea 100644 --- a/crates/biorouter-server/tests/new_chat_no_user_key.rs +++ b/crates/biorouter-server/tests/new_chat_no_user_key.rs @@ -135,7 +135,11 @@ async fn a_keyless_daemon_starts_a_new_chat_on_its_configured_private_model() { .expect("the started session carries an id") .to_string(); - let row = state.session_manager().get_session(&id, false).await.unwrap(); + let row = state + .session_manager() + .get_session(&id, false) + .await + .unwrap(); assert_eq!(row.provider_name.as_deref(), Some("versa_azure")); assert_eq!( row.model_config.map(|config| config.model_name).as_deref(), @@ -188,10 +192,7 @@ async fn a_keyless_daemon_will_not_move_a_new_chat_to_a_private_model_nobody_con // A loopback Ollama is Private (`self_hosted_tier`), and constructing one // opens no connection, so port 1 is never dialled. let (status, body) = with_config_overrides( - HashMap::from([( - "OLLAMA_HOST".to_string(), - "http://127.0.0.1:1".to_string(), - )]), + HashMap::from([("OLLAMA_HOST".to_string(), "http://127.0.0.1:1".to_string())]), post_json( biorouter_server::routes::agent::routes(Arc::clone(&state)), "/agent/update_provider", @@ -210,7 +211,11 @@ async fn a_keyless_daemon_will_not_move_a_new_chat_to_a_private_model_nobody_con "refused, but not by the tier gate: {body}" ); - let row = state.session_manager().get_session(&id, false).await.unwrap(); + let row = state + .session_manager() + .get_session(&id, false) + .await + .unwrap(); assert_eq!( row.provider_name.as_deref(), Some("versa_azure"), @@ -260,7 +265,10 @@ async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { }; let sse = format!( "data: {}\n\ndata: {}\n\ndata: [DONE]\n\n", - chunk(json!({ "role": "assistant", "content": "ready" }), Value::Null), + chunk( + json!({ "role": "assistant", "content": "ready" }), + Value::Null + ), chunk(json!({ "content": "" }), json!("stop")), ); Mock::given(method("POST")) @@ -308,7 +316,11 @@ async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { .as_str() .unwrap() .to_string(); - let before = state.session_manager().get_session(&id, false).await.unwrap(); + let before = state + .session_manager() + .get_session(&id, false) + .await + .unwrap(); assert_eq!(before.provider_name.as_deref(), Some("ollama")); assert_eq!(before.privacy_tier, SessionClassification::Public); @@ -329,7 +341,11 @@ async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { "the turn did not run on the stub: {stream}" ); - let after = state.session_manager().get_session(&id, false).await.unwrap(); + let after = state + .session_manager() + .get_session(&id, false) + .await + .unwrap(); assert_eq!( after.privacy_tier, SessionClassification::Private, From e3896e5d3e8cb1d5aef1981aa69822f1e4a31065 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 02:31:24 -0700 Subject: [PATCH 09/14] docs(desktop): say what the capability header changes for a browser tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It claimed 'not a widening'. It adds no reach for a caller holding the daemon secret, but a tab on a private-model host now opens private chats, including desktop-started ones — which SD-9 records. --- ui/desktop/src/utils/userAction.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ui/desktop/src/utils/userAction.ts b/ui/desktop/src/utils/userAction.ts index 2e6cb6c47..90c21db65 100644 --- a/ui/desktop/src/utils/userAction.ts +++ b/ui/desktop/src/utils/userAction.ts @@ -149,11 +149,13 @@ export const resetHostProviderForTests = (): void => { * the tab the moment its first reply made it private — measured: the next * request answered 403, the same request stating the host's provider 200. * - * ⚠ **Not authentication, and not a widening.** Anything holding the daemon - * secret can send that header already (`session_reach.rs` says as much); what - * this adds is that the one legitimate browser client says what is true of it. - * On a host configured with a public model it states a public one, and private - * chats stay out of reach exactly as before. + * ⚠ **Not authentication, and no new reach for anything holding the secret** — + * that caller could always send the header (`session_reach.rs` says as much). + * What changes is the browser tab's own reach: on a host configured with a + * private model it now opens private chats, including ones started in the + * desktop app, which SD-9 records as a consequence. On a host configured with a + * public model it states a public one, and private chats stay out of reach + * exactly as before. */ export const userActionHeaders = async (): Promise> => { if (isBrowserSurface()) { From 88c26d51622ce306370bcc9c6a6cf7f6cf47c3e7 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 13:59:40 -0700 Subject: [PATCH 10/14] test(desktop): stub the privacy-off note in the Hub start-failure test main's H3 note (PrivacyTiersOffNote) now renders in Hub and needs a router and two ConfigContext hooks this test's mocks do not provide; it has nothing to do with starting a chat. --- ui/desktop/src/components/Hub.startFailure.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/desktop/src/components/Hub.startFailure.test.tsx b/ui/desktop/src/components/Hub.startFailure.test.tsx index 3f9715245..21e2649b0 100644 --- a/ui/desktop/src/components/Hub.startFailure.test.tsx +++ b/ui/desktop/src/components/Hub.startFailure.test.tsx @@ -25,6 +25,9 @@ vi.mock('../toasts', async (importOriginal) => ({ toastError: mockToastError, })); vi.mock('./sessions/SessionsInsights', () => ({ SessionInsights: () => null })); +// The privacy-off note (H3) needs a router and two more ConfigContext hooks, and +// says nothing about starting a chat. +vi.mock('./privacy/PrivacyTiersOffNote', () => ({ PrivacyTiersOffNote: () => null })); vi.mock('./ConfigContext', () => ({ useConfig: () => ({ extensionsList: [], From 68cf246e0166b395b87cc34d5a1f05b3ffed02f4 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 20:40:28 -0700 Subject: [PATCH 11/14] fix(server): SD-12's keyless exemption is pinned to the launch configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1 (HIGH) of the 2026-09-12 review refuted SD-12's central claim, and this rewrites the justification as well as the code. SD-12 rested on: "/agent/start binds BIOROUTER_PROVIDER, a key only a proven person may write." The HTTP doors to that key really are shut. But config.yaml is not an HTTP resource: DR-14's filesystem read/write deny is recorded DEFERRED in privacy-tiers.md's own ledger, the agent holds developer__shell, and Config's value cache is keyed on a FileStamp it re-stats on every read — so the file is reloaded live and the bind read whatever it said at request time. A model with a shell on a keyless daemon whose operator configured a PUBLIC default could write a private provider into that file and POST /agent/start for a 200 and Private capability, where main answers 409. Measured, before this change: test a_private_provider_written_after_launch_does_not_start_a_new_chat ... FAILED left: 200, right: 409 body: "provider_name":"versa_azure", "endpoint":"https://unified-api.ucsf.edu/general" test a_private_endpoint_written_after_launch_does_not_start_a_new_chat ... FAILED left: 200, right: 409 body: "provider_name":"ollama" The second is why pinning the provider's NAME would not have been enough: self_hosted_tier reads ollama as Private exactly while its host is loopback, so OLLAMA_HOST moves the tier with BIOROUTER_PROVIDER untouched. So the exemption is pinned rather than re-derived per request. biorouter_server ::launch samples the capability-deciding configuration once, in commands::agent ::run, before AppState::new() and before any route is mounted; the exemption applies only while the live values still match, and drift refuses with a 409 that names the key and asks for a restart. The bind still reads the file, so an operator who edits it and restarts is served — which is why the exemption is pinned and not the binding. This makes SD-1's own sentence true of the door rather than merely asserted near it: "the tier implied by the operator's `biorouter configure` choice then holds for every session in that daemon." The pinned set is privacy::CAPABILITY_CONFIG_KEYS verbatim — the list /config/upsert and /config/remove already guard, kept honest by config_keys.rs's scan of the tier-input files — plus BIOROUTER_MODEL, because /agent/start binds both halves of the operator's declaration. A second hand-written list would be a third answer to a question that already has two agreeing ones. Two things this deliberately does NOT claim. It is not DR-14: a model that can write config.yaml can still read the session store and the knowledge bases as ordinary files and can start a second biorouterd of its own. What it protects is the door's stated guarantee, and the part with real teeth on a machine where DR-14 is deferred — the Gate C / Gate E roster of private connectors whose credentials live in the OS credential store rather than on disk. Both are now written into SD-12 instead of left to be inferred. Also here, because it is the same launch record read the other way (Finding 3's server half; the launcher half that makes it fire follows in the next commit): NoKeyInstalled means only "this process read no valid digest", which a desktop spawn satisfies when userActionKey is undefined or the bounded 2s stdin read times out. A launcher that means to send a key declares so, and a daemon that holds the declaration but no key keeps main's refusal plus a startup ERROR naming the consequence, rather than silently taking an exemption meant for deployments where no proof can exist. new_chat_bind_needs_user becomes new_chat_bind_decision, returning why rather than whether, because two of the three refusals are actionable by a person and one boolean answered all of them in a sentence written for a model. ⚠ This branch's own SD-12 test binary did not compile: main renamed VERSA_AZURE_DEPLOYMENT to VERSA_AZURE_DEFAULT_MODEL and the merge at f276111f left the import behind, so none of its five tests had run since. Repaired here because nothing could be measured until it did. All four uses were model names. Tests: 8 passed in tests/new_chat_no_user_key.rs (5 existing, 3 new); 608 in biorouter-server --lib; every biorouter-server integration binary; privacy_ capability / privacy_guard_wiring / privacy_toggle; clippy -D warnings and cargo fmt --check clean. Generated API contract regenerated for the /agent/start 409 description. --- CLAUDE.md | 27 +- crates/biorouter-server/src/commands/agent.rs | 33 +- crates/biorouter-server/src/launch.rs | 292 ++++++++++++++++++ crates/biorouter-server/src/lib.rs | 3 + crates/biorouter-server/src/routes/agent.rs | 265 +++++++++++++--- .../tests/new_chat_no_user_key.rs | 189 +++++++++++- crates/biorouter/src/privacy/mod.rs | 2 +- docs/deployment/serve-decisions.md | 104 ++++++- ui/desktop/openapi.json | 2 +- ui/desktop/src/api/types.gen.ts | 2 +- 10 files changed, 851 insertions(+), 68 deletions(-) create mode 100644 crates/biorouter-server/src/launch.rs diff --git a/CLAUDE.md b/CLAUDE.md index 2bad215d9..e0719c3aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1203,15 +1203,32 @@ replaced a standalone `biorouter-headless` binary and its Linux tarball, both de - **A new chat starts on the configured model without a proof, and nothing else does** (SD-12). Until it, a `serve` daemon with a private provider configured refused EVERY `/agent/start` (the 2026-09-10 QA's F1): the new-chat bind asked for a proof a keyless daemon cannot check. - Three pieces, each load-bearing — measured by removing it: on a keyless daemon - `new_chat_bind_needs_user` (`routes/agent.rs`) lets the configured default bind, while a keyed + Four pieces, each load-bearing — measured by removing it: on a keyless daemon + `new_chat_bind_decision` (`routes/agent.rs`) lets the configured default bind, while a keyed daemon still refuses a proof-less private first bind; `raise_baseline` makes a keyless daemon's `/agent/update_provider` measure every move onto a private model from Public, or the - exemption would carry sideways to a private model nobody configured; and the browser states the + exemption would carry sideways to a private model nobody configured; the browser states the host's model as `X-Caller-Provider` (`userActionHeaders()` on `isBrowserSurface()`), without - which a chat's first reply ratcheted it private and its next request 403'd. Tests: + which a chat's first reply ratcheted it private and its next request 403'd; and + `biorouter_server::launch` **pins the exemption to the configuration the daemon was launched + with**. Tests: `cargo test -p biorouter-server --test new_chat_no_user_key` (its own binary: the digest is a - process-global `OnceLock`). ⚠ **Still unreachable in a browser, and out of SD-12's scope:** + process-global `OnceLock`). + ⚠ **The exemption's first justification was FALSE and the fix is that pin.** It rested on + *"`/agent/start` binds `BIOROUTER_PROVIDER`, a key only a proven person may write"*. The HTTP + doors are shut, but `config.yaml` is not an HTTP resource: DR-14's filesystem deny is DEFERRED, + the agent holds `developer__shell`, and `Config`'s cache is keyed on a `FileStamp` it re-`stat`s + per read — so a model with a shell on a keyless daemon configured PUBLIC could write a private + provider into the file and get a 200 with Private capability where `main` answers 409 (measured + 2026-09-12). The pinned set is `privacy::CAPABILITY_CONFIG_KEYS` verbatim plus `BIOROUTER_MODEL`; + pinning the provider NAME alone is not enough, because flipping `OLLAMA_HOST` to loopback moves + `ollama`'s tier with the name untouched. ⚠ And `NoKeyInstalled` is **not** the same thing as + "this is `serve`" — a desktop spawn satisfies it when `userActionKey` is undefined or the + daemon's bounded 2s stdin read times out. That case is a repairable fault, so the desktop + launcher declares its intent in `BIOROUTER_USER_ACTION_EXPECTED` and such a daemon keeps + `main`'s refusal plus a startup `ERROR`. ⚠ `BIOROUTER_MODEL` is in neither capability-key list + by decision, not oversight: no `tier()` implementation reads the model name (all five checked), + so it is an integrity key and its row lives in `NOT_CAPABILITY_CONFIG_KEYS`. ⚠ **Still unreachable in a browser, and out of SD-12's scope:** `/agent/cancel` and `/interrupt` require the proof unconditionally, so Stop and mid-turn steering cannot work on a keyless daemon. ⚠ `privacy_ar15_is_retired.rs`'s closure scan took the FIRST `TierRaiseNeedsUser` in `routes/agent.rs`, which from `eb594ded` was the new-chat gate diff --git a/crates/biorouter-server/src/commands/agent.rs b/crates/biorouter-server/src/commands/agent.rs index 69f57565d..c7bc07573 100644 --- a/crates/biorouter-server/src/commands/agent.rs +++ b/crates/biorouter-server/src/commands/agent.rs @@ -210,13 +210,38 @@ pub async fn run(exit_with_parent: Option) -> Result<()> { // by `sysctl(KERN_PROCARGS2)`, which is not a path at all and which no // sandbox profile can gate. let user_action_digest = read_user_action_digest().await; - if user_action_digest.is_none() { - tracing::warn!( + // SD-12, Finding 3. `UserActionProof::NoKeyInstalled` means "this process read + // no valid digest", which is TWO situations wearing one name: a deployment + // where no proof can ever exist, and a launcher that meant to send one and + // did not. The launcher says which (`launch::USER_ACTION_EXPECTED_ENV`), and + // the difference decides both what is logged and — below, in + // `routes::agent::new_chat_bind_decision` — whether SD-12's exemption applies + // at all. A desktop daemon that lost its key is a fault to repair, not a + // headless deployment, so it keeps `main`'s refusal. + let launcher_declared_a_key = + biorouter_server::launch::launcher_declared_a_user_action_key_in_env(); + match (user_action_digest.is_none(), launcher_declared_a_key) { + (true, true) => tracing::error!( + "no user-action key on stdin, but this daemon's launcher declared it would send one \ + ({}=set): every request that needs proof of a person will be refused, INCLUDING one \ + made by the person at the keyboard, and a new chat on a private model will be \ + refused rather than started (SD-12). The key was either never generated or the \ + daemon's bounded 2s stdin read timed out. Quit and reopen Biorouter.", + biorouter_server::launch::USER_ACTION_EXPECTED_ENV + ), + (true, false) => tracing::warn!( "no user-action key on stdin: this daemon will refuse every request that raises an \ existing chat's privacy capability, including one made by the person at the \ - keyboard; a new chat still starts on the configured provider (SD-12)" - ); + keyboard; a new chat still starts on the provider this daemon was LAUNCHED with, \ + and is refused if that configuration has changed since (SD-12)" + ), + (false, _) => {} } + // SD-12: pin the operator's declaration. Before `AppState::new()` and before + // any route is mounted, so no request is ever served against an unrecorded + // launch state, and so the sample predates anything this process could write + // to `config.yaml` itself. + biorouter_server::launch::record_launch_state(launcher_declared_a_key); // A tool whose approval can never be granted must not be offered. `serve` // spawns this daemon with `Stdio::null()`, so it holds no key and every // proof-backed approval refuses forever — the install and delete tools take diff --git a/crates/biorouter-server/src/launch.rs b/crates/biorouter-server/src/launch.rs new file mode 100644 index 000000000..d90b5415c --- /dev/null +++ b/crates/biorouter-server/src/launch.rs @@ -0,0 +1,292 @@ +//! What this daemon was **launched** with — the operator's own declaration, +//! sampled once before any route is mounted and never re-read. +//! +//! SD-12 (`docs/deployment/serve-decisions.md`) lets a daemon that holds no +//! user-action key bind a **private** provider to a brand-new chat with no proof +//! of a person, because nobody on such a daemon can produce one. The +//! justification is that the provider being bound is the *operator's* choice, +//! made at a terminal with `biorouter configure`. +//! +//! ⚠ **That sentence is only true of a value the operator wrote, and the file it +//! lives in is agent-writable.** DR-14's general filesystem deny is DEFERRED +//! (`docs/security/privacy-tiers.md`, *"Did not ship"*), the agent holds +//! `developer__shell`, and `Config`'s value cache is keyed on a `FileStamp` it +//! re-`stat`s on every read — so `config.yaml` is reloaded live and +//! `configured_new_session_provider()` reads whatever the file says **at request +//! time**. Without this module, a model on a keyless daemon whose operator +//! configured a *public* default could write a private provider into that file +//! and then `POST /agent/start` to mint a Private-capability chat with an +//! extension set of its own choosing. +//! +//! So the exemption is **pinned** to this snapshot. The bind still reads the +//! configuration — an operator who edits the file and restarts the daemon is +//! served — but SD-12's keyless exemption applies only while the +//! capability-deciding configuration still matches what the daemon started with. +//! That is SD-1's own sentence made true of the door: *"the tier implied by the +//! operator's `biorouter configure` choice then holds for every session in that +//! daemon."* +//! +//! It also records whether the **launcher declared** it would hand over a +//! user-action key, which is what separates two states `UserActionProof` +//! deliberately collapses into one: a deployment where no proof can ever exist, +//! and a desktop daemon whose key did not arrive. +//! +//! ⚠ **This is not DR-14, and must not be read as a substitute for it.** A shell +//! is still a shell: a model that can write `config.yaml` can also read the +//! session store and the knowledge bases directly, and can start a *second* +//! `biorouterd` of its own. What this module closes is one door's *stated* +//! guarantee, so that the record does not rest on a claim the tree contradicts. + +use std::sync::{PoisonError, RwLock}; + +/// A launcher that hands this daemon a user-action digest on stdin declares +/// itself here. +/// +/// It exists so the daemon can tell *"no key was ever meant to arrive"* +/// (`biorouter serve`, which spawns with `Stdio::null()`, or a hand-run +/// `biorouterd agent`) from *"one was, and did not"* — a fault to repair rather +/// than a deployment shape. Set unconditionally on the spawn path in +/// `ui/desktop/src/biorouterd.ts`, **including** when that path finds no key to +/// send, because that case is precisely the one worth naming. +/// +/// In the environment rather than on stdin on purpose: it is not a credential and +/// nothing is authenticated by it. A value that can only make this daemon +/// *stricter* is safe to read from a place the model can see but not write. +pub const USER_ACTION_EXPECTED_ENV: &str = "BIOROUTER_USER_ACTION_EXPECTED"; + +/// Reported by [`capability_config_moved_since_launch`] when this process never +/// recorded a launch state at all. Not a config key — a sentinel, so the caller +/// fails closed instead of reading "nothing moved". +pub const NO_LAUNCH_STATE_RECORDED: &str = ""; + +#[derive(Debug)] +struct LaunchState { + /// `(key, value at launch)` for every key in [`pinned_config_keys`]. + capability_config: Vec<(&'static str, Option)>, + /// Did whoever started this daemon say it would send a user-action key? + launcher_declared_a_user_action_key: bool, +} + +static LAUNCH: RwLock> = RwLock::new(None); + +/// The configuration keys whose value at request time must still match the value +/// this daemon started with, for SD-12's keyless exemption to apply. +/// +/// [`biorouter::privacy::CAPABILITY_CONFIG_KEYS`] **verbatim** — the same list +/// `/config/upsert` and `/config/remove` already use to decide *"is this write a +/// tier raise?"* — plus `BIOROUTER_MODEL`. +/// +/// Reusing that list rather than writing a second one is the whole point: +/// `privacy::config_keys`'s scan of the tier-input files is what keeps it honest, +/// so a key that starts deciding capability is pinned here without anyone +/// remembering to, and one that stops deciding it leaves. A hand-written second +/// list would be a third answer to a question that already has two agreeing ones. +/// +/// ⚠ **The provider name alone is not enough.** Flipping `OLLAMA_HOST` to +/// loopback moves `ollama` from Public to Private with `BIOROUTER_PROVIDER` +/// untouched (`self_hosted_tier`) — the same escalation through another key. +/// +/// `BIOROUTER_MODEL` is **not** a capability key — no `tier()` implementation +/// reads the model name — and it is pinned here for a different reason: the +/// exemption is for the operator's own declaration, and `/agent/start` binds +/// *both* halves of it (`configured_new_session_provider`). Its classification +/// lives in `privacy::config_keys::NOT_CAPABILITY_CONFIG_KEYS`. +pub fn pinned_config_keys() -> impl Iterator { + biorouter::privacy::CAPABILITY_CONFIG_KEYS + .iter() + .copied() + .chain(std::iter::once("BIOROUTER_MODEL")) +} + +fn sample_capability_config() -> Vec<(&'static str, Option)> { + let config = biorouter::config::Config::global(); + pinned_config_keys() + .map(|key| (key, config.get_param::(key).ok())) + .collect() +} + +/// Record what this daemon was launched with. +/// +/// Called ONCE from `commands::agent::run`, after the stdin digest read and +/// before `AppState::new()` — so no request can be served against an unrecorded +/// launch state, and so the sample is taken before anything in this process could +/// have written `config.yaml` itself. +/// +/// It overwrites rather than being write-once, because the integration tests that +/// exercise a keyless daemon have to stand up more than one launch posture in a +/// single binary (the config overrides are a `tokio` task-local scoped to a +/// future, so the sample must be taken inside one). Production has exactly one +/// call site. +pub fn record_launch_state(launcher_declared_a_user_action_key: bool) { + let recorded = LaunchState { + capability_config: sample_capability_config(), + launcher_declared_a_user_action_key, + }; + *LAUNCH.write().unwrap_or_else(PoisonError::into_inner) = Some(recorded); +} + +/// The first pinned key whose value no longer matches what this daemon started +/// with, or `None` while the operator's declaration is unchanged. +/// +/// ⚠ **Fails closed.** A process that never recorded a launch state reports +/// [`NO_LAUNCH_STATE_RECORDED`] rather than `None`: the one caller uses this to +/// decide whether to *skip* a privacy proof, and a missing snapshot must not read +/// as a clean one. +pub fn capability_config_moved_since_launch() -> Option { + let guard = LAUNCH.read().unwrap_or_else(PoisonError::into_inner); + let Some(state) = guard.as_ref() else { + return Some(NO_LAUNCH_STATE_RECORDED.to_string()); + }; + let config = biorouter::config::Config::global(); + state + .capability_config + .iter() + .find(|(key, at_launch)| config.get_param::(key).ok() != *at_launch) + .map(|(key, _)| (*key).to_string()) +} + +/// Did whoever started this daemon declare it would hand over a user-action key? +/// +/// Read from the recorded launch state, never from the environment at request +/// time, for the reason the whole module exists: the answer is a property of the +/// launch, not of the moment. +/// +/// `false` when nothing was recorded, which is not a fail-open reading — the +/// composite gate is closed by [`capability_config_moved_since_launch`], which +/// reports drift in exactly that situation. +pub fn expected_a_user_action_key() -> bool { + LAUNCH + .read() + .unwrap_or_else(PoisonError::into_inner) + .as_ref() + .is_some_and(|state| state.launcher_declared_a_user_action_key) +} + +/// Did the launcher set [`USER_ACTION_EXPECTED_ENV`]? +/// +/// Read exactly once, by `commands::agent::run`, and then frozen into the launch +/// state. Anything other than unset, empty, `0` or `false` counts as a +/// declaration — a launcher that says anything at all here is claiming it sends a +/// key, and the stricter reading is the safe one. +pub fn launcher_declared_a_user_action_key_in_env() -> bool { + match std::env::var(USER_ACTION_EXPECTED_ENV) { + Ok(value) => { + let value = value.trim(); + !(value.is_empty() || value == "0" || value.eq_ignore_ascii_case("false")) + } + Err(_) => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The pinned set is the capability list plus the model, and it is *derived* + /// rather than transcribed — so this asserts the derivation, not a copy. + #[test] + fn the_pinned_set_is_the_capability_list_plus_the_model() { + let pinned: Vec<&str> = pinned_config_keys().collect(); + for key in biorouter::privacy::CAPABILITY_CONFIG_KEYS { + assert!( + pinned.contains(key), + "{key} decides capability but is not pinned to the launch configuration" + ); + } + assert!(pinned.contains(&"BIOROUTER_MODEL")); + assert_eq!( + pinned.len(), + biorouter::privacy::CAPABILITY_CONFIG_KEYS.len() + 1, + "the pinned set grew a key of its own; it must stay a derivation: {pinned:?}" + ); + } + + /// The reading that matters most: an unrecorded launch state is drift, never + /// agreement. + /// + /// Stated against the function rather than against the static, because this + /// binary's tests share one process and another may have recorded a state + /// already. + #[test] + fn an_unrecorded_launch_state_reads_as_drift() { + let unrecorded = LAUNCH + .read() + .unwrap_or_else(PoisonError::into_inner) + .is_none(); + if unrecorded { + assert_eq!( + capability_config_moved_since_launch().as_deref(), + Some(NO_LAUNCH_STATE_RECORDED) + ); + } + assert!( + !expected_a_user_action_key() || !unrecorded, + "an unrecorded launch state must not claim a key was expected" + ); + } + + #[test] + fn only_a_launcher_that_says_nothing_is_read_as_sending_no_key() { + for (value, declared) in [ + (Some("1"), true), + (Some("true"), true), + (Some("yes"), true), + (Some(""), false), + (Some("0"), false), + (Some("false"), false), + (Some("FALSE"), false), + (None, false), + ] { + let _guard = env_lock::lock_env([(USER_ACTION_EXPECTED_ENV, value)]); + assert_eq!( + launcher_declared_a_user_action_key_in_env(), + declared, + "{value:?} was read the wrong way" + ); + } + } + + /// A recorded launch state agrees with the configuration it was sampled + /// from, and disagrees the moment one of the pinned keys moves. + #[tokio::test] + async fn a_pinned_key_that_moves_after_launch_is_named() { + use biorouter::config::with_config_overrides; + use std::collections::HashMap; + + let launched_with = HashMap::from([ + ("BIOROUTER_PROVIDER".to_string(), "ollama".to_string()), + ("BIOROUTER_MODEL".to_string(), "stub-model".to_string()), + ( + "OLLAMA_HOST".to_string(), + "https://ollama.example".to_string(), + ), + ]); + with_config_overrides(launched_with.clone(), async { + record_launch_state(false); + assert_eq!(capability_config_moved_since_launch(), None); + }) + .await; + + // The provider name is untouched; only the endpoint moved — which is the + // case a name-only pin would have waved through. + let mut flipped = launched_with.clone(); + flipped.insert("OLLAMA_HOST".to_string(), "http://127.0.0.1:1".to_string()); + with_config_overrides(flipped, async { + assert_eq!( + capability_config_moved_since_launch().as_deref(), + Some("OLLAMA_HOST") + ); + }) + .await; + + let mut swapped = launched_with; + swapped.insert("BIOROUTER_PROVIDER".to_string(), "versa_azure".to_string()); + with_config_overrides(swapped, async { + assert_eq!( + capability_config_moved_since_launch().as_deref(), + Some("BIOROUTER_PROVIDER") + ); + }) + .await; + } +} diff --git a/crates/biorouter-server/src/lib.rs b/crates/biorouter-server/src/lib.rs index 37ebfc680..e902fbcc4 100644 --- a/crates/biorouter-server/src/lib.rs +++ b/crates/biorouter-server/src/lib.rs @@ -11,6 +11,9 @@ extern crate self as biorouter_server; pub mod auth; pub mod configuration; pub mod error; +// SD-12: what this daemon was LAUNCHED with. Lib-only for the same reason +// `auth` is — it holds a process global, and `routes::agent` is compiled twice. +pub mod launch; pub mod openapi; pub mod routes; pub mod state; diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 98316e018..1d6fceb76 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -341,15 +341,35 @@ fn configured_new_session_provider() -> Result, Er } } +/// Why a brand-new chat's bind to the operator's configured private provider may +/// not go ahead as it stands — SD-12's verdict, with the reason, because two of +/// the three refusals here are actionable by a *person* and a single boolean +/// would have answered all of them in a sentence written for a model. +#[derive(Debug, Clone, PartialEq, Eq)] +enum NewChatBind { + /// Bind it. + Allowed, + /// This daemon can check a proof, and none arrived. + NeedsUserProof, + /// SD-12 would exempt this bind, but the configuration being read is no + /// longer the one the daemon was launched with, so there is nothing left in + /// it that is the operator's choice. Carries the key that moved. + ConfigMovedSinceLaunch(String), + /// The launcher declared it would hand over a user-action key and the key + /// never arrived. A fault to repair, not a deployment where no proof can + /// exist — so the exemption does not apply. + KeyWasExpected, +} + /// SD-12 (`docs/deployment/serve-decisions.md`): does binding the operator's /// configured default provider to a brand-new chat need a person's proof? /// /// A new chat has no capability of its own yet, so a private default reads as a /// raise from Public — the reading `update_agent_provider` gives any first bind. -/// What differs is who chose the model. `/agent/start` names no provider; it -/// binds `BIOROUTER_PROVIDER`, a key only a proven person may write over HTTP -/// (DR-16, open question 24) or the operator wrote out of band with `biorouter -/// configure`. The proof is therefore asked for only where it can be given: +/// What differs is who chose the model: `/agent/start` names no provider, it binds +/// `BIOROUTER_PROVIDER` + `BIOROUTER_MODEL`, and SD-1 says the tier the operator's +/// `biorouter configure` choice implies holds for every session in that daemon. So +/// the proof is asked for only where it can be given: /// /// * `Proven` — the desktop renderer, which sends `X-User-Action` on every start /// (`ui/desktop/src/sessions.ts`). Binds. @@ -360,19 +380,57 @@ fn configured_new_session_provider() -> Result, Er /// with an extension set of its own choosing. /// * `NoKeyInstalled` — `biorouter serve` (SD-7) or a hand-run `biorouterd`. /// Nobody on this daemon can prove anything, so a refusal here refuses the -/// person too, on every new chat, always — the 2026-09-10 QA's F1. Binds. +/// person too, on every new chat, always — the 2026-09-10 QA's F1. Binds, under +/// the two conditions below. +/// +/// ⚠ **The original justification for that third arm was wrong, and both +/// conditions exist because of it** (2026-09-12 review, Findings 1 and 3): /// -/// Only the configured default is exempt, and only at creation: `/agent/start` -/// cannot name any other provider, and on a keyless daemon +/// 1. It said `BIOROUTER_PROVIDER` is *"a key only a proven person may write"*. +/// The HTTP doors are closed, but `config.yaml` is not an HTTP resource: +/// DR-14's filesystem deny is DEFERRED, the agent holds `developer__shell`, and +/// `Config` re-`stat`s and reloads the file — so the value read here is +/// whatever the file says at request time. A model with a shell on a keyless +/// daemon configured with a *public* default could write a private provider +/// there and take this exemption. So the exemption is pinned to the +/// configuration the daemon **launched** with (`launch::pinned_config_keys`); +/// drift refuses, and names the key. +/// 2. `NoKeyInstalled` is only *"this process read no valid digest"*, which a +/// desktop spawn also satisfies when its `userActionKey` is undefined or the +/// daemon's bounded 2s stdin read times out. That is a repairable fault, and on +/// `main` it degraded safely by refusing. It keeps refusing: the launcher +/// declares its intent (`launch::USER_ACTION_EXPECTED_ENV`), which is the only +/// signal that separates the two. +/// +/// Only the configured default is ever exempt, and only at creation: +/// `/agent/start` cannot name another provider, and on a keyless daemon /// `update_agent_provider` refuses every private bind ([`raise_baseline`]), so a /// new chat there never reaches a private model the operator did not choose. -fn new_chat_bind_needs_user(enforced: bool, tier: ProviderTier, proof: UserActionProof) -> bool { - enforced - && raise_needs_user_action(ProviderTier::Public, tier) - && match proof { - UserActionProof::Proven | UserActionProof::NoKeyInstalled => false, - UserActionProof::Unproven => true, +fn new_chat_bind_decision( + enforced: bool, + tier: ProviderTier, + proof: UserActionProof, + launcher_declared_a_key: bool, + config_moved_since_launch: Option, +) -> NewChatBind { + // DR-15's master opt-out, and the plain reading that a public default raises + // nothing for anybody. Neither is about who is asking. + if !enforced || !raise_needs_user_action(ProviderTier::Public, tier) { + return NewChatBind::Allowed; + } + match proof { + UserActionProof::Proven => NewChatBind::Allowed, + UserActionProof::Unproven => NewChatBind::NeedsUserProof, + UserActionProof::NoKeyInstalled => { + if launcher_declared_a_key { + NewChatBind::KeyWasExpected + } else if let Some(key) = config_moved_since_launch { + NewChatBind::ConfigMovedSinceLaunch(key) + } else { + NewChatBind::Allowed + } } + } } /// The capability `update_agent_provider` measures a raise from — SD-12's other @@ -381,7 +439,7 @@ fn new_chat_bind_needs_user(enforced: bool, tier: ProviderTier, proof: UserActio /// On a daemon that holds a user-action key it is the chat's live capability, /// as it always was. On one that holds none, no chat's private capability came /// from anything a person proved over HTTP: the only private binding such a -/// daemon hands out through its routes is [`new_chat_bind_needs_user`]'s +/// daemon hands out through its routes is [`new_chat_bind_decision`]'s /// creation-time bind to the configured default. There is no private floor for /// a request to build on, so a bind to ANY private provider is measured from /// Public, and refused, since no proof can arrive. Without this, SD-12 would let @@ -408,19 +466,54 @@ async fn bind_new_session_provider( message: format!("Failed to configure the selected provider for the new chat: {error}"), status: StatusCode::BAD_REQUEST, })?; - // DR-15's master opt-out, read inside the gate as every #56 surface does. - if new_chat_bind_needs_user( + // DR-15's master opt-out is read inside the gate, as every #56 surface does. + // The launch state is read here rather than in the gate for the same reason: + // one sample per request, threaded, so the decision cannot be made against two + // different answers. + match new_chat_bind_decision( biorouter::privacy::privacy_tiers_enabled(), provider.tier(), user_action_proof(headers), + biorouter_server::launch::expected_a_user_action_key(), + biorouter_server::launch::capability_config_moved_since_launch(), ) { - return Err(ErrorResponse { - message: PrivacyRefusal::TierRaiseNeedsUser { - requested: provider_name, - } - .to_string(), - status: StatusCode::CONFLICT, - }); + NewChatBind::Allowed => {} + NewChatBind::NeedsUserProof => { + return Err(ErrorResponse { + message: PrivacyRefusal::TierRaiseNeedsUser { + requested: provider_name, + } + .to_string(), + status: StatusCode::CONFLICT, + }); + } + // Written for the operator, not for the model. SD-8's rule: a control the + // caller cannot pass must say what would make it passable, and here that + // is a restart — no proof exists on this daemon to offer instead. + NewChatBind::ConfigMovedSinceLaunch(key) => { + return Err(ErrorResponse { + message: format!( + "This Biorouter daemon starts new chats on the model it was launched with, \ + because nothing here can confirm a request came from you. '{key}' has \ + changed in the configuration since it started, so '{provider_name}' is not \ + the model it was launched on and starting a chat on it would be a switch \ + nobody asked for. Restart the daemon to pick up the new configuration." + ), + status: StatusCode::CONFLICT, + }); + } + NewChatBind::KeyWasExpected => { + return Err(ErrorResponse { + message: format!( + "Biorouter cannot confirm that this request came from you: the application \ + that started this daemon was meant to hand it a user-action key and none \ + arrived, so a new chat on the private model '{provider_name}' is refused \ + rather than started. Quit and reopen Biorouter. If it keeps happening, the \ + daemon log records 'no user-action key on stdin'." + ), + status: StatusCode::CONFLICT, + }); + } } let agent = state .get_agent(session.id.clone()) @@ -674,7 +767,7 @@ pub struct RestartAgentResponse { (status = 200, description = "Agent started successfully", body = Session), (status = 400, description = "Bad request", body = ErrorResponse), (status = 401, description = "Unauthorized - invalid secret key"), - (status = 409, description = "The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-12). A daemon with no user-action key binds its configured provider without one.", body = ErrorResponse), + (status = 409, description = "The configured provider is private and this daemon will not bind it to a new chat as things stand (SD-12). Either the daemon holds a user-action key and the request carried no proof it came from the user; or it holds none but its launcher declared it would send one, so the missing key is a fault rather than a deployment where no proof can exist; or it holds none and a capability-deciding configuration key has changed since it started, in which case the message names the key and asks for a restart. A daemon with no user-action key, launched without that declaration, binds the provider it was launched with and needs no proof.", body = ErrorResponse), (status = 500, description = "Internal server error", body = ErrorResponse) ) )] @@ -3192,30 +3285,122 @@ mod new_session_provider_binding_tests { #[test] fn only_a_daemon_that_can_check_a_proof_asks_a_new_chat_for_one() { use UserActionProof::{NoKeyInstalled, Proven, Unproven}; - assert!(!new_chat_bind_needs_user( - true, - ProviderTier::Private, - Proven - )); - assert!(new_chat_bind_needs_user( - true, - ProviderTier::Private, - Unproven - )); - assert!( - !new_chat_bind_needs_user(true, ProviderTier::Private, NoKeyInstalled), + // A daemon launched on this configuration, by a launcher that promised no + // key: the posture SD-12's exemption is for. + let launched_here = |tier, proof| new_chat_bind_decision(true, tier, proof, false, None); + assert_eq!( + launched_here(ProviderTier::Private, Proven), + NewChatBind::Allowed + ); + assert_eq!( + launched_here(ProviderTier::Private, Unproven), + NewChatBind::NeedsUserProof + ); + assert_eq!( + launched_here(ProviderTier::Private, NoKeyInstalled), + NewChatBind::Allowed, "a keyless daemon refusing its own configured default refuses every person, always" ); for proof in [Proven, Unproven, NoKeyInstalled] { // A public default raises nothing, for anyone. - assert!(!new_chat_bind_needs_user(true, ProviderTier::Public, proof)); + assert_eq!( + launched_here(ProviderTier::Public, proof), + NewChatBind::Allowed + ); // DR-15's master opt-out turns the gate off, not the question. - assert!(!new_chat_bind_needs_user( + assert_eq!( + new_chat_bind_decision(false, ProviderTier::Private, proof, false, None), + NewChatBind::Allowed + ); + } + } + + /// Finding 1 (HIGH) of the 2026-09-12 review: the keyless exemption is for + /// the configuration the daemon was **launched** with, not for whatever + /// `config.yaml` — which the agent can write and `Config` reloads live — says + /// at request time. + /// + /// Both conditions bite only on the keyless arm. A daemon that can check a + /// proof already has one, and a person who edits the configuration and asks + /// for a chat in the same breath is not the case this closes. + #[test] + fn drift_since_launch_costs_the_keyless_exemption_and_nothing_else() { + use UserActionProof::{NoKeyInstalled, Proven, Unproven}; + let moved = || Some("BIOROUTER_PROVIDER".to_string()); + + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Private, NoKeyInstalled, false, moved()), + NewChatBind::ConfigMovedSinceLaunch("BIOROUTER_PROVIDER".to_string()), + "a private provider written into config.yaml after launch took the exemption" + ); + // The refusal names the key, because the only person who can act on it + // needs to know which value to put back or which daemon to restart. + assert_eq!( + new_chat_bind_decision( + true, + ProviderTier::Private, + NoKeyInstalled, false, + Some("OLLAMA_HOST".to_string()), + ), + NewChatBind::ConfigMovedSinceLaunch("OLLAMA_HOST".to_string()), + ); + // A public default is not a raise, so drift changes nothing about it — + // there is no exemption being taken to withdraw. + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Public, NoKeyInstalled, false, moved()), + NewChatBind::Allowed + ); + // And a daemon that can check a proof is unaffected in both directions. + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Private, Proven, true, moved()), + NewChatBind::Allowed + ); + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Private, Unproven, true, moved()), + NewChatBind::NeedsUserProof + ); + } + + /// Finding 3 (LOW): `NoKeyInstalled` is two situations wearing one name, and + /// only one of them is a deployment where no proof can exist. A desktop daemon + /// whose key never arrived keeps `main`'s refusal. + /// + /// ⚠ The launcher's declaration is checked **before** the drift, so the + /// message the person gets names the fault they can act on rather than a + /// configuration key that may be perfectly fine. + #[test] + fn a_launcher_that_promised_a_key_does_not_inherit_the_keyless_exemption() { + use UserActionProof::{NoKeyInstalled, Proven, Unproven}; + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Private, NoKeyInstalled, true, None), + NewChatBind::KeyWasExpected + ); + assert_eq!( + new_chat_bind_decision( + true, ProviderTier::Private, - proof - )); + NoKeyInstalled, + true, + Some("BIOROUTER_PROVIDER".to_string()), + ), + NewChatBind::KeyWasExpected, + "a missing key is the actionable fault; the drift message would send the person to the \ + wrong place" + ); + // The declaration says nothing about a daemon that DID get its key: those + // two arms are decided by the proof alone. + for proof in [Proven, Unproven] { + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Private, proof, true, None), + new_chat_bind_decision(true, ProviderTier::Private, proof, false, None), + ); } + // Nor about a public default, which raises nothing. + assert_eq!( + new_chat_bind_decision(true, ProviderTier::Public, NoKeyInstalled, true, None), + NewChatBind::Allowed + ); } /// SD-12's other half: a keyless daemon measures every move onto a private diff --git a/crates/biorouter-server/tests/new_chat_no_user_key.rs b/crates/biorouter-server/tests/new_chat_no_user_key.rs index 7e995f819..da76d0344 100644 --- a/crates/biorouter-server/tests/new_chat_no_user_key.rs +++ b/crates/biorouter-server/tests/new_chat_no_user_key.rs @@ -31,7 +31,10 @@ use biorouter::config::{with_config_overrides, Config}; use biorouter::conversation::message::Message; use biorouter::privacy::refusal::USER_ACTION_REFUSAL_MARKER; use biorouter::privacy::SessionClassification; -use biorouter::providers::versa_azure::VERSA_AZURE_DEPLOYMENT; +// Renamed on `main` (a MODEL, not a deployment — `VERSA_AZURE_DEPLOYMENTS` is +// the model -> deployment map that replaced it). This binary was left RED by +// the merge at f276111f, so nothing here ran until the name was repaired. +use biorouter::providers::versa_azure::VERSA_AZURE_DEFAULT_MODEL; use biorouter_server::auth::{user_action_proof, UserActionProof}; use biorouter_server::state::AppState; use serde_json::{json, Value}; @@ -47,7 +50,7 @@ fn versa_is_the_configured_default() -> HashMap { ("BIOROUTER_PROVIDER".to_string(), "versa_azure".to_string()), ( "BIOROUTER_MODEL".to_string(), - VERSA_AZURE_DEPLOYMENT.to_string(), + VERSA_AZURE_DEFAULT_MODEL.to_string(), ), ( "VERSA_AZURE_API_KEY".to_string(), @@ -56,6 +59,39 @@ fn versa_is_the_configured_default() -> HashMap { ]) } +/// A **public** configured default, for the tests that measure what happens when +/// the configuration moves after launch. `self_hosted_tier` reads `ollama` as +/// Public exactly while its host is not loopback, and constructing the provider +/// opens no connection, so `ollama.example` is never resolved. +fn a_public_ollama_is_the_configured_default() -> HashMap { + HashMap::from([ + ("BIOROUTER_PROVIDER".to_string(), "ollama".to_string()), + ("BIOROUTER_MODEL".to_string(), "stub-model".to_string()), + ( + "OLLAMA_HOST".to_string(), + "https://ollama.example".to_string(), + ), + ]) +} + +/// Stand up the launch posture SD-12's exemption is pinned to: a daemon started +/// with `configured` as its configuration, by a launcher that promised no +/// user-action key. +/// +/// Production samples this once in `commands::agent::run`. Here it has to happen +/// inside the override scope, because the overrides are a `tokio` task-local. +async fn launched_with(configured: HashMap) { + launched_by(configured, false).await; +} + +/// [`launched_with`], plus what the launcher claimed about the key it would send. +async fn launched_by(configured: HashMap, launcher_promised_a_key: bool) { + with_config_overrides(configured, async { + biorouter_server::launch::record_launch_state(launcher_promised_a_key); + }) + .await; +} + /// Every test here stands on this: the daemon under test holds no key. fn assert_the_daemon_is_keyless() { assert_eq!( @@ -113,6 +149,7 @@ async fn discard(state: &Arc, session_id: &str) { #[serial] async fn a_keyless_daemon_starts_a_new_chat_on_its_configured_private_model() { assert_the_daemon_is_keyless(); + launched_with(versa_is_the_configured_default()).await; let state = AppState::new().await.unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -143,7 +180,7 @@ async fn a_keyless_daemon_starts_a_new_chat_on_its_configured_private_model() { assert_eq!(row.provider_name.as_deref(), Some("versa_azure")); assert_eq!( row.model_config.map(|config| config.model_name).as_deref(), - Some(VERSA_AZURE_DEPLOYMENT) + Some(VERSA_AZURE_DEFAULT_MODEL) ); // O5: the ratchet fires on the first turn, never on the bind. A chat that // has touched nothing is not yet private — it is private-CAPABLE. @@ -171,6 +208,7 @@ async fn a_keyless_daemon_starts_a_new_chat_on_its_configured_private_model() { #[serial] async fn a_keyless_daemon_will_not_move_a_new_chat_to_a_private_model_nobody_configured() { assert_the_daemon_is_keyless(); + launched_with(versa_is_the_configured_default()).await; let state = AppState::new().await.unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -237,7 +275,7 @@ async fn a_keyless_daemon_still_refuses_to_change_the_configured_default() { let (status, body) = post_json( biorouter_server::routes::config_management::routes(state), "/config/set_provider", - json!({ "provider": "versa_azure", "model": VERSA_AZURE_DEPLOYMENT }), + json!({ "provider": "versa_azure", "model": VERSA_AZURE_DEFAULT_MODEL }), ) .await; assert_eq!(status, StatusCode::CONFLICT, "{body}"); @@ -302,6 +340,11 @@ async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { config.set_param("BIOROUTER_PROVIDER", "ollama").unwrap(); config.set_param("BIOROUTER_MODEL", "stub-model").unwrap(); config.set_param("OLLAMA_HOST", stub.uri()).unwrap(); + // …and that IS this daemon's launch configuration, so SD-12's exemption + // applies. Recorded after the writes rather than before, for the reason + // production records it before `AppState::new()`: the snapshot has to be the + // configuration the requests will read. + biorouter_server::launch::record_launch_state(false); let state = AppState::new().await.unwrap(); let dir = tempfile::tempdir().unwrap(); @@ -378,3 +421,141 @@ async fn the_first_turn_on_a_keyless_default_chat_ratchets_it_as_usual() { let _ = config.delete(key); } } + +/// **Finding 1 (HIGH), the review that refuted SD-12's first justification.** +/// +/// SD-12 rested on *"`/agent/start` binds `BIOROUTER_PROVIDER`, a key only a +/// proven person may write"*. The HTTP doors to that key are genuinely closed — +/// `a_keyless_daemon_still_refuses_to_change_the_configured_default` above is one +/// of them — but `config.yaml` is not an HTTP resource. DR-14's filesystem deny +/// is DEFERRED, the agent holds `developer__shell`, and `Config` re-`stat`s and +/// reloads the file, so a model could write the provider it wanted and then ask +/// for a new chat on it. +/// +/// The exemption is pinned to the launch configuration instead: the daemon here +/// started on a **public** Ollama, so the private provider that appeared in the +/// file afterwards is nobody's declaration and the bind is refused. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_private_provider_written_after_launch_does_not_start_a_new_chat() { + assert_the_daemon_is_keyless(); + launched_with(a_public_ollama_is_the_configured_default()).await; + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + + let (status, body) = with_config_overrides( + versa_is_the_configured_default(), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!( + status, + StatusCode::CONFLICT, + "a private provider written into config.yaml after this daemon launched started a chat \ + at Private capability: {body}" + ); + assert!( + body.contains("BIOROUTER_PROVIDER"), + "the refusal must name the key that moved, so the operator can act on it: {body}" + ); + assert!( + body.to_lowercase().contains("restart"), + "the refusal must say how to make the new configuration take effect: {body}" + ); +} + +/// The same escalation through a key that is **not** the provider's name, which +/// is why the pin is the whole capability list rather than `BIOROUTER_PROVIDER` +/// alone: `self_hosted_tier` reads `ollama` as Private exactly while its host is +/// loopback, so moving `OLLAMA_HOST` moves the tier with the provider name +/// untouched. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_private_endpoint_written_after_launch_does_not_start_a_new_chat() { + assert_the_daemon_is_keyless(); + launched_with(a_public_ollama_is_the_configured_default()).await; + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + + let mut flipped_to_loopback = a_public_ollama_is_the_configured_default(); + // Private, and never dialled: constructing an Ollama provider opens nothing. + flipped_to_loopback.insert("OLLAMA_HOST".to_string(), "http://127.0.0.1:1".to_string()); + + let (status, body) = with_config_overrides( + flipped_to_loopback, + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!( + status, + StatusCode::CONFLICT, + "moving the endpoint alone was enough to mint a Private-capability chat: {body}" + ); + assert!( + body.contains("OLLAMA_HOST"), + "the refusal named the wrong key: {body}" + ); +} + +/// **Finding 3 (LOW).** `NoKeyInstalled` is "this process read no valid digest", +/// which is two situations wearing one name. SD-12's exemption is for the one +/// where no proof can *ever* exist; a desktop daemon whose key never arrived — +/// `userActionKey` undefined, or the bounded 2s stdin read timing out — is a +/// fault to repair, and on `main` it degraded loudly by refusing every private +/// new chat. It must keep doing that rather than silently binding. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_daemon_whose_launcher_promised_a_key_and_got_none_refuses_the_exemption() { + assert_the_daemon_is_keyless(); + launched_by(versa_is_the_configured_default(), true).await; + let state = AppState::new().await.unwrap(); + let dir = tempfile::tempdir().unwrap(); + + let (status, body) = with_config_overrides( + versa_is_the_configured_default(), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!( + status, + StatusCode::CONFLICT, + "a daemon that expected a user-action key and got none took SD-12's exemption anyway: \ + {body}" + ); + assert!( + body.contains("user-action key"), + "the refusal must say the key is missing, not that the user did not confirm: {body}" + ); + + // …and the same configuration, launched by something that promised nothing, + // still starts the chat. Without this the test above would pass on a build + // that had simply broken the exemption. + launched_with(versa_is_the_configured_default()).await; + let (status, body) = with_config_overrides( + versa_is_the_configured_default(), + post_json( + biorouter_server::routes::agent::routes(Arc::clone(&state)), + "/agent/start", + start_request(dir.path()), + ), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + let id = serde_json::from_str::(&body).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + discard(&state, &id).await; +} diff --git a/crates/biorouter/src/privacy/mod.rs b/crates/biorouter/src/privacy/mod.rs index 2d2c618fa..02e10f789 100644 --- a/crates/biorouter/src/privacy/mod.rs +++ b/crates/biorouter/src/privacy/mod.rs @@ -49,7 +49,7 @@ pub mod visibility; pub use affiliation::{CrossAffiliation, ExtensionAffiliation, ModelAffiliation}; pub use alt_provider::assert_alt_provider_allowed; pub use capability::CallCapability; -pub use config_keys::is_capability_key; +pub use config_keys::{is_capability_key, CAPABILITY_CONFIG_KEYS}; pub use extensions::{ classify_extension, classify_extension_entry, private_extension_ids, resolve_extension, ExtensionClassification, diff --git a/docs/deployment/serve-decisions.md b/docs/deployment/serve-decisions.md index bfb4aee4a..db71ea34d 100644 --- a/docs/deployment/serve-decisions.md +++ b/docs/deployment/serve-decisions.md @@ -295,9 +295,19 @@ behaviour, so changing it means revisiting this record, not making a quiet fix. **Ruling.** On a daemon that holds no user-action key — the one `biorouter serve` starts (SD-7), or a `biorouterd` started by hand — `POST /agent/start` binds the operator's configured provider -to the new chat without asking for proof of a person, whether that provider is public or private. -Three things hold beside it: - +to the new chat without asking for proof of a person, whether that provider is public or private, +**for as long as that configuration is still the one the daemon was launched with**. Five things +hold beside it: + +- **The exemption is pinned to the launch configuration.** A daemon samples the + capability-deciding configuration once, before any route is mounted, and the exemption applies + only while the live values still match it. If any of them has moved, the bind is refused with a + 409 that names the key and says to restart the daemon. `biorouter_server::launch`. +- **A daemon that expected a key and did not get one keeps the refusal.** A launcher that hands + over a digest declares so in the environment (`BIOROUTER_USER_ACTION_EXPECTED`), so + "no proof can ever exist here" is distinguishable from "the proof went missing". The second is + a fault to repair, and it keeps the pre-SD-12 behaviour — every private new chat refused, now in + a sentence that says why — plus a startup `ERROR` naming the consequence. - On that daemon, `POST /agent/update_provider` refuses every move onto a private model, whatever the chat runs on now. The configured model is the only private model a chat there can reach. - A daemon that holds a key — the desktop application's — is unchanged. Its renderer sends the @@ -306,13 +316,54 @@ Three things hold beside it: (`X-Caller-Provider`), the way `biorouter session` already does from a terminal, so a chat its first reply made private stays reachable from the tab that started it. -**Why.** The configured model is the person's decision, made out of band. `/agent/start` names no -provider: it binds `BIOROUTER_PROVIDER`, which only a proven person may write over HTTP, or which -the operator wrote at the terminal with `biorouter configure`. Open question 24 of the privacy plan -already put the raise at that write — *a raise of every future session* — and SD-1 already says the -tier that choice implies *holds for every session in that daemon*. A new chat taking that tier is -the choice being honored, not a switch. DR-16 governs raising a chat that exists, and a chat that -did not exist a moment ago has nothing to raise. +**Why.** The configured model is the person's decision, made out of band. SD-1 already says the +tier that choice implies *holds for every session in that daemon*, and open question 24 of the +privacy plan already put the raise at the moment the choice is written — *a raise of every future +session*. A new chat taking that tier is the choice being honored, not a switch. DR-16 governs +raising a chat that exists, and a chat that did not exist a moment ago has nothing to raise. + +> ⚠ **The first version of this record justified the exemption with a claim the tree contradicts, +> and the correction is the load-bearing part.** It said: *"`/agent/start` names no provider: it +> binds `BIOROUTER_PROVIDER`, which only a proven person may write over HTTP."* The HTTP doors to +> that key really are closed — `/config/set_provider` 409s on a keyless daemon, and `/config/upsert` +> and `/config/remove` both gate `privacy::is_capability_key` behind the proof — but **`config.yaml` +> is not an HTTP resource.** §9.5's general filesystem read/write deny, DR-14, is recorded +> **DEFERRED** in [privacy tiers, *"Did not ship"*](../security/privacy-tiers.md#what-shipped-and-what-did-not); +> the agent holds `developer__shell`; and `Config`'s value cache is keyed on a `FileStamp` it +> re-`stat`s on every read, so the file is reloaded live and the bind reads whatever it says **at +> request time**. A model with a shell on a keyless daemon whose operator had configured a *public* +> default could therefore write a private provider into that file and `POST /agent/start` to get a +> 200 and Private capability — unlocking `chatrecall` over private chats, private knowledge bases +> and an unfiltered Gate E roster with an extension set of its own choosing — where `main` answers +> 409. That is not the operator's choice being honored; it is the door's own text being rewritten by +> the thing the door constrains. +> +> So the exemption is **pinned** rather than re-derived per request. The bind still reads the +> configuration — an operator who edits the file and restarts is served, which is the whole point of +> restarting — but the *exemption* survives only while the capability-deciding values still match +> what this daemon started with. That makes SD-1's sentence true of the door rather than merely +> asserted near it, and it makes a model's ability to write `config.yaml` irrelevant here. + +**What is pinned, and why that list.** `privacy::CAPABILITY_CONFIG_KEYS` verbatim — +`BIOROUTER_PROVIDER`, `BIOROUTER_LEAD_MODEL`, `BIOROUTER_LEAD_PROVIDER`, `OLLAMA_HOST`, +`LLAMACPP_EXTERNAL_HOST` — plus `BIOROUTER_MODEL`. Reusing the list `/config/upsert` already guards +is deliberate: `privacy::config_keys`'s scan of the tier-input files is what keeps it honest, so a +key that starts deciding capability is pinned here without anyone remembering to, and a second +hand-written list would be a third answer to a question that already has two agreeing ones. +⚠ **The provider's name alone would not have been enough:** `self_hosted_tier` reads `ollama` as +Private exactly while its host is loopback, so flipping `OLLAMA_HOST` moves the tier with +`BIOROUTER_PROVIDER` untouched — the same escalation through a different key. + +`BIOROUTER_MODEL` is pinned for a different reason and is **not** a capability key. No `tier()` +implementation reads the model name — all five were checked: both Versa modules resolve +`ucsf_gateway_tier(endpoint)`, `ollama` and `llamacpp` resolve `self_hosted_tier(base_url)`, and the +lead/worker composite takes the `least` of its two halves. Writing it cannot move a tier; it decides +which model runs, and `/agent/start` binds *both* halves of the operator's declaration +(`configured_new_session_provider` requires the provider **and** the model), so the pin covers what +the operator actually declared. Its classification, with that reasoning, is a row in +`privacy::config_keys::NOT_CAPABILITY_CONFIG_KEYS`; what it permits without a proof is an integrity +and availability question — silently downgrading every new chat to a different model, or making new +chats fail outright — not a tier one. On a daemon with no key, asking for the proof can only refuse everyone. The 2026-09-10 QA run measured it: with an institutional model configured, every new chat on a `serve` daemon was @@ -359,19 +410,48 @@ control answers in writing ([privacy tiers §3.1](../security/privacy-tiers.md)) makes the bind legitimate, so a provider the request picked would be a switch. `/agent/start` names none today, and a field that ever let it name one must not inherit this exemption. - *Exempt the configured model on every daemon.* Rejected; see *Why not on every daemon*. +- *Leave the justification as written and record the hole in the consequences.* Rejected. It would + have meant writing down that this door grants Private capability to anything with a shell — a + statement that is true and that nobody reading the ruling would expect from it. The cost of + closing it is one configuration comparison per new chat. +- *Bind the launch snapshot itself and never read the file again.* Rejected, though it is the + narrower rule. It also makes a **deliberate** operator edit silently ineffective: the daemon would + keep serving the old provider with nothing to say about it. Pinning the exemption rather than the + binding fails loudly instead, and names the key that moved. +- *Treat a desktop daemon whose key never arrived as a keyless deployment.* Rejected — Finding 3 of + the 2026-09-12 review. `UserActionProof::NoKeyInstalled` means only *"this process read no valid + 32-byte digest off stdin"*, which a desktop spawn satisfies when `userActionKey` is undefined + (`stdin` is `end()`ed empty, so `hex::decode("")` yields an empty vector that is not 32 bytes) or + when the daemon's bounded 2s stdin read times out. On `main` that degradation was loud and safe. + Letting it inherit the exemption would have turned a repairable fault into a silent relaxation + announced by one `WARN` in a log nobody reads. The launcher declares its intent instead, which is + the only signal that can tell the two apart, and the declaration can make this daemon *stricter* + only — so reading it from the environment is safe even though the model can see it. - *Let a keyless daemon treat its configured model as the capability of any request that states none.* Rejected. An absent header resolving to Public is the fail-safe the reach gate is built on, and a default that raised it would speak for every caller rather than for the client that says what it runs. -**Consequence to accept.** On a keyless daemon whose configured model is private, a model running +**Consequence to accept.** On a keyless daemon whose configured model *is* private, a model running in a chat on that daemon — a public-model chat resumed from the shared session store — that has recovered the daemon secret can start a private-capability chat through `/agent/start` with extensions it chose, and can reach private chats by stating the host's provider. It could already do the first through `workspace_open { new }`, and the second by spelling a provider name (the header is not authentication, as `session_reach.rs` records); and the filesystem read-deny that would stop it carrying anything back out did not ship. It is recorded rather than closed: on a -daemon that cannot tell a person from a model, closing it means refusing the person. +daemon that cannot tell a person from a model, closing it means refusing the person. What the pin +changes is that this is now confined to the tier the operator *launched* the daemon on — a public +deployment cannot be turned into a private-capability one from inside a chat. + +⚠ **And the pin is not DR-14.** It closes one door's stated guarantee, not the general property a +reader might take from it. A shell is still a shell: a model that can write `config.yaml` can read +the session store (`~/.config/biorouter/sessions/`) and the knowledge bases +(`~/.config/biorouter/knowledge/`) as ordinary files, and can start a second `biorouterd` of its +own with any configuration it likes. The privacy barrier is safety before it is security — it stops +mistakes reaching the wrong model — and on a machine where DR-14 is deferred, the part of this door +worth guarding is the Gate C / Gate E *roster*: private connectors (UCSF OMOP, CDW, SPOKE) whose +credentials live in the operating system's credential store rather than on disk. That is what the +pin protects, and it is the honest scope of the claim. And one visible change: a browser tab on a host configured with a private model now opens private chats started in the desktop application on the same machine, which it was refused before. That is diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index c076cf505..e181da461 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -689,7 +689,7 @@ "description": "Unauthorized - invalid secret key" }, "409": { - "description": "The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-12). A daemon with no user-action key binds its configured provider without one.", + "description": "The configured provider is private and this daemon will not bind it to a new chat as things stand (SD-12). Either the daemon holds a user-action key and the request carried no proof it came from the user; or it holds none but its launcher declared it would send one, so the missing key is a fault rather than a deployment where no proof can exist; or it holds none and a capability-deciding configuration key has changed since it started, in which case the message names the key and asks for a restart. A daemon with no user-action key, launched without that declaration, binds the provider it was launched with and needs no proof.", "content": { "application/json": { "schema": { diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index f67d4f7fc..2ce8da4b2 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -4726,7 +4726,7 @@ export type StartAgentErrors = { */ 401: unknown; /** - * The configured provider is private and this daemon holds a user-action key, but the request carried no proof it came from the user (SD-12). A daemon with no user-action key binds its configured provider without one. + * The configured provider is private and this daemon will not bind it to a new chat as things stand (SD-12). Either the daemon holds a user-action key and the request carried no proof it came from the user; or it holds none but its launcher declared it would send one, so the missing key is a fault rather than a deployment where no proof can exist; or it holds none and a capability-deciding configuration key has changed since it started, in which case the message names the key and asks for a restart. A daemon with no user-action key, launched without that declaration, binds the provider it was launched with and needs no proof. */ 409: ErrorResponse; /** From 8958565f345d5dd5abbd4aefcd8e884cbe482cd3 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 20:40:43 -0700 Subject: [PATCH 12/14] fix(desktop): the launcher declares the user-action key it sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 3 (LOW) of the 2026-09-12 review. The previous commit made the daemon act on the declaration; this is the declaration. new_chat_bind_decision keys the keyless exemption on UserActionProof:: NoKeyInstalled, which reads "this process read no valid 32-byte digest off stdin". Besides `biorouter serve` and a hand-run daemon, two desktop spawns satisfy it: - userActionKey undefined — biorouterd.ts writes nothing and end()s stdin, so the daemon reads "", hex::decode("") gives Ok(vec![]), and <[u8;32]>:: try_from fails; - the daemon's bounded 2s stdin read times out. On main that degradation was loud and safe: every private new-chat bind 409'd. It must not become "bind without proof, announced by one WARN in a log nobody reads" — a desktop daemon that lost its key is a repairable fault, not a deployment where no proof can exist, and the person can quit and reopen the app. Nothing in the daemon could tell the two apart, so the launcher says which: BIOROUTER_USER_ACTION_EXPECTED, set unconditionally on the spawn path. ⚠ Unconditional is the load-bearing part. `if (userActionKey)` around it would delete the signal in the only case that needs it — pinned by a second test that spawns with no key and asserts the declaration survives. In the environment rather than on stdin, which AR-11's rule permits precisely because this is not derived from the key: it authenticates nothing, and a value a model can read but not write can only make this daemon stricter. The existing "never through the environment" test keeps its /USER_ACTION/i pattern — the property it defends is that nothing derived from the KEY travels there — and exempts this one name while asserting it is the literal "1" and contains neither the key nor its digest. `biorouter serve` clears the variable rather than trusting an empty inherit: it sends no digest by design, so a value exported in the operator's shell would turn every private new chat there into a refusal nobody can clear — the exact failure SD-12 exists to remove. Tests: 5 passed in ui/desktop/src/biorouterd.test.ts (3 existing, 1 amended, 1 new); 20 in biorouter-cli --lib commands::serve; tsc --noEmit and prettier clean. --- crates/biorouter-cli/src/commands/serve.rs | 10 ++++++++ ui/desktop/src/biorouterd.test.ts | 30 ++++++++++++++++++++++ ui/desktop/src/biorouterd.ts | 19 ++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/crates/biorouter-cli/src/commands/serve.rs b/crates/biorouter-cli/src/commands/serve.rs index 9056b7ca8..2fe199f97 100644 --- a/crates/biorouter-cli/src/commands/serve.rs +++ b/crates/biorouter-cli/src/commands/serve.rs @@ -133,6 +133,16 @@ pub async fn handle_serve( ) // See the module documentation: no proof-of-user digest, on purpose. .stdin(Stdio::null()) + // SD-12: and this daemon says so. The desktop launcher sets + // `BIOROUTER_USER_ACTION_EXPECTED` to declare that it *does* send a + // digest, which makes a daemon that receives none refuse rather than + // exempt a private new chat. `serve` sends none by design, so an + // inherited value from the operator's shell would turn every private new + // chat here into a refusal nobody can clear — the failure SD-12 exists + // to remove. Cleared rather than trusted; the literal is defined once, in + // `biorouter_server::launch::USER_ACTION_EXPECTED_ENV` (this crate does + // not depend on `biorouter-server`). + .env_remove("BIOROUTER_USER_ACTION_EXPECTED") // A backstop for a panic unwinding through here. Every ordinary path // goes through `stop_daemon`, which asks before it insists. .kill_on_drop(true) diff --git a/ui/desktop/src/biorouterd.test.ts b/ui/desktop/src/biorouterd.test.ts index 38ad2f294..1d9185bfd 100644 --- a/ui/desktop/src/biorouterd.test.ts +++ b/ui/desktop/src/biorouterd.test.ts @@ -130,15 +130,45 @@ describe('startBiorouterd logging', () => { expect(spawnArgs.options.stdio[0]).toBe('pipe'); // was 'ignore' expect(spawnArgs.args).toEqual(['agent']); // not on argv either const env = spawnArgs.options.env; + // SD-12 added ONE user-action-shaped variable, and it is not a secret: it is + // the launcher declaring that it sends a digest at all, so a daemon that gets + // none can tell a fault from a `biorouter serve` deployment. Exempted by name + // and pinned to a boolean below, rather than by loosening the pattern — the + // property being defended is that nothing DERIVED FROM THE KEY travels here, + // and a regex that stopped matching `USER_ACTION` would stop defending it. + expect(env.BIOROUTER_USER_ACTION_EXPECTED).toBe('1'); for (const [k, v] of Object.entries(env)) { + if (k === 'BIOROUTER_USER_ACTION_EXPECTED') continue; expect(k).not.toMatch(/USER_ACTION/i); expect(v).not.toBe(userActionKeyForTest); // and not smuggled under another name } + // The declaration carries nothing recoverable: not the key, not its digest. + expect(env.BIOROUTER_USER_ACTION_EXPECTED).not.toContain(userActionKeyForTest); + expect(env.BIOROUTER_USER_ACTION_EXPECTED).not.toContain(sha256Hex(userActionKeyForTest)); expect(stdinWrites.join('')).toContain(sha256Hex(userActionKeyForTest)); expect(stdinWrites.join('')).not.toContain(userActionKeyForTest); // digest, never the key expect(stdinEnded).toBe(true); }); + // Finding 3 of the 2026-09-12 review: the declaration is what separates "no key + // was ever meant to arrive" from "one was and did not", so it must survive the + // case where the launcher has no key to send — the only case where the daemon + // needs it. A `if (userActionKey)` around it would delete the signal exactly + // there. + it('still declares that it sends a key when it has none to send', async () => { + const app = { + isPackaged: false, + on: vi.fn(), + } as unknown as App; + + await startBiorouterd({ app, serverSecret, dir: process.cwd() }); + + const options = mocks.spawn.mock.calls[0]?.[2] as { env: Record }; + expect(options.env.BIOROUTER_USER_ACTION_EXPECTED).toBe('1'); + expect(stdinWrites.join('')).toBe(''); // nothing to write + expect(stdinEnded).toBe(true); + }); + afterAll(() => { if (previousInheritedValue === undefined) { delete process.env[inheritedKey]; diff --git a/ui/desktop/src/biorouterd.ts b/ui/desktop/src/biorouterd.ts index 945cfcf29..c12be39e8 100644 --- a/ui/desktop/src/biorouterd.ts +++ b/ui/desktop/src/biorouterd.ts @@ -282,6 +282,8 @@ interface BiorouterProcessEnv { PATH: string; BIOROUTER_PORT: string; BIOROUTER_SERVER__SECRET_KEY?: string; + /** SD-12: this launcher sends a user-action digest on stdin. See the spawn. */ + BIOROUTER_USER_ACTION_EXPECTED?: string; BIOROUTER_DISABLE_KEYRING?: string; } @@ -353,6 +355,23 @@ export const startBiorouterd = async ( PATH: `${path.dirname(resolvedBiorouterdPath)}${path.delimiter}${process.env.PATH || ''}`, BIOROUTER_PORT: String(port), BIOROUTER_SERVER__SECRET_KEY: serverSecret, + // Issue #56 DR-16 / SD-12. This launcher writes a user-action digest down + // stdin below, and says so here. + // + // ⚠ **Unconditional, including when there is no key to send.** That case is + // exactly the one worth naming: `UserActionProof::NoKeyInstalled` means only + // "this process read no valid digest", which a `biorouter serve` daemon (no + // key by design, `Stdio::null()`) and a desktop daemon whose key never + // arrived both satisfy. SD-12 lets the first start a new chat on a private + // model without a proof, because nobody there can ever give one; the second + // is a repairable fault and must keep refusing. Declaring the intent is what + // lets the daemon tell them apart — so a `if (userActionKey)` here would + // delete the signal in the only situation that needs it. + // + // Never the key or its digest: AR-11 measured the environment to be + // recoverable in-process. This is a boolean-shaped claim that authenticates + // nothing, and a value a model could only use to make the daemon stricter. + BIOROUTER_USER_ACTION_EXPECTED: '1', // Dev Electron rebuilds should not trigger macOS Keychain prompts; packaged // builds keep the normal OS credential-store behavior. BIOROUTER_DISABLE_KEYRING: From 030e04bafe6aa25f943a86c0bb228a85d1350b0b Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 20:40:58 -0700 Subject: [PATCH 13/14] docs(privacy): BIOROUTER_MODEL decides which model runs, never which tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 2 (MEDIUM) of the 2026-09-12 review: it was in NEITHER capability list, so any caller holding only the daemon secret — the model, by AR-11 — could write it without proof, on every daemon including the desktop's. A provider is guarded; the model it runs was not. Closed by classification, not by a guard, because the guard would have had to be justified by the classification and it comes out the other way. No tier() implementation reads the model name. All five tier-input providers checked: versa_azure.rs:576 and versa_bedrock.rs:454 resolve ucsf_gateway_tier(&self. resolved_endpoint); ollama.rs:183 and llamacpp.rs:556 resolve self_hosted_tier (base_url); lead_worker.rs:545 takes ProviderTier::least of its two halves. And a model NAME cannot smuggle a persisted provider binding past BIOROUTER_PROVIDER: those markers live in ModelConfig::request_params (provider_binding.rs:196, lead_worker.rs:55), which ModelConfig::new leaves empty, and /agent/start also calls ensure_no_restore_marker. So what an unproven write permits is integrity and availability, not a tier move, and the row says so: every new chat starting on a model the operator did not choose — a cheaper or weaker one, or a different Versa deployment inside the same private gateway, whose tier is the gateway's and does not move — or no new chats at all, since configured_new_session_provider requires both halves and answers 400 when only one is set. Making it a capability key would instead have made every model switch in the GUI a user act while protecting no tier — the "a rule that fires constantly is a rule people route around" failure the list's own doc comment warns about. It IS pinned on SD-12's keyless path (launch::pinned_config_keys) for a different reason: the exemption there is for the operator's own declaration and /agent/start binds both halves of it. Like BIOROUTER_PROVIDER it is read through the config_value! macro, so the literal never appears in a get_param( call and the scan cannot see it. Seeded, with two assertions that the seed survives — it is in the NOT list, and is_capability_key says no — and the "classified but nothing reads it" loop now excuses both config_value! keys rather than one. Fail-before is a source fact: git show origin/main:crates/biorouter/src/privacy/config_keys.rs \ | grep -c BIOROUTER_MODEL -> 0 Tests: cargo test -p biorouter --lib -- privacy::config_keys, 2 passed. --- crates/biorouter/src/privacy/config_keys.rs | 54 ++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/biorouter/src/privacy/config_keys.rs b/crates/biorouter/src/privacy/config_keys.rs index 7d5fbe719..5dcd55f86 100644 --- a/crates/biorouter/src/privacy/config_keys.rs +++ b/crates/biorouter/src/privacy/config_keys.rs @@ -42,6 +42,37 @@ pub const CAPABILITY_CONFIG_KEYS: &[&str] = &[ /// Every other key the tier-input files read, each with the reason it does not /// determine capability. A key must be in exactly one of these two lists. pub const NOT_CAPABILITY_CONFIG_KEYS: &[(&str, &str)] = &[ + // The other half of `/agent/start`'s bind, and the 2026-09-12 review's + // Finding 2: it was in NEITHER list, so any caller holding only the daemon + // secret could write it without proof, on every daemon including the + // desktop's. Classified rather than guarded, because the classification is + // what the guard would have to be justified by, and it comes out the other + // way: no `tier()` implementation reads the model name. All five tier-input + // providers were checked — both Versa modules resolve + // `ucsf_gateway_tier(endpoint)`, `ollama` and `llamacpp` resolve + // `self_hosted_tier(base_url)`, and `LeadWorkerProvider` takes the `least` of + // its two halves — and a model name is a *string*, so it cannot smuggle a + // persisted provider binding either: those live in + // `ModelConfig::request_params`, which `ModelConfig::new` leaves empty. + // + // What an unproven write to it DOES permit is an integrity and availability + // problem, recorded so nobody mistakes it for nothing: + // every new chat starts on a model the operator did not choose (a cheaper or + // weaker one, or a different Versa deployment inside the same private + // gateway), or on none at all, because `configured_new_session_provider` + // requires both halves and answers `400` when only one is set. Neither moves + // a tier. ⚠ **On SD-12's keyless path it is nevertheless pinned to the launch + // configuration** (`biorouter_server::launch::pinned_config_keys`) — not + // because it decides capability, but because the exemption there is for the + // operator's own declaration and this is half of it. + // + // Read through the `config_value!` macro (base.rs), like `BIOROUTER_PROVIDER`, + // so the literal never appears in a `get_param(` call and the scan below + // cannot see it. Seeded, and the test asserts the seed survives. + ( + "BIOROUTER_MODEL", + "names which model runs, never which tier: no `tier()` reads the model name", + ), ("BIOROUTER_CONTEXT_LIMIT", "token budget, not a tier input"), ( "BIOROUTER_LEAD_TURNS", @@ -219,6 +250,21 @@ mod tests { // survives. assert!(CAPABILITY_CONFIG_KEYS.contains(&"BIOROUTER_PROVIDER")); assert_eq!(CAPABILITY_CONFIG_KEYS.len(), 5); + // The same, for the other half of `/agent/start`'s bind. Seeded into the + // NOT list by the 2026-09-12 review's Finding 2, which found it in + // neither — see its row for why the classification comes out that way. + assert!( + NOT_CAPABILITY_CONFIG_KEYS + .iter() + .any(|(key, _why)| *key == "BIOROUTER_MODEL"), + "BIOROUTER_MODEL is unclassified again: it is half of the bind /agent/start performs, \ + so leaving it out of both lists is how it went unreviewed the first time" + ); + assert!( + !is_capability_key("BIOROUTER_MODEL"), + "BIOROUTER_MODEL was made a capability key; no `tier()` reads the model name, so this \ + would make every model switch a user act without protecting a tier" + ); // …and the other way round: every classified key is still READ by a // tier-input file. Without this, a read that goes away leaves its row @@ -228,7 +274,13 @@ mod tests { .iter() .copied() .chain(NOT_CAPABILITY_CONFIG_KEYS.iter().map(|(key, _why)| *key)); - for key in classified.filter(|key| *key != "BIOROUTER_PROVIDER") { + // + // The two `config_value!` keys are excused, for the reason given above + // each of them: the scan reads `get_param("…")` literals, and neither + // literal exists in the source. + for key in + classified.filter(|key| !matches!(*key, "BIOROUTER_PROVIDER" | "BIOROUTER_MODEL")) + { assert!( scanned.iter().any(|read| read == key), "{key} is classified but no tier-input file reads it; delete its row" From 94c189606c97508b371eac4c6ed2d33645345241 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 20:41:12 -0700 Subject: [PATCH 14/14] test(privacy): AR-15's guard pins raise_baseline's wiring, not just its existence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit raise_baseline has zero occurrences on origin/main — this PR introduces it, and it is what keeps a keyless daemon's Private -> Private sideways move onto a private model nobody configured refused. Nothing pinned it: the AR-15 closure guard asserts three tokens in update_agent_provider's refusal condition (privacy_tiers_enabled(), raise_needs_user_action(, !is_user_action(), and all three survive deleting raise_baseline entirely. ⚠ raise_baseline( cannot go in that token loop. The call is a `let` on the line before the `if`, and the condition slice starts at the last " if " before the refusal — so the token is structurally outside it and adding it to the list would fail for a reason that is not a regression. Two assertions instead: raise_baseline( against the handler body, and raise_needs_user_action(baseline, against the squashed condition. The second is the half that catches a rewire back to `current` without the call being deleted. The guard asserts PRESENCE and does not count occurrences, so this raises none of the two-rows-for-one-file hazards the privacy censuses have. Fail-before, by demonstration rather than argument — deleting the two lines that wire raise_baseline into the gate and running the binary: test the_documented_closure_is_the_one_the_code_performs ... FAILED update_agent_provider no longer computes a `raise_baseline`, … test result: FAILED. 6 passed; 1 failed Only the new assertion fired; the three pre-existing tokens were all still present in the mutilated condition, so the guard as it stood went green over the deletion. Source restored and re-verified. Tests: 7 passed in tests/privacy_ar15_is_retired.rs. --- .../tests/privacy_ar15_is_retired.rs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/biorouter-server/tests/privacy_ar15_is_retired.rs b/crates/biorouter-server/tests/privacy_ar15_is_retired.rs index 076db76d9..dcbe25522 100644 --- a/crates/biorouter-server/tests/privacy_ar15_is_retired.rs +++ b/crates/biorouter-server/tests/privacy_ar15_is_retired.rs @@ -385,6 +385,34 @@ fn the_documented_closure_is_the_one_the_code_performs() { ); } + // SD-12 (PR #229) put a second thing between the raise predicate and the + // truth, and it is this PR's own protection, so this PR pins it: on a daemon + // holding no user-action key, `raise_baseline` forces the capability the raise + // is measured FROM down to Public, which is what keeps a `Private -> Private` + // sideways move onto a private model nobody configured refused. Delete it and + // every token above is still present, the condition still reads as a gate, and + // the sideways move is waved through as "not a raise". `routes/agent.rs`'s unit + // tests cover the function; nothing covered its WIRING. + // + // ⚠ Two assertions, not one token in the loop above, and the split is + // load-bearing: the call is a `let` on the line *before* the `if`, so the + // condition slice — which starts at the last ` if ` — structurally cannot + // contain it. The handler carries the call; the condition carries the half + // that matters just as much, that the predicate measures from that value + // rather than from the chat's live `current` binding. + let handler_body = cut(AGENT_ROUTE, handler, refusal); + assert!( + handler_body.contains("raise_baseline("), + "update_agent_provider no longer computes a `raise_baseline`, so a keyless daemon measures \ + a private raise from the chat's live binding again and SD-12's exemption can be carried \ + sideways onto a private model the operator never configured" + ); + assert!( + squash(condition).contains("raise_needs_user_action(baseline,"), + "the tier-raise predicate is no longer measured from `raise_baseline`'s result, so \ + computing it changes nothing.\nGuard reads: {condition}" + ); + // A negative control, so the extractor is provably not matching anything it // is handed: the same file's `update_working_dir` is not a raise channel. let elsewhere = AGENT_ROUTE