Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available. Verified Claude’s naming nit: both npm-source steps now warn on fallback but retain assertion-style names. Version checks and the forced GitHub-release fallback assertion remain intact. No additional defects or conflicts with pending next-branch work were found.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| ⚪ NIT | .github/workflows/setup-cli-smoke-test.yml:100 |
naming |
claude | Both steps still say "Verify setup-cli installed from npm", although fallback now only emits a warning and succeeds. Their names misleadingly suggest that a passing step confirms npm installation. |
Stats
Claude findings: 1 · Codex findings: 0 · Confirmed: 1 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| if [ "${INSTALL_SOURCE}" != "npm" ]; then | ||
| echo "::error::setup-cli fell back to ${INSTALL_SOURCE} because installing supabase@${VERSION#v} from npm failed; check the setup-cli step log" | ||
| exit 1 | ||
| echo "::warning::setup-cli fell back to ${INSTALL_SOURCE} because installing supabase@${VERSION#v} from npm failed; check the setup-cli step log" |
There was a problem hiding this comment.
⚪ NIT · naming · source: claude
Both steps still say "Verify setup-cli installed from npm", although fallback now only emits a warning and succeeds. Their names misleadingly suggest that a passing step confirms npm installation.
Evidence: Step names at lines 92 and 174 claim to verify npm installation, while lines 99–101 and 181–183 only emit ::warning:: when INSTALL_SOURCE differs from npm.
Suggested fix: Rename both steps to "Warn if setup-cli fell back from npm".
CLI benchmark (Lightweight Commands): no changes foundCommit
API calls: unchanged for all 7 commands. Binary size: 134.9 MB, unchanged. Details: timing ranges, API calls per command, slowest phasesTiming ranges
API calls per command
Slowest phasesNo command moved beyond noise, so there are no phase deltas. Comment |
Coly010
changed the title
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| timeout-minutes: 25 | ||
| # Only set on fallback; an empty value from another matrix leg doesn't overwrite it. | ||
| outputs: | ||
| npm_fallback: ${{ steps.npm-source.outputs.fallback }} |
There was a problem hiding this comment.
⚪ Severity: LOW
Each matrix leg publishes the same npm_fallback job output, but GitHub keeps only the output from the last matrix leg to finish. A v1/v2 leg or a v3 leg without a fallback can therefore overwrite a real fallback value, leaving the summary and release Slack notice false and hiding this release signal.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Don't use a job-level output on a matrix job to carry the fallback signal. GitHub keeps only one matrix leg's output, so an empty value from a later leg can overwrite a real fallback. There are two safer options. (1) Have the fallback-detecting leg write a small marker file, for example npm-fallback-${{ matrix.name }}.txt, and upload it with actions/upload-artifact. A downstream job then downloads the artifacts and sets npm_fallback=true if any marker exists. (2) Move the fallback detection into a separate non-matrix job, or an aggregate job that uses the GitHub API to read each leg's step result. Whichever option you pick, the aggregating job should expose npm_fallback. release-shared.yml, release.yml and the Slack notice should read it from that job. Also remove the misleading comment that says empty values from other legs don't overwrite the output.
Current Behavior
The setup-cli smoke test fails a v3 job when setup-cli installs from the GitHub release instead of npm. Because that job runs inside the release workflow, the whole release run goes red: the "released" Slack message is skipped and a generic "release failed" message is sent, even though every channel published and the fallback installed the right version.
Expected Behavior
A fallback to the GitHub release emits a warning annotation and the job stays green. The fallback is also exported as a workflow output, passed through
release-shared.ymltorelease.yml, and added as a notice to the "released" Slack message, so the signal is kept without marking the release as failed. The dedicated fallback job still checks that the fallback path works, and a fallback that installs the wrong version still fails the version check.The workflow change needs a cherry-pick to active
v*.xbranches.