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
9 changes: 7 additions & 2 deletions internal/mountsync/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6565,9 +6565,14 @@ func (s *Syncer) pullRemoteFull(ctx context.Context, conflicted map[string]struc
if client, ok := s.client.(githubWorkingTreeTarClient); ok {
used, err := s.pullRemoteFullGithubTarSeed(ctx, client, conflicted, prog)
if used {
return err
if err == nil {
return nil
}
s.logf("github tar seed failed; falling back to resumable tree pull: %v", err)
seedErr = err
Comment on lines +6571 to +6572

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 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.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +6571 to +6572

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 Badge 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 👍 / 👎.

} else {
seedErr = err
Comment on lines +6571 to +6574

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

}
seedErr = err
}
if !s.state.BootstrapComplete {
s.logf("skipping atomic export for initial bootstrap; using bounded resumable tree pull")
Expand Down
75 changes: 75 additions & 0 deletions internal/mountsync/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3001,6 +3001,81 @@ func TestPullRemoteFullGithubWorkingTreeTarSeedsAndStoresCursor(t *testing.T) {
}
}

func TestPullRemoteFullGithubWorkingTreeTarSeedFallsBackToTree(t *testing.T) {
localDir := t.TempDir()
contentsRoot := "/github/repos/AgentWorkforce/cloud/contents"
headSHA := "head123"
readme := []byte("# Cloud\n")
app := []byte("export const ok = true;\n")
readmeRemote := contentsRoot + "/README.md@" + headSHA + ".json"
appRemote := contentsRoot + "/src/app.ts@" + headSHA + ".json"
sentinelPath := "/github/repos/AgentWorkforce/cloud/.relayfile/clone.json"
client := &fakeExportClient{
fakeClient: &fakeClient{
files: map[string]RemoteFile{
sentinelPath: {
Path: sentinelPath,
Revision: "rev_1",
ContentType: "application/json",
Content: `{"headSha":"` + headSHA + `","defaultBranch":"main","sourceProfile":"complete-v1","filesExpected":2}`,
},
readmeRemote: {
Path: readmeRemote,
Revision: "rev_2",
ContentType: "application/json",
Content: string(readme),
ContentHash: hashBytes(readme),
},
appRemote: {
Path: appRemote,
Revision: "rev_3",
ContentType: "application/json",
Content: string(app),
ContentHash: hashBytes(app),
},
},
events: []FilesystemEvent{
{EventID: "evt_1", Type: "file.created", Path: readmeRemote, Revision: "rev_2", ContentHash: hashBytes(readme)},
{EventID: "evt_2", Type: "file.updated", Path: sentinelPath, Revision: "rev_1"},
},
},
tarErr: errors.New("transient github tar seed export failure"),
}
syncer, err := NewSyncer(client, SyncerOptions{
WorkspaceID: "ws_tar_seed_fallback",
RemoteRoot: contentsRoot,
LocalRoot: localDir,
StateFile: filepath.Join(localDir, ".relayfile-mount-state.json"),
WebSocket: boolPtr(false),
FullPullEvery: -1,
})
if err != nil {
t.Fatalf("NewSyncer failed: %v", err)
}
if err := syncer.Reconcile(context.Background()); err != nil {
t.Fatalf("Reconcile should fall back to tree after tar seed failure: %v", err)
}
if client.tarCalls != 1 {
t.Fatalf("expected github tar export to be attempted once, got %d", client.tarCalls)
}
if client.listTreeCalls == 0 {
t.Fatal("expected fallback tree traversal after tar seed failure")
}
gotReadme, err := os.ReadFile(filepath.Join(localDir, "README.md"))
if err != nil {
t.Fatalf("read README from tree fallback: %v", err)
}
if !bytes.Equal(gotReadme, readme) {
t.Fatalf("unexpected README content from tree fallback: %q", string(gotReadme))
}
if got := syncer.state.EventsCursor; got != "evt_2" {
t.Fatalf("expected events cursor to advance after tree fallback, got %q", got)
}
if tracked := syncer.state.Files[appRemote]; tracked.Hash != hashBytes(app) || tracked.Revision != "rev_3" {
t.Fatalf("unexpected tracked app state after tree fallback: %+v", tracked)
}
}

type unsupportedGithubTreeClient struct{ *fakeClient }

func (c *unsupportedGithubTreeClient) ListTree(context.Context, string, string, int, string) (TreeResponse, error) {
Expand Down
Loading