Conversation
…ersion
Motivation
The nightly rad CLI download workflow has produced over 130 "CLI nightly
test failed" issues over the past year, each closed by maintainers as an
unexplained transient failure. The CI log always cuts off right after
"GitHub API call successful" with no further script output before make
reports a bare "Error 1", giving maintainers nothing to diagnose.
Approach
build/test-cli-download.sh extracts the release version with:
RAD_VERSION=$(echo "$api_response" | grep "tag_name" | grep -v rc | awk ... | sed ...)
if [ -z "$RAD_VERSION" ]; then
echo "Failed to extract RAD_VERSION from API response"
exit 1
fi
The script runs under `set -euo pipefail`. When the GitHub API response
contains no "tag_name" line (for example, an API error body instead of a
releases array), the pipeline returns a non-zero status, and under
`set -e` the `RAD_VERSION=$(...)` assignment statement itself aborts the
script immediately with that status -- before the `[ -z "$RAD_VERSION" ]`
check, and its error message, are ever reached. The intended diagnostic is
dead code.
The fix adds `|| true` to the assignment so the pipeline's non-zero status
no longer kills the script early, letting execution reach the existing
empty-value check, which now also prints the raw API response body so a
future failure is actually diagnosable. The success path is unchanged:
`|| true` only affects the exit status used by `set -e`, not what the
command substitution captures on stdout.
This does not claim to fix the specific historical cause of issue radius-project#12906
(GitHub did not preserve that run's raw API response, so it can't be
confirmed) -- only that the script's own error handling was unreachable
dead code, which is fixed here and independently useful for whatever
causes the next occurrence.
Validation
- bash -n build/test-cli-download.sh: syntax OK.
- shellcheck --rcfile .github/linters/.shellcheckrc build/test-cli-download.sh:
clean, mirrors the repo's `make lint-shell` CI target and its pinned
config (`make lint-shell` itself could not run locally because its
Go-based tool installer needs disk space this sandbox does not have --
an environment limitation, not a code issue).
- Ran build/test-cli-download.sh linux amd64 rad "" "" against the live
GitHub API: unchanged happy-path output, script exits 0, and the real
rad_linux_amd64 binary downloads successfully.
- Reproduced the failure mode directly: with api_response mocked to a
GitHub-style API error body ({"message":"API rate limit exceeded ..."}),
the pre-fix script exits 1 with zero output after "GitHub API call
successful" (matching the exact log pattern from issue radius-project#12906);
the post-fix script exits 1 and prints the diagnostic
"Failed to extract RAD_VERSION from API response:" followed by the
response body.
Report: radius-project#12906
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
There was a problem hiding this comment.
🟡 Changes recommended
The script’s earlier curl failure handling is still likely unreachable under set -e when curl fails inside command substitution, leaving a remaining silent-failure path in the same workflow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves diagnosability of the nightly rad CLI download test by ensuring version extraction failures from the GitHub Releases API don’t terminate the script prematurely under set -euo pipefail, and by printing the API response body when extraction fails.
Changes:
- Prevent premature exit when the version-extraction
greppipeline finds no matches (allowing the explicit empty-value error branch to run). - Emit the raw GitHub API response when
RAD_VERSIONcannot be extracted.
File summaries
| File | Description |
|---|---|
| build/test-cli-download.sh | Makes the version-extraction step resilient to non-matching grep under set -e and prints the API response on extraction failure for easier debugging. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Extract version from API response using grep, awk, and sed. | ||
| # `|| true` stops a non-matching grep (e.g. an API error response with no | ||
| # "tag_name") from tripping `set -e` here, before the empty-value check below | ||
| # can report a useful error. | ||
| RAD_VERSION=$(echo "$api_response" | grep "tag_name" | grep -v rc | awk 'NR==1{print $2}' | sed -n 's/"\(.*\)",/\1/p') || true |
| echo "Failed to extract RAD_VERSION from API response:" | ||
| echo "$api_response" |
|
/lgtm pls address copilot comments above |
… stderr
Addresses both review comments.
The exit-code check after the API call could never run. The script uses
set -euo pipefail, so a failing curl inside api_response=$(curl ...) exits
the script at the assignment, before curl_exit_code=$? is evaluated.
Verified: with the old code against an unreachable endpoint the script exits
7 and prints nothing; the "GitHub API call failed" line is dead. It is now
a || { ... } block, which set -e does not trip, and curl uses -sS so its own
error is surfaced too.
Both error paths now write to stderr with printf rather than echo, since
they are diagnostics rather than output.
Verified against a dead endpoint (curl error plus "failed (curl exit 7)" on
stderr, exit 1) and against an unparseable body (the response echoed to
stderr, exit 1, nothing on stdout).
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-opus-5 (via Claude Code)
|
Thank you for contributing to Radius. The following commits do not currently show a GitHub Verified signature: Please follow our commit-signing guide to configure GPG, SSH, or S/MIME signing. Then re-sign the affected commits and force-push the rewritten branch with Cryptographic commit signing is separate from the DCO |
|
Thanks @lakshmimsft — both addressed in the latest commit. The first one was a real bug. The script runs under Both error paths now use |
Summary
Fixes a silent-failure bug in
build/test-cli-download.sh(used by the nightly rad CLI download workflow) where, underset -euo pipefail, a non-matchinggrepwhile extracting the release version from the GitHub API response would abort the script immediately -- before the script's own[ -z "$RAD_VERSION" ]error-handling branch (and its diagnostic message) could ever run. The fix (|| trueon the extraction pipeline) lets that branch execute as intended, and it now also prints the raw API response body so a future failure is actually diagnosable.Reason for change
The nightly rad CLI download workflow has generated 130+ "CLI nightly test failed" issues over the past year, each closed by maintainers as an unexplained transient failure because the CI log always cuts off right after "GitHub API call successful" with no further output before
makereports a bare "Error 1". The script's own error-reporting code for this exact case was unreachable dead code due to aset -e/pipefailinteraction with agreppipeline. This change does not claim to know the root cause of the underlying API failures (GitHub does not retain the raw API response from past runs, so it can't be confirmed to have been a rate-limit response specifically) -- only that the diagnostic dead-code bug is real, reproducible, and now fixed, so the next occurrence will actually be diagnosable.Fixes #12906
How to test
To see the fix in action, mock
api_response(e.g.api_response='{"message":"API rate limit exceeded"}') in place of thecurlcall and re-run the script: before the fix it exits 1 with no output after "GitHub API call successful"; after the fix it prints the response body before exiting 1.Note:
make lint-shellcould not be run directly in this sandbox because its Go-based tool installer needs disk space that isn't available here (an environment limitation, not a code issue);shellcheckwas run directly against the repo's pinned.shellcheckrcas a faithful substitute for that specific CI check.File change summary
build/test-cli-download.sh|| trueso a non-matchinggrepin the version-extraction pipeline no longer silently kills the script underset -e; print the raw API response body when version extraction fails.