Skip to content

Add fetch_interval config to prevent repeated fetches#29

Merged
agarcher merged 1 commit intomainfrom
fetch-delay
Jan 15, 2026
Merged

Add fetch_interval config to prevent repeated fetches#29
agarcher merged 1 commit intomainfrom
fetch-delay

Conversation

@agarcher
Copy link
Owner

@agarcher agarcher commented Jan 15, 2026

Summary

  • Adds fetch_interval config option to set minimum time between fetches (default: 5m)
  • When a fetch was performed recently, commands skip fetching and print "Skipping fetch (last fetch Xs ago)"
  • Last fetch time stored per-remote in .git/wt-last-fetch-<remote>
  • Setting interval to "0" disables caching (always fetch)

Test plan

  • wt config --global fetch_interval 1m sets the interval
  • wt list fetches and prints "Fetched from origin"
  • Running wt list again within 1m prints "Skipping fetch (last fetch Xs ago)"
  • wt config fetch_interval 0 disables caching for current repo
  • wt config --show-origin displays fetch_interval with source

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added fetch_interval configuration to control how often remote repositories are fetched (default: 5 minutes).
    • Remote fetches now skip if performed within the configured interval, reducing unnecessary operations.
    • Fetch intervals can be configured globally or customized per-repository.

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

Adds a configurable minimum interval between fetches (default: 5m).
When a fetch was performed recently, subsequent commands skip fetching
and print "Skipping fetch (last fetch Xs ago)" instead.

- fetch_interval config option with Go duration format (e.g., "5m", "1h")
- Last fetch time stored per-remote in .git/wt-last-fetch-<remote>
- Setting interval to "0" disables caching (always fetch)
- Works in list, cleanup, and delete commands

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

This PR implements fetch throttling for remote repositories by introducing a configurable fetch_interval setting that tracks the time of last successful fetches. When enabled, remote fetches are skipped if they occurred within the interval, reducing unnecessary network operations.

Changes

Cohort / File(s) Summary
Configuration System
internal/userconfig/userconfig.go, internal/userconfig/userconfig_test.go, internal/commands/config.go
Adds fetch_interval key to global and per-repo configuration with duration validation. Extends GetGlobal, SetGlobal, UnsetGlobal, GetForRepo, SetForRepo, UnsetForRepo methods. New GetFetchIntervalForRepo() function computes effective interval with per-repo override precedence. DefaultFetchInterval constant set to "5m". Test updated to expect 3 valid configuration keys.
Git Timestamp Storage
internal/git/git.go
Introduces GetLastFetchTime() and SetLastFetchTime() public functions to read/write fetch timestamps from .git/wt-last-fetch-<remote> files using Unix timestamp format.
Fetch Throttling Logic
internal/commands/compare.go, internal/commands/delete.go
Implements conditional fetch logic: checks elapsed time since last fetch against configured interval and skips fetch with status message if within interval. Otherwise performs fetch and records timestamp on success. Adds formatDuration() helper for human-readable duration output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 No more frantic fetches, I declare!
With intervals set, we fetch with care.
Five minutes rest, then go once more,
Timestamps whisper what came before. 📚✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% 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 accurately summarizes the main change: introducing a fetch_interval configuration to prevent repeated fetches, which aligns with the substantial modifications across compare.go, config.go, delete.go, git.go, and userconfig.go files.

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

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
internal/git/git.go (1)

255-260: Consider sanitizing remote name in filename.

If a remote name contains path separators or special characters (e.g., /), this could cause unexpected behavior or potential path traversal. While remote names are typically simple identifiers, consider sanitizing.

Suggested fix
 func SetLastFetchTime(repoRoot, remote string) error {
-	fetchFile := filepath.Join(repoRoot, ".git", "wt-last-fetch-"+remote)
+	// Sanitize remote name to avoid path issues
+	safeRemote := strings.ReplaceAll(remote, string(filepath.Separator), "_")
+	fetchFile := filepath.Join(repoRoot, ".git", "wt-last-fetch-"+safeRemote)
 	timestamp := strconv.FormatInt(time.Now().Unix(), 10)
 	return os.WriteFile(fetchFile, []byte(timestamp+"\n"), 0644)
 }

Apply the same sanitization to GetLastFetchTime.

internal/userconfig/userconfig.go (1)

159-175: Consider validating duration format when setting, not when getting.

The silent error handling (d, _ := time.ParseDuration(...)) is documented, but users who misconfigure the interval (e.g., "5minutes" instead of "5m") will silently get always-fetch behavior. Since SetGlobal and SetForRepo accept any string without validation, invalid values are only caught at runtime.

Consider adding validation in the setters:

Proposed validation helper
// ValidateFetchInterval checks if a fetch_interval value is valid
func ValidateFetchInterval(value string) error {
	if value == "" || value == "0" {
		return nil // empty means default, "0" means always fetch
	}
	_, err := time.ParseDuration(value)
	if err != nil {
		return fmt.Errorf("invalid duration format %q: %w (examples: \"5m\", \"1h\", \"30s\")", value, err)
	}
	return nil
}

Then in SetGlobal:

 case "fetch_interval":
+	if err := ValidateFetchInterval(value); err != nil {
+		return err
+	}
 	c.FetchInterval = value

And similarly in SetForRepo.


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d154fc5 and 1763ce4.

📒 Files selected for processing (6)
  • internal/commands/compare.go
  • internal/commands/config.go
  • internal/commands/delete.go
  • internal/git/git.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/delete.go
  • internal/commands/compare.go
  • internal/git/git.go
  • internal/commands/config.go
  • internal/userconfig/userconfig_test.go
  • internal/userconfig/userconfig.go
🧬 Code graph analysis (3)
internal/commands/delete.go (1)
internal/git/git.go (3)
  • GetLastFetchTime (242-253)
  • FetchRemoteQuiet (221-225)
  • SetLastFetchTime (256-260)
internal/commands/compare.go (1)
internal/git/git.go (2)
  • GetLastFetchTime (242-253)
  • SetLastFetchTime (256-260)
internal/commands/config.go (1)
internal/userconfig/userconfig.go (1)
  • DefaultFetchInterval (39-39)
🔇 Additional comments (13)
internal/git/git.go (1)

241-253: LGTM - Clean timestamp read implementation.

The function correctly handles errors by returning zero time, which callers can safely use with time.Since() (will result in a large duration, triggering a fetch).

internal/userconfig/userconfig_test.go (1)

255-266: LGTM - Test correctly updated for new config key.

The test expectations properly reflect the addition of fetch_interval as a valid configuration key.

internal/commands/delete.go (1)

149-163: LGTM - Fetch throttling correctly implemented.

The logic properly:

  1. Retrieves the configured interval and last fetch time
  2. Skips fetch when within the interval (printing to stderr per guidelines)
  3. Records successful fetch time after completion
  4. Gracefully handles missing timestamp files (zero time triggers fetch)

Note: This relies on formatDuration defined in compare.go, which is accessible since both files are in the same package.

internal/commands/compare.go (3)

63-74: LGTM - Fetch throttling logic is sound.

The implementation correctly:

  1. Checks the configured interval against time since last fetch
  2. Skips fetch when within interval (stderr output per coding guidelines)
  3. Falls through to fetchWithSpinner which handles the actual fetch and timestamp recording

137-138: LGTM - Timestamp recorded after successful fetch.

Correctly placed after the fetch succeeds but before updating remote HEAD.


149-163: Consider rounding for cleaner output.

The duration formatting truncates rather than rounds, which could show "0s" for very short durations (< 1 second). This is likely fine given the use case (minimum interval is typically minutes), but worth noting.

internal/commands/config.go (4)

36-52: LGTM - Help text and examples are comprehensive.

The documentation clearly explains the new fetch_interval key, its default value, format, and how to disable caching with "0".


109-123: LGTM - Config list output correctly extended.

Both global and per-repo fetch_interval values are properly displayed using cmd.OutOrStdout() (stdout per coding guidelines for listings).


164-171: LGTM - Show-origin correctly displays fetch_interval source.

The three-way precedence (per-repo → global → default) is correctly handled and displayed.


235-240: LGTM - Duration validation is correct.

Using time.ParseDuration ensures valid Go duration format. This correctly accepts "0" to disable caching, and rejects invalid formats with a helpful error message.

internal/userconfig/userconfig.go (3)

21-24: LGTM!

Using a pointer type for FetchInterval correctly distinguishes between "not set" (nil) and "explicitly set to empty" (""), consistent with the existing Fetch *bool pattern.


247-258: LGTM!

The cleanup logic correctly checks all three fields before removing an empty repo entry.


273-277: LGTM!

The getter methods and ValidKeys() are correctly extended to handle fetch_interval, following the established patterns for other config keys.

Also applies to: 303-306, 314-314

✏️ 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.

@agarcher agarcher merged commit f3562e1 into main Jan 15, 2026
5 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 15, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant