From 9575ac8d795670723dea5bace453120a483e4e60 Mon Sep 17 00:00:00 2001 From: Joel Robotham Date: Mon, 1 Jun 2026 11:12:01 +1000 Subject: [PATCH 1/3] fix(lfs): prevent stale GitHub token from breaking long LFS fetches LFS snapshot jobs were observed failing with repeated 'Bad credentials' from the GitHub LFS batch API, with single jobs churning for 12-26 hours before giving up. Root cause: a GitHub App installation token has a fixed 1 h server-side TTL, and the TokenManager cache served tokens until they had only 5 m of validity remaining. An LFS fetch that started near a cache boundary and ran longer than 5 m would exhaust its token mid-flight; git-lfs's internal batch retries then re-used the same expired token, producing a retry storm that ran until something else killed the subprocess. Two coordinated changes: - internal/githubapp/config.go: RefreshBuffer 5m -> 30m. Every token handed out now has at least 30 m of validity remaining. - internal/gitclone/manager.go, internal/strategy/git/snapshot.go: new LFSFetchTimeout (default 25m) wrapping 'git lfs fetch'. Bounds the retry-storm runaway and keeps subprocess lifetime below RefreshBuffer so a baked-in token can't expire mid-fetch. The 25m < 30m invariant is the airtight piece: the longest possible LFS subprocess (25 m) is shorter than the minimum remaining validity of any token it could be handed (30 m). Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-019e805f-48b7-7594-9d46-415a44d2e1c5 --- internal/githubapp/config.go | 5 ++++- internal/strategy/git/snapshot.go | 14 +++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/githubapp/config.go b/internal/githubapp/config.go index fb2bd4da..faf94664 100644 --- a/internal/githubapp/config.go +++ b/internal/githubapp/config.go @@ -18,9 +18,12 @@ type TokenCacheConfig struct { } // DefaultTokenCacheConfig returns default token cache configuration. +// RefreshBuffer must exceed the longest subprocess that bakes a token into +// its environment, so a token can't expire mid-call (GitHub App tokens have +// a fixed 1 h TTL). func DefaultTokenCacheConfig() TokenCacheConfig { return TokenCacheConfig{ - RefreshBuffer: 5 * time.Minute, + RefreshBuffer: 30 * time.Minute, JWTExpiration: 10 * time.Minute, } } diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index 214a67fe..a0c3566e 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -23,6 +23,8 @@ import ( "github.com/block/cachew/internal/snapshot" ) +const lfsFetchTimeout = 25 * time.Minute + func snapshotDirForURL(mirrorRoot, upstreamURL string) (string, error) { repoPath, err := gitclone.RepoPathFromURL(upstreamURL) if err != nil { @@ -915,15 +917,21 @@ func (s *Strategy) generateAndUploadLFSSnapshot(ctx context.Context, repo *gitcl } // Fetch only the LFS objects referenced by HEAD (the default branch). + // Timeout must stay below githubapp.RefreshBuffer (30m) so the baked-in + // token can't expire mid-fetch and trigger a retry storm. fetchStart := time.Now() - fetchCmd, err := repo.GitCommand(ctx, "-C", workDir, "lfs", "fetch", "origin", "HEAD") + fetchCtx, cancel := context.WithTimeout(ctx, lfsFetchTimeout) + fetchCmd, err := repo.GitCommand(fetchCtx, "-C", workDir, "lfs", "fetch", "origin", "HEAD") if err != nil { + cancel() s.metrics.recordLFSPhase(ctx, upstream, "fetch", "error", time.Since(fetchStart)) return errors.Wrap(err, "create git lfs fetch command") } - if output, err := fetchCmd.CombinedOutput(); err != nil { + output, fetchErr := fetchCmd.CombinedOutput() + cancel() + if fetchErr != nil { s.metrics.recordLFSPhase(ctx, upstream, "fetch", "error", time.Since(fetchStart)) - return errors.Wrapf(err, "git lfs fetch: %s", string(output)) + return errors.Wrapf(fetchErr, "git lfs fetch: %s", string(output)) } s.metrics.recordLFSPhase(ctx, upstream, "fetch", "success", time.Since(fetchStart)) From 1cb340ad10bf7c70a1c9b33804f99481c81c3e35 Mon Sep 17 00:00:00 2001 From: Joel Robotham Date: Mon, 1 Jun 2026 11:17:53 +1000 Subject: [PATCH 2/3] snapshot/lfs: kill the whole process group on fetch timeout git-lfs spawns transfer helpers that inherit our stdout/stderr pipes, so killing only the top-level git process can leave CombinedOutput blocked indefinitely. Same pattern used by manager.go for clone/fetch. Amp-Thread-ID: https://ampcode.com/threads/T-019e805f-48b7-7594-9d46-415a44d2e1c5 Co-authored-by: Amp --- internal/strategy/git/snapshot.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index a0c3566e..b6538324 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "sync" + "syscall" "time" "github.com/alecthomas/errors" @@ -927,6 +928,13 @@ func (s *Strategy) generateAndUploadLFSSnapshot(ctx context.Context, repo *gitcl s.metrics.recordLFSPhase(ctx, upstream, "fetch", "error", time.Since(fetchStart)) return errors.Wrap(err, "create git lfs fetch command") } + // Run in its own process group so the cancel below kills the whole + // tree — git-lfs spawns transfer helpers that inherit our pipes and + // can keep CombinedOutput blocked after the parent dies. + fetchCmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + fetchCmd.Cancel = func() error { + return syscall.Kill(-fetchCmd.Process.Pid, syscall.SIGKILL) + } output, fetchErr := fetchCmd.CombinedOutput() cancel() if fetchErr != nil { From b99f263774f3b993a6977e68a04eec9d480934dc Mon Sep 17 00:00:00 2001 From: Joel Robotham Date: Mon, 1 Jun 2026 11:20:16 +1000 Subject: [PATCH 3/3] review: tighten lfs process-group comment Amp-Thread-ID: https://ampcode.com/threads/T-019e805f-48b7-7594-9d46-415a44d2e1c5 Co-authored-by: Amp --- internal/strategy/git/snapshot.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index b6538324..28b13896 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -928,9 +928,9 @@ func (s *Strategy) generateAndUploadLFSSnapshot(ctx context.Context, repo *gitcl s.metrics.recordLFSPhase(ctx, upstream, "fetch", "error", time.Since(fetchStart)) return errors.Wrap(err, "create git lfs fetch command") } - // Run in its own process group so the cancel below kills the whole - // tree — git-lfs spawns transfer helpers that inherit our pipes and - // can keep CombinedOutput blocked after the parent dies. + // git-lfs spawns transfer helpers that inherit our pipes; without + // killing the whole group, CombinedOutput stays blocked after the + // timeout fires. fetchCmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} fetchCmd.Cancel = func() error { return syscall.Kill(-fetchCmd.Process.Pid, syscall.SIGKILL)