fix(publish-npm): make silent publish failures structurally impossible

[Yan Xue (yanxue06)]

Summary

publish-npm currently trusts the publisher CLI's exit code, but several common publishers — most notably bunx clean-publish and bunx-wrapped commands generally — swallow npm publish's non-zero exit. The OIDC branch also explicitly catches its own publish failures to fall through to the token path. Together, these mean a job can finish green with nothing on npm.

This is exactly what happened to spectrum-ts from v1.10.0 through v1.11.1: an invalidated NPM_TOKEN produced npm error code E404 on every publish, but the publisher's exit code stayed at 0, so four releases went green on Actions while npm latest stayed at 1.9.2. No alarms, no email, no PR comments — just silent drift.

This PR makes that class of failure structurally impossible via two layers of defense.

Layer 1 — publish steps fail directly when npm errors

Both publish steps (OIDC primary, NPM token fallback) now capture publisher output via tee, read the publisher's real exit code from ${PIPESTATUS[0]} (not the pipeline's $?, which is always 0 because of tee), and treat any ^npm (error|ERR!) line in the output as a hard failure even when the publisher itself exited 0. This catches the wrapper-swallowed-exit case directly at the publish step instead of relying on after-the-fact detection.

The OIDC branch also intentionally refuses to fall back to NPM_TOKEN on a swallowed-exit failure: a "successful" OIDC publish that printed npm errors signals a wrapper bug the token path can't fix, and falling back would just hide it again. Real OIDC failures (non-zero exit) still fall back as before.

LOG="$(mktemp)"
set +e
${{ inputs.publish-command }} ... 2>&1 | tee "$LOG"
PUBLISH_EXIT=${PIPESTATUS[0]}
set -e

if [ "$PUBLISH_EXIT" -ne 0 ]; then
  exit "$PUBLISH_EXIT"
fi

if grep -qE '^npm (error|ERR!)' "$LOG"; then
  echo "Publisher exited 0 but npm emitted error lines."
  exit 1
fi

Layer 2 — registry verification as a final backstop

Even with layer 1, a future publisher we haven't anticipated could still swallow errors in some new way. So a final step probes https://registry.npmjs.org/<name>/<version> and asserts the bytes are actually fetchable. Six retries spaced 5s apart absorb npm CDN propagation. Only runs on real publishes (skipped on --dry-run).

- name: Verify publish landed on registry
  if: ${{ inputs.dry-run != 'true' }}
  run: |
    NAME=$(jq -r .name package.json)
    VERSION=$(jq -r .version package.json)
    URL="https://registry.npmjs.org/$NAME/$VERSION"
    for attempt in 1 2 3 4 5 6; do
      STATUS=$(curl -fsS -o /dev/null -w "%{http_code}" "$URL" || true)
      [ "$STATUS" = "200" ] && exit 0
      sleep 5
    done
    exit 1

Why both layers

Layer 1 fails fast at the source — operators see "npm token publish exited non-zero" or "publisher reported success but npm emitted error lines" inline with the publish output, with no ambiguity about which step is wrong. Layer 2 is the canary — it's publisher-agnostic and asserts the only thing that actually matters: did the bytes land on the public registry. If layer 1 ever has a blind spot (e.g., a future publisher invents a new error format that doesn't match ^npm (error|ERR!)), layer 2 still catches it.

This eliminates the "publish reports success, registry has nothing" state entirely. It cannot be reached.

Backward compatibility

• Successful publishes are unchanged — both the publisher's output and the new "Verified <name>@<version> is live" message appear in the log
• Failed publishes that were already failing the step (non-zero exit) are unchanged
• Failed publishes that were silently going green now hard-fail with a clear diagnostic — this is the intended new behavior
• dry-run: true skips the registry verification (nothing to verify); the publish-step hardening is a no-op on dry-runs because dry-run output doesn't contain npm error unless packaging itself is broken (which we want to surface anyway)

Test plan

• YAML lints clean (already verified)
• Dry-run publish from a test repo: registry-verify step is skipped; publish-step hardening doesn't false-positive on dry-run output
• Real publish with a working NPM_TOKEN: all three steps succeed, log shows ✅ Published via NPM token and ✅ <name>@<version> is live on the npm registry
• Real publish with a deliberately-bad NPM_TOKEN and a publisher that swallows exits (e.g., bunx clean-publish): publish step fails red on layer 1 (Publisher reported success but npm emitted error lines), without ever reaching layer 2
• Real publish where the publisher exits non-zero: publish step fails red on the explicit exit-code check
• Sanity-check on the spectrum-ts repo's first OIDC publish after this lands

Notes for reviewers

There's a follow-up PR on spectrum-ts (photon-hq/spectrum-ts#76) that opts that repo into npm OIDC Trusted Publishing. That PR is the operational fix for the immediate spectrum-ts breakage; this PR is the systemic safety net so the same class of failure can't recur silently for any caller of publish-npm.

[coderabbitai]

📝 Walkthrough

Walkthrough

The npm publish action now includes a post-publish verification step. After publishing, the action reads the package name and version from package.json, constructs the npm registry URL, and polls the endpoint with exponential backoff retries to confirm the version has appeared on the registry before completing.

Changes

Publish Verification

Layer / File(s)	Summary
Post-publish registry verification step .github/blocks/publish-npm/action.yaml	A new step validates the published <name>@<version> is live on the npm registry by querying the expected registry URL with up to 6 retries. The step runs only when dry-run is not true and fails with diagnostics listing common causes if the version is not found.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A package flies forth to the npm sky,
Now verified landing—no need to wonder why,
Six retries with patience, a heartbeat so true,
The registry whispers: "Your version shines through!" ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding verification that a published npm version actually landed on the registry to catch silent failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/verify-npm-publish-on-registry

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

📄 README may need an update

This PR introduces changes that might not be reflected in README.md.

Reason: README.md's publish-npm block documentation is missing the public publish-command input now exposed by .github/blocks/publish-npm/action.yaml, so it does not fully reflect the current configurable API.

This is an automated check powered by AI. If the README is intentionally unchanged, feel free to ignore this.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[github-actions]

📚 Skills documentation may need an update

This PR introduces changes that might not be reflected in the skills documentation.

Reason: _skills-repo/skills/buildspace-ci-cd/SKILL.md still describes npm publishing as token-based (NPM_TOKEN) and does not mention the new publish-npm behavior supporting npm OIDC Trusted Publishing with id-token: write fallback to NPM_TOKEN plus post-publish registry verification.

This is an automated check powered by AI. If the skills are intentionally unchanged, feel free to ignore this.

[coderabbitai]

🧹 Nitpick comments (1)

.github/blocks/publish-npm/action.yaml (1)

152-153: ⚡ Quick win

Revisit the severity: scoped package URLs don’t need encoding for npm registry lookups
The npm registry accepts unencoded scoped package paths (e.g., https://registry.npmjs.org/@types/node/25.9.1 returns 200), so the proposed URL construction change isn’t a major correctness fix for scoped names. URL-encoding the package name/version remains a reasonable robustness improvement, but it should be treated as optional refactoring rather than a correctness issue.

🤖 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/blocks/publish-npm/action.yaml around lines 152 - 153, The change
treating scoped package URL-encoding as a correctness fix is overstated; revert
the required encoding and keep the registry lookup using the existing URL
construction (the URL variable built from NAME and VERSION:
URL="https://registry.npmjs.org/$NAME/$VERSION") so scoped names like
`@types/node` resolve correctly, and if you want to keep encoding as a robustness
option implement it as a non-mandatory refactor (e.g., optional flag or a
separate helper) rather than changing the default behavior; update the echo
line/message if needed to reflect that encoding is optional.

🤖 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.

Nitpick comments:
In @.github/blocks/publish-npm/action.yaml:
- Around line 152-153: The change treating scoped package URL-encoding as a
correctness fix is overstated; revert the required encoding and keep the
registry lookup using the existing URL construction (the URL variable built from
NAME and VERSION: URL="https://registry.npmjs.org/$NAME/$VERSION") so scoped
names like `@types/node` resolve correctly, and if you want to keep encoding as a
robustness option implement it as a non-mandatory refactor (e.g., optional flag
or a separate helper) rather than changing the default behavior; update the echo
line/message if needed to reflect that encoding is optional.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b054323b-d39d-4660-b099-fd4df2074fe8

📥 Commits

Reviewing files that changed from the base of the PR and between 15ee35b and 3898396.

📒 Files selected for processing (1)

• .github/blocks/publish-npm/action.yaml

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: copilot-pull-request-reviewer
• GitHub Check: check-readme / check-readme
• GitHub Check: check-skills / check-skills