Skip to content

feat(reports): count archived repos on ADO (#74) + cross-platform scan summary#96

Merged
fabio-gos-sonarsource merged 2 commits into
mainfrom
feat/ado-archived-and-scan-summary
Jul 8, 2026
Merged

feat(reports): count archived repos on ADO (#74) + cross-platform scan summary#96
fabio-gos-sonarsource merged 2 commits into
mainfrom
feat/ado-archived-and-scan-summary

Conversation

@fabio-gos-sonarsource

Copy link
Copy Markdown
Contributor

Closes #74.

ADO archived repos (#74)

The archived-repo handling in getazure was inert:

  • listReposForProject never detected archived/disabled repos (archivedCount was declared but never incremented).
  • A variable-shadowing bug in getRepoAnalyse (a loop-scoped := re-declaring emptyRepos) meant both the empty and archived counts were silently lost and always returned 0; emptyOrArchivedCount was misnamed dead code.

Azure DevOps has no "archived" repo state — the equivalent is a disabled repo (no default branch, uncloneable) — and the pinned SDK (azure-devops-go-api v1.0.0-b5) drops isDisabled from GitRepository. So disabled repos are now detected via a raw REST call (GET …/git/repositories?api-version=6.0, PAT basic auth, shared utils.HTTPClient) that degrades gracefully (logs a warning, treats none as disabled) on permission/network errors so a scan never fails. Disabled repos are counted and skipped, and the empty/archived accounting is corrected.

Other platforms

GitHub, Bitbucket Cloud, and GitLab already detected + counted archived correctly. Bitbucket DC skipped archived repos without counting them — added an archived/excluded counter to fetchAllRepos to close that reporting gap.

Cross-platform Scan Summary

Per-run repository breakdown — Scanned / Analyzed / Archived / Empty / Excluded / Skipped — surfaced in the ResultsAll web page and the global PDF report, for all platforms.

  • New pkg/utils/summary.go mirrors the existing skipped.go persistence pattern: ScanSummaryResults/config/analysis_summary.json, with Scanned derived so the breakdown always reconciles (Scanned = Analyzed + Archived + Empty + Excluded + Skipped).
  • Missing file → nil, so older result sets still render.
  • /api/scan-summary endpoint added alongside the existing API handlers.
  • Analyzed is adjusted down by analysis-phase failures, which surface as Skipped.

Tests

  • pkg/utils/summary_test.go: save/load round-trip, Scanned derivation, missing/invalid file, PDF section render (nil/zero/populated).
  • pkg/devops/getazure/getazure_disabled_test.go: isDisabled parse + graceful non-200/bad-JSON fallback.
  • pkg/devops/getbitbucketdc: fetchAllRepos archived/excluded counts.
  • scan_summary_test.go: buildScanSummaryView logic + template renders/omits.

All builds (./..., -tags webui, -tags resultsall) and the full test suite pass. A live ADO scan against a real disabled repo was not run (needs org credentials); the REST parse, counting, and rendering are covered by unit tests.

🤖 Generated with Claude Code

ADO archived handling was inert: listReposForProject never detected
archived/disabled repos, and a variable-shadowing bug in getRepoAnalyse
silently lost both the empty and archived counts. Azure DevOps has no
"archived" repo state and the pinned SDK drops isDisabled, so detect
disabled repos via a raw REST call (graceful on error) and count/skip
them. Also close the equivalent reporting gap in Bitbucket DC, which
skipped archived repos without counting them.

Add a cross-platform scan summary (Scanned / Analyzed / Archived /
Empty / Excluded / Skipped) persisted to
Results/config/analysis_summary.json and rendered in the ResultsAll web
page and the global PDF report. Mirrors the existing skipped-repos
persistence/render pattern; missing file degrades to nil so older
result sets still render.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread pkg/devops/getazure/getazure.go
Comment thread pkg/devops/getazure/getazure.go Outdated
Address the Gitar review findings on the scan-summary PR:

- fetchDisabledRepoIDs bounded its request with context.Background() and the
  shared HTTP client, which has no request timeout — a stalled Azure endpoint
  could hang the whole scan. Wrap the call in a 30s context deadline so it
  degrades gracefully as intended.
- The Azure summary derived Scanned from len(importantBranches), so repos
  dropped inside the analysis loop (no usable default branch, or single-branch
  selection) fell into no category and understated the true total. Add a
  Skipped bucket (discovery-phase drops) to ScanSummary; Scanned now reconciles
  as Analyzed + Archived + Empty + Excluded + Skipped, and the reports fold the
  discovery-phase skips together with the analysis-phase skips.

Also consolidate the per-platform summary persistence behind
utils.NewScanSummary + utils.PersistScanSummary (removing duplicated struct
literals and the repeated error-log string), and add tests covering the
disabled-repo detection, listReposForProject filtering, fetchAllRepos
archived/excluded counting, the reconciliation math, and the render helpers.

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

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
79.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Comment thread ResultsAll.go
Comment on lines +955 to +969
func buildScanSummaryView(summary *utils.ScanSummary, skippedCount int) *ScanSummaryView {
if summary == nil {
return nil
}
analyzed := summary.Analyzed - skippedCount
if analyzed < 0 {
analyzed = 0
}
return &ScanSummaryView{
Scanned: summary.Scanned,
Analyzed: analyzed,
Archived: summary.Archived,
Empty: summary.Empty,
Excluded: summary.Excluded,
Skipped: summary.Skipped + skippedCount,

@gitar-bot gitar-bot Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Scan summary row fails to reconcile when Analyzed floors at 0

The stated invariant for the scan summary is Scanned = Analyzed + Archived + Empty + Excluded + Skipped. In both buildScanSummaryView (ResultsAll.go:955-971) and renderScanSummarySection (pkg/utils/globalreport.go:337-355) the displayed Analyzed is computed as summary.Analyzed - skippedCount and floored at 0, while displayed Skipped is summary.Skipped + skippedCount and Scanned is passed through unchanged.

When skippedCount > summary.Analyzed (more analysis-phase failures than projected analyzed repos), Analyzed is clamped to 0 but the full skippedCount is still added into the Skipped column and Scanned is unchanged. The row then over-sums by skippedCount - summary.Analyzed and no longer reconciles — contradicting the invariant the reconciliation test (scan_summary_test.go:36-38) asserts for the normal case.

This is a low-probability edge (skippedCount normally cannot exceed the projected analyzed count) and only affects display totals, hence minor. Consider clamping the amount folded into Skipped to what was actually subtracted from Analyzed so the row always balances.

Only fold the portion of skippedCount that was actually subtracted from Analyzed into the displayed Skipped, keeping the row balanced when analysis failures exceed projected analyzed repos.:

used := skippedCount
if used > summary.Analyzed {
    used = summary.Analyzed
}
analyzed := summary.Analyzed - used
// ...
Skipped: summary.Skipped + used,

Was this helpful? React with 👍 / 👎

@fabio-gos-sonarsource
fabio-gos-sonarsource merged commit 33c82fd into main Jul 8, 2026
3 of 4 checks passed
@fabio-gos-sonarsource
fabio-gos-sonarsource deleted the feat/ado-archived-and-scan-summary branch July 8, 2026 13:59
@gitar-bot

gitar-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Implements repository accounting for ADO and Bitbucket DC alongside a new cross-platform scan summary feature. Address the reconciliation invariant when Analyzed counts floor at zero to ensure consistent scan breakdown reporting.

💡 Edge Case: Scan summary row fails to reconcile when Analyzed floors at 0

📄 ResultsAll.go:955-969 📄 pkg/utils/globalreport.go:337-351

The stated invariant for the scan summary is Scanned = Analyzed + Archived + Empty + Excluded + Skipped. In both buildScanSummaryView (ResultsAll.go:955-971) and renderScanSummarySection (pkg/utils/globalreport.go:337-355) the displayed Analyzed is computed as summary.Analyzed - skippedCount and floored at 0, while displayed Skipped is summary.Skipped + skippedCount and Scanned is passed through unchanged.

When skippedCount > summary.Analyzed (more analysis-phase failures than projected analyzed repos), Analyzed is clamped to 0 but the full skippedCount is still added into the Skipped column and Scanned is unchanged. The row then over-sums by skippedCount - summary.Analyzed and no longer reconciles — contradicting the invariant the reconciliation test (scan_summary_test.go:36-38) asserts for the normal case.

This is a low-probability edge (skippedCount normally cannot exceed the projected analyzed count) and only affects display totals, hence minor. Consider clamping the amount folded into Skipped to what was actually subtracted from Analyzed so the row always balances.

Only fold the portion of skippedCount that was actually subtracted from Analyzed into the displayed Skipped, keeping the row balanced when analysis failures exceed projected analyzed repos.
used := skippedCount
if used > summary.Analyzed {
    used = summary.Analyzed
}
analyzed := summary.Analyzed - used
// ...
Skipped: summary.Skipped + used,
✅ 2 resolved
Bug: Disabled-repo REST call can hang the scan indefinitely

📄 pkg/devops/getazure/getazure.go:543-557
fetchDisabledRepoIDs builds its request with parms.Context (which is context.Background(), set in GetRepoAzureList) and executes it with the shared utils.HTTPClient, which is configured with connection-pool timeouts only and no Timeout. If the Azure DevOps endpoint accepts the connection but never responds, HTTPClient.Do will block forever. This undermines the PR's stated goal that disabled-repo detection 'degrades gracefully … so a scan never fails' — a network stall (as opposed to an error/non-200) will hang the entire scan rather than logging a warning and continuing. Consider giving the request a bounded deadline.

Edge Case: Azure Analyzed count omits repos skipped mid-loop, breaking reconciliation

📄 pkg/devops/getazure/getazure.go:338-346 📄 pkg/devops/getazure/getazure.go:464-478 📄 pkg/utils/summary.go:39-51
For Azure the persisted Analyzed is len(importantBranches), and SaveScanSummary derives Scanned = Analyzed + Archived + Empty + Excluded. However, in getRepoAnalyse a repo can be dropped inside the analysis loop without being appended to importantBranches and without being counted as archived/empty/excluded: when branch analysis fails and the repo has no usable default branch (continue at getazure.go:479), or when SingleBranch is set and the most-important branch does not match (continue at getazure.go:486). Those repos vanish from every category, so the displayed Scanned total silently understates the repositories actually discovered and the breakdown no longer reconciles. This is an edge case (only triggered by no-default-branch failures or SingleBranch filtering), so impact is limited, but the summary card advertises an exact reconciliation.

🤖 Prompt for agents
Code Review: Implements repository accounting for ADO and Bitbucket DC alongside a new cross-platform scan summary feature. Address the reconciliation invariant when Analyzed counts floor at zero to ensure consistent scan breakdown reporting.

1. 💡 Edge Case: Scan summary row fails to reconcile when Analyzed floors at 0
   Files: ResultsAll.go:955-969, pkg/utils/globalreport.go:337-351

   The stated invariant for the scan summary is `Scanned = Analyzed + Archived + Empty + Excluded + Skipped`. In both `buildScanSummaryView` (ResultsAll.go:955-971) and `renderScanSummarySection` (pkg/utils/globalreport.go:337-355) the displayed Analyzed is computed as `summary.Analyzed - skippedCount` and floored at 0, while displayed Skipped is `summary.Skipped + skippedCount` and Scanned is passed through unchanged.
   
   When `skippedCount > summary.Analyzed` (more analysis-phase failures than projected analyzed repos), Analyzed is clamped to 0 but the full `skippedCount` is still added into the Skipped column and Scanned is unchanged. The row then over-sums by `skippedCount - summary.Analyzed` and no longer reconciles — contradicting the invariant the reconciliation test (scan_summary_test.go:36-38) asserts for the normal case.
   
   This is a low-probability edge (skippedCount normally cannot exceed the projected analyzed count) and only affects display totals, hence minor. Consider clamping the amount folded into Skipped to what was actually subtracted from Analyzed so the row always balances.

   Fix (Only fold the portion of skippedCount that was actually subtracted from Analyzed into the displayed Skipped, keeping the row balanced when analysis failures exceed projected analyzed repos.):
   used := skippedCount
   if used > summary.Analyzed {
       used = summary.Analyzed
   }
   analyzed := summary.Analyzed - used
   // ...
   Skipped: summary.Skipped + used,

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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.

Remove archived repos from the calculation for ADO

1 participant