Skip to content

fix(azure): analyze repos with no default branch instead of panicking#95

Merged
fabio-gos-sonarsource merged 3 commits into
mainfrom
fix/azure-nil-default-branch-panic
Jul 8, 2026
Merged

fix(azure): analyze repos with no default branch instead of panicking#95
fabio-gos-sonarsource merged 3 commits into
mainfrom
fix/azure-nil-default-branch-panic

Conversation

@fabio-gos-sonarsource

@fabio-gos-sonarsource fabio-gos-sonarsource commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

A large Azure DevOps org scan crashes partway through branch identification with:

panic: runtime error: invalid memory address or nil pointer dereference
Analysis failed: exit status 2

The analysis dies mid-run (e.g. at repo 432/522), even though the offending repository looks accessible and valid in the Azure DevOps UI.

Root cause

In the Azure DevOps Go API, git.GitRepository.DefaultBranch (as well as Name and Size) is a *string tagged omitempty, so GetRepository can legitimately return it as nil — typically for a disabled/archived repository, or one whose HEAD ref was never initialized (yet still has branches, so it passes the empty-repo check).

getMostImportantBranch dereferenced *repo.DefaultBranch unconditionally:

defaultBranch = *repo.DefaultBranch   // 💥 panic when DefaultBranch == nil

Because the run was in default-branch mode, that one repo took down the entire scan. There were two further unguarded derefs of these omitempty pointers (getRepoAnalyse error fallback, and *repo.Size).

Fix

Rather than skip a repo with no default branch, this routes it through the branch-scanning engine the tool already has (handleNonDefaultBranch), which picks the most active branch.

  • Add defaultBranchName helper for nil-safe default-branch extraction (strips refs/heads/).
  • getMostImportantBranch: when a repo has no default branch, fall back to scanning every branch and picking the most active one — so the repo is still analyzed, not dropped or crashed on.
  • Harden handleNonDefaultBranch: skip nil branch names, and keep a first-branch fallback so a repo with no recent commits and no default branch still resolves to a real branch instead of an empty string.
  • Guard the getRepoAnalyse error fallback and the *repo.Size dereference.
  • Add TestDefaultBranchName.

Behavior

For a disabled/archived-style repo with a nil DefaultBranch:

  • Before: panic → whole run dies at exit status 2.
  • After: picks the largest/most-active branch and continues analyzing all repos.

Testing

  • go build ./...
  • go test ./pkg/devops/getazure/ ✅ (incl. new TestDefaultBranchName)

fabio-gos-sonarsource and others added 2 commits July 8, 2026 13:41
GitRepository.DefaultBranch (and Name/Size) are omitempty pointers, so the
Azure DevOps API can return them as nil — e.g. for disabled/archived repos or
repos whose HEAD ref was never initialized. getMostImportantBranch dereferenced
*repo.DefaultBranch unconditionally, crashing the entire analysis with
"panic: runtime error: invalid memory address or nil pointer dereference"
partway through a large org scan.

- Add defaultBranchName helper for nil-safe default-branch extraction.
- When a repo has no default branch, fall back to the existing biggest-branch
  engine (handleNonDefaultBranch) so the repo is still analyzed, not skipped.
- Harden handleNonDefaultBranch: skip nil branch names, and keep a first-branch
  fallback so a repo with no recent commits and no default branch still resolves
  to a real branch rather than an empty string.
- Guard the getRepoAnalyse error fallback and the *repo.Size dereference.
- Add TestDefaultBranchName.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a fake git.Client test double (embeds the 121-method interface, overrides
the 5 calls exercised) and unit tests covering the new code from the previous
commit, which was failing the SonarQube new-code coverage condition (7.9% < 80%):

- handleNonDefaultBranch: biggest-branch pick, first-branch fallback,
  default-branch fallback, nil-branch-name skip, no-analyzable-branch error.
- getMostImportantBranch: default-branch mode with/without a default present,
  single-branch mode, GetRepository error propagation.
- handleDefaultOrSingleBranch: nil-Size guard (and Size-present path).
- getRepoAnalyse error fallback: skip repo with nil default branch, use the
  default when present.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread pkg/devops/getazure/getazure_test.go
…ides)

The doc comment predated adding GetRepositories/GetItems for the getRepoAnalyse
test. Update it to reflect all five overridden methods. Addresses Gitar review
feedback on PR #95.

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

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@fabio-gos-sonarsource
fabio-gos-sonarsource merged commit e3fa2ea into main Jul 8, 2026
4 checks passed
@fabio-gos-sonarsource
fabio-gos-sonarsource deleted the fix/azure-nil-default-branch-panic branch July 8, 2026 12:23
@gitar-bot

gitar-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds nil-safe handling for Azure DevOps repositories with missing default branches, preventing panics and ensuring analysis completion. The stale documentation in the fakeGitClient was also corrected.

✅ 1 resolved
Quality: fakeGitClient doc comment says "three calls" but overrides five

📄 pkg/devops/getazure/getazure_test.go:18-32
The comment states the fake "only overrides the three calls exercised by the branch-resolution logic," but the struct actually overrides five methods: GetRepository, GetRepositories, GetItems, GetBranches, and GetCommits. This is a harmless but misleading doc string that could confuse future maintainers about which calls are stubbed. Suggest updating the count (or making it generic, e.g. "only overrides the calls exercised by the branch-resolution and repo-analysis logic").

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.

1 participant