Add fetch_interval config with caching

[agarcher]

Summary

• Add fetch_interval config option to control fetch frequency when remote comparison is enabled
• Default to 5-minute caching between fetches to avoid hitting remote on every command
• Support never to disable fetching entirely, 0 to always fetch
• Remove separate fetch boolean - all fetch behavior controlled through fetch_interval

Configuration

wt config --global fetch_interval 10m   # Fetch at most every 10 minutes
wt config --global fetch_interval 0     # Always fetch (no caching)
wt config fetch_interval never          # Disable fetch for current repo

Test plan

• Verify wt list with remote shows "Skipping fetch" when within interval
• Verify wt list fetches after interval expires
• Verify fetch_interval: never disables fetching
• Verify fetch_interval: 0 always fetches
• Verify invalid duration values are rejected

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Replaced fetch configuration with fetch_interval for more granular control over data fetching intervals.

• Documentation

  • Updated configuration documentation to reflect new fetch_interval setting supporting duration values (e.g., "10m"), "0" for always fetch, and "never" to disable fetching.
  • Updated examples showing global and per-repo configuration overrides for the new setting.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces a boolean fetch configuration flag with a duration-based fetch_interval system. The new mechanism controls minimum time between repository fetches, supporting "never" to disable, "0" to always fetch, and standard durations (e.g., "5m"). Changes span configuration data model, command logic, tests, and documentation.

Changes

Cohort / File(s)	Summary
Documentation README.md	Updated examples and descriptions to reflect fetch_interval (default 5m) with value semantics: "never" disables, "0" always fetches, standard durations control minimum time between fetches. Removed references to the boolean fetch flag.
Configuration Data Model internal/userconfig/userconfig.go	Removed Fetch bool field from UserConfig and Fetch *bool from RepoConfig. Added FetchIntervalNever sentinel constant. Replaced GetFetchForRepo() with GetFetchIntervalForRepo() returning a duration. Removed "fetch" key handling from all config getters/setters; ValidKeys() now returns only ["remote", "fetch_interval"].
Config Command internal/commands/config.go	Removed all fetch key handling from get, set, and unset operations. Updated validation to accept "never" and numeric durations for fetch_interval. Removed warnings about fetch without remote. Updated documentation and messaging.
Fetch Logic in Commands internal/commands/compare.go, internal/commands/delete.go	Replaced GetFetchForRepo() boolean checks with unconditional GetFetchIntervalForRepo() calls; fetch is skipped only when interval equals FetchIntervalNever, otherwise existing time-based throttling applies.
Test Files internal/commands/commands_test.go, internal/userconfig/userconfig_test.go	Updated all test cases to use "fetch_interval" key with duration string values ("10m", "never", "0") instead of "fetch" with boolean values. Renamed TestConfigFetchWithoutRemoteWarning() to TestConfigFetchIntervalValidation(). Removed per-repo fetch boolean override tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Add fetch_interval config to prevent repeated fetches #29: Implements the identical fetch-to-fetch_interval refactoring, replacing boolean fetch with duration-based interval mechanism across the same files.
• Add remote comparison mode with auto-fetch and wt config command #25: Prior work that introduced or modified config/fetch features; this PR updates and replaces its boolean fetch with a new duration-based approach.

Poem

🐰 A flag once boolean, true or false so plain,
Now dances with durations, like intervals of rain—
Five minutes by default, or "never" to forbid,
A fetch that waits its moment, just as it's configured did! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add fetch_interval config with caching' accurately describes the main change: introducing a new fetch_interval configuration option with caching behavior based on time intervals.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

internal/commands/compare.go (1)

30-31: Consider directing informational messages to stderr.

Per coding guidelines, cmd.Println() should be used for stderr and fmt.Fprintln(cmd.OutOrStdout(), ...) for stdout output (paths, listings). The "Repository:" and "Comparing to:" messages on lines 31 and 90 are informational status messages rather than the actual listing output, so they might be better suited for stderr.

♻️ Suggested change

-	cmd.Printf("Repository: %s\n", repoRoot)
+	cmd.PrintErrf("Repository: %s\n", repoRoot)

And similarly for line 90:

-	cmd.Printf("Comparing to: %s\n", comparisonRef)
-	cmd.Println()
+	cmd.PrintErrf("Comparing to: %s\n", comparisonRef)
+	cmd.PrintErrln()

Based on coding guidelines.

internal/userconfig/userconfig_test.go (1)

223-234: Consider adding test coverage for GetFetchIntervalForRepo.

The tests cover setting/getting/unsetting fetch_interval, but there's no explicit test for GetFetchIntervalForRepo which handles the duration parsing, "never" sentinel, and per-repo override logic. Consider adding tests for:

• Returns FetchIntervalNever when set to "never"
• Returns 0 when set to "0"
• Per-repo override takes precedence over global
• Invalid duration falls back to 0

🧪 Suggested test addition

func TestGetFetchIntervalForRepo(t *testing.T) {
	tenMin := "10m"
	never := "never"
	zero := "0"
	
	tests := []struct {
		name           string
		globalInterval string
		repoInterval   *string
		repoPath       string
		want           time.Duration
	}{
		{"default when empty", "", nil, "/repo", 5 * time.Minute},
		{"global 10m", "10m", nil, "/repo", 10 * time.Minute},
		{"global never", "never", nil, "/repo", FetchIntervalNever},
		{"global 0 always fetch", "0", nil, "/repo", 0},
		{"per-repo override", "10m", &never, "/repo", FetchIntervalNever},
		{"per-repo 0 override", "10m", &zero, "/repo", 0},
	}
	
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			cfg := &UserConfig{
				FetchInterval: tt.globalInterval,
				Repos: map[string]RepoConfig{
					"/repo": {FetchInterval: tt.repoInterval},
				},
			}
			got := cfg.GetFetchIntervalForRepo(tt.repoPath)
			if got != tt.want {
				t.Errorf("GetFetchIntervalForRepo() = %v, want %v", got, tt.want)
			}
		})
	}
}

internal/commands/config.go (1)

188-193: Consider adding a default case for future-proofing.

The switch statement handles only remote and fetch_interval. If ValidKeys() is extended in the future without updating this switch, the function would silently succeed without printing any value.

Suggested fix

 		switch key {
 		case "remote":
 			_, _ = fmt.Fprintln(cmd.OutOrStdout(), cfg.GetRemoteForRepo(repoRoot))
 		case "fetch_interval":
 			_, _ = fmt.Fprintln(cmd.OutOrStdout(), cfg.GetFetchIntervalForRepo(repoRoot))
+		default:
+			return fmt.Errorf("key %q is not yet implemented for per-repo get", key)
 		}

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3562e1 and b925a79.

📒 Files selected for processing (7)

• README.md
• internal/commands/commands_test.go
• internal/commands/compare.go
• internal/commands/config.go
• internal/commands/delete.go
• internal/userconfig/userconfig.go
• internal/userconfig/userconfig_test.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Use cmd.Println() to write to stderr; use fmt.Fprintln(cmd.OutOrStdout(), ...) for stdout output (paths, listings) in Cobra command implementations

Files:

• internal/commands/compare.go
• internal/userconfig/userconfig_test.go
• internal/commands/config.go
• internal/commands/delete.go
• internal/commands/commands_test.go
• internal/userconfig/userconfig.go

🧠 Learnings (1)

📚 Learning: 2026-01-13T19:27:50.183Z

Learnt from: CR
Repo: agarcher/wt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T19:27:50.183Z
Learning: Hooks should receive standardized environment variables: WT_NAME, WT_PATH, WT_BRANCH, WT_REPO_ROOT, WT_WORKTREE_DIR, WT_INDEX

Applied to files:

• README.md

🧬 Code graph analysis (3)

internal/commands/compare.go (1)

internal/userconfig/userconfig.go (1)

• FetchIntervalNever (147-147)

internal/userconfig/userconfig_test.go (1)

internal/userconfig/userconfig.go (4)

• DefaultFetchInterval (36-36)
• UserConfig (26-33)
• RepoConfig (20-23)
• ValidKeys (288-290)

internal/commands/delete.go (1)

internal/userconfig/userconfig.go (1)

• FetchIntervalNever (147-147)

🔇 Additional comments (20)

README.md (1)

303-344: LGTM!

Documentation updates are clear and consistent with the implementation. The examples effectively demonstrate the three modes: duration-based caching (10m), always fetch (0), and disabled (never).

internal/userconfig/userconfig.go (4)

146-147: LGTM!

Using a sentinel value (FetchIntervalNever = -1) is a clean approach to represent the "disabled" state while keeping the return type as time.Duration.

---

29-30: LGTM!

The comment and field definition correctly document the supported values for FetchInterval.

---

241-245: LGTM!

The cleanup logic correctly removes the repo entry when both Remote and FetchInterval are empty/nil, preventing stale entries in the config.

---

163-170: This validation already exists in the config command handler.

Duration strings are validated in internal/commands/config.go in the setConfig() function (lines 205-211) before SetGlobal() or SetForRepo() are called. Invalid durations are rejected with a user-friendly error message: "fetch_interval must be a valid duration (e.g., '5m', '1h', '0') or 'never'". The validation is tested and working as shown in commands_test.go where invalid duration "invalid" is properly rejected.

The current architecture is correct: validation happens at the command handler level (input time), and SetGlobal/SetForRepo are low-level data setters that appropriately don't duplicate validation.

Likely an incorrect or invalid review comment.

internal/commands/delete.go (1)

149-164: LGTM!

The fetch interval logic correctly handles all three modes:

• FetchIntervalNever: Skip fetch entirely
• 0: Always fetch (bypass interval check)
• > 0: Duration-based caching with skip message

The implementation is consistent with compare.go and properly uses cmd.PrintErrf for status messages to stderr.

internal/commands/compare.go (1)

61-75: LGTM!

The fetch interval logic is correctly implemented and consistent with delete.go. All three modes (never, always-fetch, duration-based) are properly handled.

internal/userconfig/userconfig_test.go (2)

15-17: LGTM!

The test correctly verifies that DefaultUserConfig() sets FetchInterval to DefaultFetchInterval ("5m").

---

91-117: LGTM!

Good test coverage for the unset flow, verifying that:

1. Unsetting one field preserves the other
2. Unsetting both fields removes the entire repo entry

internal/commands/config.go (6)

28-51: LGTM!

The command definition and help text are well-documented, clearly explaining the fetch_interval options including durations, 0 for always fetch, and never to disable. The examples cover the key use cases.

---

93-115: LGTM!

The function correctly uses cmd.OutOrStdout() for listing output as per coding guidelines. The handling of both global and per-repo fetch_interval values is appropriate, with the pointer type for per-repo allowing distinction between unset and empty values.

---

117-166: LGTM!

The show-origin logic correctly determines the source of fetch_interval with proper precedence (per-repo override > global > default). Output is correctly directed to stdout.

---

199-237: LGTM!

The validation logic for fetch_interval is correct. It properly handles the special never value and delegates to time.ParseDuration for duration strings (which correctly accepts 0 as a valid zero duration). The error message is clear and helpful.

---

239-269: LGTM!

The unset logic correctly handles both global and per-repo configurations with proper validation and persistence.

---

271-278: LGTM!

Simple and correct key validation implementation.

internal/commands/commands_test.go (5)

1081-1101: LGTM!

The help test correctly verifies that key configuration terms appear in the output. The check for "fetch" will match the fetch_interval documentation in the help text.

---

1133-1147: LGTM!

The test correctly validates the set/get flow for fetch_interval with a valid duration value.

---

1185-1202: LGTM!

The test correctly verifies that --list output includes the configured fetch_interval value with the expected format.

---

1205-1229: LGTM!

The show-origin test correctly verifies that both remote and fetch (matching fetch_interval) appear in the output.

---

1278-1310: LGTM!

Excellent test coverage for fetch_interval validation. The test covers all key scenarios: valid durations, the never special value, 0 for always-fetch behavior, and proper rejection of invalid inputs.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}