Retry the sandbox clone when GitHub token replication lags - #667
Conversation
A Daytona-backed run could fail five seconds after start when the sandbox git clone hit a transient GitHub "Repository not found" error. Clone-based providers mint an installation access token and clone with it in the same breath, but GitHub replicates a new token to its edge cache sites asynchronously. A clone that starts within a second of the mint can be rejected before the token is visible to the site serving it, and on a private repo that rejection arrives as "Repository not found" because GitHub answers unauthorized reads with 404. Nothing retried the clone, and the failure classified as `deterministic`, which is the one category `loop_restart` refuses to restart. An identical run relaunched 46 seconds later succeeded with no changes. A successful mint is what makes the message safe to retry. `resolve_clone_credentials` already fails loudly on every deterministic explanation for a clone 404: the installation lookup 404s when the App is not installed for the owner, and token creation 422s when the installation does not cover the repo. Once credentials are in hand, "not found" from the clone itself cannot mean "no access". Add `clone_retry` and use it from both clone-based providers: 3 attempts with 3s then 9s backoff, reusing the same token so replication keeps making progress instead of restarting the clock. Token-replication signatures retry only when credentials are present, so a public clone of a wrong URL still fails fast. Infrastructure failures retry either way. The Docker provider had the identical single-shot clone and is the default runtime provider, so it is covered too. Also fix two nearby issues found while reading the area: - The GitHub-URL-parse path in the Daytona clone skipped `fail_init`, unlike every sibling path, so `InitializeFailed` was never emitted. - The `classify_exec_failure` hint for "repository not found" asserted the App installation may not cover the repo. After a successful scoped mint that diagnosis is impossible, and it sent operators hunting a configuration problem that did not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Notes for whoever picks this upDraft because the core failure mode is not reproducible on demand — it depends on GitHub's internal replication timing. Everything here is reasoned from the event stream of one incident plus GitHub's own statements. Read the argument before trusting the fix. The load-bearing claimEverything rests on this: a successful mint rules out every deterministic reason for a clone 404. If that is wrong, this PR retries failures that will never succeed and adds 12s to every one of them. The argument is in Places I made a judgment call you may want to revisitBackoff of 3s → 9s (12s worst case). From GitHub Support's guidance in the Amplify thread (~3s, then 10s). No measurement behind it — we have exactly one data point, where a retry 46s later worked. If clone failures show up in telemetry still exhausting all three attempts, the window is too short and the right move is more attempts, not longer ones (replication only makes progress). Retrying
Strictly speaking this cleanup guards a different failure (clone killed mid-flight) than the one we are fixing — for the auth-race case git cleans up after itself. I included it because without it the retry can turn a retryable error into a confusing "destination path already exists", which would make the fix look broken. Reasonable to argue it belongs in its own PR. What I could not verify
Deliberately left alone
Reviewing efficientlyRead in this order:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a shared retry loop for the initial repository clone performed during sandbox initialization, addressing transient “repository not found” failures caused by GitHub installation-token replication lag (observed in Daytona, and applicable to Docker as well).
Changes:
- Introduces a shared
clone_retrymodule that retries clone operations (3 attempts with 3s/9s backoff) based on structured retry reasons (TransientInfravsTokenReplication). - Updates Docker and Daytona clone paths to use the shared retry loop and to clean up partial checkouts between attempts.
- Improves clone error classification and fixes a Daytona init failure path to emit
InitializeFailedviafail_init.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/components/fabro-sandbox/src/lib.rs | Wires in the new clone_retry module behind feature/test cfgs. |
| lib/components/fabro-sandbox/src/error.rs | Expands the “repository not found” exec hint to include token replication lag as a possible cause. |
| lib/components/fabro-sandbox/src/docker.rs | Wraps Docker clone execution in the shared retry loop and clears partial clones between attempts. |
| lib/components/fabro-sandbox/src/daytona/mod.rs | Wraps Daytona clone in the shared retry loop, adds retry classification, and fixes missing fail_init on a clone-credential error path. |
| lib/components/fabro-sandbox/src/clone_retry.rs | New shared retry/backoff + message-based classification with unit tests. |
Comments suppressed due to low confidence (1)
lib/components/fabro-sandbox/src/docker.rs:761
GitCloneFailednow emitserr.to_string()anderr.causes()directly. If the underlying Docker exec error (or its cause chain) includes the authenticated clone URL, this can leak the freshly minted GitHub installation token into sandbox events/logs. The git stderr path is redacted, but this error path is not.
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url,
error: err.to_string(),
causes: err.causes(),
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Pushed review cleanup in 4727ee8. Changes:
Local verification passed:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
lib/components/fabro-sandbox/src/docker.rs:766
retry_cloneretries the samegit clonecommand but doesn’t cleanlayout.primary_repo_pathbetween attempts. If the first attempt creates a partial checkout, the next attempt will likely fail with "destination path ... already exists" (classified as permanent), making the retry loop ineffective for mid-clone transient failures.
let command = git_clone_command(clone_url, branch.as_deref(), &layout.primary_repo_path);
let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT;
let clone_result = clone_retry::retry_clone(
SandboxProviderKind::Docker,
Some(clone_deadline),
lib/components/fabro-sandbox/src/daytona/mod.rs:1151
- The Daytona clone retry loop doesn’t appear to clean
layout.primary_repo_pathbetween attempts. If an attempt fails after creating a partial checkout, later attempts can fail immediately with a non-empty destination (e.g. "destination path ... already exists"), which would undermine the intended retry behavior for transient mid-clone failures.
let clone_result = clone_retry::retry_clone(
SandboxProviderKind::Daytona,
None,
|_attempt| {
let git_svc = &git_svc;
Problem
A Daytona-backed run failed five seconds after start when the sandbox
git clonehit a transient GitHubRepository not founderror. Run01KYM99DF27JRRW4XSYZBP27K7:sandbox.snapshot.ready(31 ms, cache hit)sandbox.git.startedsandbox.git.failed—["repository not found: Repository not found."]run.failed, categorydeterministicAn identical run relaunched 46 seconds later succeeded with no changes, and a different run cloned the same repo successfully 8 minutes earlier. The repo existed and the App installation covered it.
Three things combined:
Repository not foundbecause GitHub answers unauthorized reads with 404 rather than 403.git_svc.clone()was single-shot,initialize()is called exactly once, and node-level retry policies never apply because sandbox init happens before any node runs.deterministic."repository not found"matches nothing inTRANSIENT_INFRA_HINTS, andloop_restartis hard-blocked for every category exceptTransientInfra— so the one recovery mechanism that could have absorbed this was disabled for it.Why a post-mint 404 is provably transient
resolve_clone_credentialsalready fails loudly on every deterministic explanation for a clone 404:A mint failure surfaces as "Failed to get GitHub App credentials for clone", which is not what the failed run emitted. The mint succeeded, which rules out app-not-installed, repo-not-covered, and repo-does-not-exist. Whatever
Repository not foundmeans after a successful scoped mint, it is not deterministic.The fix
New
clone_retrymodule: 3 attempts, 3s then 9s backoff, reusing the same token. Replication of a given token only makes progress, so each attempt strictly improves the odds — re-minting would restart the clock. This matches GitHub Support's own guidance (see references).The retry condition splits two ways rather than gating everything on credentials:
TransientInfra(network, 5xx, rate limit) retries always.TokenReplication(repository not found,authentication failed) retries only when credentials are present. On a public clone, "not found" means the URL names a repo that is not there, so it still fails fast rather than burning 12s of backoff.The GitHub 404 does not arrive as an HTTP 404 on the Daytona API call — git runs inside the sandbox, so its stderr comes back through the toolbox as the error message. The credential race has to be matched on text; Daytona's own transport failures are visible structurally.
Scope: both clone-based providers
The bug report scoped this to Daytona, but the Docker provider had the identical single-shot clone-after-fresh-mint in
clone_github_repo, anddefaults.tomlmakes Docker the default runtime provider. Fixing only Daytona would have left most users exposed. Each provider supplies its own classifier; the retry loop and backoff are shared.Two additions beyond the report
git clonerefuses to write into a non-empty directory. Without this, the retry would convert a retryable failure into a confusing "destination path already exists". Best-effort, retries only.docs/internal/logging-strategy.mdprohibits raw command stderr in tracing. Each retry logsreason = token_replication,attempt, anddelay_ms; the full error still surfaces if attempts run out.Secondary issues fixed
fail_init: the GitHub-URL-parse path in the Daytona clone returned its error withoutself.fail_init(...), unlike every sibling path, soSandboxEvent::InitializeFailedwas never emitted for that case.classify_exec_failuremapped"repository not found"to "the App installation may not include this repo". After a successful scoped mint that diagnosis is impossible, and it sent operators hunting a configuration problem that did not exist.Deliberately not changed
Reclassifying
"repository not found"asTransientInfrainclassify_failure_reason. A blanket mapping would be wrong — without credentials it is genuinely deterministic. The signal that makes it transient is "a mint succeeded immediately before", which only the clone site knows. With retries in place, the classification of the exhausted case matters much less.Testing
01KYM99DF27JRRW4XSYZBP27K7.#[tokio::test(start_paused = true)], so 12s of backoff costs no wall-clock.cargo nextest run --workspace— 7413 passed.fmt --checkclean, andcargo build -p fabro-sandboxwith default features only confirms the new module's feature gating.Not verified against a live replication lag. The failure is timing-dependent and cannot be reproduced on demand, so the classifier is covered by unit tests against the recorded error string rather than by racing GitHub.
Nothing wraps
sandbox.initialize()in a timeout, so the added ~12s worst case cannot blow an init deadline.References
🤖 Generated with Claude Code