feat(ci): add post-publish verification for npm package#323
Conversation
Adds automated verification that tests the published agent-relay package across Node.js 18, 20, and 22 using: - Global npm install - npx execution - Local project install Includes Docker-based isolation and GitHub Actions workflow that runs automatically after successful publish. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if [ "$PARALLEL" = true ]; then | ||
| log_info "Running verification in parallel..." | ||
| docker compose up --abort-on-container-exit $SERVICES | ||
| EXIT_CODE=$? |
There was a problem hiding this comment.
🔴 Parallel mode captures exit code but never uses it, always reports success
In parallel mode, the script captures EXIT_CODE=$? but never uses it. The final summary logic only checks FAILED_VERSIONS array, which is never populated in parallel mode.
Click to expand
How it happens
Lines 94-97 run parallel mode:
if [ "$PARALLEL" = true ]; then
docker compose up --abort-on-container-exit $SERVICES
EXIT_CODE=$?But the summary at lines 121-126 only checks FAILED_VERSIONS:
if [ ${#FAILED_VERSIONS[@]} -eq 0 ]; then
log_success "All Node.js versions passed verification!"
exit 0FAILED_VERSIONS is only populated in the sequential (else) branch, never in parallel mode.
Impact
When running with --parallel, the script will always report success and exit 0 even if all tests fail.
Recommendation: Use EXIT_CODE in parallel mode: after line 97, add if [ $EXIT_CODE -ne 0 ]; then exit $EXIT_CODE; fi or incorporate it into the final summary logic.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for i in {1..30}; do | ||
| if npm view agent-relay@${{ inputs.version }} version 2>/dev/null; then | ||
| echo "Package found on npm registry" | ||
| break | ||
| fi | ||
| echo "Attempt $i: Package not yet available, waiting 10s..." | ||
| sleep 10 | ||
| done |
There was a problem hiding this comment.
🔴 npm propagation wait loop silently continues if package is never found
The wait loop attempts 30 times to find the package, but if all attempts fail, it silently continues to the next step instead of failing the workflow.
Click to expand
How it happens
Lines 66-73 loop but never fail:
for i in {1..30}; do
if npm view agent-relay@${{ inputs.version }} version 2>/dev/null; then
echo "Package found on npm registry"
break
fi
echo "Attempt $i: Package not yet available, waiting 10s..."
sleep 10
doneAfter 30 failed attempts (5 minutes), execution continues to the install step which will then fail with a confusing error.
Impact
The verification will proceed against a non-existent package version, causing cryptic npm install failures instead of a clear "package not found after timeout" error.
Recommendation: Add a flag to track if the package was found, and exit 1 after the loop if not found: FOUND=false; for i in ...; do if npm view ...; then FOUND=true; break; fi; done; if [ "$FOUND" = false ]; then echo "Package not found after 30 attempts"; exit 1; fi
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| # Wait for npm to propagate the package | ||
| - name: Wait for npm propagation | ||
| if: inputs.version != 'latest' |
There was a problem hiding this comment.
🔴 Wait step runs with empty version on pull_request trigger causing invalid npm command
The condition if: inputs.version != 'latest' evaluates to true when inputs.version is empty (which happens on pull_request trigger), causing the npm view command to run with an empty version.
Click to expand
How it happens
On pull_request trigger (lines 12-15), no inputs are provided, so inputs.version is empty/null. The condition at line 63:
if: inputs.version != 'latest'evaluates to true because empty string != 'latest'.
This causes line 67 to execute:
if npm view agent-relay@${{ inputs.version }} versionWhich becomes npm view agent-relay@ version - an invalid command.
Impact
The workflow will fail with a confusing npm error when triggered on PRs that modify the workflow files.
Recommendation: Change the condition to also check for empty: if: inputs.version && inputs.version != 'latest' or use the resolved VERSION variable from the previous step.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo "| Node 18 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARY | ||
| echo "| Node 20 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARY | ||
| echo "| Node 22 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
🟡 Workflow summary displays same status for all Node versions instead of individual results
The verification summary in verify-publish.yml displays misleading per-Node-version status information.
Click to expand
Issue
The summary section (lines 212-214 and 219-221) shows three separate rows for Node 18, 20, and 22, but all rows display the same value from needs.verify.result:
echo "| Node 18 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARY
echo "| Node 20 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARY
echo "| Node 22 | ${{ needs.verify.result == 'success' && '✅' || '❌' }} |" >> $GITHUB_STEP_SUMMARYIn GitHub Actions, needs.job.result for a matrix job returns the aggregate result, not individual matrix instance results. This means if Node 18 passes but Node 20 fails, all three rows will show ❌ (because the aggregate is failure).
Actual vs Expected
- Actual: All three Node version rows show identical status (the aggregate matrix result)
- Expected: Each row should show the individual result for that specific Node version, or the summary should clarify it's showing aggregate status
Impact
This is misleading to users reviewing the workflow summary - they may think all versions failed when only one did, or vice versa. The overall pass/fail logic is correct, but the granular display is inaccurate.
Recommendation: Either (1) remove the per-version table and just show aggregate pass/fail, (2) add a note clarifying this is aggregate status, or (3) use job outputs to capture individual matrix results for accurate per-version reporting.
Was this helpful? React with 👍 or 👎 to provide feedback.
… hooks - Import ConsensusType, VoteValue, ProposalStatus from @agent-relay/protocol in daemon - Rename hooks' InboxMessage to ParsedInboxMessage to avoid conflict with protocol - Fix imports in enhanced-features.ts and consensus-integration.ts
- Use --package flag for explicit npx package specification - Add PATH debugging and ensure npm bin is in PATH in Docker script - Add binary existence check for debugging
- Use 'npx package -- args' syntax instead of --package flag - Remove set -e so all tests run and report results - Add explicit exit code capture for npm install
1 participant
Summary
Test plan
🤖 Generated with Claude Code