fix(scan): bound clones with a timeout + rolling worker pool, surface skipped repos

[fabio-gos-sonarsource]

Problem

1. No clone timeout. gogit.Getrepos used go-git PlainClone with no deadline, so a stalled TCP connection to the git server (VPN drop, proxy, server under load) hung the cloning goroutine forever. It also swallowed clone errors — it logged them but still returned a valid path with a nil error, so a failed clone was treated as an empty-but-successful repo.
2. Batch barrier in the worker pool. AnalyseReposList launched fixed batches of Workers and waited for all of a batch to finish before starting the next. One slow/hung repo therefore stalled up to Workers-1 idle workers and froze overall progress.

A GitHub Enterprise (and any platform) org-wide scan could freeze for 30+ minutes on a single repository, showing a stuck progress bar and repeated Waiting for workers.... Two independent root causes, both in the shared cross-platform analysis path:

Together: a single bad repo could hang an entire org scan with no recovery and no record.

Fix

• Clone timeout — gogit.Getrepos now takes a timeout and uses PlainCloneContext. On failure it removes the partial clone and returns the error; a deadline surfaces as an explicit clone timed out after <d> message.
• CloneTimeout setting (minutes; default 15, 0 disables) threaded through RepoParams → goloc.Params → gogit. Added to config_sample.json, the webui platform defaults, and the advanced-options form.
• Rolling worker pool — AnalyseReposList rewritten as a semaphore-bounded pool + WaitGroup. Same max concurrency as before, but every finished repo frees its slot immediately, so one slow repo can no longer block the others (no more batch barrier / Waiting for workers...).
• Skipped repositories surfaced — repos that can't be analyzed (clone timeout/failure or analysis error) are recorded per run and persisted to Results/config/analysis_skipped.json, then shown in:
  • the ResultsAll web page (new "Skipped Repositories" section + GET /api/skipped-repositories)
  • the GlobalReport PDF (new "Skipped Repositories" section)

All of this is platform-agnostic — GitHub/GHE, GitLab, Bitbucket Cloud, Bitbucket DC, and Azure all flow through the same performRepoAnalysis / gogit.Getrepos path, so the fix applies to every platform.

Scope note

The "Skipped Repositories" report lists analysis-phase failures (the actionable ones tied to this bug). Intentional pre-analysis skips (archived/empty/excluded) remain summarized as counts, as before.

Tests

• pkg/gogit: clone-timeout returns an error and leaves no temp clone behind; existing clone tests updated for the new signature.
• golc (tag): getCloneTimeout defaulting/disable logic; concurrency-safe skipRecorder.
• pkg/utils: skipped-repos save/load roundtrip, empty-clears-stale, missing-file; PDF section renders (empty / one / long-list-with-truncation) without panic.
• Full CI-parity run green: go test ./pkg/..., -tags=golc, -tags=resultsall; both binaries build with -trimpath.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
93.9% Coverage on New Code
1.1% Duplication on New Code

See analysis details on SonarQube Cloud

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Implements a rolling worker pool and repository clone timeouts to prevent scan hangs, while introducing a skipped repository reporting mechanism. A data race on the progress counter and silent deadline disabling were both addressed.

✅ 2 resolved

✅ Bug: Data race on shared count pointer in rolling worker pool

📄 golc.go:377-387 📄 golc.go:701 📄 golc.go:729 📄 golc.go:745 📄 golc.go:754 📄 golc.go:764 📄 golc.go:774
In AnalyseReposList a single count := 1 is declared and its address is passed (via analyseRepoFunc) into every worker goroutine. With the new rolling worker pool, up to concurrency goroutines run performRepoAnalysis simultaneously, and each one does *count++ (golc.go:701,729,745,754,764) and reads *count in the success log line (golc.go:774) without any mutex or atomic. This is an unsynchronized concurrent read/write of the same int — a genuine data race (undefined behavior; go test -race/go build -race on this path would flag it, contradicting the "CI green" claim). Impact is limited to incorrect/torn progress numbers in the log line rather than control flow (the function returns total, not count), but it should be fixed while this concurrency code is being rewritten. Note the same shared-pointer pattern also exists in AnalyseReposListFile/analyseDirectory (golc.go:924). Fix by making the counter atomic (e.g. var count int64 + atomic.AddInt64) or guarding it with a mutex.

✅ Edge Case: Cleared CloneTimeout field silently disables the deadline

📄 webui.go:1863-1864
In webui.go gatherConfig(), cfg.CloneTimeout = ctRaw ? (parseInt(ctRaw.value, 10) || 0) : 15;. If a user clears the "Clone timeout" input (empty string), parseInt('') is NaN, so NaN || 0 yields 0, which getCloneTimeout interprets as "deadline disabled" rather than the intended default of 15. This is the exact hang scenario the PR set out to prevent. Consider treating an empty/blank field as the default (15) instead of 0, reserving 0 for an explicit user-entered 0.

Options

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

Comment with these commands to change:

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

_{Was this helpful? React with 👍 / 👎 | Gitar}