fix(changelog-site): retry the GitHub tag fetch and fail instead of degrading - #645
Open
balajinvda wants to merge 1 commit into
Open
fix(changelog-site): retry the GitHub tag fetch and fail instead of degrading#645balajinvda wants to merge 1 commit into
balajinvda wants to merge 1 commit into
Conversation
…egrading The published changelog flapped: services appeared and disappeared between half-hourly rebuilds, all of them together. fetchGitHubTags runs git ls-remote and git fetch against GitHub with no retry, and the caller only warned on failure. One transient network failure therefore dropped the entire GitHub tag set, and the site republished with every GitHub-only release missing -- overwriting a good deployment with a worse one. The next successful run restored it. Nothing reported an error, because the job exits zero either way. Retry both network calls three times with a short backoff so a blip does not decide the contents of the site, and make an unavailable GitHub tag set fatal so a degraded build is never published and Pages keeps serving the last good deployment. --allow-missing-github-tags opts back into publishing without them. Verified locally against a full GitLab clone plus the GitHub mirror: happy path 37 services, exit 0, github-origin releases present failure path exits non-zero, writes no output at all Co-authored-by: Balaji Ganesan <[email protected]>
📝 WalkthroughWalkthroughThe changelog site retries GitHub tag discovery and fetching up to three times with incremental backoff. GitHub tag failures now stop the build by default. The ChangesGitHub tag retrieval
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ChangelogSite
participant GitCommands
participant GitHub
ChangelogSite->>GitCommands: Run tag discovery or fetch
GitCommands->>GitHub: Retrieve GitHub tags
GitHub-->>GitCommands: Return tags or error
GitCommands-->>ChangelogSite: Return result
ChangelogSite->>GitCommands: Retry failed command up to three times
ChangelogSite-->>ChangelogSite: Abort by default or publish with --allow-missing-github-tags
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/changelog-site/main.go`:
- Around line 453-474: Update the local git command runner around run to use
exec.CommandContext with a per-attempt timeout, canceling each context after the
attempt completes. Set GIT_TERMINAL_PROMPT=0 on every command so credential
prompts cannot block, while preserving the existing retry, stderr capture, and
error-reporting behavior.
- Around line 453-474: Update fetchGitHubTags to fetch GitHub tag refs into a
dedicated, non-canonical namespace rather than refs/tags/*, preventing stale
refs from contaminating the checkout. Update buildReleases to read GitHub tags
explicitly from that isolated namespace while leaving gitlabTags sourced only
from canonical tags; preserve --allow-missing-github-tags behavior without
promoting stale GitHub refs.
- Around line 453-474: Extend TestFetchGitHubTagsLabelsOrigin with focused
coverage for retry recovery, retry exhaustion, token scrubbing, failure modes,
output preservation, and --allow-missing-github-tags using an injectable or fake
Git runner. Update the GitHub tag synchronization flow around the run helper and
its refspec so deleted remote tags are pruned or isolated before releases are
built, and add a two-run test verifying deletion is reflected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: df16f37a-8ec8-407c-a04b-b417ca46bee8
📒 Files selected for processing (1)
tools/changelog-site/main.go
Comment on lines
+453
to
+474
| // Both calls below cross the network to GitHub. A single transient failure | ||
| // used to drop the entire GitHub tag set, and because the caller only | ||
| // warned, the site republished with every GitHub-only release missing -- | ||
| // services appeared and disappeared between half-hourly rebuilds. Retry | ||
| // with a short backoff so a blip does not decide the contents of the site. | ||
| run := func(args ...string) (string, error) { | ||
| cmd := exec.Command("git", append([]string{"-C", repoDir}, args...)...) | ||
| var stdout, stderr strings.Builder | ||
| cmd.Stdout = &stdout | ||
| cmd.Stderr = &stderr | ||
| if err := cmd.Run(); err != nil { | ||
| return "", fmt.Errorf("git %s: %w: %s", args[0], err, scrub(stderr.String())) | ||
| var lastErr error | ||
| for attempt := 0; attempt < 3; attempt++ { | ||
| if attempt > 0 { | ||
| time.Sleep(time.Duration(attempt) * 2 * time.Second) | ||
| } | ||
| cmd := exec.Command("git", append([]string{"-C", repoDir}, args...)...) | ||
| var stdout, stderr strings.Builder | ||
| cmd.Stdout = &stdout | ||
| cmd.Stderr = &stderr | ||
| if err := cmd.Run(); err != nil { | ||
| lastErr = fmt.Errorf("git %s: %w: %s", args[0], err, scrub(stderr.String())) | ||
| continue | ||
| } | ||
| return stdout.String(), nil | ||
| } | ||
| return stdout.String(), nil | ||
| return "", lastErr |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: network-facing git commands use CommandContext with a bounded timeout.
rg -n -C 4 \
'exec\.Command(Context)?|context\.WithTimeout|GIT_TERMINAL_PROMPT|time\.Sleep' \
tools/changelog-site --glob '*.go'Repository: NVIDIA/nvcf
Length of output: 2907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- imports and retry helper ---'
sed -n '1,45p;430,490p' tools/changelog-site/main.go
printf '%s\n' '--- call sites and related Git operations ---'
sed -n '370,425p;490,560p' tools/changelog-site/main.go
printf '%s\n' '--- focused tests ---'
sed -n '1,180p' tools/changelog-site/main_test.goRepository: NVIDIA/nvcf
Length of output: 15303
Bound each Git attempt with a deadline.
Because fetchGitHubTags runs network operations with exec.Command, git ls-remote or git fetch can block on network I/O or credential handling. Use exec.CommandContext with a per-attempt timeout and set GIT_TERMINAL_PROMPT=0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/changelog-site/main.go` around lines 453 - 474, Update the local git
command runner around run to use exec.CommandContext with a per-attempt timeout,
canceling each context after the attempt completes. Set GIT_TERMINAL_PROMPT=0 on
every command so credential prompts cannot block, while preserving the existing
retry, stderr capture, and error-reporting behavior.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
git init -q "$tmp/umbrella"
git -C "$tmp/umbrella" config user.name review
git -C "$tmp/umbrella" config user.email [email protected]
printf 'initial\n' > "$tmp/umbrella/file"
git -C "$tmp/umbrella" add file
git -C "$tmp/umbrella" commit -qm initial
git -C "$tmp/umbrella" tag svc/v1.0.0
git clone -q "$tmp/umbrella" "$tmp/github"
git -C "$tmp/github" tag svc/v2.0.0
git -C "$tmp/umbrella" fetch --quiet "$tmp/github" '+refs/tags/*:refs/tags/*'
git -C "$tmp/umbrella" tag -lRepository: NVIDIA/nvcf
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target file outline ---'
ast-grep outline tools/changelog-site/main.go --match 'func $_($_) $_ { $$$ }' --view summary || true
printf '%s\n' '--- fetch and tag-discovery code ---'
sed -n '430,570p' tools/changelog-site/main.go
printf '%s\n' '--- related symbols and call sites ---'
rg -n -C 4 'allow-missing-github-tags|refs/tags|gitlabTags|githubTags|fetch' tools/changelog-siteRepository: NVIDIA/nvcf
Length of output: 18044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- release discovery and tag consumption ---'
rg -n -C 8 'func buildReleases|buildReleases\(|git\(\*repo|git\([^)]*"tag"|tag -l|originFor' tools/changelog-site/main.go
printf '%s\n' '--- relevant tests and CI invocation ---'
sed -n '1,180p' tools/changelog-site/main_test.go
rg -n -C 5 'changelog-site|allow-missing-github-tags|--repo|repo:' .github tools ci 2>/dev/null || true
printf '%s\n' '--- concise file map ---'
git ls-files tools/changelog-siteRepository: NVIDIA/nvcf
Length of output: 33186
Isolate GitHub tag refs from the canonical tag namespace.
fetchGitHubTags writes to refs/tags/*, while buildReleases scans that namespace. On a reused checkout, stale GitHub-only tags enter gitlabTags before the fetch and can be labeled both. With --allow-missing-github-tags, they can be published as gitlab.
Use an isolated ref namespace and consume it explicitly. --atomic does not prevent cross-run contamination.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/changelog-site/main.go` around lines 453 - 474, Update fetchGitHubTags
to fetch GitHub tag refs into a dedicated, non-canonical namespace rather than
refs/tags/*, preventing stale refs from contaminating the checkout. Update
buildReleases to read GitHub tags explicitly from that isolated namespace while
leaving gitlabTags sourced only from canonical tags; preserve
--allow-missing-github-tags behavior without promoting stale GitHub refs.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 \
'fetchGitHubTags|allow-missing-github-tags|github tags unavailable|changelog\.json|index\.html' \
tools/changelog-site --glob '*_test.go'Repository: NVIDIA/nvcf
Length of output: 1885
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test file outline ---'
ast-grep outline tools/changelog-site/main_test.go --lang go
printf '%s\n' '--- production symbols and relevant call sites ---'
ast-grep outline tools/changelog-site/main.go --lang go
printf '%s\n' '--- focused production sections ---'
sed -n '400,570p' tools/changelog-site/main.go
printf '%s\n' '--- focused test sections ---'
sed -n '1,220p' tools/changelog-site/main_test.go
printf '%s\n' '--- repository diff summary ---'
git diff --statRepository: NVIDIA/nvcf
Length of output: 17047
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- remaining main flow ---'
sed -n '505,675p' tools/changelog-site/main.go
printf '%s\n' '--- release scanning implementation ---'
sed -n '245,345p' tools/changelog-site/main.go
printf '%s\n' '--- test inventory and package metadata ---'
rg -n '^(func Test|func Benchmark)|^module |^go ' tools/changelog-site --glob '*_test.go' --glob 'go.mod'
printf '%s\n' '--- relevant references outside the test file ---'
rg -n -C 3 'allow-missing-github-tags|fetchGitHubTags|github tags unavailable|os\.WriteFile|MkdirAll|RemoveAll|index\.html|changelog\.json' tools/changelog-siteRepository: NVIDIA/nvcf
Length of output: 22124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("tools/changelog-site/main.go")
s = p.read_text()
for needle in [
'func fetchGitHubTags',
'func main()',
'git(*repo, "tag", "-l")',
'buildReleases(',
'os.WriteFile',
'os.MkdirAll',
]:
print(f"{needle}: {s.count(needle)} occurrence(s)")
PYRepository: NVIDIA/nvcf
Length of output: 348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Exercise the Git refspec used by fetchGitHubTags in isolated temporary
# repositories. This does not execute repository code or alter the checkout.
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
remote="$tmp/remote.git"
work="$tmp/work"
checkout="$tmp/checkout"
git init --bare -q "$remote"
git init -q -b main "$work"
git -C "$work" -c user.email=t@t -c user.name=t commit --allow-empty -q -m base
git -C "$work" tag svc/v1.0.0
git -C "$work" push -q "$remote" main 'refs/tags/*'
git clone -q "$remote" "$checkout"
git -C "$checkout" fetch --quiet "$remote" '+refs/tags/*:refs/tags/*'
printf '%s\n' 'tags after first fetch:'
git -C "$checkout" tag --list 'svc/*'
# Remove the remote tag, then repeat the exact non-pruning refspec.
git -C "$work" push -q "$remote" ':refs/tags/svc/v1.0.0'
git -C "$checkout" fetch --quiet "$remote" '+refs/tags/*:refs/tags/*'
printf '%s\n' 'tags after second fetch without --prune:'
git -C "$checkout" tag --list 'svc/*'
printf '%s\n' 'remote tags after deletion:'
git --git-dir="$remote" tag --list 'svc/*'Repository: NVIDIA/nvcf
Length of output: 331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- fatal and output helpers ---'
sed -n '675,710p' tools/changelog-site/main.go
printf '%s\n' '--- full tracked test list for this tool ---'
git ls-files tools/changelog-site '*_test.go'Repository: NVIDIA/nvcf
Length of output: 50367
Add focused tests and synchronize reused GitHub tag refs.
TestFetchGitHubTagsLabelsOrigin covers only success. Add tests for retry recovery, exhaustion, token scrubbing, failure modes, output preservation, and --allow-missing-github-tags using a fake or injectable Git runner. The current refspec does not prune deleted tags. Add a two-run deletion test and prune or isolate GitHub tag refs before building releases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/changelog-site/main.go` around lines 453 - 474, Extend
TestFetchGitHubTagsLabelsOrigin with focused coverage for retry recovery, retry
exhaustion, token scrubbing, failure modes, output preservation, and
--allow-missing-github-tags using an injectable or fake Git runner. Update the
GitHub tag synchronization flow around the run helper and its refspec so deleted
remote tags are pruned or isolated before releases are built, and add a two-run
test verifying deletion is reflected.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The published changelog flapped: services appeared and disappeared between half-hourly rebuilds, and always all of them together.
fetchGitHubTagsrunsgit ls-remoteandgit fetchagainst GitHub with no retry, and the caller only warned on failure:So one transient network failure dropped the entire GitHub tag set, and the site republished with every GitHub-only release missing, overwriting a good deployment with a worse one. The next successful run restored it. Nothing reported an error, because the job exits zero either way.
That "degrade gracefully" is wrong for a publisher. Serving a stale-but-complete site is strictly better than serving a fresh-but-half-empty one, and the failure is invisible in the output.
What changed
--allow-missing-github-tagsopts back into the old behaviour deliberately.Testing
Run against a full GitLab clone plus the GitHub mirror, both paths:
{gitlab: 1058, both: 36, github: 960}Spot-checked the services that were reported flapping:
Notes
Two things found while testing, neither fixed here:
both. The first run fetches GitHub tags into the clone, so the second run'sgit tag -lbaseline already contains them. CI is unaffected because it clones fresh, but it makes local iteration misleading.tools/ci/subproject-validations.yamlfrom the frozen GitLab umbrella, which lists no Java subprojects because they were added after the freeze. That needs a decision about where the service list should live and is not addressable in this repo alone.tools/changelog-site/changelog-siteis a compiled binary committed by accident in #554. Left untouched here rather than committing a rebuilt one.Summary by CodeRabbit
Bug Fixes
New Features