From a38baf4443e207786d410bff760203d9cfab5a7a Mon Sep 17 00:00:00 2001 From: Fabio Date: Mon, 23 Feb 2026 11:02:44 +0100 Subject: [PATCH 1/4] feat: add HealthFailed category for repos where commit fetching errors When ListCommits returns an error (network, auth, rate limiting), repos are now classified as "failed" instead of being silently marked "abandoned". Failed repos are excluded from code percentage calculations in the health summary to avoid skewing active/maintained/abandoned ratios. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 +- README.md | 12 ++------- cmd/codemium/main.go | 15 +++++++++-- internal/health/health.go | 14 +++++++--- internal/health/health_test.go | 48 ++++++++++++++++++++++++++++++---- internal/health/summary.go | 7 ++++- internal/model/model.go | 2 ++ internal/output/markdown.go | 13 ++++++++- 8 files changed, 89 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6ad3247..fe94935 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,7 @@ internal/ - **Clone strategy**: Shallow clone (depth 1, single branch, no tags) to temp dir, deleted after analysis. - **scc initialization**: `processor.ProcessConstants()` called via `sync.Once` since scc requires global initialization. - **AI estimation**: When `--ai-estimate` is used, a second pass fetches commit history via provider REST APIs. `provider.CommitLister` interface provides `ListCommits` and `CommitStats`. `aidetect.Detect` classifies commits, `aiestimate.Estimate` orchestrates per-repo. Results attach to existing report model as optional fields. -- **Health classification**: When `--health` is used, repos are classified as Active (<180d), Maintained (180-365d), or Abandoned (>365d) based on last commit date. `--health-details` adds deep analysis: per-window author counts, code churn, bus factor, and velocity trend. Uses the same `CommitLister` interface. +- **Health classification**: When `--health` is used, repos are classified as Active (<180d), Maintained (180-365d), or Abandoned (>365d) based on last commit date. Repos where commit history cannot be fetched (API errors, permissions) are classified as Failed. `--health-details` adds deep analysis: per-window author counts, code churn, bus factor, and velocity trend. Uses the same `CommitLister` interface. - **Vendor/generated filtering**: Always-on filtering using `go-enry` to skip vendor, generated, and binary files during analysis. `FilteredFiles` count is tracked per repo and in report totals. - **License detection**: After analysis, `license.Detect` scans the cloned repo directory for SPDX license identifiers (e.g., "MIT", "Apache-2.0"). Results appear in the per-repo License column. - **Code churn / hotspots**: Opt-in via `--churn` flag. Uses provider REST APIs to fetch per-file change data (`--churn-limit N` sets max commits, default 500). `churn.Analyze` collects per-file change frequencies; `churn.ComputeHotspots` ranks files by churn x complexity. Top 20 hotspots shown per repo. diff --git a/README.md b/README.md index c1f0f46..d12f7a6 100644 --- a/README.md +++ b/README.md @@ -111,12 +111,6 @@ export CODEMIUM_GITHUB_TOKEN=your_personal_access_token ``` This prompts for your token, verifies it against the GitLab API, and stores it at `~/.config/codemium/credentials.json`. -For self-hosted GitLab instances, set `CODEMIUM_GITLAB_URL`: -```bash -export CODEMIUM_GITLAB_URL=https://gitlab.example.com -codemium auth login --provider gitlab -``` - **Option 2: glab CLI** If you have the [GitLab CLI](https://gitlab.com/gitlab-org/cli) installed and authenticated, codemium can use its token automatically: @@ -183,9 +177,6 @@ codemium analyze --provider gitlab --group myorg/mysubgroup # Specific repos codemium analyze --provider gitlab --group mygroup --repos api,frontend - -# Self-hosted GitLab -CODEMIUM_GITLAB_URL=https://gitlab.example.com codemium analyze --provider gitlab --group mygroup ``` ### Analyze trends over time @@ -267,7 +258,7 @@ This is especially useful when Bitbucket project codes or repo naming convention ### Repository health classification -Classify repositories as Active, Maintained, or Abandoned based on last commit date: +Classify repositories as Active, Maintained, Abandoned, or Failed based on commit history: ```bash # Quick health check (1 API call per repo, no cloning) @@ -284,6 +275,7 @@ Health categories: - **Active**: last commit < 180 days ago - **Maintained**: 180–365 days ago - **Abandoned**: > 365 days ago +- **Failed**: commit history could not be fetched (API error, permissions, etc.) ### Additional flags diff --git a/cmd/codemium/main.go b/cmd/codemium/main.go index 67e9f43..4ba8100 100644 --- a/cmd/codemium/main.go +++ b/cmd/codemium/main.go @@ -548,7 +548,14 @@ func runAnalyze(cmd *cobra.Command, args []string) error { healthResults := worker.RunWithProgress(ctx, repoList, concurrency, func(ctx context.Context, repo model.Repo) (*model.RepoStats, error) { commits, err := commitLister.ListCommits(ctx, repo, commitLimit) if err != nil { - return nil, err + // If we can't fetch commits, mark as failed rather than skipping + return &model.RepoStats{ + Repository: repo.Slug, + Health: &model.RepoHealth{ + Category: model.HealthFailed, + DaysSinceCommit: -1, + }, + }, nil } h := health.ClassifyFromCommits(commits, now) @@ -557,7 +564,11 @@ func runAnalyze(cmd *cobra.Command, args []string) error { if healthDetailsFlag && len(commits) > 0 { details, err = health.AnalyzeDetails(ctx, commitLister, repo, commits, now) if err != nil { - return nil, err + // Return health without details if details fail + return &model.RepoStats{ + Repository: repo.Slug, + Health: h, + }, nil } } diff --git a/internal/health/health.go b/internal/health/health.go index 5f5d4b5..05e3249 100644 --- a/internal/health/health.go +++ b/internal/health/health.go @@ -37,11 +37,13 @@ func Classify(lastCommitDate, now time.Time) *model.RepoHealth { } } -// ClassifyFromCommits classifies a repo based on a list of commits. -// It finds the most recent commit date. Returns nil if commits is empty. func ClassifyFromCommits(commits []provider.CommitInfo, now time.Time) *model.RepoHealth { if len(commits) == 0 { - return nil + return &model.RepoHealth{ + Category: model.HealthAbandoned, + LastCommitDate: "", + DaysSinceCommit: -1, + } } latest := commits[0].Date @@ -52,7 +54,11 @@ func ClassifyFromCommits(commits []provider.CommitInfo, now time.Time) *model.Re } if latest.IsZero() { - return nil + return &model.RepoHealth{ + Category: model.HealthAbandoned, + LastCommitDate: "", + DaysSinceCommit: -1, + } } return Classify(latest, now) diff --git a/internal/health/health_test.go b/internal/health/health_test.go index 938038f..a0132b5 100644 --- a/internal/health/health_test.go +++ b/internal/health/health_test.go @@ -42,8 +42,14 @@ func TestClassifyBoundaries(t *testing.T) { func TestClassifyFromCommitsEmpty(t *testing.T) { now := time.Date(2026, 2, 23, 0, 0, 0, 0, time.UTC) result := ClassifyFromCommits(nil, now) - if result != nil { - t.Error("expected nil for empty commits") + if result == nil { + t.Fatal("expected non-nil result for empty commits") + } + if result.Category != model.HealthAbandoned { + t.Errorf("expected abandoned for empty commits, got %s", result.Category) + } + if result.DaysSinceCommit != -1 { + t.Errorf("expected -1 days for empty commits, got %d", result.DaysSinceCommit) } } @@ -107,6 +113,38 @@ func TestSummarizeMixed(t *testing.T) { } } +func TestSummarizeWithFailed(t *testing.T) { + repos := []model.RepoStats{ + { + Repository: "active-repo", + Totals: model.Stats{Code: 1000}, + Health: &model.RepoHealth{Category: model.HealthActive}, + }, + { + Repository: "failed-repo", + Totals: model.Stats{Code: 500}, + Health: &model.RepoHealth{Category: model.HealthFailed, DaysSinceCommit: -1}, + }, + } + + summary := Summarize(repos) + if summary == nil { + t.Fatal("expected non-nil summary") + } + if summary.Active.Repos != 1 { + t.Errorf("expected 1 active repo, got %d", summary.Active.Repos) + } + if summary.Failed.Repos != 1 { + t.Errorf("expected 1 failed repo, got %d", summary.Failed.Repos) + } + if summary.Failed.Code != 500 { + t.Errorf("expected 500 failed code, got %d", summary.Failed.Code) + } + if summary.Active.CodePercent != 100.0 { + t.Errorf("expected 100%% active code (failed excluded from total), got %.1f%%", summary.Active.CodePercent) + } +} + func TestSummarizeNoHealth(t *testing.T) { repos := []model.RepoStats{ {Repository: "repo-1", Totals: model.Stats{Code: 1000}}, @@ -119,9 +157,9 @@ func TestSummarizeNoHealth(t *testing.T) { // mockCommitLister implements provider.CommitLister for testing. type mockCommitLister struct { - commits []provider.CommitInfo - statsMap map[string][2]int64 // hash -> [additions, deletions] - statsErr error + commits []provider.CommitInfo + statsMap map[string][2]int64 // hash -> [additions, deletions] + statsErr error } func (m *mockCommitLister) ListCommits(_ context.Context, _ model.Repo, _ int) ([]provider.CommitInfo, error) { diff --git a/internal/health/summary.go b/internal/health/summary.go index ff8e916..afc8c4d 100644 --- a/internal/health/summary.go +++ b/internal/health/summary.go @@ -17,18 +17,23 @@ func Summarize(repos []model.RepoStats) *model.HealthSummary { continue } hasHealth = true - totalCode += r.Totals.Code switch r.Health.Category { case model.HealthActive: summary.Active.Repos++ summary.Active.Code += r.Totals.Code + totalCode += r.Totals.Code case model.HealthMaintained: summary.Maintained.Repos++ summary.Maintained.Code += r.Totals.Code + totalCode += r.Totals.Code case model.HealthAbandoned: summary.Abandoned.Repos++ summary.Abandoned.Code += r.Totals.Code + totalCode += r.Totals.Code + case model.HealthFailed: + summary.Failed.Repos++ + summary.Failed.Code += r.Totals.Code } } diff --git a/internal/model/model.go b/internal/model/model.go index be32cd7..2211c36 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -114,6 +114,7 @@ const ( HealthActive HealthCategory = "active" HealthMaintained HealthCategory = "maintained" HealthAbandoned HealthCategory = "abandoned" + HealthFailed HealthCategory = "failed" ) // RepoHealth holds the health classification for a repository. @@ -151,6 +152,7 @@ type HealthSummary struct { Active HealthCategorySummary `json:"active"` Maintained HealthCategorySummary `json:"maintained"` Abandoned HealthCategorySummary `json:"abandoned"` + Failed HealthCategorySummary `json:"failed"` } // Filters records what filters were applied to the analysis. diff --git a/internal/output/markdown.go b/internal/output/markdown.go index f11e4de..408ae1c 100644 --- a/internal/output/markdown.go +++ b/internal/output/markdown.go @@ -69,6 +69,10 @@ func WriteMarkdown(w io.Writer, report model.Report) error { report.HealthSummary.Maintained.Repos, report.HealthSummary.Maintained.Code, report.HealthSummary.Maintained.CodePercent) fmt.Fprintf(w, "| Abandoned (>365d) | %d | %d | %.1f%% |\n", report.HealthSummary.Abandoned.Repos, report.HealthSummary.Abandoned.Code, report.HealthSummary.Abandoned.CodePercent) + if report.HealthSummary.Failed.Repos > 0 { + fmt.Fprintf(w, "| Failed (error) | %d | %d | — |\n", + report.HealthSummary.Failed.Repos, report.HealthSummary.Failed.Code) + } fmt.Fprintln(w) } @@ -111,7 +115,14 @@ func WriteMarkdown(w io.Writer, report model.Report) error { if hasHealth { healthStr := "\u2014" if repo.Health != nil { - healthStr = fmt.Sprintf("%s (%dd)", capitalize(string(repo.Health.Category)), repo.Health.DaysSinceCommit) + switch { + case repo.Health.Category == model.HealthFailed: + healthStr = "Failed (error)" + case repo.Health.DaysSinceCommit >= 0: + healthStr = fmt.Sprintf("%s (%dd)", capitalize(string(repo.Health.Category)), repo.Health.DaysSinceCommit) + default: + healthStr = fmt.Sprintf("%s (no commits)", capitalize(string(repo.Health.Category))) + } } fmt.Fprintf(w, " | %s", healthStr) } From 39fb1119f989c84d4aab2dfac878e42134064391 Mon Sep 17 00:00:00 2001 From: Fabio Date: Mon, 23 Feb 2026 11:17:45 +0100 Subject: [PATCH 2/4] fix: use Bearer auth and updated glab CLI command for GitLab Switch from PRIVATE-TOKEN header to Authorization Bearer for GitLab API requests, and use `glab config get token` instead of `glab auth token` for CLI token retrieval. Co-Authored-By: Claude Opus 4.6 --- internal/auth/gitlab.go | 2 +- internal/provider/gitlab.go | 2 +- internal/provider/gitlab_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/auth/gitlab.go b/internal/auth/gitlab.go index 326195f..23b532b 100644 --- a/internal/auth/gitlab.go +++ b/internal/auth/gitlab.go @@ -9,7 +9,7 @@ import ( // GlabCLIToken attempts to get a GitLab token from the glab CLI tool. // Returns the token and true if successful, or empty string and false otherwise. func GlabCLIToken() (string, bool) { - out, err := exec.Command("glab", "auth", "token").Output() + out, err := exec.Command("glab", "config", "get", "token", "--host", "gitlab.com").Output() if err != nil { return "", false } diff --git a/internal/provider/gitlab.go b/internal/provider/gitlab.go index 8c85802..ecb44aa 100644 --- a/internal/provider/gitlab.go +++ b/internal/provider/gitlab.go @@ -99,7 +99,7 @@ func (g *GitLab) doGet(ctx context.Context, reqURL string) (*http.Response, erro if err != nil { return nil, err } - req.Header.Set("PRIVATE-TOKEN", g.token) + req.Header.Set("Authorization", "Bearer "+g.token) return g.client.Do(req) } diff --git a/internal/provider/gitlab_test.go b/internal/provider/gitlab_test.go index 4959c85..f9df085 100644 --- a/internal/provider/gitlab_test.go +++ b/internal/provider/gitlab_test.go @@ -153,8 +153,8 @@ func TestGitLabExcludeSlugFilter(t *testing.T) { func TestGitLabAuthHeader(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if got := r.Header.Get("PRIVATE-TOKEN"); got != "my-pat" { - t.Errorf("expected PRIVATE-TOKEN header my-pat, got %q", got) + if got := r.Header.Get("Authorization"); got != "Bearer my-pat" { + t.Errorf("expected Authorization header 'Bearer my-pat', got %q", got) } json.NewEncoder(w).Encode([]map[string]any{}) })) From 3a42a2e4d3b445f2bb87049f66593bad4991785e Mon Sep 17 00:00:00 2001 From: Fabio Date: Mon, 23 Feb 2026 12:19:53 +0100 Subject: [PATCH 3/4] feat: add error.log for API failures with categorized entries Write a .error.log file (e.g., report.error.log) next to the JSON report when API errors occur. Each line is prefixed with a category ([health], [health-details], [ai-estimate], [ai-estimate-detail]) for easy filtering with grep. Also adds RepoHealth.Error field to surface failure reasons in JSON output, and propagates partial errors from AnalyzeDetails and Estimate. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 3 +- README.md | 2 + cmd/codemium/main.go | 68 +++++++++++++++++++++++++--- internal/aiestimate/estimate.go | 12 +++-- internal/aiestimate/estimate_test.go | 8 ++-- internal/health/details.go | 14 +++--- internal/health/health_test.go | 39 +++++++++++++++- internal/model/model.go | 1 + internal/provider/gitlab.go | 12 ++--- 9 files changed, 127 insertions(+), 32 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fe94935..2d4bd73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,8 @@ internal/ - **Clone strategy**: Shallow clone (depth 1, single branch, no tags) to temp dir, deleted after analysis. - **scc initialization**: `processor.ProcessConstants()` called via `sync.Once` since scc requires global initialization. - **AI estimation**: When `--ai-estimate` is used, a second pass fetches commit history via provider REST APIs. `provider.CommitLister` interface provides `ListCommits` and `CommitStats`. `aidetect.Detect` classifies commits, `aiestimate.Estimate` orchestrates per-repo. Results attach to existing report model as optional fields. -- **Health classification**: When `--health` is used, repos are classified as Active (<180d), Maintained (180-365d), or Abandoned (>365d) based on last commit date. Repos where commit history cannot be fetched (API errors, permissions) are classified as Failed. `--health-details` adds deep analysis: per-window author counts, code churn, bus factor, and velocity trend. Uses the same `CommitLister` interface. +- **Health classification**: When `--health` is used, repos are classified as Active (<180d), Maintained (180-365d), or Abandoned (>365d) based on last commit date. Repos where commit history cannot be fetched (API errors, permissions) are classified as Failed with the error message stored in `RepoHealth.Error`. `--health-details` adds deep analysis: per-window author counts, code churn, bus factor, and velocity trend. Uses the same `CommitLister` interface. +- **Error logging**: API errors from health, health-details, AI estimation, and partial commit stat failures are collected and written to `.error.log` (derived from the report path, e.g. `report.error.log` for `report.json`) when any errors occur. Each line is prefixed with a category for easy filtering. `AnalyzeDetails` and `aiestimate.Estimate` return `(result, []string, error)` where `[]string` contains partial error messages. - **Vendor/generated filtering**: Always-on filtering using `go-enry` to skip vendor, generated, and binary files during analysis. `FilteredFiles` count is tracked per repo and in report totals. - **License detection**: After analysis, `license.Detect` scans the cloned repo directory for SPDX license identifiers (e.g., "MIT", "Apache-2.0"). Results appear in the per-repo License column. - **Code churn / hotspots**: Opt-in via `--churn` flag. Uses provider REST APIs to fetch per-file change data (`--churn-limit N` sets max commits, default 500). `churn.Analyze` collects per-file change frequencies; `churn.ComputeHotspots` ranks files by churn x complexity. Top 20 hotspots shown per repo. diff --git a/README.md b/README.md index d12f7a6..5edd513 100644 --- a/README.md +++ b/README.md @@ -277,6 +277,8 @@ Health categories: - **Abandoned**: > 365 days ago - **Failed**: commit history could not be fetched (API error, permissions, etc.) +When API errors occur during health classification, AI estimation, or detailed analysis, an error log is automatically written next to the JSON report (e.g., `output/report.error.log` for `output/report.json`). Each line is prefixed with a category (`[health]`, `[health-details]`, `[ai-estimate]`, `[ai-estimate-detail]`) for easy filtering with `grep`. + ### Additional flags ```bash diff --git a/cmd/codemium/main.go b/cmd/codemium/main.go index 4ba8100..03ec477 100644 --- a/cmd/codemium/main.go +++ b/cmd/codemium/main.go @@ -12,6 +12,7 @@ import ( "path/filepath" "sort" "strings" + "sync" "time" tea "github.com/charmbracelet/bubbletea" @@ -20,11 +21,11 @@ import ( "github.com/dsablic/codemium/internal/aiestimate" "github.com/dsablic/codemium/internal/analyzer" - "github.com/dsablic/codemium/internal/churn" - "github.com/dsablic/codemium/internal/license" "github.com/dsablic/codemium/internal/auth" + "github.com/dsablic/codemium/internal/churn" "github.com/dsablic/codemium/internal/health" "github.com/dsablic/codemium/internal/history" + "github.com/dsablic/codemium/internal/license" "github.com/dsablic/codemium/internal/model" "github.com/dsablic/codemium/internal/narrative" "github.com/dsablic/codemium/internal/output" @@ -39,6 +40,13 @@ var ( date = "unknown" ) +// errorEntry represents a diagnostic error for the error log. +type errorEntry struct { + Category string + Repo string + Message string +} + func main() { root := &cobra.Command{ Use: "codemium", @@ -441,6 +449,10 @@ func runAnalyze(cmd *cobra.Command, args []string) error { program = nil } + // Diagnostic error collection (written to error.log if non-empty) + var diagErrors []errorEntry + var diagMu sync.Mutex + // AI estimation phase aiEstimateFlag, _ := cmd.Flags().GetBool("ai-estimate") aiCommitLimit, _ := cmd.Flags().GetInt("ai-commit-limit") @@ -471,7 +483,14 @@ func runAnalyze(cmd *cobra.Command, args []string) error { } aiResults := worker.RunWithProgress(ctx, repoList, concurrency, func(ctx context.Context, repo model.Repo) (*model.RepoStats, error) { - est, err := aiestimate.Estimate(ctx, commitLister, repo, aiCommitLimit) + est, partialErrs, err := aiestimate.Estimate(ctx, commitLister, repo, aiCommitLimit) + if len(partialErrs) > 0 { + diagMu.Lock() + for _, pe := range partialErrs { + diagErrors = append(diagErrors, errorEntry{Category: "ai-estimate-detail", Repo: repo.Slug, Message: pe}) + } + diagMu.Unlock() + } if err != nil { return nil, err } @@ -491,7 +510,11 @@ func runAnalyze(cmd *cobra.Command, args []string) error { // Attach AI estimates to analysis results aiByRepo := make(map[string]*model.AIEstimate) for _, r := range aiResults { - if r.Err == nil && r.Stats != nil && r.Stats.AIEstimate != nil { + if r.Err != nil { + diagErrors = append(diagErrors, errorEntry{Category: "ai-estimate", Repo: r.Repo.Slug, Message: r.Err.Error()}) + continue + } + if r.Stats != nil && r.Stats.AIEstimate != nil { aiByRepo[r.Repo.Slug] = r.Stats.AIEstimate } } @@ -548,12 +571,15 @@ func runAnalyze(cmd *cobra.Command, args []string) error { healthResults := worker.RunWithProgress(ctx, repoList, concurrency, func(ctx context.Context, repo model.Repo) (*model.RepoStats, error) { commits, err := commitLister.ListCommits(ctx, repo, commitLimit) if err != nil { - // If we can't fetch commits, mark as failed rather than skipping + diagMu.Lock() + diagErrors = append(diagErrors, errorEntry{Category: "health", Repo: repo.Slug, Message: err.Error()}) + diagMu.Unlock() return &model.RepoStats{ Repository: repo.Slug, Health: &model.RepoHealth{ Category: model.HealthFailed, DaysSinceCommit: -1, + Error: err.Error(), }, }, nil } @@ -562,9 +588,19 @@ func runAnalyze(cmd *cobra.Command, args []string) error { var details *model.RepoHealthDetails if healthDetailsFlag && len(commits) > 0 { - details, err = health.AnalyzeDetails(ctx, commitLister, repo, commits, now) + var partialErrs []string + details, partialErrs, err = health.AnalyzeDetails(ctx, commitLister, repo, commits, now) + if len(partialErrs) > 0 { + diagMu.Lock() + for _, pe := range partialErrs { + diagErrors = append(diagErrors, errorEntry{Category: "health-details", Repo: repo.Slug, Message: pe}) + } + diagMu.Unlock() + } if err != nil { - // Return health without details if details fail + diagMu.Lock() + diagErrors = append(diagErrors, errorEntry{Category: "health-details", Repo: repo.Slug, Message: err.Error()}) + diagMu.Unlock() return &model.RepoStats{ Repository: repo.Slug, Health: h, @@ -659,6 +695,24 @@ func runAnalyze(cmd *cobra.Command, args []string) error { } } + // Write error.log if there were any diagnostic errors + if len(diagErrors) > 0 { + ext := filepath.Ext(outputPath) + errorLogPath := strings.TrimSuffix(outputPath, ext) + ".error.log" + if err := os.MkdirAll(filepath.Dir(errorLogPath), 0o755); err != nil { + return fmt.Errorf("create error log directory: %w", err) + } + f, err := os.Create(errorLogPath) + if err != nil { + return fmt.Errorf("create error log: %w", err) + } + for _, e := range diagErrors { + fmt.Fprintf(f, "[%s] %s | %s\n", e.Category, e.Repo, e.Message) + } + f.Close() + fmt.Fprintf(os.Stderr, "Error log written to %s (%d entries)\n", errorLogPath, len(diagErrors)) + } + // Build report — use user/group as organization in metadata when set reportOrg := org if user != "" { diff --git a/internal/aiestimate/estimate.go b/internal/aiestimate/estimate.go index 3dac743..c9c8a04 100644 --- a/internal/aiestimate/estimate.go +++ b/internal/aiestimate/estimate.go @@ -2,6 +2,7 @@ package aiestimate import ( "context" + "fmt" "sync" "github.com/dsablic/codemium/internal/aidetect" @@ -12,10 +13,11 @@ import ( const statsConcurrency = 10 // Estimate computes AI attribution metrics for a single repo. -func Estimate(ctx context.Context, cl provider.CommitLister, repo model.Repo, commitLimit int) (*model.AIEstimate, error) { +// It returns the estimate, a list of partial error messages (per-commit stat failures), and a fatal error. +func Estimate(ctx context.Context, cl provider.CommitLister, repo model.Repo, commitLimit int) (*model.AIEstimate, []string, error) { commits, err := cl.ListCommits(ctx, repo, commitLimit) if err != nil { - return nil, err + return nil, nil, err } est := &model.AIEstimate{ @@ -68,10 +70,12 @@ func Estimate(ctx context.Context, cl provider.CommitLister, repo model.Repo, co } wg.Wait() + var partialErrors []string for i, fc := range flagged { d := details[i] if d.err != nil { - continue // partial failure — skip this commit's stats + partialErrors = append(partialErrors, fmt.Sprintf("CommitStats %s: %v", fc.info.Hash, d.err)) + continue } est.AIAdditions += d.additions @@ -95,5 +99,5 @@ func Estimate(ctx context.Context, cl provider.CommitLister, repo model.Repo, co }) } - return est, nil + return est, partialErrors, nil } diff --git a/internal/aiestimate/estimate_test.go b/internal/aiestimate/estimate_test.go index 7805649..df8fa11 100644 --- a/internal/aiestimate/estimate_test.go +++ b/internal/aiestimate/estimate_test.go @@ -40,7 +40,7 @@ func TestEstimateBasic(t *testing.T) { } repo := model.Repo{Slug: "test-repo", URL: "https://github.com/org/test-repo"} - estimate, err := aiestimate.Estimate(context.Background(), mock, repo, 500) + estimate, _, err := aiestimate.Estimate(context.Background(), mock, repo, 500) if err != nil { t.Fatalf("Estimate: %v", err) } @@ -68,7 +68,7 @@ func TestEstimateNoAICommits(t *testing.T) { } repo := model.Repo{Slug: "test-repo", URL: "https://github.com/org/test-repo"} - estimate, err := aiestimate.Estimate(context.Background(), mock, repo, 500) + estimate, _, err := aiestimate.Estimate(context.Background(), mock, repo, 500) if err != nil { t.Fatalf("Estimate: %v", err) } @@ -95,7 +95,7 @@ func TestEstimatePercentCalculation(t *testing.T) { } repo := model.Repo{Slug: "r", URL: "https://github.com/o/r"} - est, err := aiestimate.Estimate(context.Background(), mock, repo, 500) + est, _, err := aiestimate.Estimate(context.Background(), mock, repo, 500) if err != nil { t.Fatalf("Estimate: %v", err) } @@ -116,7 +116,7 @@ func TestEstimateFirstLineMessage(t *testing.T) { } repo := model.Repo{Slug: "r", URL: "https://github.com/o/r"} - est, err := aiestimate.Estimate(context.Background(), mock, repo, 500) + est, _, err := aiestimate.Estimate(context.Background(), mock, repo, 500) if err != nil { t.Fatalf("Estimate: %v", err) } diff --git a/internal/health/details.go b/internal/health/details.go index 6f7f062..c3b795d 100644 --- a/internal/health/details.go +++ b/internal/health/details.go @@ -3,6 +3,7 @@ package health import ( "context" + "fmt" "strings" "sync" "time" @@ -21,12 +22,13 @@ const ( ) // AnalyzeDetails performs deep health analysis on a repo's commits. -func AnalyzeDetails(ctx context.Context, lister provider.CommitLister, repo model.Repo, commits []provider.CommitInfo, now time.Time) (*model.RepoHealthDetails, error) { +// It returns the details, a list of partial error messages (e.g. per-commit stat failures), and a fatal error. +func AnalyzeDetails(ctx context.Context, lister provider.CommitLister, repo model.Repo, commits []provider.CommitInfo, now time.Time) (*model.RepoHealthDetails, []string, error) { if len(commits) == 0 { return &model.RepoHealthDetails{ AuthorsByWindow: map[string]int{}, ChurnByWindow: map[string]model.WindowChurnStats{}, - }, nil + }, nil, nil } sixMoAgo := now.AddDate(0, -6, 0) @@ -63,7 +65,7 @@ func AnalyzeDetails(ctx context.Context, lister provider.CommitLister, repo mode sem := make(chan struct{}, statsConcurrency) var wg sync.WaitGroup var mu sync.Mutex - var firstErr error + var partialErrors []string for i, c := range commits { if ctx.Err() != nil { @@ -78,9 +80,7 @@ func AnalyzeDetails(ctx context.Context, lister provider.CommitLister, repo mode adds, dels, err := lister.CommitStats(ctx, repo, hash) if err != nil { mu.Lock() - if firstErr == nil { - firstErr = err - } + partialErrors = append(partialErrors, fmt.Sprintf("CommitStats %s: %v", hash, err)) mu.Unlock() return } @@ -145,7 +145,7 @@ func AnalyzeDetails(ctx context.Context, lister provider.CommitLister, repo mode ChurnByWindow: churnByWindow, BusFactor: busFactor, VelocityTrend: velocityTrend, - }, nil + }, partialErrors, nil } func commitWindow(date time.Time, sixMoAgo, twelveMoAgo time.Time) string { diff --git a/internal/health/health_test.go b/internal/health/health_test.go index a0132b5..25758bd 100644 --- a/internal/health/health_test.go +++ b/internal/health/health_test.go @@ -2,6 +2,8 @@ package health import ( "context" + "fmt" + "strings" "testing" "time" @@ -196,10 +198,13 @@ func TestAnalyzeDetails(t *testing.T) { } repo := model.Repo{Slug: "test-repo", URL: "https://github.com/org/test-repo"} - details, err := AnalyzeDetails(context.Background(), lister, repo, commits, now) + details, partialErrs, err := AnalyzeDetails(context.Background(), lister, repo, commits, now) if err != nil { t.Fatalf("AnalyzeDetails: %v", err) } + if len(partialErrs) > 0 { + t.Errorf("unexpected partial errors: %v", partialErrs) + } // 0-6mo: Alice, Bob (2 authors, 2 commits) if details.AuthorsByWindow[Window0to6] != 2 { @@ -245,7 +250,7 @@ func TestAnalyzeDetailsEmpty(t *testing.T) { lister := &mockCommitLister{} repo := model.Repo{Slug: "empty-repo"} - details, err := AnalyzeDetails(context.Background(), lister, repo, nil, now) + details, _, err := AnalyzeDetails(context.Background(), lister, repo, nil, now) if err != nil { t.Fatalf("AnalyzeDetails: %v", err) } @@ -256,3 +261,33 @@ func TestAnalyzeDetailsEmpty(t *testing.T) { t.Errorf("expected empty authors, got %v", details.AuthorsByWindow) } } + +func TestAnalyzeDetailsPartialErrors(t *testing.T) { + now := time.Date(2026, 2, 23, 0, 0, 0, 0, time.UTC) + commits := []provider.CommitInfo{ + {Hash: "a1", Author: "Alice ", Date: now.AddDate(0, -1, 0)}, + } + + lister := &mockCommitLister{ + commits: commits, + statsErr: fmt.Errorf("gitlab commit detail API returned status 403"), + } + + repo := model.Repo{Slug: "test-repo", URL: "https://github.com/org/test-repo"} + details, partialErrs, err := AnalyzeDetails(context.Background(), lister, repo, commits, now) + if err != nil { + t.Fatalf("AnalyzeDetails: %v", err) + } + if details == nil { + t.Fatal("expected non-nil details") + } + if len(partialErrs) != 1 { + t.Fatalf("expected 1 partial error, got %d", len(partialErrs)) + } + if !strings.Contains(partialErrs[0], "CommitStats a1") { + t.Errorf("expected partial error to reference hash a1, got %q", partialErrs[0]) + } + if !strings.Contains(partialErrs[0], "403") { + t.Errorf("expected partial error to contain status code, got %q", partialErrs[0]) + } +} diff --git a/internal/model/model.go b/internal/model/model.go index 2211c36..3d46c5b 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -122,6 +122,7 @@ type RepoHealth struct { Category HealthCategory `json:"category"` LastCommitDate string `json:"last_commit_date"` DaysSinceCommit int `json:"days_since_commit"` + Error string `json:"error,omitempty"` } // RepoHealthDetails holds deep health analysis for a repository. diff --git a/internal/provider/gitlab.go b/internal/provider/gitlab.go index ecb44aa..782c86c 100644 --- a/internal/provider/gitlab.go +++ b/internal/provider/gitlab.go @@ -52,8 +52,6 @@ type gitlabProject struct { } `json:"namespace"` } -// ListRepos fetches all projects in a GitLab group, handling -// pagination automatically. func (g *GitLab) ListRepos(ctx context.Context, opts ListOpts) ([]model.Repo, error) { var allRepos []model.Repo @@ -217,11 +215,11 @@ func gitlabProjectID(repoURL string) string { } type gitlabCommit struct { - ID string `json:"id"` - AuthorName string `json:"author_name"` - AuthorEmail string `json:"author_email"` - Message string `json:"message"` - CommittedDate string `json:"committed_date"` + ID string `json:"id"` + AuthorName string `json:"author_name"` + AuthorEmail string `json:"author_email"` + Message string `json:"message"` + CommittedDate string `json:"committed_date"` } // ListCommits fetches up to limit commits for a repo via the GitLab API. From 3e7f823444952d9c954f808bf9e0e4b9862fab4a Mon Sep 17 00:00:00 2001 From: Fabio Date: Mon, 23 Feb 2026 14:19:41 +0100 Subject: [PATCH 4/4] feat: add rate limiting and 429 retry for API requests Add RateLimitTransport (http.RoundTripper) with configurable rate limiting and automatic retry on 429 responses with exponential backoff. Pass shared *http.Client to all provider constructors via --rate-limit CLI flag. Tested against 399-repo GitLab group: 0 rate limit errors (previously 867). Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 + README.md | 3 + cmd/codemium/main.go | 21 +++-- internal/provider/bitbucket.go | 7 +- internal/provider/bitbucket_projects_test.go | 4 +- internal/provider/bitbucket_test.go | 12 +-- internal/provider/github.go | 7 +- internal/provider/github_test.go | 14 ++-- internal/provider/gitlab.go | 7 +- internal/provider/gitlab_test.go | 18 ++-- internal/provider/ratelimit.go | 87 ++++++++++++++++++++ internal/provider/ratelimit_test.go | 65 +++++++++++++++ 12 files changed, 211 insertions(+), 36 deletions(-) create mode 100644 internal/provider/ratelimit.go create mode 100644 internal/provider/ratelimit_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 2d4bd73..ab69d70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,6 +26,7 @@ internal/ gitlab.go glab CLI token fallback provider/ Repository listing from APIs provider.go Provider interface definition + ratelimit.go Rate-limited HTTP transport (429 retry + token-bucket) bitbucket.go Bitbucket Cloud REST API v2.0 github.go GitHub REST API gitlab.go GitLab REST API v4 @@ -70,6 +71,7 @@ internal/ - **Provider abstraction**: `provider.Provider` interface allows adding new git hosting providers. Each provider implements `ListRepos(ctx, ListOpts)`. - **Worker pool**: Bounded goroutine pool with semaphore pattern. Configurable concurrency via `--concurrency` flag. +- **Rate limiting**: `RateLimitTransport` in `provider/ratelimit.go` implements `http.RoundTripper` with token-bucket rate limiting and 429 retry (exponential backoff, `Retry-After` header). Injected via `--rate-limit` flag (default: 0 = unlimited, retry-only). All providers accept `*http.Client` to share the transport. - **Partial failure**: Repos that fail to clone or analyze are recorded as errors in the report; the run continues. - **Auth**: Credentials stored at `~/.config/codemium/credentials.json` (0600 perms). Resolution order: env vars (`CODEMIUM__TOKEN`) → saved credentials → CLI fallback (`gh auth token` for GitHub, `glab auth token` for GitLab). - **Clone strategy**: Shallow clone (depth 1, single branch, no tags) to temp dir, deleted after analysis. diff --git a/README.md b/README.md index 5edd513..426e4d2 100644 --- a/README.md +++ b/README.md @@ -277,12 +277,15 @@ Health categories: - **Abandoned**: > 365 days ago - **Failed**: commit history could not be fetched (API error, permissions, etc.) +API requests that receive a 429 (Too Many Requests) response are automatically retried with exponential backoff (up to 5 retries). Use `--rate-limit` to proactively throttle requests and avoid hitting rate limits (e.g., `--rate-limit 5` for GitLab's 300 req/min raw endpoint limit). + When API errors occur during health classification, AI estimation, or detailed analysis, an error log is automatically written next to the JSON report (e.g., `output/report.error.log` for `output/report.json`). Each line is prefixed with a category (`[health]`, `[health-details]`, `[ai-estimate]`, `[ai-estimate-detail]`) for easy filtering with `grep`. ### Additional flags ```bash --concurrency 10 # Parallel workers (default: 5) +--rate-limit 5 # Max API requests per second (default: unlimited) --include-archived # Include archived repos (excluded by default) --include-forks # Include forked repos (excluded by default) --ai-estimate # Estimate AI-generated code via commit history analysis diff --git a/cmd/codemium/main.go b/cmd/codemium/main.go index 03ec477..b79753f 100644 --- a/cmd/codemium/main.go +++ b/cmd/codemium/main.go @@ -273,6 +273,7 @@ func newAnalyzeCmd() *cobra.Command { cmd.Flags().Int("health-commit-limit", 500, "Max commits to scan per repo for health details (0 = unlimited)") cmd.Flags().Bool("churn", false, "Analyze code churn and hotspots") cmd.Flags().Int("churn-limit", 500, "Max commits to scan per repo for churn analysis (0 = unlimited)") + cmd.Flags().Float64("rate-limit", 0, "Max API requests per second (0 = unlimited)") cmd.MarkFlagRequired("provider") @@ -295,6 +296,10 @@ func runAnalyze(cmd *cobra.Command, args []string) error { includeForks, _ := cmd.Flags().GetBool("include-forks") concurrency, _ := cmd.Flags().GetInt("concurrency") outputPath, _ := cmd.Flags().GetString("output") + rateLimit, _ := cmd.Flags().GetFloat64("rate-limit") + + // Create rate-limited HTTP client + httpClient := &http.Client{Transport: &provider.RateLimitTransport{ReqPerSec: rateLimit}} // Load credentials store := auth.NewFileStore(auth.DefaultStorePath()) @@ -322,7 +327,7 @@ func runAnalyze(cmd *cobra.Command, args []string) error { if workspace == "" { return fmt.Errorf("--workspace is required for bitbucket") } - prov = provider.NewBitbucket(cred.AccessToken, cred.Username, "") + prov = provider.NewBitbucket(cred.AccessToken, cred.Username, "", httpClient) case "github": if org != "" && user != "" { return fmt.Errorf("--org and --user are mutually exclusive for github") @@ -330,13 +335,13 @@ func runAnalyze(cmd *cobra.Command, args []string) error { if org == "" && user == "" { return fmt.Errorf("--org or --user is required for github") } - prov = provider.NewGitHub(cred.AccessToken, "") + prov = provider.NewGitHub(cred.AccessToken, "", httpClient) case "gitlab": if group == "" { return fmt.Errorf("--group is required for gitlab") } baseURL := os.Getenv("CODEMIUM_GITLAB_URL") - prov = provider.NewGitLab(cred.AccessToken, baseURL) + prov = provider.NewGitLab(cred.AccessToken, baseURL, httpClient) default: return fmt.Errorf("unsupported provider: %s", providerName) } @@ -946,6 +951,7 @@ func newTrendsCmd() *cobra.Command { cmd.Flags().Bool("include-forks", false, "Include forked repos") cmd.Flags().Int("concurrency", 5, "Number of parallel workers") cmd.Flags().String("output", "output/report.json", "Write JSON to file") + cmd.Flags().Float64("rate-limit", 0, "Max API requests per second (0 = unlimited)") cmd.MarkFlagRequired("provider") cmd.MarkFlagRequired("since") @@ -972,6 +978,7 @@ func runTrends(cmd *cobra.Command, args []string) error { includeForks, _ := cmd.Flags().GetBool("include-forks") concurrency, _ := cmd.Flags().GetInt("concurrency") outputPath, _ := cmd.Flags().GetString("output") + rateLimit, _ := cmd.Flags().GetFloat64("rate-limit") if interval != "monthly" && interval != "weekly" { return fmt.Errorf("--interval must be 'monthly' or 'weekly'") @@ -994,13 +1001,15 @@ func runTrends(cmd *cobra.Command, args []string) error { store.Save(providerName, cred) } + httpClient := &http.Client{Transport: &provider.RateLimitTransport{ReqPerSec: rateLimit}} + var prov provider.Provider switch providerName { case "bitbucket": if workspace == "" { return fmt.Errorf("--workspace is required for bitbucket") } - prov = provider.NewBitbucket(cred.AccessToken, cred.Username, "") + prov = provider.NewBitbucket(cred.AccessToken, cred.Username, "", httpClient) case "github": if org != "" && user != "" { return fmt.Errorf("--org and --user are mutually exclusive for github") @@ -1008,13 +1017,13 @@ func runTrends(cmd *cobra.Command, args []string) error { if org == "" && user == "" { return fmt.Errorf("--org or --user is required for github") } - prov = provider.NewGitHub(cred.AccessToken, "") + prov = provider.NewGitHub(cred.AccessToken, "", httpClient) case "gitlab": if group == "" { return fmt.Errorf("--group is required for gitlab") } baseURL := os.Getenv("CODEMIUM_GITLAB_URL") - prov = provider.NewGitLab(cred.AccessToken, baseURL) + prov = provider.NewGitLab(cred.AccessToken, baseURL, httpClient) default: return fmt.Errorf("unsupported provider: %s", providerName) } diff --git a/internal/provider/bitbucket.go b/internal/provider/bitbucket.go index 80fa6ba..cd8cfc7 100644 --- a/internal/provider/bitbucket.go +++ b/internal/provider/bitbucket.go @@ -32,15 +32,18 @@ type Project struct { // NewBitbucket creates a new Bitbucket provider. If baseURL is empty, // the default Bitbucket Cloud API endpoint is used. If username is // non-empty, Basic Auth is used instead of Bearer token auth. -func NewBitbucket(token, username, baseURL string) *Bitbucket { +func NewBitbucket(token, username, baseURL string, client *http.Client) *Bitbucket { if baseURL == "" { baseURL = bitbucketAPIBase } + if client == nil { + client = &http.Client{} + } return &Bitbucket{ token: token, username: username, baseURL: baseURL, - client: &http.Client{}, + client: client, } } diff --git a/internal/provider/bitbucket_projects_test.go b/internal/provider/bitbucket_projects_test.go index f3e890a..67d7954 100644 --- a/internal/provider/bitbucket_projects_test.go +++ b/internal/provider/bitbucket_projects_test.go @@ -35,7 +35,7 @@ func TestBitbucketListProjects(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("token", "user", server.URL) + bb := provider.NewBitbucket("token", "user", server.URL, nil) projects, err := bb.ListProjects(context.Background(), "myws") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -59,7 +59,7 @@ func TestBitbucketListProjectsAuth(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("token", "user", server.URL) + bb := provider.NewBitbucket("token", "user", server.URL, nil) bb.ListProjects(context.Background(), "ws") if len(gotAuth) < 6 || gotAuth[:6] != "Basic " { diff --git a/internal/provider/bitbucket_test.go b/internal/provider/bitbucket_test.go index e779763..866368b 100644 --- a/internal/provider/bitbucket_test.go +++ b/internal/provider/bitbucket_test.go @@ -58,7 +58,7 @@ func TestBitbucketListRepos(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("test-token", "", server.URL) + bb := provider.NewBitbucket("test-token", "", server.URL, nil) repos, err := bb.ListRepos(context.Background(), provider.ListOpts{ Workspace: "myworkspace", }) @@ -87,7 +87,7 @@ func TestBitbucketFilterByProject(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("test-token", "", server.URL) + bb := provider.NewBitbucket("test-token", "", server.URL, nil) bb.ListRepos(context.Background(), provider.ListOpts{ Workspace: "myworkspace", Projects: []string{"PROJ1", "PROJ2"}, @@ -106,7 +106,7 @@ func TestBitbucketBasicAuth(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("app-pass", "myuser", server.URL) + bb := provider.NewBitbucket("app-pass", "myuser", server.URL, nil) bb.ListRepos(context.Background(), provider.ListOpts{Workspace: "ws"}) if gotAuth == "" { @@ -150,7 +150,7 @@ func TestBitbucketExcludeForks(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("test-token", "", server.URL) + bb := provider.NewBitbucket("test-token", "", server.URL, nil) repos, _ := bb.ListRepos(context.Background(), provider.ListOpts{ Workspace: "ws", @@ -193,7 +193,7 @@ func TestBitbucketListCommits(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("test-token", "", server.URL) + bb := provider.NewBitbucket("test-token", "", server.URL, nil) commits, err := bb.ListCommits(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://bitbucket.org/myworkspace/repo-1", @@ -230,7 +230,7 @@ func TestBitbucketCommitStats(t *testing.T) { })) defer server.Close() - bb := provider.NewBitbucket("test-token", "", server.URL) + bb := provider.NewBitbucket("test-token", "", server.URL, nil) additions, deletions, err := bb.CommitStats(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://bitbucket.org/myworkspace/repo-1", diff --git a/internal/provider/github.go b/internal/provider/github.go index 1409ec5..225ad6b 100644 --- a/internal/provider/github.go +++ b/internal/provider/github.go @@ -24,14 +24,17 @@ type GitHub struct { // NewGitHub creates a new GitHub provider. If baseURL is empty, // the default GitHub API endpoint is used. -func NewGitHub(token string, baseURL string) *GitHub { +func NewGitHub(token string, baseURL string, client *http.Client) *GitHub { if baseURL == "" { baseURL = githubAPIBase } + if client == nil { + client = &http.Client{} + } return &GitHub{ token: token, baseURL: baseURL, - client: &http.Client{}, + client: client, } } diff --git a/internal/provider/github_test.go b/internal/provider/github_test.go index 99f5825..8dfafc2 100644 --- a/internal/provider/github_test.go +++ b/internal/provider/github_test.go @@ -47,7 +47,7 @@ func TestGitHubListRepos(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) repos, err := gh.ListRepos(context.Background(), provider.ListOpts{ Organization: "myorg", }) @@ -72,7 +72,7 @@ func TestGitHubListReposUser(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) repos, err := gh.ListRepos(context.Background(), provider.ListOpts{ User: "myuser", }) @@ -97,7 +97,7 @@ func TestGitHubExcludeForksAndArchived(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) repos, _ := gh.ListRepos(context.Background(), provider.ListOpts{ Organization: "org", }) @@ -134,7 +134,7 @@ func TestGitHubListCommits(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) commits, err := gh.ListCommits(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://github.com/myorg/repo-1", @@ -175,7 +175,7 @@ func TestGitHubCommitStats(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) additions, deletions, err := gh.CommitStats(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://github.com/myorg/repo-1", @@ -208,7 +208,7 @@ func TestGitHubCommitFileStats(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) files, err := gh.CommitFileStats(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://github.com/myorg/repo-1", }, "abc123") @@ -241,7 +241,7 @@ func TestGitHubListCommitsLimit(t *testing.T) { })) defer server.Close() - gh := provider.NewGitHub("test-token", server.URL) + gh := provider.NewGitHub("test-token", server.URL, nil) commits, err := gh.ListCommits(context.Background(), model.Repo{ Slug: "repo-1", URL: "https://github.com/myorg/repo-1", diff --git a/internal/provider/gitlab.go b/internal/provider/gitlab.go index 782c86c..5c69154 100644 --- a/internal/provider/gitlab.go +++ b/internal/provider/gitlab.go @@ -24,14 +24,17 @@ type GitLab struct { // NewGitLab creates a new GitLab provider. If baseURL is empty, // the default GitLab.com endpoint is used. -func NewGitLab(token string, baseURL string) *GitLab { +func NewGitLab(token string, baseURL string, client *http.Client) *GitLab { if baseURL == "" { baseURL = gitlabAPIBase } + if client == nil { + client = &http.Client{} + } return &GitLab{ token: token, baseURL: baseURL, - client: &http.Client{}, + client: client, } } diff --git a/internal/provider/gitlab_test.go b/internal/provider/gitlab_test.go index f9df085..6e94547 100644 --- a/internal/provider/gitlab_test.go +++ b/internal/provider/gitlab_test.go @@ -53,7 +53,7 @@ func TestGitLabListRepos(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) repos, err := gl.ListRepos(context.Background(), provider.ListOpts{ Organization: "mygroup", }) @@ -103,7 +103,7 @@ func TestGitLabExcludeForksAndArchived(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) repos, _ := gl.ListRepos(context.Background(), provider.ListOpts{ Organization: "g", IncludeArchived: true, // server-side filter disabled so we test client-side fork filter @@ -122,7 +122,7 @@ func TestGitLabIncludeSlugFilter(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) repos, _ := gl.ListRepos(context.Background(), provider.ListOpts{ Organization: "g", Repos: []string{"repo-a"}, @@ -141,7 +141,7 @@ func TestGitLabExcludeSlugFilter(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) repos, _ := gl.ListRepos(context.Background(), provider.ListOpts{ Organization: "g", Exclude: []string{"repo-b"}, @@ -160,7 +160,7 @@ func TestGitLabAuthHeader(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("my-pat", server.URL) + gl := provider.NewGitLab("my-pat", server.URL, nil) gl.ListRepos(context.Background(), provider.ListOpts{Organization: "g"}) } @@ -173,7 +173,7 @@ func TestGitLabSelfHosted(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) _, err := gl.ListRepos(context.Background(), provider.ListOpts{Organization: "mygroup"}) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -205,7 +205,7 @@ func TestGitLabListCommits(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) commits, err := gl.ListCommits(context.Background(), model.Repo{ Slug: "repo-1", URL: server.URL + "/mygroup/repo-1", @@ -246,7 +246,7 @@ func TestGitLabCommitStats(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) additions, deletions, err := gl.CommitStats(context.Background(), model.Repo{ Slug: "repo-1", URL: server.URL + "/mygroup/repo-1", @@ -282,7 +282,7 @@ func TestGitLabListCommitsLimit(t *testing.T) { })) defer server.Close() - gl := provider.NewGitLab("test-token", server.URL) + gl := provider.NewGitLab("test-token", server.URL, nil) commits, err := gl.ListCommits(context.Background(), model.Repo{ Slug: "repo-1", URL: server.URL + "/mygroup/repo-1", diff --git a/internal/provider/ratelimit.go b/internal/provider/ratelimit.go new file mode 100644 index 0000000..cd1f581 --- /dev/null +++ b/internal/provider/ratelimit.go @@ -0,0 +1,87 @@ +package provider + +import ( + "io" + "net/http" + "strconv" + "sync" + "time" +) + +const defaultMaxRetries = 5 + +// RateLimitTransport wraps an http.RoundTripper with rate limiting and 429 retry. +type RateLimitTransport struct { + ReqPerSec float64 // 0 = unlimited (retry-only) + Base http.RoundTripper // nil = http.DefaultTransport + + once sync.Once + limiter chan struct{} +} + +func (t *RateLimitTransport) init() { + if t.ReqPerSec > 0 { + t.limiter = make(chan struct{}, 1) + interval := time.Duration(float64(time.Second) / t.ReqPerSec) + go func() { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for range ticker.C { + select { + case t.limiter <- struct{}{}: + default: + } + } + }() + } +} + +func (t *RateLimitTransport) base() http.RoundTripper { + if t.Base != nil { + return t.Base + } + return http.DefaultTransport +} + +// RoundTrip implements http.RoundTripper with rate limiting and 429 retry. +func (t *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.once.Do(t.init) + + for attempt := 0; ; attempt++ { + // Wait for rate limiter token if configured + if t.limiter != nil { + select { + case <-req.Context().Done(): + return nil, req.Context().Err() + case <-t.limiter: + } + } + + resp, err := t.base().RoundTrip(req) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusTooManyRequests || attempt >= defaultMaxRetries { + return resp, nil + } + + // Drain and close body before retry + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + + // Backoff: use Retry-After header or exponential (1s, 2s, 4s...) + delay := time.Duration(1< 0 { + delay = time.Duration(secs) * time.Second + } + } + + select { + case <-req.Context().Done(): + return nil, req.Context().Err() + case <-time.After(delay): + } + } +} diff --git a/internal/provider/ratelimit_test.go b/internal/provider/ratelimit_test.go new file mode 100644 index 0000000..d7bc4bf --- /dev/null +++ b/internal/provider/ratelimit_test.go @@ -0,0 +1,65 @@ +package provider_test + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/dsablic/codemium/internal/provider" +) + +func TestRateLimitTransportRetryOn429(t *testing.T) { + var attempts int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := atomic.AddInt32(&attempts, 1) + if n <= 2 { + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte("ok")) + })) + defer server.Close() + + client := &http.Client{ + Transport: &provider.RateLimitTransport{ReqPerSec: 0}, + } + + resp, err := client.Get(server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("expected 200, got %d", resp.StatusCode) + } + if got := atomic.LoadInt32(&attempts); got != 3 { + t.Errorf("expected 3 attempts, got %d", got) + } +} + +func TestRateLimitTransportContextCancellation(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "60") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer server.Close() + + client := &http.Client{ + Transport: &provider.RateLimitTransport{ReqPerSec: 0}, + } + + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + _, err := client.Do(req) + if err == nil { + t.Fatal("expected error from context cancellation") + } +}