fix(mount): fall back when GitHub tar seed fails - #514
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughA failed GitHub working-tree tar seed is now recoverable. The full pull logs and records the seed error, then uses the tree pull path. The test verifies repository materialization and cursor advancement. ChangesGitHub tar seed fallback
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit watched the tar seed fail Comment |
| s.logf("github tar seed failed; falling back to resumable tree pull: %v", err) | ||
| seedErr = err |
There was a problem hiding this comment.
🔴 Slow tar seeds defeat fallback
When the tar request exhausts the bootstrap watchdog, err arrives after ctx is canceled. pullRemoteFullTree reuses that context and fails immediately, so slow tar seeds still block initial materialization.
Learn more
The tar export is one atomic HTTP request with no progress callbacks. The bootstrap watchdog cancels its context when that request remains idle too long. The new fallback starts only after the tar call returns, but it passes the same canceled context to pullRemoteFullTree. Every tree request then fails without making progress, and the next reconcile repeats the tar attempt.
Example: A tar export stalls until the two-minute bootstrap watchdog cancels ctx. The export returns context deadline exceeded; fallback calls ListTree with that canceled context and fails immediately. The next initial-sync cycle starts the same sequence instead of hydrating through the tree API.
Recommended fix: Give ExportGithubWorkingTreeTar its own sub-deadline strictly below the active bootstrap watchdog, as pullRemoteFullExport does. Fall back only while the parent bootstrap context remains live; propagate parent cancellation directly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| s.logf("github tar seed failed; falling back to resumable tree pull: %v", err) | ||
| seedErr = err | ||
| } else { | ||
| seedErr = err |
There was a problem hiding this comment.
🟡 Completed mounts miss tree fallback
With BootstrapComplete true, a tar error runs pullRemoteFullExport before the tree fallback. A transient export error then aborts the cycle, even when ListTree remains healthy.
Learn more
A completed mount normally tries the generic snapshot export before the resumable tree traversal. The new tar failure path rejoins that sequence instead of selecting the promised tree fallback. pullRemoteFullExport returns used=true for ordinary HTTP failures, causing pullRemoteFull to return before pullRemoteFullTree runs.
Example: A periodic full pull gets HTTP 500 from the GitHub tar export and HTTP 500 from the generic export endpoint, while ListTree and ReadFile remain healthy. Reconciliation returns the generic export error without trying the tree traversal.
Recommended fix: When seedErr is non-nil, bypass pullRemoteFullExport and proceed directly to pullRemoteFullTree. Add coverage with an already-complete mount where both export endpoints fail but tree reads succeed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cd5e7c225
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| s.logf("github tar seed failed; falling back to resumable tree pull: %v", err) | ||
| seedErr = err |
There was a problem hiding this comment.
Skip atomic export after the tar seed fails
When an already-bootstrapped GitHub mount performs a periodic or forced full pull, this branch claims to fall back to the resumable tree, but BootstrapComplete remains true, so control next enters pullRemoteFullExport; if that export returns any ordinary error with used=true, line 6582 returns it and the tree traversal never runs. Because the production HTTPClient implements both export interfaces, an outage affecting both export variants still prevents the intended fallback for existing mounts; bypass the atomic export when seedErr != nil, or otherwise ensure its failure continues to pullRemoteFullTree.
Useful? React with 👍 / 👎.
Summary
Verification
Agent37 context
Production Agent37 public sandbox proof is failing during Relayfile initial sync with relayfile_mount_invoked_exited / initial_sync_process / exit 1. Prior sanitized evidence identified github_tar_seed on the same mount path; this keeps tar seed as an optimization instead of letting it abort initial repo materialization.
Note
Medium Risk
Changes initial/full GitHub mount hydration behavior, but only replaces a hard failure with a documented fallback to the established tree path.
Overview
GitHub full pulls no longer abort when the working-tree tar seed export fails. If tar seed was attempted and errors, the syncer logs the failure and continues with the existing resumable tree pull instead of returning immediately. The fast path is unchanged: a successful tar seed still finishes the full pull right away.
A new integration test simulates a transient tar export error and asserts reconciliation completes via tree listing, local files match remote content, and sync state (events cursor and tracked revisions) is updated correctly.
Reviewed by Cursor Bugbot for commit 5cd5e7c. Bugbot is set up for automated code reviews on this repo. Configure here.