Conversation
…ajor The version guard treated any 'npm view' failure as 'not published', letting a transient network or registry error proceed to a publish attempt. Only a definitive not-found (E404 / no match) may proceed now; any other failure logs the npm output and fails the job. Success also requires non-empty output, covering npm versions that report a missing version as exit 0 with nothing on stdout. Pin the npm upgrade to the 11.x major so a future npm major release can't change publish behavior mid-job.
WalkthroughThe npm publish workflow is hardened with a pinned npm major version and a rewritten idempotency guard. The guard now explicitly distinguishes between already-published versions, not-yet-published versions, and transient failures before running ChangesNPM Publish Workflow Hardening
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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: 1
🤖 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 @.github/workflows/npm-publish.yml:
- Around line 64-73: The current guard captures both stdout and stderr into
OUTPUT (OUTPUT="$(npm view ... 2>&1)"), so warnings on stderr can make [ -n
"$OUTPUT" ] true and set published=true incorrectly; change the npm view
invocation to capture stdout and stderr separately (store stdout in OUTPUT and
stderr in a different variable or temp file), preserve the command exit code in
STATUS, then base the published=true check only on the stdout variable (OUTPUT)
being non-empty and STATUS being zero while using the stderr variable only for
logging/diagnosis and for the alternative E404/not-found checks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d94a51b-b3e6-455e-9152-a4dcfbf2e176
📒 Files selected for processing (1)
.github/workflows/npm-publish.yml
Comment on lines
+64
to
73
| if OUTPUT="$(npm view "@gravitykit/block-mcp@$VERSION" version 2>&1)"; then | ||
| STATUS=0 | ||
| else | ||
| STATUS=$? | ||
| fi | ||
| if [ "$STATUS" -eq 0 ] && [ -n "$OUTPUT" ]; then | ||
| echo "published=true" >> "$GITHUB_OUTPUT" | ||
| echo "@gravitykit/block-mcp@$VERSION is already on npm — nothing to publish." | ||
| else | ||
| elif [ "$STATUS" -eq 0 ] || printf '%s' "$OUTPUT" | grep -qiE 'E404|not found|no match found'; then | ||
| echo "published=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify behavior on the CI runner/npm version in use:
# Expected:
# - Existing version => stdout non-empty
# - Missing version => either non-zero + 404-like stderr OR zero + empty stdout
# - stderr notices must not influence stdout-based published check
set -euo pipefail
PKG="`@gravitykit/block-mcp`"
EXISTING="$(npm view "$PKG" version)"
MISSING="0.0.0-coderabbit-verify-$(date +%s)"
for V in "$EXISTING" "$MISSING"; do
echo "=== Testing $PKG@$V ==="
OUT="$(npm view "$PKG@$V" version 2>/tmp/npm-view.err || true)"
STATUS=$?
ERR="$(cat /tmp/npm-view.err || true)"
printf 'status=%s\nstdout="%s"\nstderr="%s"\n\n' "$STATUS" "$OUT" "$ERR"
doneRepository: GravityKit/block-mcp
Length of output: 397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/npm-publish.yml"
echo "== File = $FILE =="
nl -ba "$FILE" | sed -n '1,140p'Repository: GravityKit/block-mcp
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/npm-publish.yml"
echo "== File = $FILE =="
# show around the referenced lines; use cat -n (nl may not exist)
cat -n "$FILE" | sed -n '40,110p'Repository: GravityKit/block-mcp
Length of output: 2409
Split stdout/stderr in the npm view guard to prevent incorrect published=true results.
In .github/workflows/npm-publish.yml (lines 64-73), OUTPUT="$(npm view ... version 2>&1)" merges stderr into OUTPUT, but the logic later treats non-empty OUTPUT as evidence the version exists. Any npm warning/error emitted to stderr (even when stdout has no version) can make [ -n "$OUTPUT" ] pass and incorrectly set published=true.
Suggested fix
- if OUTPUT="$(npm view "`@gravitykit/block-mcp`@$VERSION" version 2>&1)"; then
+ if OUTPUT="$(npm view "`@gravitykit/block-mcp`@$VERSION" version 2>/tmp/npm-view.err)"; then
STATUS=0
else
STATUS=$?
fi
- if [ "$STATUS" -eq 0 ] && [ -n "$OUTPUT" ]; then
+ ERR_OUTPUT="$(cat /tmp/npm-view.err || true)"
+ if [ "$STATUS" -eq 0 ] && [ -n "$OUTPUT" ]; then
echo "published=true" >> "$GITHUB_OUTPUT"
echo "`@gravitykit/block-mcp`@$VERSION is already on npm — nothing to publish."
- elif [ "$STATUS" -eq 0 ] || printf '%s' "$OUTPUT" | grep -qiE 'E404|not found|no match found'; then
+ elif [ "$STATUS" -eq 0 ] || printf '%s' "$ERR_OUTPUT" | grep -qiE 'E404|not found|no match found'; then
echo "published=false" >> "$GITHUB_OUTPUT"
else
echo "::error::Could not determine whether `@gravitykit/block-mcp`@$VERSION is published (npm view exit $STATUS):" >&2
- printf '%s\n' "$OUTPUT" >&2
+ printf '%s\n' "$ERR_OUTPUT" >&2
exit 1
fi🤖 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 @.github/workflows/npm-publish.yml around lines 64 - 73, The current guard
captures both stdout and stderr into OUTPUT (OUTPUT="$(npm view ... 2>&1)"), so
warnings on stderr can make [ -n "$OUTPUT" ] true and set published=true
incorrectly; change the npm view invocation to capture stdout and stderr
separately (store stdout in OUTPUT and stderr in a different variable or temp
file), preserve the command exit code in STATUS, then base the published=true
check only on the stdout variable (OUTPUT) being non-empty and STATUS being zero
while using the stderr variable only for logging/diagnosis and for the
alternative E404/not-found checks.
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
1 participant
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.
Review follow-ups to the npm publish workflow:
E404/no match found) proceeds to publish; any othernpm viewfailure (network, registry outage) logs the output and fails the job instead of slipping past the idempotency guard. Success additionally requires non-empty output, covering npm versions that report a missing version as exit 0 with empty stdout.npm@11instead ofnpm@latestso a future npm major can't change publish behavior mid-release-job.Note: 2.0.1 already published successfully via this workflow (trusted publishing was pre-registered) — merging this PR exercises the guard's skip path live.
Summary by CodeRabbit
💾 Build file (a5cdabe).