fix(auth): dual-write API key to keyring + config so stale keyring stops shadowing onboarding (#593)

Reproduction (from the user who filed #593, also the reporter of #586):

1. At any prior point, the user runs `deepseek auth set --provider deepseek`,
   which writes to the OS keyring under the `deepseek` slot.
2. The key is later rotated, the prior install is replaced, or the user
   moves to a different account.
3. The user opens the TUI, gets the in-TUI onboarding screen, and pastes
   their fresh API key.
4. `submit_api_key` → `save_api_key` writes only to `~/.deepseek/config.toml`.
5. At request time, `Secrets::resolve` follows the documented
   `keyring → env → config-file` precedence, and the **stale** keyring
   entry shadows the fresh config.toml value.
6. API call goes out with the dead key, gets a 401, the TUI shows
   "no response" with no obvious diagnostic.

The fix
=======

`save_api_key` now writes to **both** layers when a keyring backend is
reachable:

* The config file remains the durable, inspectable record of the
  active key (works in npm installs, IDE terminals, headless CI —
  everywhere). v0.8.8 made this the canonical location for a reason.
* The OS keyring entry is rewritten on every onboarding submit so a
  stale credential from a prior install is overwritten in place.

`SavedCredential` gains a new `KeyringAndConfigFile { backend, path }`
variant; the existing `ConfigFile(PathBuf)` variant remains the
fallback when no keyring backend is reachable (or under `cfg(test)`,
so the unit suite never pollutes the host keyring). The onboarding
toast naturally reports the actual outcome via
`SavedCredential::describe`, which now reads
`OS keyring (system keyring) and ~/.deepseek/config.toml` for the
common case.

`save_api_key_for` (the multi-provider entry point) is updated to
extract the path from either variant, so non-DeepSeek providers
(OpenRouter / Novita / Fireworks / NIM / SGLang) continue writing
provider-table entries to config.toml only, with no behavior change.

`deepseek doctor` warning
=========================

`run_doctor` now compares the keyring's `deepseek` slot against the
config file's `api_key` slot. When both are present and differ, the
report surfaces the discrepancy with copy-paste remediation —
`deepseek auth set --provider deepseek` rewrites both layers in one
shot, and the in-TUI onboarding now does the same. The check skips
keyring probes for other providers because they don't write to the
keyring today; probing absent slots only triggers macOS Always-Allow
prompts for nothing.

Why dual-write rather than keyring-only
=======================================

A previous attempt (`4e360274`, never merged to main) swapped the
write path to keyring-only. That hides the key from anyone who
expected to see it under `~/.deepseek/config.toml` and breaks the
"deepseek-tui works in every folder, in npm installs, in IDE
terminals" promise of v0.8.8. Dual-write keeps the inspectable copy
and adds the layered override that defeats stale-shadow without
changing the visible mental model.

Tests
=====

* `saved_credential_describe_lists_both_targets_for_keyring_and_config`
  pins the toast text shape so the user sees both targets after
  onboarding.
* The existing `save_api_key_writes_config_file_under_cfg_test` and
  `test_save_api_key_doesnt_match_similar_keys` continue to pass —
  under `cfg(test)` the keyring path is gated out, so the
  config-only outcome remains the test-time contract.

Verification
============

* `cargo fmt --all -- --check` clean.
* `cargo clippy -p deepseek-tui --bin deepseek-tui --all-features
  --locked -- -D warnings` clean.
* `cargo test -p deepseek-tui --bin deepseek-tui --locked` →
  2029 passed, 2 ignored.

Closes #593.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

crates/tui/src/config.rs

@@ -2162,7 +2162,24 @@ pub fn ensure_parent_dir(path: &Path) -> Result<()> {
 /// the caller can show a confirmation message without leaking the key.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum SavedCredential {
-    /// Stored in the deepseek config file at the given path.
+    /// Stored in **both** the OS keyring and the deepseek config file.
+    /// This is the default outcome on platforms with a working keyring
+    /// backend: writing both layers defeats the
+    /// `keyring → env → config-file` resolution-order shadow that
+    /// would otherwise let a stale OS-keyring entry from a previous
+    /// install hide the freshly-entered key (#593). The `backend`
+    /// label is the value of [`deepseek_secrets::Secrets::backend_name`]
+    /// at write time so the toast text can name the actual backend
+    /// (`"system keyring"`, `"file-based (~/.deepseek/secrets/)"`).
+    KeyringAndConfigFile {
+        /// `Secrets::backend_name()` at write time.
+        backend: String,
+        /// Absolute path to the config file that was also updated.
+        path: PathBuf,
+    },
+    /// Stored in the deepseek config file only. Fallback when no
+    /// keyring backend is reachable, or under `cfg(test)` so unit
+    /// tests don't pollute the host keyring.
     ConfigFile(PathBuf),
 }
 
@@ -2172,23 +2189,76 @@ impl SavedCredential {
     #[must_use]
     pub fn describe(&self) -> String {
         match self {
+            Self::KeyringAndConfigFile { backend, path } => {
+                format!("OS keyring ({backend}) and {}", path.display())
+            }
             Self::ConfigFile(path) => path.display().to_string(),
         }
     }
 }
 
-/// Save the active provider's API key to `~/.deepseek/config.toml`.
+/// Save the active provider's API key.
 ///
-/// v0.8.8 intentionally uses the shared config file as the default
-/// setup path. It works in every folder, in npm installs, in IDE
-/// terminals, and without platform credential prompts.
+/// **Dual-write strategy (#593):** writes to `~/.deepseek/config.toml`
+/// (always) and to the OS keyring via [`deepseek_secrets::Secrets`]
+/// (when a backend is reachable). The runtime resolves credentials in
+/// `keyring → env → config-file` order; writing to the config file
+/// alone — as v0.8.8 through v0.8.10 did — let a stale keyring entry
+/// from a prior install silently shadow the fresh value the user just
+/// typed during in-TUI onboarding, producing the "no response" symptom
+/// reported in #593.
+///
+/// The config file remains the inspectable durable record (works in
+/// npm installs, IDE terminals, and headless boxes alike), and the
+/// keyring acts as the layered override that defeats stale-shadow on
+/// the resolution path. When the keyring write fails (no backend, OS
+/// permission denied, etc.) the config-file write still stands and
+/// the function reports a [`SavedCredential::ConfigFile`] outcome —
+/// callers should not treat that as a failure.
+///
+/// Skipped under `cfg(test)` so the suite never touches the host
+/// keyring. The `secrets` crate has its own test coverage for
+/// keyring set/get.
 pub fn save_api_key(api_key: &str) -> Result<SavedCredential> {
     let trimmed = api_key.trim();
     if trimmed.is_empty() {
         anyhow::bail!("Refusing to save an empty API key.");
     }
 
+    // Always write the inspectable copy first. The config file is the
+    // durable record everyone — including macOS Keychain-prompted
+    // first-run, headless CI, and IDE terminals — can rely on.
     let path = save_api_key_to_config_file(trimmed)?;
+
+    // Then mirror to the OS keyring when one is reachable. This
+    // overwrites any stale entry from a prior install so
+    // `Secrets::resolve` (keyring → env → config-file) no longer
+    // shadows the fresh key. Skipped under `cfg(test)` so unit tests
+    // can't pollute the host keyring (macOS Always-Allow prompts,
+    // cross-test contamination).
+    #[cfg(not(test))]
+    {
+        let secrets = deepseek_secrets::Secrets::auto_detect();
+        match secrets.set("deepseek", trimmed) {
+            Ok(()) => {
+                let backend = secrets.backend_name().to_string();
+                log_sensitive_event(
+                    "credential.save",
+                    json!({
+                        "backend": backend.clone(),
+                        "config_path": path.display().to_string(),
+                        "dual_write": true,
+                    }),
+                );
+                return Ok(SavedCredential::KeyringAndConfigFile { backend, path });
+            }
+            Err(err) => {
+                tracing::warn!("OS keyring write failed; key saved to config.toml only: {err}");
+                // Fall through to the ConfigFile-only outcome below.
+            }
+        }
+    }
+
     Ok(SavedCredential::ConfigFile(path))
 }
 
@@ -2349,7 +2419,8 @@ pub fn has_api_key_for(config: &Config, provider: ApiProvider) -> bool {
 pub fn save_api_key_for(provider: ApiProvider, api_key: &str) -> Result<PathBuf> {
     if matches!(provider, ApiProvider::Deepseek | ApiProvider::DeepseekCN) {
         return match save_api_key(api_key)? {
-            SavedCredential::ConfigFile(path) => Ok(path),
+            SavedCredential::KeyringAndConfigFile { path, .. }
+            | SavedCredential::ConfigFile(path) => Ok(path),
         };
     }
 
@@ -2721,6 +2792,23 @@ mod tests {
         assert_eq!(cf.describe(), "/tmp/x.toml");
     }
 
+    /// #593: the dual-write outcome describes both targets so the
+    /// onboarding toast (`API key saved to {describe}`) tells the user
+    /// the key landed in *both* the keyring and the config file —
+    /// which is the whole point of the fix (defeats stale-keyring
+    /// shadow while keeping the config file inspectable).
+    #[test]
+    fn saved_credential_describe_lists_both_targets_for_keyring_and_config() {
+        let dual = SavedCredential::KeyringAndConfigFile {
+            backend: "system keyring".to_string(),
+            path: PathBuf::from("/tmp/x.toml"),
+        };
+        assert_eq!(
+            dual.describe(),
+            "OS keyring (system keyring) and /tmp/x.toml"
+        );
+    }
+
     #[test]
     fn has_api_key_detects_in_memory_override_and_env_var() -> Result<()> {
         // Pins the v0.8.8 contract: `has_api_key` covers the prompt-free

crates/tui/src/main.rs

@@ -1580,6 +1580,45 @@ async fn run_doctor(config: &Config, workspace: &Path, config_path_override: Opt
     }
     println!("  · credential sources: env, ~/.deepseek/config.toml");
 
+    // #593: surface keyring/config disagreement explicitly. The runtime
+    // resolution order is `keyring → env → config-file`, so a stale
+    // keyring entry from a prior install can shadow the value the user
+    // sees in `~/.deepseek/config.toml`. We only check the DeepSeek
+    // slot — other providers don't write to the keyring today, and
+    // probing entries that aren't there triggers macOS keychain
+    // prompts for nothing.
+    let secrets = deepseek_secrets::Secrets::auto_detect();
+    let keyring_key = secrets.get("deepseek").ok().flatten();
+    let config_key = config
+        .api_key
+        .as_ref()
+        .filter(|v| !v.trim().is_empty() && v.as_str() != "__KEYRING__")
+        .map(|s| s.to_string());
+    match (keyring_key.as_deref(), config_key.as_deref()) {
+        (Some(k), Some(c)) if k.trim() != c.trim() => {
+            println!();
+            println!(
+                "  {} `deepseek`: OS keyring and config.toml hold different values.",
+                "⚠".truecolor(red_r, red_g, red_b)
+            );
+            println!(
+                "    Resolution order is keyring → env → config-file, so the keyring value wins."
+            );
+            println!("    Reconcile by overwriting both with the current key:");
+            println!("        deepseek auth set --provider deepseek");
+            println!(
+                "    (Or paste the key into the in-TUI onboarding screen — it now writes both layers.)"
+            );
+        }
+        (Some(_), None) => {
+            println!(
+                "  {} `deepseek`: key is in OS keyring only (config.toml has no copy).",
+                "·".dimmed()
+            );
+        }
+        _ => {}
+    }
+
     let has_api_key = if config.deepseek_api_key().is_ok() {
         println!(
             "  {} active provider key resolved",