fix(changelog-site): retry the GitHub tag fetch and fail instead of degrading - #645
Merged
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 <bganesan@nvidia.com>
📝 WalkthroughWalkthroughThe changelog site retrieves GitHub tags with timeout-bound retries and incremental backoff. Tag retrieval failures 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 timeout-bound 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 or publish based on --allow-missing-github-tags
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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
sbaum1994
approved these changes
Aug 5, 2026
fetchGitHubTags runs two network-facing git commands through exec.Command with no deadline. A hung connection or a credential prompt blocks until the CI job times out an hour later, and the retry loop added in this PR never gets to run, so the retry does not help in the case it was written for. Give each attempt its own context with a two-minute timeout, and report a timeout distinctly from a git error so the cause is visible in the log. Three attempts plus backoff still finish well inside a CI job. Set GIT_TERMINAL_PROMPT=0 on the command rather than relying on the environment. The publishing job does export it, but this tool is also run by hand, and a prompt waiting on a closed stdin is the exact stall the deadline then has to clean up. Raised by CodeRabbit on !645. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/changelog-site/main_test.go (1)
324-334: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest command behavior instead of scanning source text.
The assertions at Lines 329-333 search the entire
main.gofile. They can pass whenGIT_TERMINAL_PROMPT=0orexec.CommandContextappears in unrelated code. They do not verify that both Git commands receive the environment setting and a deadline.Run
fetchGitHubTagswith a fakegitexecutable or an injected command runner. Assert the child environment and cancellation behavior forls-remoteandfetch. The suppliedtools/changelog-site/main.goimplementation places these guarantees in the sharedrunclosure.🤖 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_test.go` around lines 324 - 334, Replace the source-text assertions in TestFetchGitHubTagsDisablesCredentialPrompt with behavioral coverage of fetchGitHubTags, using a fake git executable or injected command runner. Verify both ls-remote and fetch receive GIT_TERMINAL_PROMPT=0 and are executed through the shared run closure with cancellation/deadline behavior from exec.CommandContext.
🤖 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_test.go`:
- Around line 315-317: Update the retry-budget test around gitNetworkTimeout to
assert it equals exactly 2*time.Minute, then calculate the worst-case budget for
both retry sequences used by fetchGitHubTags: ls-remote and fetch, each with its
full retry schedule. Keep the existing 30-minute CI budget assertion while
ensuring both fetchGitHubTags call paths are represented.
---
Nitpick comments:
In `@tools/changelog-site/main_test.go`:
- Around line 324-334: Replace the source-text assertions in
TestFetchGitHubTagsDisablesCredentialPrompt with behavioral coverage of
fetchGitHubTags, using a fake git executable or injected command runner. Verify
both ls-remote and fetch receive GIT_TERMINAL_PROMPT=0 and are executed through
the shared run closure with cancellation/deadline behavior from
exec.CommandContext.
🪄 Autofix
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: 25907577-3e03-46cc-8af1-4b9787c38ad2
📒 Files selected for processing (3)
tools/changelog-site/changelog-sitetools/changelog-site/main.gotools/changelog-site/main_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/changelog-site/main.go
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
3 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