fix(credentials): retry transient Windows FS errors when persisting auth-profiles.json (#3355) (#3364)

src/openhuman/credentials/profiles.rs

@@ -1,4 +1,5 @@
 use crate::openhuman::keyring::SecretStore;
+use crate::openhuman::util::retry_with_backoff;
 use anyhow::{Context, Result};
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
@@ -9,6 +10,11 @@ use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::{Duration, Instant};
 
+#[cfg(test)]
+use std::sync::atomic::{AtomicUsize, Ordering};
+#[cfg(test)]
+use std::sync::Arc;
+
 const CURRENT_SCHEMA_VERSION: u32 = 1;
 
 /// Compact secret payload stored as a single keychain entry per auth profile.
@@ -51,6 +57,15 @@ const MALFORMED_LOCK_GRACE_MS: u64 = 2_000;
 /// and be reclaimed before surfacing a lock timeout to the caller.
 const LOCK_TIMEOUT_MS: u64 = STALE_LOCK_AGE_MS + 5_000;
 
+/// Retry budget for the JSON write + rename in `write_persisted_locked`.
+/// Same shape as the lock-create call at the bottom of `acquire_lock` (which
+/// is what closed Sentry OPENHUMAN-TAURI-H1 / H8 in #1641 / #2085). 6 attempts
+/// at base 100ms doubles up to ~6.3s worst-case before surfacing. Sized to
+/// stay well inside `LOCK_TIMEOUT_MS` so concurrent acquire_lock callers
+/// never time out behind a single retry-loop owner.
+const PERSIST_RETRY_ATTEMPTS: u32 = 6;
+const PERSIST_RETRY_BASE_MS: u64 = 100;
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub enum AuthProfileKind {
@@ -199,6 +214,13 @@ pub struct AuthProfilesStore {
     /// Whether the OS keychain is available on this machine.
     /// Cached at construction time to avoid repeated probes.
     use_keychain: bool,
+    /// `#[cfg(test)]` failure injection — when non-zero, the next retryable
+    /// FS call inside `write_persisted_locked` consumes one count and returns
+    /// a `__TEST_TRANSIENT__` error so the `is_transient_fs_error` classifier
+    /// treats it as retryable (`src/openhuman/util.rs:618`). Production
+    /// binaries never see this field.
+    #[cfg(test)]
+    force_transient_failures: Arc<AtomicUsize>,
 }
 
 impl AuthProfilesStore {
@@ -240,6 +262,8 @@ impl AuthProfilesStore {
             secret_store: SecretStore::new(state_dir, encrypt_secrets),
             user_id,
             use_keychain,
+            #[cfg(test)]
+            force_transient_failures: Arc::new(AtomicUsize::new(0)),
         }
     }
 
@@ -928,14 +952,41 @@ impl AuthProfilesStore {
         );
         let tmp_path = self.path.with_file_name(tmp_name);
 
-        fs::write(&tmp_path, &json).with_context(|| {
+        // Windows AV / Search-Indexer / Defender may briefly hold a handle on
+        // the destination, returning transient `ERROR_SHARING_VIOLATION (32)`,
+        // `ERROR_ACCESS_DENIED (5)`, or `ERROR_DELETE_PENDING (303)` —
+        // recognised as retryable by `is_transient_fs_error`. Mirror the
+        // lock-create retry budget at the bottom of `acquire_lock` so the
+        // JSON write+rename path absorbs the same transient family that
+        // closed Sentry OPENHUMAN-TAURI-H1 / H8 for the lock path. Outer
+        // `with_context` preserved so the Sentry fingerprint shape is stable
+        // across releases. (Sentry TAURI-RUST-92J / #3355.)
+        retry_with_backoff(
+            "write auth profile tmp",
+            PERSIST_RETRY_ATTEMPTS,
+            PERSIST_RETRY_BASE_MS,
+            || {
+                self.consume_test_transient_failure()?;
+                fs::write(&tmp_path, &json).context("write auth profile tmp")
+            },
+        )
+        .with_context(|| {
             format!(
                 "Failed to write temporary auth profile file at {}",
                 tmp_path.display()
             )
         })?;
 
-        fs::rename(&tmp_path, &self.path).with_context(|| {
+        retry_with_backoff(
+            "replace auth profile store",
+            PERSIST_RETRY_ATTEMPTS,
+            PERSIST_RETRY_BASE_MS,
+            || {
+                self.consume_test_transient_failure()?;
+                fs::rename(&tmp_path, &self.path).context("rename auth profile tmp -> store")
+            },
+        )
+        .with_context(|| {
             format!(
                 "Failed to replace auth profile store at {}",
                 self.path.display()
@@ -945,6 +996,42 @@ impl AuthProfilesStore {
         Ok(())
     }
 
+    /// Consume one test-injected transient FS failure if any are queued.
+    /// No-op in production builds.
+    #[cfg(test)]
+    fn consume_test_transient_failure(&self) -> Result<()> {
+        let remaining = self.force_transient_failures.load(Ordering::SeqCst);
+        if remaining == 0 {
+            return Ok(());
+        }
+        self.force_transient_failures.fetch_sub(1, Ordering::SeqCst);
+        Err(anyhow::anyhow!(
+            "__TEST_TRANSIENT__ injected transient FS failure"
+        ))
+    }
+
+    #[cfg(not(test))]
+    #[inline(always)]
+    fn consume_test_transient_failure(&self) -> Result<()> {
+        Ok(())
+    }
+
+    /// Queue `n` test-only forced transient FS failures. The next `n`
+    /// retryable calls inside `write_persisted_locked` return a
+    /// `__TEST_TRANSIENT__` error before the underlying FS op runs; the
+    /// retry helper treats them as retryable.
+    #[cfg(test)]
+    pub(super) fn force_next_transient_failures(&self, n: usize) {
+        self.force_transient_failures.store(n, Ordering::SeqCst);
+    }
+
+    /// Test introspection: how many forced transient failures are still
+    /// queued. Lets tests verify the retry helper drained the queue.
+    #[cfg(test)]
+    pub(super) fn remaining_forced_failures(&self) -> usize {
+        self.force_transient_failures.load(Ordering::SeqCst)
+    }
+
     fn encrypt_optional(&self, value: Option<&str>) -> Result<Option<String>> {
         match value {
             Some(value) if !value.is_empty() => self.secret_store.encrypt(value).map(Some),

src/openhuman/credentials/profiles_tests.rs

@@ -689,3 +689,102 @@ fn auth_profile_kind_serde_roundtrip() {
     let json = serde_json::to_string(&AuthProfileKind::Token).unwrap();
     assert_eq!(json, "\"token\"");
 }
+
+// ── Regression coverage for Sentry TAURI-RUST-92J / #3355 ─────────────────
+//
+// `write_persisted_locked` retries transient Windows FS errors
+// (`is_transient_fs_error` family — `ERROR_SHARING_VIOLATION` (32),
+// `ERROR_ACCESS_DENIED` (5), `ERROR_DELETE_PENDING` (303), etc.) via
+// `retry_with_backoff`. Matches the sibling `.lock`-create retry that
+// already closed OPENHUMAN-TAURI-H1 / H8 — the JSON `fs::write` +
+// `fs::rename` path was the missing partial.
+//
+// `force_next_transient_failures` is the `#[cfg(test)]`-only injection point
+// — it consumes one queued failure per retry attempt and returns an error
+// whose chain contains `__TEST_TRANSIENT__`, which `is_transient_fs_error`
+// recognises as retryable on every platform (see `src/openhuman/util.rs`).
+
+#[test]
+fn write_persisted_locked_retries_one_shot_transient() {
+    let tmp = TempDir::new().unwrap();
+    let store = AuthProfilesStore::new(tmp.path(), false);
+
+    // Queue one forced transient FS failure — the first retry attempt
+    // returns `__TEST_TRANSIENT__`, the second runs the real `fs::write` and
+    // succeeds. `upsert_profile` therefore returns Ok and the queue drains.
+    store.force_next_transient_failures(1);
+
+    let profile = AuthProfile::new_token("anthropic", "default", "tok-1".into());
+    store
+        .upsert_profile(profile.clone(), true)
+        .expect("retry should absorb the single transient failure");
+
+    assert_eq!(
+        store.remaining_forced_failures(),
+        0,
+        "retry helper must have consumed the queued forced failure"
+    );
+
+    // Round-trip the profile to prove the store wrote real bytes after the retry.
+    let data = store.load().unwrap();
+    assert!(data.profiles.contains_key(&profile.id));
+}
+
+#[test]
+fn write_persisted_locked_absorbs_burst_of_transients() {
+    let tmp = TempDir::new().unwrap();
+    let store = AuthProfilesStore::new(tmp.path(), false);
+
+    // Queue 5 forced transient failures — fewer than the retry budget
+    // (PERSIST_RETRY_ATTEMPTS = 6) so the 6th attempt succeeds. Covers the
+    // common "AV holds destination for a few hundred ms" case which was the
+    // root cause of TAURI-RUST-92J — the file genuinely lands on disk after
+    // the helper waits out the transient.
+    store.force_next_transient_failures(5);
+
+    let profile = AuthProfile::new_token("anthropic", "default", "tok-burst".into());
+    store
+        .upsert_profile(profile.clone(), true)
+        .expect("retry must absorb a burst of transient failures within budget");
+
+    assert_eq!(
+        store.remaining_forced_failures(),
+        0,
+        "retry helper must drain every queued failure before succeeding"
+    );
+
+    let data = store.load().unwrap();
+    let loaded = data
+        .profiles
+        .get(&profile.id)
+        .expect("profile must round-trip after retry");
+    assert_eq!(loaded.token.as_deref(), Some("tok-burst"));
+}
+
+#[test]
+fn write_persisted_locked_exhausts_retries_on_persistent_transient() {
+    let tmp = TempDir::new().unwrap();
+    let store = AuthProfilesStore::new(tmp.path(), false);
+
+    // Queue more forced failures than the retry budget for the write stage
+    // (PERSIST_RETRY_ATTEMPTS = 6) — every retry returns the test sentinel,
+    // so `retry_with_backoff` ultimately surfaces the failed-after-N-attempts
+    // error. Genuinely unrecoverable failures still surface to Sentry as
+    // honest signal; this is not a noise-suppression layer.
+    store.force_next_transient_failures(6);
+
+    let profile = AuthProfile::new_token("anthropic", "default", "tok-2".into());
+    let err = store
+        .upsert_profile(profile, true)
+        .expect_err("persistent transient must exhaust retries and surface as Err");
+
+    let chain = format!("{err:?}");
+    assert!(
+        chain.contains("Failed to write temporary auth profile file"),
+        "outer with_context must be preserved for Sentry fingerprint stability: {chain}"
+    );
+    assert!(
+        chain.contains("write auth profile tmp failed after"),
+        "retry helper must annotate the exhausted attempts count: {chain}"
+    );
+}