diff --git a/cmd/relayfile-mount/main.go b/cmd/relayfile-mount/main.go index 0912ddec..66a1e280 100644 --- a/cmd/relayfile-mount/main.go +++ b/cmd/relayfile-mount/main.go @@ -432,7 +432,7 @@ func runSinglePollingMount(rootCtx context.Context, cfg mountConfig) error { log.Printf("%s", mountStartupLogLine(cfg)) log.Printf("Mirror started at %s. Sync interval %s +/- %.0f%%. Public state: %s", cfg.localDir, cfg.interval.Round(time.Second), cfg.intervalJitter*100, filepath.Join(cfg.localDir, ".relay", "state.json")) - run := func(reconcile bool) { + run := func(reconcile bool) error { ctx, cancel := context.WithTimeout(rootCtx, cfg.timeout) defer cancel() var err error @@ -442,19 +442,30 @@ func runSinglePollingMount(rootCtx context.Context, cfg mountConfig) error { err = syncer.SyncOnce(ctx) } if err != nil { + var stalled *mountsync.BootstrapStalledError + if errors.As(err, &stalled) { + // This is an operator-actionable hard stop, not a transient + // cycle failure. Returning it terminates this runner (and, for + // scoped layouts, cancels sibling runners) instead of letting + // the polling ticker retry the same persisted checkpoint forever. + return err + } if errors.Is(err, context.DeadlineExceeded) { if synced, total, ok := readBootstrapProgress(cfg.localDir); ok { log.Printf("mount bootstrapping: %d/%d files (in progress)", synced, total) - return + return nil } } log.Printf("mount sync cycle failed: %v", err) - return + return nil } log.Printf("mount sync cycle completed") + return nil } - run(true) + if err := run(true); err != nil { + return err + } if cfg.once { return nil } @@ -503,7 +514,9 @@ func runSinglePollingMount(rootCtx context.Context, cfg mountConfig) error { cycle, ) if reconcile { - run(true) + if err := run(true); err != nil { + return err + } } timer.Reset(jitteredIntervalWithSample(cfg.interval, cfg.intervalJitter, rng.Float64())) } diff --git a/cmd/relayfile-mount/main_test.go b/cmd/relayfile-mount/main_test.go index 66dd43c1..7f37bd59 100644 --- a/cmd/relayfile-mount/main_test.go +++ b/cmd/relayfile-mount/main_test.go @@ -2,7 +2,9 @@ package main import ( "context" + "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "os" @@ -415,6 +417,142 @@ func TestInstallCredsFileRefreshToleratesParseFailureWithoutRetry(t *testing.T) } } +// TestRunSinglePollingMountStopsOnBootstrapStall proves the typed hard failure +// leaves the polling runner immediately. main turns this returned error into a +// nonzero process exit, so the ticker cannot retry the same checkpoint. +func TestRunSinglePollingMountStopsOnBootstrapStall(t *testing.T) { + t.Setenv("RELAYFILE_BOOTSTRAP_STALL_CYCLES", "1") + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + http.Error(w, "stuck", http.StatusBadGateway) + })) + defer server.Close() + + err := runSinglePollingMount(context.Background(), mountConfig{ + baseURL: server.URL, + token: "test-token", + workspaceID: "ws_bootstrap_stall", + remotePath: "/", + localDir: t.TempDir(), + stateDir: t.TempDir(), + mountKind: mountsync.MountKindDaemon, + syncMode: syncModeMirror, + interval: time.Hour, + timeout: time.Second, + websocketEnabled: false, + }) + var stalled *mountsync.BootstrapStalledError + if !errors.As(err, &stalled) { + t.Fatalf("expected bootstrap stall to escape polling runner, got %v", err) + } + if got := calls.Load(); got == 0 { + t.Fatal("expected initial full-tree request before runner exited") + } +} + +// TestRunSinglePollingMountKeepsNormalCycleFailuresNonFatal verifies that a +// normal cloud error keeps its historical per-cycle retry behavior rather +// than terminating the mount runner. +func TestRunSinglePollingMountKeepsNormalCycleFailuresNonFatal(t *testing.T) { + t.Setenv("RELAYFILE_BOOTSTRAP_STALL_CYCLES", "2") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "transient", http.StatusBadGateway) + })) + defer server.Close() + + err := runSinglePollingMount(context.Background(), mountConfig{ + baseURL: server.URL, + token: "test-token", + workspaceID: "ws_normal_retry", + remotePath: "/", + localDir: t.TempDir(), + stateDir: t.TempDir(), + mountKind: mountsync.MountKindDaemon, + syncMode: syncModeMirror, + interval: time.Hour, + timeout: time.Second, + websocketEnabled: false, + once: true, + }) + if err != nil { + t.Fatalf("normal cycle failure must remain nonfatal to the runner, got %v", err) + } +} + +// TestRunSinglePollingMountStopsOnTimerBootstrapStall exercises the polling +// timer path, not just the initial cycle. The first page commits a partial +// checkpoint and its next-page error remains nonfatal; the following timer +// reconcile fails at that unchanged cursor and terminates the runner. +func TestRunSinglePollingMountStopsOnTimerBootstrapStall(t *testing.T) { + t.Setenv("RELAYFILE_BOOTSTRAP_STALL_CYCLES", "1") + var rootTreeCalls atomic.Int32 + var cursorTreeCalls atomic.Int32 + var readCalls atomic.Int32 + entries := make([]mountsync.TreeEntry, 0, 10) + for i := 0; i < 10; i++ { + entries = append(entries, mountsync.TreeEntry{Path: fmt.Sprintf("/f/%05d.txt", i), Type: "file"}) + } + nextCursor := entries[len(entries)-1].Path + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/fs/tree"): + if r.URL.Query().Get("cursor") != "" { + cursorTreeCalls.Add(1) + // A 400 is intentionally not retried by HTTPClient, so exactly + // two calls prove the initial nonfatal cycle plus one timer cycle. + http.Error(w, "stuck cursor", http.StatusBadRequest) + return + } + rootTreeCalls.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(mountsync.TreeResponse{Entries: entries, NextCursor: &nextCursor}) + case strings.Contains(r.URL.Path, "/fs/file"): + readCalls.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(mountsync.RemoteFile{ + Path: r.URL.Query().Get("path"), + ContentType: "text/plain", + Content: "content", + }) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + started := time.Now() + err := runSinglePollingMount(context.Background(), mountConfig{ + baseURL: server.URL, + token: "test-token", + workspaceID: "ws_timer_bootstrap_stall", + remotePath: "/", + localDir: t.TempDir(), + stateDir: t.TempDir(), + mountKind: mountsync.MountKindDaemon, + syncMode: syncModeMirror, + interval: time.Millisecond, // enforced to the 5s poll floor + timeout: time.Second, + websocketEnabled: false, + }) + var stalled *mountsync.BootstrapStalledError + if !errors.As(err, &stalled) { + t.Fatalf("expected timer-path bootstrap stall to escape runner, got %v", err) + } + if got := rootTreeCalls.Load(); got != 1 { + t.Fatalf("root tree calls = %d, want one initial partial traversal", got) + } + if got := readCalls.Load(); got != int32(len(entries)) { + t.Fatalf("ReadFile calls = %d, want %d first-page files", got, len(entries)) + } + if got := cursorTreeCalls.Load(); got != 2 { + t.Fatalf("cursor tree calls = %d, want initial nonfatal + timer hard-stop", got) + } + if elapsed := time.Since(started); elapsed < minMountPollInterval { + t.Fatalf("runner returned before a timer cycle (%s < %s)", elapsed, minMountPollInterval) + } +} + func TestNormalizeRemotePathsDedupesRepeatedFlagValues(t *testing.T) { got := normalizeRemotePaths( []string{"/github/repos/acme/cloud", "github/repos/acme/cloud/", "/slack/channels/proj-cloud"}, diff --git a/internal/mountsync/bootstrap_test.go b/internal/mountsync/bootstrap_test.go index 1f6823a4..37f951e6 100644 --- a/internal/mountsync/bootstrap_test.go +++ b/internal/mountsync/bootstrap_test.go @@ -3,6 +3,7 @@ package mountsync import ( "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -365,6 +366,173 @@ func TestBootstrapResumesFromPersistedCursor(t *testing.T) { } } +// TestBootstrapStallCycleGuardPersistsAndFailsHard reproduces the +// non-converging unversioned partial-bootstrap shape: the first page is +// persisted, then every resumed cycle fails at the same next-page cursor. +// The empty revision is non-destructive and not the reason completion fails; +// the unchanged checkpoint is. The guard turns the otherwise infinite +// "non-empty state without completed bootstrap" loop into a hard failure, +// even when every attempt is a fresh Syncer process. +func TestBootstrapStallCycleGuardPersistsAndFailsHard(t *testing.T) { + client := newBootstrapClient(30, 10) + for path, file := range client.files { + file.Revision = "" + client.files[path] = file + } + client.listTreeFailAt = 2 + client.listTreeFailErr = &HTTPError{StatusCode: 502, Code: "bad_gateway", Message: "stuck"} + localDir := t.TempDir() + opts := SyncerOptions{RootCtx: context.Background(), BootstrapStallCycles: 2} + + s := newBootstrapSyncer(t, client, localDir, opts) + err := s.Reconcile(context.Background()) + if err == nil { + t.Fatalf("expected partial bootstrap cycle to return its cloud error") + } + var stalled *BootstrapStalledError + if errors.As(err, &stalled) { + t.Fatalf("checkpoint-advancing cycle must stay below the hard limit: %v", err) + } + st := loadPersistedState(t, localDir) + if st.BootstrapStallCycles != 0 || st.BootstrapCursor == "" || st.BootstrapFilesSynced == 0 { + t.Fatalf("expected persisted partial checkpoint with reset stall count, state=%#v", st) + } + if got := countLocalFiles(t, localDir); got != 10 { + t.Fatalf("expected first page to stay materialized, got %d files", got) + } + + // A fresh Syncer resumes at the same failing cursor. This is the first + // checkpoint-stable stalled cycle, so it increments but stays retryable. + s = newBootstrapSyncer(t, client, localDir, opts) + err = s.Reconcile(context.Background()) + if err == nil || errors.As(err, &stalled) { + t.Fatalf("first checkpoint-stable cycle must return the cloud error, got %v", err) + } + st = loadPersistedState(t, localDir) + if st.BootstrapStallCycles != 1 { + t.Fatalf("BootstrapStallCycles after first unchanged cycle = %d, want 1", st.BootstrapStallCycles) + } + + // The next fresh Syncer sees the same persisted partial checkpoint and + // reaches the configured limit, returning a typed, status-visible failure. + s = newBootstrapSyncer(t, client, localDir, opts) + err = s.Reconcile(context.Background()) + if !errors.As(err, &stalled) { + t.Fatalf("expected BootstrapStalledError at the hard limit, got %v", err) + } + if stalled.Cycles != 2 || stalled.Limit != 2 { + t.Fatalf("BootstrapStalledError = %#v, want cycles=2 limit=2", stalled) + } + st = loadPersistedState(t, localDir) + if st.BootstrapComplete { + t.Fatalf("hard stall must not force BootstrapComplete") + } + if st.BootstrapStallCycles != 2 { + t.Fatalf("persisted BootstrapStallCycles = %d, want 2", st.BootstrapStallCycles) + } + if st.LastError == nil || st.LastError.Kind != "bootstrap_stalled" || st.LastError.Code != "bootstrap_stall_cycle_limit" { + t.Fatalf("expected structured persisted bootstrap stall error, got %#v", st.LastError) + } +} + +func TestBootstrapStallCycleGuardIgnoresCanceledContext(t *testing.T) { + s := newBootstrapSyncer(t, newBootstrapClient(1, 1), t.TempDir(), SyncerOptions{ + RootCtx: context.Background(), + BootstrapStallCycles: 2, + }) + s.state.BootstrapCursor = "resume-cursor" + s.state.BootstrapStallCycles = 1 + + wrappedCanceled := fmt.Errorf("mount shutting down: %w", context.Canceled) + if err := s.recordBootstrapCycle("resume-cursor", nil, wrappedCanceled); err != nil { + t.Fatalf("canceled context must not trip the stall guard: %v", err) + } + if got := s.state.BootstrapStallCycles; got != 1 { + t.Fatalf("canceled context changed stall count to %d, want 1", got) + } + + // DeadlineExceeded is a failed retry at the same checkpoint, so it still + // consumes the remaining attempt and produces the typed hard stop. + err := s.recordBootstrapCycle("resume-cursor", nil, context.DeadlineExceeded) + var stalled *BootstrapStalledError + if !errors.As(err, &stalled) { + t.Fatalf("deadline exceeded must count toward the stall limit, got %v", err) + } + if stalled.Cycles != 2 || stalled.Limit != 2 { + t.Fatalf("BootstrapStalledError = %#v, want cycles=2 limit=2", stalled) + } +} + +// TestBootstrapStallCycleGuardResetsOnCheckpointAdvanceAndCompletion proves +// that the counter reflects lack of traversal progress, not merely errors. +func TestBootstrapStallCycleGuardResetsOnCheckpointAdvanceAndCompletion(t *testing.T) { + client := newBootstrapClient(30, 10) + client.listTreeFailAt = 2 + client.listTreeFailErr = &HTTPError{StatusCode: 502, Code: "bad_gateway", Message: "page failure"} + localDir := t.TempDir() + s := newBootstrapSyncer(t, client, localDir, SyncerOptions{ + RootCtx: context.Background(), + BootstrapStallCycles: 3, + }) + + // Page one advances the cursor before page two fails, so the count resets. + if err := s.Reconcile(context.Background()); err == nil { + t.Fatalf("expected first bootstrap attempt to fail on page 2") + } + if st := loadPersistedState(t, localDir); st.BootstrapStallCycles != 0 || st.BootstrapCursor == "" { + t.Fatalf("cursor advance must reset stall count; state=%#v", st) + } + + // Fail at the same persisted cursor once: count increments. + if err := s.Reconcile(context.Background()); err == nil { + t.Fatalf("expected unchanged checkpoint attempt to fail") + } + if st := loadPersistedState(t, localDir); st.BootstrapStallCycles != 1 { + t.Fatalf("unchanged checkpoint count = %d, want 1", st.BootstrapStallCycles) + } + + // Let page two advance, then fail on page three. The new cursor resets the + // persisted count despite the returned cloud error. + client.mu.Lock() + client.listTreeFailAt = 3 + client.mu.Unlock() + if err := s.Reconcile(context.Background()); err == nil { + t.Fatalf("expected bootstrap attempt to fail on page 3") + } + if st := loadPersistedState(t, localDir); st.BootstrapStallCycles != 0 { + t.Fatalf("advanced checkpoint must reset stall count, got %d", st.BootstrapStallCycles) + } + + client.mu.Lock() + client.listTreeFailAt = 0 + client.mu.Unlock() + if err := s.Reconcile(context.Background()); err != nil { + t.Fatalf("completion after checkpoint advances: %v", err) + } + st := loadPersistedState(t, localDir) + if !st.BootstrapComplete || st.BootstrapStallCycles != 0 { + t.Fatalf("completion must clear checkpoint and stall count, state=%#v", st) + } +} + +func TestBootstrapStallCycleLimitUsesOptionThenEnvThenDefault(t *testing.T) { + newSyncer := func(t *testing.T, opts SyncerOptions) *Syncer { + t.Helper() + return newBootstrapSyncer(t, newBootstrapClient(1, 1), t.TempDir(), opts) + } + t.Setenv("RELAYFILE_BOOTSTRAP_STALL_CYCLES", "7") + if got := newSyncer(t, SyncerOptions{}).bootstrapStallCycles; got != 7 { + t.Fatalf("env bootstrap stall limit = %d, want 7", got) + } + if got := newSyncer(t, SyncerOptions{BootstrapStallCycles: 3}).bootstrapStallCycles; got != 3 { + t.Fatalf("explicit bootstrap stall limit = %d, want 3", got) + } + t.Setenv("RELAYFILE_BOOTSTRAP_STALL_CYCLES", "not-a-number") + if got := newSyncer(t, SyncerOptions{}).bootstrapStallCycles; got != defaultBootstrapStallCycles { + t.Fatalf("invalid env bootstrap stall limit = %d, want default %d", got, defaultBootstrapStallCycles) + } +} + // TestBootstrapCompleteGatesFastPath: a hand-seeded state with Files + // LastEventAt but BootstrapComplete=false MUST NOT short-circuit; the // full pull runs and the mirror converges (the rw_517d60b6 repro). diff --git a/internal/mountsync/syncer.go b/internal/mountsync/syncer.go index 8165fb36..d252a560 100644 --- a/internal/mountsync/syncer.go +++ b/internal/mountsync/syncer.go @@ -89,6 +89,11 @@ const defaultFullPullEvery = 20 const ( defaultBootstrapTimeout = 0 * time.Second defaultBootstrapIdleTimeout = 90 * time.Second + // defaultBootstrapStallCycles is the maximum number of consecutive + // bootstrap cycles allowed to leave the persisted traversal checkpoint + // unchanged. It turns a permanently failing resume point into an explicit + // operator-visible failure instead of silently retrying forever. + defaultBootstrapStallCycles = 20 defaultCursorTimeout = 60 * time.Second // defaultExportTimeout bounds the single atomic full-tree export so a slow // or 429-contended export yields to the resumable pullRemoteFullTree BEFORE @@ -144,6 +149,27 @@ func resolveDurationEnv(opt time.Duration, env string, def time.Duration, logger return def } +// resolveBootstrapStallCycles returns the explicit option when set, otherwise +// RELAYFILE_BOOTSTRAP_STALL_CYCLES, otherwise the default. A non-positive +// value is invalid because the guard must remain a bounded hard stop. +func resolveBootstrapStallCycles(opt int, logger Logger) int { + if opt > 0 { + return opt + } + if opt < 0 && logger != nil { + logger.Printf("ignoring invalid BootstrapStallCycles=%d; using env/default", opt) + } + const env = "RELAYFILE_BOOTSTRAP_STALL_CYCLES" + if raw := strings.TrimSpace(os.Getenv(env)); raw != "" { + if parsed, err := strconv.Atoi(raw); err == nil && parsed > 0 { + return parsed + } else if logger != nil { + logger.Printf("ignoring invalid %s=%q; expected a positive integer", env, raw) + } + } + return defaultBootstrapStallCycles +} + var providerLayoutAliasSegments = []string{ "by-title", "by-id", @@ -195,6 +221,27 @@ func (e *IncrementalReadNotReadyError) Error() string { return fmt.Sprintf("changed event for %s is not readable yet: %s", normalizeRemotePath(e.Path), message) } +// BootstrapStalledError reports that a resumable bootstrap has retried the +// same persisted checkpoint too many times. It is deliberately a typed error +// so callers can distinguish an operator-actionable hard stop from a normal +// transient cloud failure. +type BootstrapStalledError struct { + Cycles int + Limit int + Cursor string + Cause error +} + +func (e *BootstrapStalledError) Error() string { + message := fmt.Sprintf("bootstrap stalled for %d consecutive checkpoint-stable cycles (limit %d, cursor %q)", e.Cycles, e.Limit, e.Cursor) + if e.Cause != nil { + return message + ": " + e.Cause.Error() + } + return message +} + +func (e *BootstrapStalledError) Unwrap() error { return e.Cause } + type TreeEntry struct { Path string `json:"path"` Type string `json:"type"` @@ -808,6 +855,10 @@ type SyncerOptions struct { // sentinel (<=0): the bootstrap runs to completion as long as it keeps // applying files within the idle window. BootstrapTimeout time.Duration + // BootstrapStallCycles is the maximum number of consecutive bootstrap + // cycles that may leave the persisted traversal checkpoint unchanged. 0 + // falls back to RELAYFILE_BOOTSTRAP_STALL_CYCLES, then the default (20). + BootstrapStallCycles int // CursorTimeout bounds each resolveLatestEventCursor attempt with its OWN // deadline derived from RootCtx. Timeout-class failures are retried with // backoff before the caller decides whether a full pull is safe. @@ -1000,6 +1051,7 @@ type Syncer struct { outboxFlushTimeout time.Duration bootstrapTimeout time.Duration bootstrapIdleTimeout time.Duration + bootstrapStallCycles int readNotReadyTTL time.Duration forceFullReconcile bool incrementalCycles int @@ -1143,6 +1195,10 @@ type mountState struct { BootstrapFilesSynced int `json:"bootstrapFilesSynced,omitempty"` BootstrapFilesTotal int `json:"bootstrapFilesTotal,omitempty"` BootstrapStartedAt string `json:"bootstrapStartedAt,omitempty"` + // BootstrapStallCycles counts consecutive bootstrap cycles that did not + // advance the resumable directory/cursor checkpoint. It is persisted so a + // process restart cannot evade the hard-stall guard. + BootstrapStallCycles int `json:"bootstrapStallCycles,omitempty"` // QuarantinedPaths holds remote paths that cannot be materialized locally // due to file/directory name collisions. Persisted across cycle restarts // so the daemon does not re-fetch these paths from the cloud until the @@ -1479,6 +1535,7 @@ func NewSyncer(client RemoteClient, opts SyncerOptions) (*Syncer, error) { if bootstrapIdleTimeout <= 0 { bootstrapIdleTimeout = defaultBootstrapIdleTimeout } + bootstrapStallCycles := resolveBootstrapStallCycles(opts.BootstrapStallCycles, opts.Logger) exportTimeout := resolveDurationEnv(opts.ExportTimeout, "RELAYFILE_EXPORT_TIMEOUT", defaultExportTimeout, opts.Logger) if exportTimeout <= 0 { exportTimeout = defaultExportTimeout @@ -1610,6 +1667,7 @@ func NewSyncer(client RemoteClient, opts SyncerOptions) (*Syncer, error) { outboxFlushTimeout: outboxFlushTimeout, bootstrapTimeout: bootstrapTimeout, bootstrapIdleTimeout: bootstrapIdleTimeout, + bootstrapStallCycles: bootstrapStallCycles, readNotReadyTTL: readNotReadyTTL, forceFullReconcile: forceFullReconcile, oversizedLogged: map[string]struct{}{}, @@ -2812,10 +2870,18 @@ func (s *Syncer) syncReserved(ctx context.Context, forcePoll bool) error { s.markBootstrapComplete() } } else if !s.state.BootstrapComplete || s.forceFullReconcile { - if err := s.pullRemote(ctx, conflicted); err != nil { - s.markSyncError(err) + previousBootstrapCursor := s.state.BootstrapCursor + previousBootstrapDirectories := append([]string(nil), s.state.BootstrapDirectories...) + pullErr := s.pullRemote(ctx, conflicted) + if stallErr := s.recordBootstrapCycle(previousBootstrapCursor, previousBootstrapDirectories, pullErr); stallErr != nil { + s.markSyncError(stallErr) _ = s.saveState() - return err + return stallErr + } + if pullErr != nil { + s.markSyncError(pullErr) + _ = s.saveState() + return pullErr } s.bootstrapped = true didPoll = true @@ -4479,6 +4545,7 @@ func (s *Syncer) markBootstrapComplete() { s.state.BootstrapStartedAt = "" s.state.BootstrapFilesSynced = 0 s.state.BootstrapFilesTotal = 0 + s.state.BootstrapStallCycles = 0 // Clear persisted quarantine so a fixed adapter gets a clean slate. s.state.QuarantinedPaths = nil s.clearAllIncrementalReadNotReady() @@ -4488,6 +4555,58 @@ func (s *Syncer) markBootstrapComplete() { s.forceFullReconcile = false } +// bootstrapCheckpointAdvanced reports whether this cycle moved the persisted +// resumable traversal checkpoint. Directory queue changes matter just as much +// as a page cursor change: bounded-depth traversals can finish a directory and +// begin the next one with an empty page cursor. +func (s *Syncer) bootstrapCheckpointAdvanced(previousCursor string, previousDirectories []string) bool { + if strings.TrimSpace(s.state.BootstrapCursor) != strings.TrimSpace(previousCursor) { + return true + } + if len(s.state.BootstrapDirectories) != len(previousDirectories) { + return true + } + for i, directory := range s.state.BootstrapDirectories { + if directory != previousDirectories[i] { + return true + } + } + return false +} + +// recordBootstrapCycle updates the persisted bootstrap stall guard after a +// bootstrap attempt. It resets only when the traversal checkpoint advances or +// completes, so retries across process restarts cannot spin forever at the +// same cursor. At the configured limit it returns a typed hard failure and +// intentionally leaves BootstrapComplete false. +func (s *Syncer) recordBootstrapCycle(previousCursor string, previousDirectories []string, cause error) error { + if s.state.BootstrapComplete || s.bootstrapCheckpointAdvanced(previousCursor, previousDirectories) { + s.state.BootstrapStallCycles = 0 + return nil + } + // Root-context cancellation means this runner is intentionally stopping, + // not that the remote checkpoint has made a retryable failed attempt. Keep + // the persisted count intact so shutdown/restart does not consume a stall + // budget. A deadline remains a real failed attempt and continues below. + if errors.Is(cause, context.Canceled) { + return nil + } + s.state.BootstrapStallCycles++ + limit := s.bootstrapStallCycles + if limit <= 0 { + limit = defaultBootstrapStallCycles + } + if s.state.BootstrapStallCycles < limit { + return nil + } + return &BootstrapStalledError{ + Cycles: s.state.BootstrapStallCycles, + Limit: limit, + Cursor: strings.TrimSpace(s.state.BootstrapCursor), + Cause: cause, + } +} + // snapshotDeleteUnsafe reports whether running snapshot-driven deletes is // unsafe given how many files the fresh remote listing returned versus how // many we currently track. It guards against a degraded cloud response @@ -6316,6 +6435,7 @@ func (s *Syncer) loadState() error { s.state.BootstrapStartedAt = "" s.state.BootstrapFilesSynced = 0 s.state.BootstrapFilesTotal = 0 + s.state.BootstrapStallCycles = 0 } if s.githubWorkingTree != nil && strings.TrimSpace(s.state.GithubWorkingTreeHeadSHA) != "" { s.githubWorkingTree.HeadSHA = strings.TrimSpace(s.state.GithubWorkingTreeHeadSHA) @@ -6804,6 +6924,12 @@ func classifyStatusError(err error) *statusError { Message: err.Error(), At: time.Now().UTC().Format(time.RFC3339Nano), } + var bootstrapStalled *BootstrapStalledError + if errors.As(err, &bootstrapStalled) { + status.Kind = "bootstrap_stalled" + status.Code = "bootstrap_stall_cycle_limit" + return status + } var httpErr *HTTPError if errors.As(err, &httpErr) { status.StatusCode = httpErr.StatusCode diff --git a/internal/mountsync/syncer_test.go b/internal/mountsync/syncer_test.go index d531da67..914f3fcc 100644 --- a/internal/mountsync/syncer_test.go +++ b/internal/mountsync/syncer_test.go @@ -9829,6 +9829,7 @@ func TestLoadStateResetsBootstrapCompleteOnlyOnWriteOnlyToMirrorSyncMode(t *test BootstrapStartedAt: time.Now().UTC().Format(time.RFC3339Nano), BootstrapFilesSynced: 3, BootstrapFilesTotal: 9, + BootstrapStallCycles: 2, SyncMode: tc.syncMode, }); err != nil { t.Fatalf("seed state: %v", err) @@ -9858,6 +9859,9 @@ func TestLoadStateResetsBootstrapCompleteOnlyOnWriteOnlyToMirrorSyncMode(t *test if syncer.state.BootstrapFilesTotal != 0 { t.Fatalf("BootstrapFilesTotal = %d, want 0", syncer.state.BootstrapFilesTotal) } + if syncer.state.BootstrapStallCycles != 0 { + t.Fatalf("BootstrapStallCycles = %d, want 0", syncer.state.BootstrapStallCycles) + } } }) } diff --git a/internal/mountsync/tombstones_test.go b/internal/mountsync/tombstones_test.go index 93f1f6bd..5dbe6a59 100644 --- a/internal/mountsync/tombstones_test.go +++ b/internal/mountsync/tombstones_test.go @@ -134,6 +134,65 @@ func TestRevisionGateRefusesOlderListing(t *testing.T) { } } +// TestRevisionGateRefusesUnversionedListingAcrossRepeatedPulls proves that +// an unversioned listing can never authorize destructive snapshots. +// Two partial listings without revisions must neither create nor confirm a +// tombstone for the absent tracked path. +func TestRevisionGateRefusesUnversionedListingAcrossRepeatedPulls(t *testing.T) { + disableWS := false + localDir := t.TempDir() + + keepPath := filepath.Join(localDir, "keep.md") + if err := os.WriteFile(keepPath, []byte("# keep"), 0o644); err != nil { + t.Fatalf("seed keep: %v", err) + } + otherPath := filepath.Join(localDir, "other.md") + if err := os.WriteFile(otherPath, []byte("# other"), 0o644); err != nil { + t.Fatalf("seed other: %v", err) + } + stateFile := filepath.Join(localDir, ".relayfile-mount-state.json") + if err := writeMountState(stateFile, mountState{ + Files: map[string]trackedFile{ + "/keep.md": {ContentType: "text/markdown", Hash: hashString("# keep")}, + "/other.md": {ContentType: "text/markdown", Hash: hashString("# other")}, + }, + }); err != nil { + t.Fatalf("seed state: %v", err) + } + + // The cloud omits /keep.md and emits no revisions for /other.md. This is + // indistinguishable from an unversioned partial listing, never a delete + // authorization. + fc := &fakeClient{files: map[string]RemoteFile{ + "/other.md": {Path: "/other.md", ContentType: "text/markdown", Content: "# other"}, + }, eventsUnsupported: true} + syncer, err := NewSyncer(fc, SyncerOptions{ + WorkspaceID: "ws_unversioned_gate", + RemoteRoot: "/", + LocalRoot: localDir, + WebSocket: &disableWS, + StateFile: stateFile, + }) + if err != nil { + t.Fatalf("new syncer: %v", err) + } + + for i := 0; i < 2; i++ { + if err := syncer.SyncOnce(context.Background()); err != nil { + t.Fatalf("sync %d: %v", i+1, err) + } + } + if _, err := os.Stat(keepPath); err != nil { + t.Fatalf("unversioned listing deleted tracked file: %v", err) + } + if syncer.state.Counters.SnapshotDeleteBlocked < 2 { + t.Fatalf("SnapshotDeleteBlocked = %d, want at least two blocked pulls", syncer.state.Counters.SnapshotDeleteBlocked) + } + if _, err := os.Stat(filepath.Join(localDir, ".relay", "pending-deletes")); !os.IsNotExist(err) { + t.Fatalf("unversioned listing must not create tombstones, stat err=%v", err) + } +} + // TestRevisionAdvances exercises the numeric and lexicographic paths. func TestRevisionAdvances(t *testing.T) { if revisionAdvances("rev_5", "") {