Skip to content

fix(ci): propagate per-package publish failures in ci:publish - #499

Open
karim-en wants to merge 1 commit into
mainfrom
fix/ci-publish-propagate-errors
Open

fix(ci): propagate per-package publish failures in ci:publish#499
karim-en wants to merge 1 commit into
mainfrom
fix/ci-publish-propagate-errors

Conversation

@karim-en

Copy link
Copy Markdown
Collaborator

Summary

The ci:publish script iterates over packages/* and publishes each in a subshell. Previously, per-package failures were silently swallowed because the subshell's exit code was not propagated out of the for loop — the step's exit code was determined by changeset tag at the very end.

The last release (Version Packages #498) showed this: @omni-bridge/aptos@0.16.0, @omni-bridge/hypercore@0.16.0, and @omni-bridge/sdk@0.16.0 all got 404 Not Found - PUT from npm (OIDC trusted publisher not configured for these new package names), yet the workflow reported success.

Change

Add || exit 1 after the per-package subshell so the first failing publish aborts the loop and the workflow fails visibly.

Test plan

  • Trigger a release with a misconfigured package and confirm the workflow fails on the failing package rather than silently marching on

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@karim-en
karim-en requested a review from a team July 27, 2026 11:36
@karim-en
karim-en requested a review from a team as a code owner July 27, 2026 11:36
@changeset-bot

changeset-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 2f54932

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Greptile Summary

The PR updates the ci:publish script to stop immediately when a per-package publish subshell fails, preventing later commands from masking npm publication failures.

Confidence Score: 5/5

The PR appears safe to merge.

The added failure branch correctly terminates the publish script when packing, publishing, cleanup, or package metadata processing fails, while successful and already-published package paths continue unchanged.

Reviews (1): Last reviewed commit: "fix(ci): propagate per-package publish f..." | Re-trigger Greptile

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Pull request overview

This PR fixes a silent-failure bug in the root ci:publish script. The script loops over packages/* publishing each in a subshell, but the subshell's exit code was discarded by the for loop, so the step's success was determined solely by the trailing changeset tag. Per-package npm publish failures (e.g. the 404 OIDC errors on the new aptos/hypercore/sdk packages in Version Packages #498) were swallowed and the workflow reported success. The fix appends || exit 1 so the first failing package aborts the run and fails the step visibly.

Changes:

  • package.json: add || exit 1 after the per-package publish subshell in ci:publish.

Reviewed changes

Per-file summary
File Description
package.json Propagate per-package publish subshell failures out of the for loop so ci:publish fails visibly instead of only reporting the exit code of the trailing changeset tag.

Findings

I traced the exit-code semantics for each path and the fix is correct:

  • already-publishednpm view exits 0, then/echo exits 0, subshell exits 0, || exit 1 not triggered.
  • publish succeedselse chain pack && publish && rm exits 0, subshell exits 0.
  • publish failselse chain exits non-zero, subshell exits non-zero, || exit 1 fires and terminates the top-level shell (the for loop itself is not in a subshell), so changeset tag is skipped and the step fails.

It also correctly stops changeset tag from creating git tags for a partially-published release, which the old behavior masked. No changeset is needed since the root package.json is private: true.

Non-blocking (suggestion):

  • package.json:19 — Fail-fast means a persistently-broken package early in the glob order blocks every later package from publishing. Glob order is alphabetical (aptos, btc, core, evm, hypercore, near, sdk, solana, starknet), so aptos runs first — and aptos is exactly one of the packages that failed with 404 in the incident this PR cites. If its OIDC trusted-publisher config isn't fixed, this change would abort before core/evm/near/etc. ever publish, even when those have valid version bumps and working OIDC. That's arguably still preferable to silent success, but an accumulate-and-fail-at-end pattern would publish all healthy packages while still failing the workflow, e.g.:
    "ci:publish": "bun run build && failed=0; for dir in packages/*; do (cd \"$dir\" && PKG_NAME=$(jq -r .name package.json) && PKG_VERSION=$(jq -r .version package.json) && if npm view \"$PKG_NAME@$PKG_VERSION\" version 2>/dev/null; then echo \"$PKG_NAME@$PKG_VERSION already published, skipping\"; else bun pm pack && npm publish *.tgz --access public && rm *.tgz; fi) || failed=1; done; [ \"$failed\" -eq 0 ] && changeset tag || exit 1",
    
    Your call — the current fail-fast approach is a legitimate choice and matches the PR's stated intent.

✅ Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant