Skip to content

perf(plugin): add timeout, offline fallback, and pinned caching for remote plugins - #3029

Merged
mikeland73 merged 2 commits into
mainfrom
mikeland73/perf-remote-plugin-cache
Oct 29, 2026
Merged

mikeland73 merged 2 commits into
mainfrom
mikeland73/perf-remote-plugin-cache

Conversation

@mikeland73

Copy link
Copy Markdown
Collaborator

Summary

Implements item 11 of specs/perf-improvements.md. Remote (GitHub and git) plugins are cached on disk for 24h. When that cache expired, devbox made a blocking network call with no timeout and failed outright if the call failed. Git plugins pinned to a commit also did one full clone per file.

Timeout

  • GitHub plugin files are fetched with a shared githubHTTPClient that has a 15s timeout. Before, it was &http.Client{} with no timeout. There was no shared client or timeout constant to reuse. Other call sites use anything from 3s (nixpkgs check) to 10min (launcher download).

Offline fallback

  • New getOrFetch helper in internal/plugin/cache.go, used by both GitHub and git plugins in place of filecache.GetOrSet. If the cached entry has expired and the fetch fails, it returns the expired copy and logs a slog.Warn (only shown with DEVBOX_DEBUG=1, like the rest of the plugin logging). If nothing is cached, the fetch error is returned as before.
  • filecache didn't need to change: Get already returns the decoded value together with filecache.Expired.

Commit-pinned content is cached indefinitely

  • If the plugin's Rev is a full 40-character hex commit hash, files are cached with a ~100 year TTL and DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL is ignored. Branches, tags, short hashes, and no ref still use 24h (or the env var).
  • This applies to git plugins (?rev=) and GitHub plugins (github:owner/repo/<sha> or ?rev=). For GitHub, the URL already uses the rev (raw.githubusercontent.com/owner/repo/<sha>/...), so the content can't change.
  • The TTL is still part of the cache key, so pinned entries use new keys. Each pinned file gets fetched once more after upgrading.

One clone per repo/ref

  • Git plugins now keep one clone per (URL, ref, rev) for the life of the process and read every file from it. Before, every FileContent call that missed the cache did a full clone (plugin.json, then one per create_files entry). Plugins in the same repo with different dirs share a clone. Clone args, GIT_SSH_COMMAND handling, and checkout of rev are unchanged.
  • A failed clone is remembered too, so an unreachable remote is only tried once per command. The other files then use their expired cached copies.
  • Clones go in temp dirs as before. boxcli.Execute defers plugin.RemoveGitClones() to delete them. Before, each clone was deleted right after reading the one file.

Testing

  • New tests in internal/plugin/cache_test.go. Caches point at a temp dir via filecache.WithCacheDir. git clones are stubbed and GitHub is an httptest server, so nothing uses the network:
    • getOrFetch: fresh hit doesn't fetch, expired entry is refreshed, expired entry is used when the fetch fails, error when nothing is cached.
    • remoteCacheTTL: full hashes (lower and upper case) are pinned. Short hashes, branches, and empty are not. The env var only applies to unpinned refs.
    • git: one clone across 4 files and two dirs in the same repo/ref, and a second clone for another ref. Commit-pinned content doesn't expire with a 1ns TTL. Branch and short-hash refs do. Stale fallback when the clone fails. Error when nothing is cached, with only one clone attempt.
    • GitHub: client has a timeout. Commit-pinned content isn't refetched. Branch content is. Stale fallback on a 500 and with the server closed. Error for a file that was never cached.
  • Updated the existing git tests to use a temp cache dir. Before, they cleared the real user cache. TestGitPluginFileContentCacheRespectsEnvVar expected an error after expiry with the repo deleted. It's now TestGitPluginFileContentExpiredCloneFails and expects the stale content, using a real file:// clone.
  • go test -race ./internal/plugin/ ./internal/boxcli/ and go test ./internal/... pass.
  • go tool golangci-lint run ./internal/plugin/... ./internal/boxcli/: 0 issues. The golangci-lint on PATH is built with go1.24 and refuses to load this go1.27 module, so I used the repo's pinned go tool version, same as devbox run lint.

Not done

  • Any fetch failure falls back to stale content, including a 404 or auth error, not only network errors. If a file is removed upstream from a branch, devbox keeps using the cached copy (with a debug-level warning) instead of failing.
  • After a failed refetch, the stale entry isn't re-stamped, so every command retries the fetch while offline. That's at most one clone attempt per repo, or one request per GitHub file bounded by the 15s timeout. Usually it fails quickly (DNS or connection refused).
  • git clone still has no timeout.
  • A rev clone is still a full clone followed by git checkout. A shallow fetch of a single commit would be faster, but not every git server allows fetching an arbitrary SHA.

🤖 Generated with Claude Code

mikeland73 and others added 2 commits October 29, 2026 14:07
…emote plugins

Implements item 11 of specs/perf-improvements.md.

- GitHub plugin files are fetched with a shared http.Client with a 15s
  timeout instead of a bare client with no timeout.
- When a cached GitHub or git plugin file has expired and the refetch
  fails, the expired copy is used and a slog.Warn is logged. If nothing
  is cached, the error is returned as before.
- Content fetched at a full 40-character commit hash (git `?rev=` or
  GitHub path/`?rev=`) is cached indefinitely and ignores
  DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL. Branches, tags, and short hashes
  still expire after 24h.
- Git plugins clone each repo/ref/rev once per process and read every
  file from that clone, instead of cloning once per file. A failed clone
  is remembered for the rest of the process. boxcli.Execute deletes the
  clones on exit.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikeland73

Copy link
Copy Markdown
Collaborator Author

@claude ptal

@claude

claude Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Claude finished @mikeland73's task in 29s —— View job


Review of #3029

I read the non-test diff (cache.go, git.go, github.go, root.go). I didn't build the code or run the tests, so the CI results you listed are untested on my side. I found no blocking issues. The change matches the description, and the three pieces fit together cleanly.

What looks right

  • getOrFetch is small and handles the cases correctly. A fresh entry is returned without fetching. An expired entry is used only when the fetch fails. When nothing is cached, the fetch error is returned.
  • isFullCommitHash reuses isBranchName, which already checks for exactly 40 hex characters in either case. The "pinned" check therefore stays consistent with the existing git logic.
  • The clone cache key is (baseURL, Ref, Rev), so plugins that differ only by dir share one clone. readFromClone still joins p.ref.Dir, so each plugin reads from its own directory.
  • A failed clone() removes its temp dir before returning. Failed clones are cached in gitClones, so an unreachable remote is only tried once.
  • The cache key still includes the TTL, as your description says. Pinned entries get new keys and are fetched once more after upgrading, which is the right trade-off.

Minor points, none blocking

  1. Clone cleanup. defer plugin.RemoveGitClones() in boxcli.Execute (internal/boxcli/root.go:106) won't run if something calls os.Exit or re-execs before Execute returns. The vercheck/launcher re-run path is one candidate. A clone made before such an exit would leak a devbox-git-plugin-* dir in the temp directory. Previously each clone was deleted right after its read. This is probably rare, but it's worth checking those paths, or accepting the leak.
  2. Stale fallback on permanent errors. You listed this under "Not done". A 404 or auth failure on GitHub also returns stale content, so a plugin file deleted upstream stays usable for as long as it's cached. For pinned commits the cache never expires, so this only matters for branch refs. Restricting the fallback to transport errors and 5xx responses would be a small follow-up. You would have to distinguish them inside the fetch closures.
  3. Return value on Set failure. return content, cache.Set(key, content, ttl) in getOrFetch (internal/plugin/cache.go) returns both the content and an error when the cache write fails. Callers that check err will drop good content over a disk-cache write problem. Logging the Set error at warn level and returning the content would be more forgiving, since the fetch itself succeeded. This is a judgement call, and it may match what GetOrSet did before.
  4. Corrupt cache entries. getOrFetch returns the cache error directly when it isn't a miss or an expiry (!filecache.IsCacheMiss). A corrupt cache entry therefore fails the plugin instead of triggering a refetch. If GetOrSet handled this differently before, it's worth confirming the behavior hasn't regressed.
  5. Clone lock. cloneDir holds the global gitClones mutex during the clone. That's fine for the current sequential use. If plugins are ever fetched concurrently, clones of different repos would be serialized.

Everything else in "Not done" is a reasonable scope cut, including the missing git clone timeout and the full clone plus checkout for rev.
· branch mikeland73/perf-remote-plugin-cache

@mikeland73
mikeland73 merged commit 8c74cc5 into main Oct 29, 2026
28 checks passed
@mikeland73
mikeland73 deleted the mikeland73/perf-remote-plugin-cache branch October 29, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant