diff --git a/internal/cli/setup.go b/internal/cli/setup.go index 718071767..0ba1340cf 100644 --- a/internal/cli/setup.go +++ b/internal/cli/setup.go @@ -351,11 +351,12 @@ func oauthLoggedInProviders() map[string]bool { } // firstUsableProvider returns the saved provider best suited to run without -// onboarding: the first usable (inline credential present, or no-auth/local) -// non-local provider, else the first usable local one. It lets the CLI fall back -// to an already-configured login when the active provider happens to lack a -// credential, instead of re-running onboarding every launch. +// onboarding: the first usable (inline credential, stored OAuth login, or +// no-auth/local) non-local provider, else the first usable local one. It lets +// the CLI fall back to an already-configured login when the active provider +// happens to lack a credential, instead of re-running onboarding every launch. func firstUsableProvider(providers []config.ProviderProfile) (config.ProviderProfile, bool) { + logins := oauthLoggedInProviders() var localFallback config.ProviderProfile haveLocal := false for _, profile := range providers { @@ -371,7 +372,11 @@ func firstUsableProvider(providers []config.ProviderProfile) (config.ProviderPro continue } } - if _, missing := setupMissingCredentialEnv(profile); missing { + // A stored OAuth login (e.g. `zero auth login xai`) is a credential too, even + // when the profile has no inline key / env var — mirrors setupRequired and + // usableSavedProviders so this fallback doesn't force onboarding for a + // provider the user is already authenticated with. + if _, missing := setupMissingCredentialEnv(profile); missing && !providerHasOAuthLogin(profile, logins) { continue } if providerProfileIsLocal(profile) { diff --git a/internal/cli/setup_fallback_test.go b/internal/cli/setup_fallback_test.go index 7c87d1384..692b93600 100644 --- a/internal/cli/setup_fallback_test.go +++ b/internal/cli/setup_fallback_test.go @@ -1,9 +1,12 @@ package cli import ( + "path/filepath" "testing" + "time" "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/oauth" ) func TestFirstUsableProviderPrefersRemoteKeyed(t *testing.T) { @@ -66,6 +69,31 @@ func TestFirstUsableProviderSkipsUnresolvableCatalogWithoutBaseURL(t *testing.T) } } +// An OAuth-only provider (no inline key, no env var) must be selectable as a +// fallback, matching setupRequired/usableSavedProviders — otherwise a fully +// authenticated user gets forced back into onboarding when activeProvider +// goes stale. +func TestFirstUsableProviderRecognizesOAuthLogin(t *testing.T) { + path := filepath.Join(t.TempDir(), "tok.json") + t.Setenv("ZERO_OAUTH_STORAGE", "file") // an inherited "keyring" would ignore the temp path and hit the OS keychain + t.Setenv("ZERO_OAUTH_TOKENS_PATH", path) + store, err := oauth.NewStore(oauth.StoreOptions{FilePath: path}) + if err != nil { + t.Fatalf("NewStore: %v", err) + } + if err := store.Save(oauth.ProviderKey("xai"), oauth.Token{AccessToken: "tok", ExpiresAt: time.Now().Add(time.Hour)}); err != nil { + t.Fatalf("seed token: %v", err) + } + + providers := []config.ProviderProfile{ + {Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"}, // no inline key/env, but logged in via OAuth + } + got, ok := firstUsableProvider(providers) + if !ok || got.Name != "xai" { + t.Fatalf("want OAuth-logged-in provider (xai), got %q ok=%v", got.Name, ok) + } +} + func TestProviderProfileIsLocal(t *testing.T) { cases := []struct { name string diff --git a/internal/sessions/exec_session.go b/internal/sessions/exec_session.go index d21cd6626..44b3d6c19 100644 --- a/internal/sessions/exec_session.go +++ b/internal/sessions/exec_session.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "strings" + "unicode/utf8" ) type ExecMode string @@ -221,11 +222,24 @@ func summarizePayload(payload any) string { text = string(data) } if len(text) > 500 { - return text[:500] + return truncateUTF8(text, 500) } return text } +// truncateUTF8 returns the longest prefix of s that is at most n bytes, +// backing off to the nearest rune boundary so a multi-byte character isn't +// split — a split rune here would embed invalid UTF-8 into the exec prompt. +func truncateUTF8(s string, n int) string { + if len(s) <= n { + return s + } + for n > 0 && !utf8.RuneStart(s[n]) { + n-- + } + return s[:n] +} + func extractText(value any) string { switch typed := value.(type) { case string: diff --git a/internal/sessions/store_test.go b/internal/sessions/store_test.go index 4786555b0..acd4dd9cc 100644 --- a/internal/sessions/store_test.go +++ b/internal/sessions/store_test.go @@ -11,6 +11,7 @@ import ( "sync" "testing" "time" + "unicode/utf8" ) func TestStoreCreatesAppendsListsAndReadsEvents(t *testing.T) { @@ -640,6 +641,23 @@ func TestFormatExecPromptTruncatesConversationMessagesAfterFilteringNoise(t *tes } } +// A truncation cut at a raw byte offset can land in the middle of a +// multi-byte UTF-8 rune (e.g. CJK text), embedding invalid UTF-8 into the +// exec prompt. summarizePayload must back off to a rune boundary instead. +func TestSummarizePayloadTruncatesOnRuneBoundary(t *testing.T) { + content := strings.Repeat("中文", 300) // 900 bytes of 3-byte runes + payload := json.RawMessage(fmt.Sprintf(`{"role":"user","content":%q}`, content)) + + got := summarizePayload(payload) + + if !utf8.ValidString(got) { + t.Fatalf("summarizePayload produced invalid UTF-8: %q", got) + } + if len(got) > 500 { + t.Fatalf("expected summary to be at most 500 bytes, got %d", len(got)) + } +} + func TestPrepareExecPersistsSpecialistMetadataForNewSession(t *testing.T) { store := NewStore(StoreOptions{RootDir: t.TempDir(), Now: fixedClock("2026-06-04T14:30:00Z")}) diff --git a/internal/tui/binding_test.go b/internal/tui/binding_test.go index 52f1a0391..d7c124947 100644 --- a/internal/tui/binding_test.go +++ b/internal/tui/binding_test.go @@ -289,3 +289,122 @@ func TestDispatchOptionO(t *testing.T) { t.Errorf("model.keyMatch should NOT match ctrl+o when option+o is configured") } } + +// A configured binding equal to a reserved hardcoded chord must be reverted +// to default rather than silently making the hardcoded action unreachable +// (e.g. configuring toggleDetailed to ctrl+f would otherwise swallow the +// /model picker's "favorite" shortcut). +func TestSanitizeKeyBindingsDropsReservedCollision(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleDetailed: "ctrl+f", // collides with the hardcoded favorite-model shortcut + ToggleMouse: "ctrl+e", // unaffected, should survive untouched + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if !sanitized.toggleDetailed.isZero() { + t.Errorf("toggleDetailed should be reverted to default, got %q", sanitized.toggleDetailed.Label()) + } + if sanitized.toggleMouse.isZero() || sanitized.toggleMouse.Label() != "Ctrl+E" { + t.Errorf("toggleMouse should be untouched, got %q", sanitized.toggleMouse.Label()) + } + if len(warnings) != 1 { + t.Fatalf("want exactly 1 warning, got %d: %v", len(warnings), warnings) + } +} + +// Two configurable actions bound to the same chord must not both fire — +// sanitizeKeyBindings should keep the first and revert the rest to default. +func TestSanitizeKeyBindingsDropsMutualCollision(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleDetailed: "ctrl+o", + CycleReasoning: "ctrl+o", // collides with toggleDetailed above + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if sanitized.toggleDetailed.isZero() || sanitized.toggleDetailed.Label() != "Ctrl+O" { + t.Errorf("toggleDetailed (first claimant) should keep ctrl+o, got %q", sanitized.toggleDetailed.Label()) + } + if !sanitized.cycleReasoning.isZero() { + t.Errorf("cycleReasoning (second claimant) should be reverted to default, got %q", sanitized.cycleReasoning.Label()) + } + if len(warnings) != 1 { + t.Fatalf("want exactly 1 warning, got %d: %v", len(warnings), warnings) + } +} + +// Non-colliding configured bindings must pass through unchanged with no +// warnings. +func TestSanitizeKeyBindingsNoCollisions(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleDetailed: "option+o", + ToggleSidebar: "option+b", + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if len(warnings) != 0 { + t.Fatalf("want no warnings, got %v", warnings) + } + if sanitized.toggleDetailed.Label() != "Alt+O" || sanitized.toggleSidebar.Label() != "Alt+B" { + t.Fatalf("bindings should be unchanged: toggleDetailed=%q toggleSidebar=%q", + sanitized.toggleDetailed.Label(), sanitized.toggleSidebar.Label()) + } +} + +// A configured binding must not shadow another action that is still on its +// built-in default chord. +func TestSanitizeKeyBindingsDropsCollisionWithOtherDefault(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleDetailed: "ctrl+t", // collides with cycleReasoning default + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if !sanitized.toggleDetailed.isZero() { + t.Errorf("toggleDetailed should be reverted to default, got %q", sanitized.toggleDetailed.Label()) + } + if !sanitized.cycleReasoning.isZero() { + t.Errorf("cycleReasoning should remain default (zero), got %q", sanitized.cycleReasoning.Label()) + } + if len(warnings) != 1 { + t.Fatalf("want exactly 1 warning, got %d: %v", len(warnings), warnings) + } +} + +// A reversion to default made during one collision pass must be +// re-evaluated against entries already processed earlier in that same pass: +// toggleMouse="ctrl+p" doesn't collide with togglePlan while togglePlan is +// still explicitly "ctrl+t", but once togglePlan reverts to its own default +// of ctrl+p (because ctrl+t collides with cycleReasoning's default), +// toggleMouse's explicit ctrl+p would otherwise silently shadow it. +func TestSanitizeKeyBindingsResolvesChainedDefaultCollision(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleMouse: "ctrl+p", // collides with togglePlan's default once togglePlan reverts + TogglePlan: "ctrl+t", // collides with cycleReasoning's default, reverts to togglePlan's own default (ctrl+p) + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if !sanitized.toggleMouse.isZero() { + t.Errorf("toggleMouse should be reverted to default once togglePlan adopts ctrl+p, got %q", sanitized.toggleMouse.Label()) + } + if !sanitized.togglePlan.isZero() { + t.Errorf("togglePlan should be reverted to default, got %q", sanitized.togglePlan.Label()) + } + if len(warnings) != 2 { + t.Fatalf("want exactly 2 warnings, got %d: %v", len(warnings), warnings) + } +} + +// Bare hardcoded keys handled directly in updateModel (e.g. Tab navigation) +// are reserved too and must not be shadowed by configurable bindings. +func TestSanitizeKeyBindingsDropsBareHardcodedCollision(t *testing.T) { + cfg := config.KeyBindingsConfig{ + ToggleDetailed: "tab", + } + sanitized, warnings := sanitizeKeyBindings(resolveKeyBindings(cfg)) + + if !sanitized.toggleDetailed.isZero() { + t.Errorf("toggleDetailed should be reverted to default, got %q", sanitized.toggleDetailed.Label()) + } + if len(warnings) != 1 { + t.Fatalf("want exactly 1 warning, got %d: %v", len(warnings), warnings) + } +} diff --git a/internal/tui/keybindings.go b/internal/tui/keybindings.go index 42f2b9ac9..caf74eb1a 100644 --- a/internal/tui/keybindings.go +++ b/internal/tui/keybindings.go @@ -1,6 +1,7 @@ package tui import ( + "fmt" "strings" "unicode/utf8" @@ -263,3 +264,114 @@ func (m model) keyMatch(b parsedBinding, msg tea.KeyMsg, defaultFn func(tea.KeyM } return defaultFn(msg) } + +// reservedBindings lists hardcoded (non-configurable) chords handled directly in +// model.go's key dispatch. If a configurable binding uses one of these chords, +// one of the actions becomes unreachable (depending on switch order), so +// sanitizeKeyBindings reverts the configurable binding back to its default. +var reservedBindings = []struct { + binding parsedBinding + description string +}{ + {parseBinding("ctrl+c"), "cancel / exit"}, + {parseBinding("esc"), "cancel / close"}, + {parseBinding("enter"), "submit"}, + {parseBinding("shift+tab"), "cycle permission mode"}, + {parseBinding("tab"), "navigation / completion"}, + {parseBinding("backspace"), "composer edit / attachment removal"}, + {parseBinding("up"), "history/navigation"}, + {parseBinding("down"), "history/navigation"}, + {parseBinding("pgup"), "transcript scroll"}, + {parseBinding("pgdown"), "transcript scroll"}, + {parseBinding("ctrl+f"), "favorite model (in the /model picker)"}, + {parseBinding("?"), "help overlay"}, +} + +// sanitizeKeyBindings drops (reverts to default) any configured binding that +// collides with a reserved hardcoded chord above, or with another +// configured binding, since either collision would silently make one of the +// two actions permanently unreachable. Returns the sanitized bindings plus a +// human-readable warning for each dropped binding, for the caller to surface +// as a startup notice. +func sanitizeKeyBindings(b keyBindings) (keyBindings, []string) { + entries := []struct { + name string + binding *parsedBinding + defaultBinding parsedBinding + }{ + {"toggleDetailed", &b.toggleDetailed, parseBinding("ctrl+o")}, + {"toggleMouse", &b.toggleMouse, parseBinding("ctrl+e")}, + {"cycleReasoning", &b.cycleReasoning, parseBinding("ctrl+t")}, + {"togglePlan", &b.togglePlan, parseBinding("ctrl+p")}, + {"toggleSidebar", &b.toggleSidebar, parseBinding("ctrl+b")}, + } + + // Each pass below can revert a binding to its default, which can newly + // collide with another entry that was already checked earlier in the + // same pass (e.g. toggleMouse="ctrl+p" doesn't collide with togglePlan + // while togglePlan="ctrl+t" is explicit, but once togglePlan reverts to + // its own default of ctrl+p later in the same pass, toggleMouse's + // explicit ctrl+p now silently shadows it). Repeat all three checks to a + // fixed point so a reversion is always re-evaluated against the others. + var warnings []string + for { + changed := false + + for _, e := range entries { + if e.binding.isZero() { + continue + } + for _, other := range entries { + if other.name == e.name || !other.binding.isZero() { + continue + } + if *e.binding == other.defaultBinding { + warnings = append(warnings, fmt.Sprintf( + "keybindings.%s (%s) conflicts with keybindings.%s default (%s); using the default instead.", + e.name, e.binding.Label(), other.name, other.defaultBinding.Label())) + *e.binding = parsedBinding{} + changed = true + break + } + } + } + + for _, e := range entries { + if e.binding.isZero() { + continue + } + for _, reserved := range reservedBindings { + if *e.binding == reserved.binding { + warnings = append(warnings, fmt.Sprintf( + "keybindings.%s (%s) conflicts with the built-in %s shortcut; using the default instead.", + e.name, e.binding.Label(), reserved.description)) + *e.binding = parsedBinding{} + changed = true + break + } + } + } + + claimedBy := map[parsedBinding]string{} + for _, e := range entries { + if e.binding.isZero() { + continue + } + if other, ok := claimedBy[*e.binding]; ok { + warnings = append(warnings, fmt.Sprintf( + "keybindings.%s (%s) conflicts with keybindings.%s; using the default instead.", + e.name, e.binding.Label(), other)) + *e.binding = parsedBinding{} + changed = true + continue + } + claimedBy[*e.binding] = e.name + } + + if !changed { + break + } + } + + return b, warnings +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 45dc66e90..a6cc1c6a3 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -745,6 +745,8 @@ func newModel(ctx context.Context, options Options) model { notify.MaybeAddWebhookSink(notifier, os.Getenv, nil) notifier.SetFocused(true) + resolvedKeyBindings, keyBindingWarnings := sanitizeKeyBindings(resolveKeyBindings(options.KeyBindings)) + m := model{ ctx: ctx, cwd: cwd, @@ -783,7 +785,7 @@ func newModel(ctx context.Context, options Options) model { permissionMode: permissionMode, reasoningEffort: options.ReasoningEffort, responseStyle: defaultedResponseStyle(options.ResponseStyle), - keyBindings: resolveKeyBindings(options.KeyBindings), + keyBindings: resolvedKeyBindings, themeMode: resolveThemeMode(options.Theme, os.Getenv("ZERO_THEME"), options.SavedTheme), hasDarkBg: true, userAgent: options.UserAgent, @@ -819,6 +821,9 @@ func newModel(ctx context.Context, options Options) model { m.lspManager = lsp.NewManager(cwd) } m.refreshMCPViewState() + for _, warning := range keyBindingWarnings { + m = m.appendSystemNotice(warning) + } return m } diff --git a/internal/update/replace_windows.go b/internal/update/replace_windows.go index 073cbdcb3..2afb839b3 100644 --- a/internal/update/replace_windows.go +++ b/internal/update/replace_windows.go @@ -5,6 +5,12 @@ package update import ( "fmt" "os" + "time" +) + +const ( + renameRetryAttempts = 10 + renameRetryDelay = 100 * time.Millisecond ) // replaceBinary installs newPath over targetPath. Windows will not let a @@ -17,8 +23,13 @@ func replaceBinary(targetPath string, newPath string) error { if err := os.Rename(targetPath, oldPath); err != nil { return fmt.Errorf("rename running binary aside: %w", err) } - if err := os.Rename(newPath, targetPath); err != nil { - if restoreErr := os.Rename(oldPath, targetPath); restoreErr != nil { + // Retry both the install and the restore: a transient Windows file lock + // (antivirus/indexer scanning the just-renamed file, a lingering handle) + // can make either rename fail momentarily, and on the restore side + // failure means targetPath is left missing entirely rather than merely + // stale — worth a short retry to avoid that. + if err := renameWithRetry(newPath, targetPath); err != nil { + if restoreErr := renameWithRetry(oldPath, targetPath); restoreErr != nil { return fmt.Errorf("install new binary: %w; additionally failed to restore the original binary: %v (original preserved at %s)", err, restoreErr, oldPath) } return fmt.Errorf("install new binary: %w", err) @@ -26,9 +37,32 @@ func replaceBinary(targetPath string, newPath string) error { return nil } +func renameWithRetry(oldPath string, newPath string) error { + var lastErr error + for attempt := 0; attempt < renameRetryAttempts; attempt++ { + lastErr = os.Rename(oldPath, newPath) + if lastErr == nil { + return nil + } + if attempt < renameRetryAttempts-1 { + time.Sleep(renameRetryDelay) + } + } + return lastErr +} + // CleanupStaleBinary best-effort removes a ".old" file left behind by a // previous replaceBinary call once the old process holding it has exited. // Callers should invoke this once at startup for the current executable. +// +// Guarded on targetPath still existing: if a previous replaceBinary call +// failed to install AND failed to restore the original, targetPath is +// missing and oldPath is the only surviving copy of a working binary -- +// removing it here would destroy the last recoverable copy instead of just +// clearing redundant backup left by a successful replace. func CleanupStaleBinary(targetPath string) { + if _, err := os.Stat(targetPath); err != nil { + return + } _ = os.Remove(targetPath + ".old") } diff --git a/internal/update/replace_windows_test.go b/internal/update/replace_windows_test.go new file mode 100644 index 000000000..ffe1d9441 --- /dev/null +++ b/internal/update/replace_windows_test.go @@ -0,0 +1,136 @@ +//go:build windows + +package update + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestReplaceBinaryReplacesRunningBinary(t *testing.T) { + dir := t.TempDir() + targetPath := filepath.Join(dir, "zero.exe") + newPath := filepath.Join(dir, "zero.exe.new") + + if err := os.WriteFile(targetPath, []byte("old-binary"), 0o755); err != nil { + t.Fatalf("WriteFile target: %v", err) + } + if err := os.WriteFile(newPath, []byte("new-binary"), 0o755); err != nil { + t.Fatalf("WriteFile new: %v", err) + } + + if err := replaceBinary(targetPath, newPath); err != nil { + t.Fatalf("replaceBinary: %v", err) + } + + data, err := os.ReadFile(targetPath) + if err != nil { + t.Fatalf("ReadFile target: %v", err) + } + if string(data) != "new-binary" { + t.Fatalf("target content = %q, want %q", data, "new-binary") + } + if _, err := os.Stat(targetPath + ".old"); err != nil { + t.Fatalf("expected the original binary to be preserved at %s.old: %v", targetPath, err) + } +} + +// When the install rename fails (newPath doesn't exist) but the restore +// succeeds, replaceBinary must put the original binary back at targetPath +// and report only the install failure, not a combined failure. +func TestReplaceBinaryRestoresOriginalWhenInstallFails(t *testing.T) { + dir := t.TempDir() + targetPath := filepath.Join(dir, "zero.exe") + newPath := filepath.Join(dir, "zero.exe.new") // deliberately never created + + if err := os.WriteFile(targetPath, []byte("old-binary"), 0o755); err != nil { + t.Fatalf("WriteFile target: %v", err) + } + + err := replaceBinary(targetPath, newPath) + if err == nil { + t.Fatal("expected replaceBinary to fail when newPath does not exist") + } + if strings.Contains(err.Error(), "additionally failed to restore") { + t.Fatalf("expected the restore to succeed, got a combined failure: %v", err) + } + + data, err := os.ReadFile(targetPath) + if err != nil { + t.Fatalf("ReadFile target after restore: %v", err) + } + if string(data) != "old-binary" { + t.Fatalf("target content after restore = %q, want %q", data, "old-binary") + } + if _, err := os.Stat(targetPath + ".old"); err == nil { + t.Fatal("expected .old file to be gone after a successful restore") + } +} + +// If a previous replaceBinary call failed to both install and restore, +// targetPath is missing and oldPath is the only surviving copy of a working +// binary. CleanupStaleBinary must not delete it in that case. +func TestCleanupStaleBinaryPreservesOnlyRecoverableCopy(t *testing.T) { + dir := t.TempDir() + targetPath := filepath.Join(dir, "zero.exe") // never created: simulates a failed install+restore + oldPath := targetPath + ".old" + if err := os.WriteFile(oldPath, []byte("old-binary"), 0o755); err != nil { + t.Fatalf("WriteFile old: %v", err) + } + + CleanupStaleBinary(targetPath) + + if _, err := os.Stat(oldPath); err != nil { + t.Fatalf("expected .old backup to survive when targetPath is missing: %v", err) + } +} + +// The normal case: targetPath exists (a previous replace succeeded), so the +// redundant .old backup should be cleaned up. +func TestCleanupStaleBinaryRemovesBackupWhenTargetExists(t *testing.T) { + dir := t.TempDir() + targetPath := filepath.Join(dir, "zero.exe") + oldPath := targetPath + ".old" + if err := os.WriteFile(targetPath, []byte("new-binary"), 0o755); err != nil { + t.Fatalf("WriteFile target: %v", err) + } + if err := os.WriteFile(oldPath, []byte("old-binary"), 0o755); err != nil { + t.Fatalf("WriteFile old: %v", err) + } + + CleanupStaleBinary(targetPath) + + if _, err := os.Stat(oldPath); err == nil { + t.Fatal("expected .old backup to be removed when targetPath exists") + } +} + +func TestRenameWithRetrySucceedsImmediately(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src") + dst := filepath.Join(dir, "dst") + if err := os.WriteFile(src, []byte("data"), 0o644); err != nil { + t.Fatalf("WriteFile src: %v", err) + } + + if err := renameWithRetry(src, dst); err != nil { + t.Fatalf("renameWithRetry: %v", err) + } + if _, err := os.Stat(dst); err != nil { + t.Fatalf("expected dst to exist after rename: %v", err) + } +} + +// A permanently-failing rename (source never appears) must exhaust its +// retries and surface the underlying error, rather than retrying forever. +func TestRenameWithRetryFailsAfterExhaustingAttempts(t *testing.T) { + dir := t.TempDir() + missing := filepath.Join(dir, "does-not-exist") + dst := filepath.Join(dir, "dst") + + if err := renameWithRetry(missing, dst); err == nil { + t.Fatal("expected renameWithRetry to fail for a source that never appears") + } +}