From a819564959f351507ba750e78f486bbb540e076b Mon Sep 17 00:00:00 2001 From: Joey L Date: Tue, 5 May 2026 05:01:40 +0000 Subject: [PATCH 1/4] add reclone as fallback --- go/cmd/gitter/gitter.go | 120 ++++++++++++++++++++++++++--------- go/cmd/gitter/gitter_test.go | 35 ++++++++++ 2 files changed, 126 insertions(+), 29 deletions(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 406ccbc8072..a8c74dd15b5 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -312,15 +312,6 @@ func isAuthError(err error) bool { (strings.Contains(strings.ToLower(errString), "repository") && strings.Contains(strings.ToLower(errString), "not found")) } -func isIndexLockError(err error) bool { - if err == nil { - return false - } - errString := err.Error() - - return strings.Contains(errString, "index.lock") && strings.Contains(errString, "File exists") -} - // Helper function to unmarshal request body based on Content-Type (protobuf or JSON) func unmarshalRequest(r *http.Request, body proto.Message) error { data, err := io.ReadAll(r.Body) @@ -407,6 +398,73 @@ func getFreshRepo(ctx context.Context, w http.ResponseWriter, repoURL string, fo return repo, nil } +func isIndexLockError(err error) bool { + if err == nil { + return false + } + errString := err.Error() + + return strings.Contains(errString, "index.lock") && strings.Contains(errString, "File exists") +} + +func isRefConflictError(err error) bool { + if err == nil { + return false + } + errString := err.Error() + + return strings.Contains(errString, "refname conflict") || + (strings.Contains(errString, "some local refs could not be updated") && strings.Contains(errString, "try running 'git remote prune origin'")) +} + +// Attempt to recover from git fetch + reset errors +// Returns true if recovery was attempted and we should retry fetch + reset +func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { + if err == nil { + return false + } + + // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. + // We can remove the file and retry + if isIndexLockError(err) { + logger.WarnContext(ctx, "index.lock exists, attempting to remove") + indexLockPath := filepath.Join(repoPath, ".git", "index.lock") + if err := os.Remove(indexLockPath); err != nil { + logger.ErrorContext(ctx, "failed to remove index.lock", slog.Any("err", err)) + return false + } + return true + } + + // Refname conflict, likely name conflict between local and remote refs + // We can try removing stale remote-tracking branches and retry + if isRefConflictError(err) { + logger.WarnContext(ctx, "ref conflict detected, running git remote prune origin") + if err := runCmd(ctx, repoPath, nil, "git", "remote", "prune", "origin"); err != nil { + logger.ErrorContext(ctx, "failed to prune origin", slog.Any("err", err)) + return false + } + return true + } + + return false +} + +// Helper function to group git fetch and git reset --hard together +func fetchAndReset(ctx context.Context, repoPath string) error { + err := runCmd(ctx, repoPath, nil, "git", "fetch", "origin") + if err != nil { + return fmt.Errorf("git fetch failed: %w", err) + } + + err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD") + if err != nil { + return fmt.Errorf("git reset failed: %w", err) + } + + return nil +} + func FetchRepo(ctx context.Context, repoURL string, forceUpdate bool) error { logger.InfoContext(ctx, "Starting fetch repo") start := time.Now() @@ -424,7 +482,6 @@ func FetchRepo(ctx context.Context, repoURL string, forceUpdate bool) error { // Check if we need to fetch if forceUpdate || !ok || time.Since(accessTime) > fetchTimeout { - logger.InfoContext(ctx, "Fetching git blob", slog.Duration("sinceAccessTime", time.Since(accessTime))) if _, err := os.Stat(filepath.Join(repoPath, ".git")); os.IsNotExist(err) { // Clone logger.InfoContext(ctx, "Cloning git repository", slog.Duration("sinceAccessTime", time.Since(accessTime))) @@ -433,28 +490,33 @@ func FetchRepo(ctx context.Context, repoURL string, forceUpdate bool) error { return fmt.Errorf("git clone failed: %w", err) } } else { - // Fetch/Pull - implementing simple git pull for now, might need reset --hard if we want exact mirrors - // For a generic "get latest", pull is usually sufficient if we treat it as read-only. - // Ideally safely: git fetch origin && git reset --hard origin/HEAD + // Fetch and reset logger.InfoContext(ctx, "Fetching git repository", slog.Duration("sinceAccessTime", time.Since(accessTime))) - err := runCmd(ctx, repoPath, nil, "git", "fetch", "origin") + err := fetchAndReset(ctx, repoPath) + + // Attempt recovery and fallback if err != nil { - return fmt.Errorf("git fetch failed: %w", err) - } - err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD") - if err != nil && isIndexLockError(err) { - // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. - // We can remove the file and retry the command - logger.WarnContext(ctx, "index.lock exists, attempting to remove and retry") - indexLockPath := filepath.Join(repoPath, ".git", "index.lock") - if err := os.Remove(indexLockPath); err != nil { - return fmt.Errorf("failed to remove index.lock in %s: %w", repoPath, err) + logger.WarnContext(ctx, "Initial fetch and reset failed, attempting to recover", slog.Any("err", err)) + + // Attempt recovery and retry fetch and reset if successful + if attemptGitRecovery(ctx, repoPath, err) { + logger.InfoContext(ctx, "Retrying fetch and reset after recovery") + err = fetchAndReset(ctx, repoPath) + } + + // If still failing or recovery wasn't attempted, reclone the repo as final fallback + if err != nil { + logger.WarnContext(ctx, "Fetch and reset failed after recovery attempt, deleting repo and recloning", slog.Any("err", err)) + if err := os.RemoveAll(repoPath); err != nil { + return fmt.Errorf("failed to remove repo directory for reclone: %w", err) + } + + logger.InfoContext(ctx, "Cloning git repository after fallback", slog.Duration("sinceAccessTime", time.Since(accessTime))) + err := runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", repoURL, repoPath) + if err != nil { + return fmt.Errorf("git clone failed after fallback: %w", err) + } } - // One more attempt at git reset - err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD") - } - if err != nil { - return fmt.Errorf("git reset failed: %w", err) } } diff --git a/go/cmd/gitter/gitter_test.go b/go/cmd/gitter/gitter_test.go index a31d8fc8b57..f3fc3c2b4ef 100644 --- a/go/cmd/gitter/gitter_test.go +++ b/go/cmd/gitter/gitter_test.go @@ -140,6 +140,41 @@ func TestIsAuthError(t *testing.T) { } } +func TestIsIndexLockError(t *testing.T) { + tests := []struct { + err error + expected bool + }{ + {errors.New("fatal: Unable to create '/path/to/repo.git/index.lock': File exists."), true}, + {errors.New("some other error"), false}, + {nil, false}, + } + + for _, tt := range tests { + if result := isIndexLockError(tt.err); result != tt.expected { + t.Errorf("isIndexLockError(%v) = %v, expected %v", tt.err, result, tt.expected) + } + } +} + +func TestIsRefConflictError(t *testing.T) { + tests := []struct { + err error + expected bool + }{ + {errors.New("error: some local refs could not be updated; try running 'git remote prune origin' to remove any old, conflicting branches"), true}, + {errors.New("error: fetching ref refs/remotes/some-ref-name failed: refname conflict"), true}, + {errors.New("some other error"), false}, + {nil, false}, + } + + for _, tt := range tests { + if result := isRefConflictError(tt.err); result != tt.expected { + t.Errorf("isRefConflictError(%v) = %v, expected %v", tt.err, result, tt.expected) + } + } +} + func TestGitHandler_InvalidURL(t *testing.T) { tests := []struct { url string From 5b666f247448b631adba3eb0b4f2e9e191fdbc20 Mon Sep 17 00:00:00 2001 From: Joey L Date: Tue, 5 May 2026 05:26:12 +0000 Subject: [PATCH 2/4] The venn diagram of lint and mean girls reference is two circles. --- go/cmd/gitter/gitter.go | 2 ++ go/cmd/gitter/gitter_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index a8c74dd15b5..49c839e0154 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -433,6 +433,7 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { logger.ErrorContext(ctx, "failed to remove index.lock", slog.Any("err", err)) return false } + return true } @@ -444,6 +445,7 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { logger.ErrorContext(ctx, "failed to prune origin", slog.Any("err", err)) return false } + return true } diff --git a/go/cmd/gitter/gitter_test.go b/go/cmd/gitter/gitter_test.go index f3fc3c2b4ef..00dec0ac66c 100644 --- a/go/cmd/gitter/gitter_test.go +++ b/go/cmd/gitter/gitter_test.go @@ -145,7 +145,7 @@ func TestIsIndexLockError(t *testing.T) { err error expected bool }{ - {errors.New("fatal: Unable to create '/path/to/repo.git/index.lock': File exists."), true}, + {errors.New("fatal: Unable to create '/path/to/repo.git/index.lock': File exists"), true}, {errors.New("some other error"), false}, {nil, false}, } From 5d57c8d974a732489236ab8f84feb0227fc113a7 Mon Sep 17 00:00:00 2001 From: Joey L Date: Tue, 5 May 2026 05:31:57 +0000 Subject: [PATCH 3/4] you're going in my burn book lint >:( --- go/cmd/gitter/gitter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 49c839e0154..110c51cae44 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -445,7 +445,7 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { logger.ErrorContext(ctx, "failed to prune origin", slog.Any("err", err)) return false } - + return true } From 89ec5b61a7c56714f67a3e94f92fa282fa83b10c Mon Sep 17 00:00:00 2001 From: Joey L Date: Wed, 6 May 2026 04:19:59 +0000 Subject: [PATCH 4/4] Only keep the refname conflict fix attempt. --- go/cmd/gitter/gitter.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 110c51cae44..495a2e66351 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -424,19 +424,6 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { return false } - // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. - // We can remove the file and retry - if isIndexLockError(err) { - logger.WarnContext(ctx, "index.lock exists, attempting to remove") - indexLockPath := filepath.Join(repoPath, ".git", "index.lock") - if err := os.Remove(indexLockPath); err != nil { - logger.ErrorContext(ctx, "failed to remove index.lock", slog.Any("err", err)) - return false - } - - return true - } - // Refname conflict, likely name conflict between local and remote refs // We can try removing stale remote-tracking branches and retry if isRefConflictError(err) { @@ -449,6 +436,12 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { return true } + // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. + // We want to reclone as fallback but log a separate warning (for stats) + if isIndexLockError(err) { + logger.WarnContext(ctx, "index.lock exists, will reclone instead") + } + return false }