feat(workflow): keep push credentials fresh across long ACP turns

lib/crates/fabro-agent/src/lib.rs

@@ -55,8 +55,8 @@ pub use question_tools::{
 };
 pub use read_before_write_sandbox::ReadBeforeWriteSandbox;
 pub use sandbox::{
-    CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox,
-    SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
+    CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, RefreshOutcome,
+    Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
     WorktreeEvent, WorktreeEventCallback, WorktreeOptions, WorktreeSandbox, format_lines_numbered,
     shell_quote,
 };

lib/crates/fabro-agent/src/sandbox.rs

@@ -2,8 +2,8 @@
 // Re-export the delegate_sandbox! macro at crate root so existing
 // `crate::delegate_sandbox!` invocations continue to work.
 pub use fabro_sandbox::{
-    CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox,
-    SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
+    CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, RefreshOutcome,
+    Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
     StdioProcessTermination, WorktreeEvent, WorktreeEventCallback, WorktreeOptions,
     WorktreeSandbox, delegate_sandbox, format_lines_numbered, shell_quote,
 };

lib/crates/fabro-sandbox/src/daytona/mod.rs

@@ -26,7 +26,7 @@ use tokio_util::sync::CancellationToken;
 
 use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
 use crate::redact::redact_auth_url;
-use crate::sandbox::{optional_timeout, resolve_path};
+use crate::sandbox::{RefreshOutcome, optional_timeout, resolve_path};
 use crate::{
     CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox,
     SandboxEvent, SandboxEventCallback, StdioProcess, glob_match, managed_labels, shell_quote,
@@ -1357,16 +1357,22 @@ impl Sandbox for DaytonaSandbox {
         Ok(Some((preview.url, headers)))
     }
 
-    async fn refresh_push_credentials(&self) -> crate::Result<()> {
+    async fn refresh_push_credentials(&self) -> crate::Result<RefreshOutcome> {
         if !self.repo_cloned() {
-            return Ok(());
+            return Ok(RefreshOutcome::Skipped);
         }
         let Some(origin_url) = self.origin_url.get() else {
-            return Ok(()); // no authenticated origin — nothing to refresh
+            return Ok(RefreshOutcome::Skipped); // no authenticated origin — nothing to refresh
         };
         let Some(creds) = &self.github_app else {
-            return Ok(());
+            return Ok(RefreshOutcome::Skipped);
         };
+        // Only a GitHub App installation token can be re-minted; a static PAT or
+        // a pre-minted Installation token is fixed, so re-embedding it changes
+        // nothing. Short-circuit to Skipped before the resolve + set-url exec.
+        if !matches!(creds, GitHubCredentials::App(_)) {
+            return Ok(RefreshOutcome::Skipped);
+        }
 
         let auth_url = fabro_github::resolve_authenticated_url(
             &fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
@@ -1394,7 +1400,9 @@ impl Sandbox for DaytonaSandbox {
             ));
         }
 
-        Ok(())
+        // Static creds were short-circuited to Skipped above; reaching here means
+        // a GitHub App installation token was freshly minted.
+        Ok(RefreshOutcome::Refreshed)
     }
 
     async fn set_autostop_interval(&self, minutes: i32) -> crate::Result<()> {

lib/crates/fabro-sandbox/src/docker.rs

@@ -28,7 +28,7 @@ use tokio_util::sync::CancellationToken;
 use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
 use crate::managed_labels::{self, MANAGED_LABEL, RUN_ID_LABEL};
 use crate::redact::redact_auth_url;
-use crate::sandbox::{StdioProcessControl, optional_timeout, resolve_path};
+use crate::sandbox::{RefreshOutcome, StdioProcessControl, optional_timeout, resolve_path};
 use crate::{
     CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
     ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector,
@@ -1952,16 +1952,22 @@ impl Sandbox for DockerSandbox {
         self.origin_url.get().map(String::as_str)
     }
 
-    async fn refresh_push_credentials(&self) -> crate::Result<()> {
+    async fn refresh_push_credentials(&self) -> crate::Result<RefreshOutcome> {
         if !self.repo_cloned() {
-            return Ok(());
+            return Ok(RefreshOutcome::Skipped);
         }
         let Some(origin_url) = self.origin_url.get() else {
-            return Ok(());
+            return Ok(RefreshOutcome::Skipped);
         };
         let Some(creds) = &self.github_app else {
-            return Ok(());
+            return Ok(RefreshOutcome::Skipped);
         };
+        // Only a GitHub App installation token can be re-minted; a static PAT or
+        // a pre-minted Installation token is fixed, so re-embedding it changes
+        // nothing. Short-circuit to Skipped before the resolve + set-url exec.
+        if !matches!(creds, GitHubCredentials::App(_)) {
+            return Ok(RefreshOutcome::Skipped);
+        }
 
         let auth_url = fabro_github::resolve_authenticated_url(
             &fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()),
@@ -1986,7 +1992,9 @@ impl Sandbox for DockerSandbox {
             ));
         }
 
-        Ok(())
+        // Static creds were short-circuited to Skipped above; reaching here means
+        // a GitHub App installation token was freshly minted.
+        Ok(RefreshOutcome::Refreshed)
     }
 }
 

lib/crates/fabro-sandbox/src/lib.rs

@@ -55,8 +55,8 @@ pub use read_guard::ReadBeforeWriteSandbox;
 pub use reconnect::{reconnect, reconnect_for_run, reconnect_for_run_with_callback};
 pub use sandbox::{
     CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
-    ExecStreamingResult, GitRunInfo, GitSetupIntent, GrepOptions, Sandbox, SandboxEvent,
-    SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
+    ExecStreamingResult, GitRunInfo, GitSetupIntent, GrepOptions, RefreshOutcome, Sandbox,
+    SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,
     StdioProcessTermination, format_lines_numbered, git_push_via_exec, redacted_output_tail,
     setup_git_via_exec, shell_quote,
 };

lib/crates/fabro-sandbox/src/sandbox.rs

@@ -197,7 +197,7 @@ macro_rules! delegate_sandbox {
                 self.$field.snapshot_info()
             }
 
-            async fn refresh_push_credentials(&self) -> $crate::Result<()> {
+            async fn refresh_push_credentials(&self) -> $crate::Result<$crate::RefreshOutcome> {
                 self.$field.refresh_push_credentials().await
             }
 
@@ -812,6 +812,18 @@ pub struct GrepOptions {
     pub max_results:      Option<usize>,
 }
 
+/// Outcome of [`Sandbox::refresh_push_credentials`]: whether a fresh token was
+/// actually minted and applied to the origin remote, or the call was a no-op
+/// (no clone, no authenticated origin, or no GitHub App credentials to rotate).
+/// Lets callers log accurately instead of assuming every `Ok` re-minted.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum RefreshOutcome {
+    /// A fresh token was minted and the origin remote URL was updated.
+    Refreshed,
+    /// Nothing to refresh (no clone / no origin / no managed credentials).
+    Skipped,
+}
+
 #[async_trait]
 pub trait Sandbox: Send + Sync {
     async fn read_file_bytes(&self, path: &str) -> crate::Result<Vec<u8>>;
@@ -960,10 +972,11 @@ pub trait Sandbox: Send + Sync {
     }
 
     /// Refresh git push credentials (e.g. rotate an expiring GitHub App token).
-    /// Default is a no-op; Daytona overrides to update the remote URL with a
-    /// fresh token.
-    async fn refresh_push_credentials(&self) -> crate::Result<()> {
-        Ok(())
+    /// Default is a no-op; Docker/Daytona override to update the remote URL
+    /// with a fresh token. Returns [`RefreshOutcome`] so callers can tell
+    /// an actual re-mint from a skipped no-op.
+    async fn refresh_push_credentials(&self) -> crate::Result<RefreshOutcome> {
+        Ok(RefreshOutcome::Skipped)
     }
 
     /// Set the auto-stop interval in minutes (0 to disable).

lib/crates/fabro-sandbox/src/worktree.rs

@@ -361,7 +361,7 @@ impl Sandbox for WorktreeSandbox {
         self.inner.sandbox_info()
     }
 
-    async fn refresh_push_credentials(&self) -> crate::Result<()> {
+    async fn refresh_push_credentials(&self) -> crate::Result<crate::RefreshOutcome> {
         self.inner.refresh_push_credentials().await
     }
 

lib/crates/fabro-server/src/spawn_env.rs

@@ -13,6 +13,11 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[
     EnvVars::FABRO_LOG,
     EnvVars::FABRO_HOME,
     EnvVars::FABRO_STORAGE_ROOT,
+    // Push-credential refresh-ahead tunables (FABRO_PUSH_CRED_REFRESH_*).
+    // `run_turn` executes in the worker, so these must survive `env_clear()` to
+    // reach the refresh-ahead loop in the ACP handler.
+    EnvVars::FABRO_PUSH_CRED_REFRESH_AHEAD,
+    EnvVars::FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS,
     EnvVars::TERM,
     EnvVars::NO_COLOR,
     EnvVars::CLICOLOR,
@@ -113,6 +118,11 @@ mod tests {
                 "FABRO_STORAGE_ROOT".to_string(),
                 "/tmp/fabro-storage".to_string(),
             ),
+            ("FABRO_PUSH_CRED_REFRESH_AHEAD".to_string(), "0".to_string()),
+            (
+                "FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS".to_string(),
+                "1800".to_string(),
+            ),
             ("TERM".to_string(), "xterm-256color".to_string()),
             ("NO_COLOR".to_string(), "1".to_string()),
             ("CLICOLOR".to_string(), "0".to_string()),
@@ -147,6 +157,20 @@ mod tests {
         assert_eq!(actual.get("PATH").map(String::as_str), Some("/bin"));
         assert_eq!(actual.get("HOME").map(String::as_str), Some("/tmp/home"));
         assert_eq!(actual.get("FABRO_LOG").map(String::as_str), Some("debug"));
+        // Push-credential refresh-ahead tunables must survive env_clear() into
+        // the worker so run_turn's refresh-ahead loop can read them.
+        assert_eq!(
+            actual
+                .get("FABRO_PUSH_CRED_REFRESH_AHEAD")
+                .map(String::as_str),
+            Some("0")
+        );
+        assert_eq!(
+            actual
+                .get("FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS")
+                .map(String::as_str),
+            Some("1800")
+        );
         assert_eq!(
             actual.get("TERM").map(String::as_str),
             Some("xterm-256color")

lib/crates/fabro-static/src/env_vars.rs

@@ -22,6 +22,9 @@ impl EnvVars {
     pub const FABRO_LOG: &'static str = "FABRO_LOG";
     pub const FABRO_LOG_DESTINATION: &'static str = "FABRO_LOG_DESTINATION";
     pub const FABRO_NO_UPGRADE_CHECK: &'static str = "FABRO_NO_UPGRADE_CHECK";
+    pub const FABRO_PUSH_CRED_REFRESH_AHEAD: &'static str = "FABRO_PUSH_CRED_REFRESH_AHEAD";
+    pub const FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS: &'static str =
+        "FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS";
     pub const FABRO_QUIET: &'static str = "FABRO_QUIET";
     pub const FABRO_SERVER: &'static str = "FABRO_SERVER";
     pub const FABRO_SERVER_MAX_CONCURRENT_RUNS: &'static str = "FABRO_SERVER_MAX_CONCURRENT_RUNS";
@@ -167,6 +170,8 @@ mod tests {
             EnvVars::FABRO_LOG,
             EnvVars::FABRO_LOG_DESTINATION,
             EnvVars::FABRO_NO_UPGRADE_CHECK,
+            EnvVars::FABRO_PUSH_CRED_REFRESH_AHEAD,
+            EnvVars::FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS,
             EnvVars::FABRO_QUIET,
             EnvVars::FABRO_SERVER,
             EnvVars::FABRO_SERVER_MAX_CONCURRENT_RUNS,