fix(update): retry sanity-exec on ETXTBSY — deflake coverage and harden self-update against the fork/exec fd race (#140)

* test(update): regression test for the ETXTBSY sanity-exec race (RED)

Reproduces the coverage-job flake deterministically: a write fd held open
on the staged binary while sanity_exec runs — the shape a sibling
thread's fork() leaves behind via fd inheritance — makes the exec fail
with "Text file busy" (Linux enforces ETXTBSY; the test is linux-gated).

Also tightens the existing strictness test to assert each rejection's
REASON instead of bare is_err(), which previously let an ETXTBSY spawn
failure masquerade as the expected rejection one line before the flake
surfaced.

This commit is intentionally pushed without the fix so CI demonstrates
the failure; the follow-up commit makes it pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(update): retry sanity-exec on ETXTBSY (fork/exec fd-inheritance race)

Between a sibling thread's fork() and its exec(), the child briefly
inherits every open fd — including a write fd on the binary staged
moments ago — and exec'ing the file during that window fails with
"Text file busy". Retry the spawn on ErrorKind::ExecutableFileBusy
(10 attempts, 25 ms linear backoff, <=1.4 s worst case) instead of
failing a fully SHA-verified download; all other spawn errors still
fail immediately and the 10 s hang timeout applies per attempt. Same
dance Go's os/exec and cargo do.

Turns the previous commit's RED regression test green and deflakes the
coverage job (first bitten on PR #139: llvm-cov widens the race window,
which is why `test`/`test-release` never caught it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-core/src/update/download.rs

@@ -259,24 +259,45 @@ async fn sanity_exec(
     expected: &semver::Version,
     strict: bool,
 ) -> Result<Option<String>, UpdateError> {
-    let mut cmd = tokio::process::Command::new(staged);
-    cmd.arg("--version")
-        .stdin(std::process::Stdio::null())
-        .stdout(std::process::Stdio::piped())
-        .stderr(std::process::Stdio::null())
-        .kill_on_drop(true);
-    let output = tokio::time::timeout(std::time::Duration::from_secs(10), cmd.output())
-        .await
-        .map_err(|_| {
-            UpdateError::VerifyFailed(
-                "downloaded binary hung during its --version self-check".to_string(),
-            )
-        })?
-        .map_err(|e| {
-            UpdateError::VerifyFailed(format!(
-                "downloaded binary failed to execute (wrong architecture?): {e}"
-            ))
-        })?;
+    // ETXTBSY retry: between a sibling thread's fork() and its exec(), the
+    // child briefly inherits every open fd — including a write fd on the
+    // binary staged moments ago — and exec'ing the file during that window
+    // fails with "Text file busy". The window is real for any multi-threaded
+    // process (and bites the parallel test binary under coverage), so ride
+    // it out with short sleeps instead of failing a fully verified download
+    // — the same dance Go's os/exec and cargo do.
+    const ETXTBSY_ATTEMPTS: u64 = 10;
+    let mut attempt = 0u64;
+    let output = loop {
+        let mut cmd = tokio::process::Command::new(staged);
+        cmd.arg("--version")
+            .stdin(std::process::Stdio::null())
+            .stdout(std::process::Stdio::piped())
+            .stderr(std::process::Stdio::null())
+            .kill_on_drop(true);
+        let result = tokio::time::timeout(std::time::Duration::from_secs(10), cmd.output())
+            .await
+            .map_err(|_| {
+                UpdateError::VerifyFailed(
+                    "downloaded binary hung during its --version self-check".to_string(),
+                )
+            })?;
+        match result {
+            Ok(output) => break output,
+            Err(e)
+                if e.kind() == std::io::ErrorKind::ExecutableFileBusy
+                    && attempt < ETXTBSY_ATTEMPTS =>
+            {
+                attempt += 1;
+                tokio::time::sleep(std::time::Duration::from_millis(25 * attempt)).await;
+            }
+            Err(e) => {
+                return Err(UpdateError::VerifyFailed(format!(
+                    "downloaded binary failed to execute (wrong architecture?): {e}"
+                )));
+            }
+        }
+    };
     if !output.status.success() {
         return Err(UpdateError::VerifyFailed(format!(
             "downloaded binary's --version self-check exited with {}",
@@ -537,17 +558,23 @@ mod tests {
             path
         };
 
+        // Each rejection asserts the REASON, not just is_err(): an unrelated
+        // spawn failure (e.g. the ETXTBSY race covered by the test below)
+        // must not masquerade as the expected rejection.
         // Wrong program name: hard error in both modes.
         let imposter = write_script("imposter", "#!/bin/sh\necho other-tool 9.9.9\n");
-        assert!(sanity_exec(&imposter, &expected, false).await.is_err());
+        let err = sanity_exec(&imposter, &expected, false).await.unwrap_err();
+        assert!(err.to_string().contains("identifies as"), "got: {err}");
 
         // Non-zero exit: hard error.
         let failing = write_script("failing", "#!/bin/sh\necho socket-patch 9.9.9\nexit 3\n");
-        assert!(sanity_exec(&failing, &expected, true).await.is_err());
+        let err = sanity_exec(&failing, &expected, true).await.unwrap_err();
+        assert!(err.to_string().contains("exited with"), "got: {err}");
 
         // Version mismatch: fatal in strict mode, warning otherwise.
         let mismatched = write_script("mismatch", "#!/bin/sh\necho socket-patch 1.0.0\n");
-        assert!(sanity_exec(&mismatched, &expected, true).await.is_err());
+        let err = sanity_exec(&mismatched, &expected, true).await.unwrap_err();
+        assert!(err.to_string().contains("instead of version"), "got: {err}");
         let warning = sanity_exec(&mismatched, &expected, false).await.unwrap();
         assert!(warning.unwrap().contains("1.0.0"));
 
@@ -558,6 +585,34 @@ mod tests {
         // Exec-format failure (not executable at all): hard error.
         let garbage = tmp.path().join("garbage");
         std::fs::write(&garbage, b"\x00\x01\x02").unwrap();
-        assert!(sanity_exec(&garbage, &expected, false).await.is_err());
+        let err = sanity_exec(&garbage, &expected, false).await.unwrap_err();
+        assert!(err.to_string().contains("failed to execute"), "got: {err}");
+    }
+
+    // Regression test for the coverage-job flake: between a sibling thread's
+    // fork() and its exec(), the child inherits every open fd — including a
+    // write fd on the just-staged binary — and exec'ing the binary during
+    // that window fails with ETXTBSY ("Text file busy"). Simulate the
+    // inherited fd with a write handle held open briefly on another thread;
+    // sanity_exec must ride it out instead of failing a verified download.
+    // Linux-only: other platforms don't reliably enforce ETXTBSY.
+    #[cfg(target_os = "linux")]
+    #[tokio::test]
+    async fn sanity_exec_retries_when_binary_briefly_text_busy() {
+        use std::os::unix::fs::PermissionsExt;
+        let tmp = tempfile::tempdir().unwrap();
+        let path = tmp.path().join("busy");
+        std::fs::write(&path, "#!/bin/sh\necho socket-patch 9.9.9\n").unwrap();
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();
+
+        let held = std::fs::OpenOptions::new().append(true).open(&path).unwrap();
+        let dropper = std::thread::spawn(move || {
+            std::thread::sleep(std::time::Duration::from_millis(150));
+            drop(held);
+        });
+
+        let result = sanity_exec(&path, &semver::Version::new(9, 9, 9), true).await;
+        dropper.join().unwrap();
+        assert_eq!(result.unwrap(), None);
     }
 }