fix(publish-npm): make publish steps fail directly when npm errors (follow-up to #82) - #83
Conversation
Layered on top of the registry-verification step: harden both publish
steps so they detect failures *during* publish instead of relying solely
on the after-the-fact registry probe.
Both the OIDC and token publish steps now:
- Capture publisher output via `tee` (still streams live to the log)
- Read the publisher's real exit code from `${PIPESTATUS[0]}` instead
of the pipeline's $? (which would always be 0 because of tee)
- Treat any `^npm (error|ERR!)` line in the output as a hard failure,
even when the publisher itself exited 0 — this catches the case
where `bunx clean-publish` swallowed npm's non-zero exit, which is
exactly what kept spectrum-ts 1.10.0..1.11.1 silent for four releases
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 that the token path can't fix, and falling
back would hide it again. Real OIDC failures (non-zero exit) still fall
back to the token path, preserving the original opt-in semantics.
The registry-verification step from the previous commit stays as a final
backstop. Defense-in-depth: publish step fails directly → verify step
fails on missing version → no green CI without bytes on npm.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Your plan currently allows 3 reviews/hour. Refill in 3 minutes and 16 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
📚 Skills documentation may need an updateThis PR introduces changes that might not be reflected in the skills documentation. Reason:
|
📄 README may need an updateThis PR introduces changes that might not be reflected in Reason:
|
1 participant
Summary
Follow-up to #82. That PR added Layer 2 (verify on registry) and #82 was squash-merged before this Layer 1 hardening commit landed on the same branch — so
maincurrently has only the post-publish backstop, not the in-step guard.This PR adds Layer 1: the publish steps themselves now fail when
npm publisherrored, even when the publisher CLI swallows the non-zero exit (which is the spectrum-ts1.10.0..1.11.1failure mode and also the new1.11.2provenance-verification failure mode —bunx clean-publishprintednpm error code E422and still exited 0).How it works
Both publish steps (OIDC primary, NPM token fallback) now:
tee(still streams live to the GitHub Actions log).${PIPESTATUS[0]}instead of the pipeline's$?(which is always 0 because oftee).^npm (error|ERR!)even if the publisher exited 0.The OIDC branch additionally 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.
Real evidence this is needed
The v1.11.2 spectrum-ts publish run shows the exact pathology:
That
✅ Publishedcame from the un-hardened code onmainafter this PR's parent (#82) was merged without the hardening. With this PR, the same input would produce:Layer 2 (registry verify) catches it eventually, but at the cost of a misleading
✅first. Layer 1 fails at the right step with the right message.Why both layers
Layer 1 fails fast at the source — operators see "Publisher reported success but npm emitted error lines" inline with the publish output, no ambiguity about which step is wrong. Layer 2 is the publisher-agnostic canary that asserts the only thing that actually matters: did the bytes land on the registry. If 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.
Backward compatibility
dry-run: trueis unaffected (publish-step hardening is a no-op on dry-run output unless packaging itself is broken, which is exactly when we want to surface it)Test plan
npm error: publish step fails red on the grep check (this is the spectrum-ts 1.11.2 case)✅ Published via OIDC Trusted Publishing.followed by✅ <name>@<version> is live on the npm registry.Notes for reviewers
The cherry-pick is clean against current
mainsince #82 only landed Layer 2. After this merges,publish-npmwill have the full two-layer defense as described in #82's body.Made with Cursor
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.