Mask provider secrets; a coding agent without its CLI is not Configured (QA F5, F6)

CLAUDE.md

@@ -483,6 +483,13 @@ compliance page is required reading before research data goes near either.
   (`routes/coding_agents.rs`) backs the onboarding card
   `onboarding/CodingAgentInlineCard.tsx`, wired beside `LlamaServerInlineCard` in
   `ProviderGuard.tsx`. `CLAUDE_CODE_COMMAND` / `CODEX_COMMAND` override discovery.
+- **"Configured" means the key is saved AND the CLI resolves.** `check_provider_configured`
+  (`routes/utils.rs`) asks `discovery::resolve_configured` — the lookup the status probe
+  uses — so a command key naming a missing CLI is served `is_configured: false` with
+  `unavailable_reason`, and `SwitchModelModal` shows that row disabled with the reason rather
+  than offering a bind `from_env` would refuse. Sign-in is deliberately NOT part of it: learning
+  it spawns the CLI, and `GET /config/providers` runs for every provider. See
+  [`docs/desktop-ui/provider-catalog.md`](docs/desktop-ui/provider-catalog.md).
 - **Tests:** `cargo test -p biorouter --lib providers::coding_agent`,
   `cargo test -p biorouter-server --test tool_bridge_routes`, and the vitest
   suite for the onboarding card. The live end-to-end tests need the real vendor

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

@@ -1,4 +1,4 @@
-use crate::routes::utils::check_provider_configured;
+use crate::routes::utils::{check_provider_configured, provider_readiness, ProviderReadiness};
 use crate::state::AppState;
 use axum::routing::put;
 use axum::{
@@ -129,6 +129,20 @@ pub struct ProviderDetails {
     /// `extensionPairingRefused` documents the same rule on its side.
     #[serde(default)]
     pub resolved_tier: Option<ProviderTier>,
+    /// Why a provider the user HAS set up cannot run right now: a one-line
+    /// sentence for the model picker to print on the row it disables.
+    ///
+    /// Set only when [`Self::is_configured`] is false for a reason other than a
+    /// missing key — today, a coding agent whose command key is saved and whose
+    /// CLI does not resolve (see `routes::utils::provider_readiness`). `None` for
+    /// every usable provider and for every provider that is simply not set up,
+    /// which the picker leaves out rather than greys out.
+    ///
+    /// ⚠ **Only what can be learned without spawning.** A signed-out CLI is not
+    /// reported here: finding that out means running it, and this route runs
+    /// for every provider on every settings open.
+    #[serde(default)]
+    pub unavailable_reason: Option<String>,
 }
 
 #[derive(Serialize, ToSchema)]
@@ -991,33 +1005,45 @@ pub async fn providers() -> Result<Json<Vec<ProviderDetails>>, StatusCode> {
     // Concurrently, because each row may construct a provider and a serial pass
     // would add every constructor's latency together on a route the settings
     // grid blocks on.
-    let providers_response: Vec<ProviderDetails> =
-        futures::future::join_all(providers.into_iter().map(
-            |(metadata, provider_type)| async move {
-                let is_configured = check_provider_configured(&metadata, provider_type);
-                // Issue #56, DR-26. Both resolved from the instance, never from
-                // the name — see `resolve_provider_axes`.
-                let (resolved_tier, affiliation) = if is_configured {
-                    resolve_provider_axes(&metadata).await
-                } else {
-                    (None, None)
-                };
-
-                ProviderDetails {
-                    name: metadata.name.clone(),
-                    metadata,
-                    is_configured,
-                    provider_type,
-                    affiliation,
-                    resolved_tier,
-                }
-            },
-        ))
-        .await;
+    let providers_response: Vec<ProviderDetails> = futures::future::join_all(
+        providers
+            .into_iter()
+            .map(|(metadata, provider_type)| provider_details(metadata, provider_type)),
+    )
+    .await;
 
     Ok(Json(providers_response))
 }
 
+/// One row of `GET /config/providers`.
+async fn provider_details(
+    metadata: ProviderMetadata,
+    provider_type: ProviderType,
+) -> ProviderDetails {
+    let (is_configured, unavailable_reason) = match provider_readiness(&metadata, provider_type) {
+        ProviderReadiness::Configured => (true, None),
+        ProviderReadiness::NotConfigured => (false, None),
+        ProviderReadiness::Unavailable(reason) => (false, Some(reason)),
+    };
+    // Issue #56, DR-26. Both resolved from the instance, never from the name —
+    // see `resolve_provider_axes`.
+    let (resolved_tier, affiliation) = if is_configured {
+        resolve_provider_axes(&metadata).await
+    } else {
+        (None, None)
+    };
+
+    ProviderDetails {
+        name: metadata.name.clone(),
+        metadata,
+        is_configured,
+        provider_type,
+        affiliation,
+        resolved_tier,
+        unavailable_reason,
+    }
+}
+
 #[utoipa::path(
     get,
     path = "/config/providers/{name}/models",
@@ -2202,6 +2228,7 @@ mod affiliation_wire_tests {
             provider_type: ProviderType::Builtin,
             affiliation,
             resolved_tier,
+            unavailable_reason: None,
         }
     }
 
@@ -2382,3 +2409,82 @@ mod privacy_disclosure_tests {
         );
     }
 }
+
+/// F6 of the 2026-09-10 provider QA run, at the route: a coding agent whose CLI
+/// is missing is served `is_configured: false` WITH the reason the model picker
+/// prints on the row it disables — and an ordinary row carries an explicit
+/// `null` in the same key.
+///
+/// ⚠ Exercised through `provider_details`, the one function `providers()` maps
+/// over, rather than through the whole route: `GET /config/providers` builds
+/// every configured provider in the developer's real config, which no unit test
+/// should do. The command key is pinned through the environment under
+/// `env_lock`, so the real config file never decides the outcome.
+#[cfg(test)]
+mod readiness_wire_tests {
+    use super::*;
+    use biorouter::providers::base::Provider;
+    use biorouter::providers::codex::CodexProvider;
+    use biorouter::providers::coding_agent::CodingAgentKind;
+
+    #[tokio::test]
+    async fn a_codex_row_whose_cli_is_missing_is_unconfigured_and_says_why() {
+        let dir = tempfile::tempdir().unwrap();
+        let missing = dir.path().join("nonexistent").join("codex");
+        let _env = env_lock::lock_env([("CODEX_COMMAND", Some(missing.to_str().unwrap()))]);
+
+        let row = provider_details(CodexProvider::metadata(), ProviderType::Builtin).await;
+
+        assert!(
+            !row.is_configured,
+            "the badge and the picker both key on this"
+        );
+        assert_eq!(
+            row.unavailable_reason.as_deref(),
+            Some(CodingAgentKind::Codex.not_installed_summary().as_str())
+        );
+        // Nothing was constructed for a provider that cannot be bound.
+        assert!(row.resolved_tier.is_none() && row.affiliation.is_none());
+
+        let json = serde_json::to_value(row).unwrap();
+        assert_eq!(json["is_configured"], serde_json::json!(false));
+        assert_eq!(
+            json["unavailable_reason"],
+            serde_json::json!(CodingAgentKind::Codex.not_installed_summary())
+        );
+    }
+
+    /// The control: a codex row whose CLI resolves is configured and carries no
+    /// reason — so the test above cannot pass for a route that refuses Codex
+    /// outright.
+    #[tokio::test]
+    async fn a_codex_row_whose_cli_resolves_is_configured_with_no_reason() {
+        let dir = tempfile::tempdir().unwrap();
+        let exe = dir.path().join("codex");
+        std::fs::write(&exe, b"#!/bin/sh\n").unwrap();
+        let _env = env_lock::lock_env([("CODEX_COMMAND", Some(exe.to_str().unwrap()))]);
+
+        let row = provider_details(CodexProvider::metadata(), ProviderType::Builtin).await;
+
+        assert!(row.is_configured);
+        assert_eq!(row.unavailable_reason, None);
+    }
+
+    /// Usable and not-set-up rows alike serve the key as `null`, never omit it:
+    /// an absent key is indistinguishable from a daemon that predates the field.
+    #[test]
+    fn a_row_with_nothing_to_explain_serialises_an_explicit_null() {
+        let row = ProviderDetails {
+            name: "openai".to_string(),
+            metadata: ProviderMetadata::empty(),
+            is_configured: false,
+            provider_type: ProviderType::Builtin,
+            affiliation: None,
+            resolved_tier: None,
+            unavailable_reason: None,
+        };
+        let json = serde_json::to_value(row).unwrap();
+        assert!(json.as_object().unwrap().contains_key("unavailable_reason"));
+        assert!(json["unavailable_reason"].is_null());
+    }
+}

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

@@ -1,9 +1,64 @@
 use biorouter::config::declarative_providers::load_provider;
 use biorouter::config::Config;
 use biorouter::providers::base::{ConfigKey, ProviderMetadata, ProviderType};
+use biorouter::providers::coding_agent::discovery::{self, CodingAgentKind};
 use std::env;
 
+/// Whether a provider can be used, as `GET /config/providers` reports it.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ProviderReadiness {
+    /// Its keys are saved, and nothing it needs that can be checked cheaply is
+    /// missing. The only state `is_configured` is true for.
+    Configured,
+    /// Not set up: a key it requires has not been saved.
+    NotConfigured,
+    /// The user set it up, but something it needs at runtime is missing — the
+    /// one-line reason. Today that is a coding agent whose command key is saved
+    /// and whose CLI does not resolve.
+    ///
+    /// ⚠ **Only what a `stat` can see.** Signed-out is deliberately NOT a reason
+    /// here: learning it means spawning the vendor CLI, and this runs for every
+    /// provider on every `GET /config/providers` (see
+    /// `coding_agent::discovery`'s module header). The catalog's status pill says
+    /// it, and a turn that reaches a signed-out CLI fails with the vendor's own
+    /// login command.
+    Unavailable(String),
+}
+
 pub fn check_provider_configured(metadata: &ProviderMetadata, provider_type: ProviderType) -> bool {
+    provider_readiness(metadata, provider_type) == ProviderReadiness::Configured
+}
+
+/// [`check_provider_configured`], with the reason when a provider the user set
+/// up still cannot run.
+pub fn provider_readiness(
+    metadata: &ProviderMetadata,
+    provider_type: ProviderType,
+) -> ProviderReadiness {
+    if !keys_are_saved(metadata, provider_type) {
+        return ProviderReadiness::NotConfigured;
+    }
+
+    // A coding agent's one key only NAMES a command, and a saved name is not an
+    // installed CLI. Reporting it configured anyway is how the row read "Not
+    // installed" and "Configured" on one line, and how the model picker offered
+    // a provider whose `from_env` would refuse the bind. The same
+    // `resolve_configured` backs `/coding_agents/status`, so the two answers
+    // agree by construction rather than by a test remembering to compare them.
+    if provider_type == ProviderType::Builtin {
+        if let Some(kind) = CodingAgentKind::from_provider_id(&metadata.name) {
+            if discovery::resolve_configured(kind).is_none() {
+                return ProviderReadiness::Unavailable(kind.not_installed_summary());
+            }
+        }
+    }
+
+    ProviderReadiness::Configured
+}
+
+/// Whether every key the provider requires has been saved — the whole of what
+/// "configured" meant before a saved key could fail to be enough.
+fn keys_are_saved(metadata: &ProviderMetadata, provider_type: ProviderType) -> bool {
     let config = Config::global();
 
     if provider_type == ProviderType::Custom || provider_type == ProviderType::Declarative {
@@ -89,3 +144,113 @@ pub fn check_provider_configured(metadata: &ProviderMetadata, provider_type: Pro
         is_set_in_env || is_set_in_config
     })
 }
+
+#[cfg(test)]
+mod tests {
+    //! Issue F6 of the 2026-09-10 provider QA run: with `CODEX_COMMAND` pointed
+    //! at a path that does not exist, the Codex row read "Not installed" and
+    //! "✓ Configured" on one line, and Codex stayed selectable in the model
+    //! picker.
+    //!
+    //! ⚠ **Every case sets the command key through the environment**, under
+    //! `env_lock`'s one process-wide mutex. `Config::get_param` reads the
+    //! environment before the config file, so neither half of the check ever
+    //! reaches the developer's real `~/.config/biorouter` — and a saved
+    //! `CODEX_COMMAND` there cannot decide what these tests see.
+
+    use super::*;
+    use biorouter::providers::base::Provider;
+    use biorouter::providers::claude_code::ClaudeCodeProvider;
+    use biorouter::providers::codex::CodexProvider;
+
+    /// Pin `kind`'s command key to `value` for the life of the guard.
+    fn command_pinned_to(kind: CodingAgentKind, value: &str) -> env_lock::EnvGuard<'static> {
+        env_lock::lock_env([(kind.command_config_key(), Some(value))])
+    }
+
+    fn metadata_for(kind: CodingAgentKind) -> ProviderMetadata {
+        match kind {
+            CodingAgentKind::ClaudeCode => ClaudeCodeProvider::metadata(),
+            CodingAgentKind::Codex => CodexProvider::metadata(),
+        }
+    }
+
+    /// The reported defect. A saved key naming a CLI that is not there is not a
+    /// configured provider, and it says why.
+    #[test]
+    fn a_coding_agent_whose_cli_is_missing_is_not_configured() {
+        let dir = tempfile::tempdir().unwrap();
+        let missing = dir.path().join("no-such-dir").join("codex");
+
+        for kind in CodingAgentKind::all() {
+            let _env = command_pinned_to(kind, missing.to_str().unwrap());
+            let metadata = metadata_for(kind);
+
+            assert!(
+                !check_provider_configured(&metadata, ProviderType::Builtin),
+                "{kind:?} pointed at {} must not report is_configured",
+                missing.display()
+            );
+            assert_eq!(
+                provider_readiness(&metadata, ProviderType::Builtin),
+                ProviderReadiness::Unavailable(kind.not_installed_summary()),
+                "{kind:?}: the reason is the not-installed sentence, not a silent false"
+            );
+        }
+    }
+
+    /// Without this, the test above passes for a check that simply refuses every
+    /// coding agent. A command that resolves is configured, with no reason.
+    #[test]
+    fn a_coding_agent_whose_cli_resolves_is_configured() {
+        let dir = tempfile::tempdir().unwrap();
+        let exe = dir.path().join("codex");
+        std::fs::write(&exe, b"#!/bin/sh\n").unwrap();
+
+        for kind in CodingAgentKind::all() {
+            let _env = command_pinned_to(kind, exe.to_str().unwrap());
+            let metadata = metadata_for(kind);
+
+            assert!(check_provider_configured(&metadata, ProviderType::Builtin));
+            assert_eq!(
+                provider_readiness(&metadata, ProviderType::Builtin),
+                ProviderReadiness::Configured
+            );
+        }
+    }
+
+    /// ⚠ **The row and the status route must agree.** The pill beside the name
+    /// comes from `/coding_agents/status` (`probe`), the check beside it from this
+    /// function; F6 was the two disagreeing on one line. A pinned path that does
+    /// not exist resolves nothing, so the probe returns without spawning.
+    #[tokio::test]
+    async fn not_configured_is_exactly_what_the_status_route_calls_not_installed() {
+        let dir = tempfile::tempdir().unwrap();
+        let missing = dir.path().join("codex");
+        let _env = command_pinned_to(CodingAgentKind::Codex, missing.to_str().unwrap());
+
+        let status = discovery::probe(CodingAgentKind::Codex).await;
+        assert_eq!(status.auth, discovery::AuthState::NotInstalled);
+        assert!(!check_provider_configured(
+            &metadata_for(CodingAgentKind::Codex),
+            ProviderType::Builtin
+        ));
+    }
+
+    /// The CLI requirement is the coding agents' alone. A provider with the very
+    /// same key shape — one required key with a default, which is how
+    /// `llamacpp` reports configured through `LLAMACPP_PORT` — is still judged
+    /// on the saved key and nothing else.
+    #[test]
+    fn the_cli_requirement_applies_to_the_coding_agents_only() {
+        let mut metadata = ProviderMetadata::empty();
+        metadata.name = "same_shape_as_a_coding_agent_f6".to_string();
+        metadata.config_keys = vec![ConfigKey::new("F6_SAME_SHAPE_PORT", true, false, Some("1"))];
+        let _env = env_lock::lock_env([("F6_SAME_SHAPE_PORT", Some("11543"))]);
+
+        assert_eq!(
+            provider_readiness(&metadata, ProviderType::Builtin),
+            ProviderReadiness::Configured
+        );
+    }
+}