Repository navigation
fix(mount): request raw tar on github working-tree seed - #537
Conversation
The tar seed pins gzip=1, which only affects server-GENERATED tars — the retained source archive is served from R2 either way. Generated tars over 128 MiB (gzipped or not) are held to the buffered export ceiling server-side, so the cloud repo's ~174 MiB working tree 413s on the base-snapshot and merged-manifest paths — including the ACL deny fall-through — and the mount drops to ~6k bulk reads that cannot fit the fleet readiness deadline. Request gzip=0: raw tar streams under the multi-GiB ceiling and skips the CompressionStream CPU the buffered ceiling exists to protect. The client already sniffs Content-Type for its gzip reader, so the stored archive path is unaffected.
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 configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe GitHub working-tree seed request now sets ChangesGitHub tar seed
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The seed request retains the source-archive setting while selecting raw tar, and the supplied test context covers the request and response handling. No actionable merge risk is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 checks the tar’s new way, Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
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. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/mountsync/syncer.go">
<violation number="1" location="internal/mountsync/syncer.go:6795">
P2: This sends `gzip=0` together with `sourceArchive=1`, a combination no code path in this repo exercises today. `ExportGithubWorkingTreeTar` only emits `gzip=0` when `!seed.Gzip`, and the existing contract test `TestHTTPClientExportGithubWorkingTreeTarRequestsSourceArchive` (internal/mountsync/http_client_test.go) asserts the opposite convention for source archives: a retained archive request carries no `gzip` parameter so the R2 object keeps its native gzip encoding. The comment claims the server ignores `gzip` for retained R2 archives, but that behavior ships in relayfile-cloud#312, which this PR says is still pending deployment. Against the currently deployed server, which has only ever seen source-archive requests without a `gzip` param (this call previously used `Gzip: true`), a large repo with a retained archive could be served the generated-tar path or a 413 — exactly the regression this flip is meant to cure. The local tar reader does adapt via Content-Type detection, so the risk is confined to server semantics, but the fix's correctness is unverified and untested: the raw-tar test uses `Gzip: false` with `SourceArchive` unset, so the shipped `Gzip: false, SourceArchive: true` request shape is never covered. Confirm the server ignores `gzip=0` when `sourceArchive=1` (once #312 deploys) and add a test asserting the combined request/response contract before relying on this path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // fall-through) and drops to per-file pulls. Raw tar gets the | ||
| // multi-GiB streaming ceiling and skips the CompressionStream | ||
| // CPU the ceiling exists to protect. | ||
| Gzip: false, |
There was a problem hiding this comment.
P2: This sends gzip=0 together with sourceArchive=1, a combination no code path in this repo exercises today. ExportGithubWorkingTreeTar only emits gzip=0 when !seed.Gzip, and the existing contract test TestHTTPClientExportGithubWorkingTreeTarRequestsSourceArchive (internal/mountsync/http_client_test.go) asserts the opposite convention for source archives: a retained archive request carries no gzip parameter so the R2 object keeps its native gzip encoding. The comment claims the server ignores gzip for retained R2 archives, but that behavior ships in relayfile-cloud#312, which this PR says is still pending deployment. Against the currently deployed server, which has only ever seen source-archive requests without a gzip param (this call previously used Gzip: true), a large repo with a retained archive could be served the generated-tar path or a 413 — exactly the regression this flip is meant to cure. The local tar reader does adapt via Content-Type detection, so the risk is confined to server semantics, but the fix's correctness is unverified and untested: the raw-tar test uses Gzip: false with SourceArchive unset, so the shipped Gzip: false, SourceArchive: true request shape is never covered. Confirm the server ignores gzip=0 when sourceArchive=1 (once #312 deploys) and add a test asserting the combined request/response contract before relying on this path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At internal/mountsync/syncer.go, line 6795:
<comment>This sends `gzip=0` together with `sourceArchive=1`, a combination no code path in this repo exercises today. `ExportGithubWorkingTreeTar` only emits `gzip=0` when `!seed.Gzip`, and the existing contract test `TestHTTPClientExportGithubWorkingTreeTarRequestsSourceArchive` (internal/mountsync/http_client_test.go) asserts the opposite convention for source archives: a retained archive request carries no `gzip` parameter so the R2 object keeps its native gzip encoding. The comment claims the server ignores `gzip` for retained R2 archives, but that behavior ships in relayfile-cloud#312, which this PR says is still pending deployment. Against the currently deployed server, which has only ever seen source-archive requests without a `gzip` param (this call previously used `Gzip: true`), a large repo with a retained archive could be served the generated-tar path or a 413 — exactly the regression this flip is meant to cure. The local tar reader does adapt via Content-Type detection, so the risk is confined to server semantics, but the fix's correctness is unverified and untested: the raw-tar test uses `Gzip: false` with `SourceArchive` unset, so the shipped `Gzip: false, SourceArchive: true` request shape is never covered. Confirm the server ignores `gzip=0` when `sourceArchive=1` (once #312 deploys) and add a test asserting the combined request/response contract before relying on this path.</comment>
<file context>
@@ -6784,7 +6784,15 @@ func (s *Syncer) pullRemoteFullGithubTarSeed(ctx context.Context, client githubW
+ // fall-through) and drops to per-file pulls. Raw tar gets the
+ // multi-GiB streaming ceiling and skips the CompressionStream
+ // CPU the ceiling exists to protect.
+ Gzip: false,
SourceArchive: true,
})
</file context>
1 participant
Summary
gzip=0(raw tar) instead of pinninggzip=1.Content-Typefor its gzip reader.Why
Generated tars are held to the server's 128 MiB buffered export ceiling when gzipped — the ceiling exists to protect CompressionStream CPU. Repos past that size (the cloud workspace's working tree is ~174 MiB) 413 on every generated-tar path, including the ACL-deny fall-through to the merged manifest — so the mount drops to ~6k bulk-read fetches that cannot fit the fleet mount readiness deadline. Observed live after relayfile-cloud#312's fall-through fix would otherwise let the request through.
Raw tar streams under the multi-GiB ceiling with no per-byte CPU cost.
Test plan
internal/mountsyncsuite green (77s) — seed test updated to assertSourceArchive+ raw targo vet,gofmtcleanGenerated with Devin
Note
Medium Risk
Changes bulk seeding behavior for large GitHub-backed mounts; wrong encoding handling could break sync, but the client already sniffs content type and the change targets a known production failure mode.
Overview
GitHub working-tree tar seeding now requests raw (uncompressed) tar (
Gzip: false) instead of server-generated gzip when callingExportGithubWorkingTreeTarwithSourceArchive.That avoids the server’s 128 MiB buffered ceiling on gzipped generated exports, which was causing 413 responses for large working trees (~174 MiB) and forcing a fallback to thousands of per-file pulls that miss mount readiness deadlines. Raw tar uses the multi-GiB streaming path without
CompressionStreamCPU cost; retained R2 source archives are unchanged.The reconcile seed test now asserts
SourceArchivewithGzip: false.Reviewed by Cursor Bugbot for commit e1a9ac0. Bugbot is set up for automated code reviews on this repo. Configure here.