Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion internal/mountsync/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6784,7 +6784,15 @@ func (s *Syncer) pullRemoteFullGithubTarSeed(ctx context.Context, client githubW
PathPrefix: s.githubWorkingTree.ContentsRoot,
HeadSHA: headSHA,
SourceProfile: manifest.SourceProfile,
Gzip: true,
// Raw tar, not gzip: the flag only affects server-generated
// tars — the retained source archive is served from R2 either
// way. A gzipped generated tar is held to the server's 128 MiB
// buffered ceiling, so any repo past that size 413s on the
// merged/base-snapshot paths (including the ACL deny
// 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

SourceArchive: true,
})
})
Expand Down
4 changes: 2 additions & 2 deletions internal/mountsync/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3107,8 +3107,8 @@ func TestPullRemoteFullGithubWorkingTreeTarSeedsAndStoresCursor(t *testing.T) {
if client.lastTarSeed.SourceProfile != "complete-v1" {
t.Fatalf("expected complete source profile on tar export, got %q", client.lastTarSeed.SourceProfile)
}
if !client.lastTarSeed.SourceArchive || !client.lastTarSeed.Gzip {
t.Fatalf("expected native gzip source archive request, got %+v", client.lastTarSeed)
if !client.lastTarSeed.SourceArchive || client.lastTarSeed.Gzip {
t.Fatalf("expected source archive request with raw (gzip=0) generated tar, got %+v", client.lastTarSeed)
}
gotReadme, err := os.ReadFile(filepath.Join(localDir, "README.md"))
if err != nil {
Expand Down
Loading