Skip to content

chore(release): scripts/release.sh - #171

Merged
krassx merged 4 commits into
mainfrom
chore/release-script
Oct 30, 2026
Merged

krassx merged 4 commits into
mainfrom
chore/release-script

Conversation

@krassx

@krassx krassx commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

One script for the release steps: preflight (main, clean, CI green), first-publish of packages never on npm (the only step needing npm auth/2FA), Release workflow dry run then real, registry verification, git tag and GitHub release. Resumable. Motivated by beta.4, where the new @bugsee/trackers could not be first-published through OIDC.

🤖 Generated with Claude Code

…ish, verify and tag

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f6a75e54-78da-4e5d-bed0-8d7248df88a7
Commits

Reviewing files that changed from the base of the PR and between 633dc81 and 5fecae6.

Files selected for processing: 4
  • docs/dev-environment.md
  • scripts/npm-trust-all.mjs
  • scripts/publish-all.mjs
  • scripts/release.sh
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 1m 5s —— View job


Review of PR #171: scripts/release.sh

I read scripts/release.sh against publish-all.mjs, npm-trust-all.mjs, check-publishable.mjs and release.yml. I did not run the script and no tests were run. I posted five inline comments and made no changes to files.

Overall risk: Medium. The script is tooling, not shipped code, and it covers the right flow. Two problems affect its main purpose.

Merge recommendation: Fix the P1 and the P2s, then merge. The P1 is likely to break the bootstrap step on first use.

Findings to fix:

  1. P1, bootstrap publish cannot take 2FA (release.sh:104). publish-all.mjs spawns npm publish with stdin ignored and stderr captured, so an interactive OTP or web-auth URL is not possible. The prompt on line 99 says you will be asked for 2FA. Per publish-local.sh, this path needs a bypass-2FA token or NODE_AUTH_TOKEN.
  2. P2, resume skips the trust attach (release.sh:93). The trust step only runs when a package is missing from npm. If it fails after a successful first publish, a re-run skips it and the package stays untrusted. The next release then fails for that package.
  3. P2, dispatched commit can differ from the tagged commit (release.sh:48). The workflow runs against main at dispatch time, but the tag is on the preflight $sha. The run lookup also doesn't filter by event or headSha.
  4. P3, tail steps are not resumable (release.sh:80). A failure after the tag push makes the re-run die on "tag already exists". A prerelease version can also go out under latest unnoticed.
  5. P3, --help and npm whoami (release.sh:29). --help prints two code lines, and an npm login is required even when no bootstrap is needed.

Positives:

  • Preflight is thorough: branch, clean tree, origin/main match, CI on the exact commit, and the publishable gate.
  • The tag is checked before anything is published.
  • The registry verify tolerates delayed visibility after the 202 response.
  • Workflow inputs are passed safely.
  • The dry run before the real publish, with a confirm prompt, is a good safeguard.

Comment thread scripts/release.sh Resolved
Comment thread scripts/release.sh Resolved
Comment thread scripts/release.sh Resolved
Comment thread scripts/release.sh Resolved
Comment thread scripts/release.sh Resolved
… pinned dispatch commit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 1m 8s —— View job


Re-review of PR #171 after b57aaea7

I re-read scripts/release.sh, publish-all.mjs and npm-trust-all.mjs at the new head. I did not run the release flow against npm or GitHub. I did run one small bash check: the todo merge loop (release.sh:99-103) does not trip set -e. No files were changed.

Earlier findings:

# Finding Status
1 P1: bootstrap publish could not take 2FA Fixed. --interactive inherits stdio and the prompt text is accurate. The cost is the new P3 below.
2 P2: resume skipped the trust attach Fixed. The release-trust-pending marker in the git dir is kept until npm-trust-all.mjs --only succeeds.
3 P2: dispatched commit could differ from the tagged commit Mostly fixed. release.sh:49-50 checks origin/main == $sha right before dispatch, and the run lookup filters on event and --commit. A window of a few milliseconds remains because --ref main is still a branch name. That is not worth more code.
4 P3: tail steps not resumable Fixed. The tag, push and GitHub release are each skipped when already done. Still open: nothing stops a prerelease version like 0.1.0-beta.5 from going out under the default --tag latest. Given beta.4 was the motivation, a guard that dies when $version contains - and TAG=latest would be cheap.
5 P3: --help range and npm whoami Fixed. The help range is 2,17p, and npm whoami now only runs when a bootstrap is needed.

New finding (1 inline comment):

  • P3, --interactive loses the "accepted but not readable yet" skip (scripts/publish-all.mjs:105). With stdio: 'inherit' the thrown error has no stderr, so the "cannot publish over the previously published versions" branch can never match. A re-run shortly after an interrupted bootstrap hits E403 on the already-accepted package and aborts until the registry catches up.

Overall risk: Low. This is release tooling, and the earlier P1 and P2 issues are resolved.

Merge recommendation: Merge. Optionally add the prerelease-under-latest guard and handle the P3 above in this PR or a follow-up.

Most important remaining items:

  1. The prerelease-under-latest guard in release.sh.
  2. The interactive publish error handling in publish-all.mjs.

Positives:

  • The marker-file approach for resuming the trust step is simple and survives crashes.
  • The registry wait loops tolerate the 202-then-readable delay.
  • Every tail step is idempotent.
  • --only on both helper scripts keeps the bootstrap narrow.

Comment thread scripts/publish-all.mjs Resolved
…-but-unreadable package

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 1m 14s —— View job


Re-review of PR #171 after b63a55e1

I read scripts/release.sh, publish-all.mjs, npm-trust-all.mjs and release.yml at the new head. I did not run the release flow against npm or GitHub, and no tests were run. No files were changed.

Earlier findings

Finding Status
Bootstrap publish couldn't take 2FA Fixed. --interactive inherits stdio.
Resume skipped the trust attach Fixed. The release-trust-pending marker is removed only after npm-trust-all.mjs --only succeeds.
Dispatched commit could differ from the tagged commit Fixed. origin/main is re-checked before each dispatch, and the run is matched by event and --commit.
Tail steps not resumable Fixed. The tag, push and GitHub release are each skipped when already done.
--help range and npm whoami Fixed.
--interactive loses the "accepted but not readable" skip Acknowledged. It can't be captured with an inherited terminal. The new message in b63a55e1 tells the user to wait and re-run, which is enough.
Prerelease version under --tag latest Still open, P3. Nothing stops 0.1.0-beta.5 from going out under the default latest. A guard that dies when $version contains - and TAG=latest would be cheap.

New finding (1 inline comment)

  • P3, lost failure message (scripts/release.sh:62). If gh run view --log-failed itself exits non-zero, set -e and pipefail stop the script before die runs, so "Release workflow run … failed (dry_run=…)" is never printed. Appending || true to the log dump fixes it.

Overall risk: Low. This is release tooling. I found no new correctness or security issues beyond the P3 above.

Merge recommendation: Merge. Optionally add the prerelease-under-latest guard and the || true before merging.

Most important remaining items

  1. The prerelease-under-latest guard.
  2. || true on the --log-failed dump.

Positives

  • The dispatch is pinned to the preflight commit and refused if main moves.
  • The run is matched by event and commit, so another person's run is never watched.
  • The marker file keeps the trust step resumable, and every tail step is idempotent.
  • Registry waits tolerate the 202-then-readable delay.
  • --only on both helper scripts keeps the bootstrap narrow.

Comment thread scripts/release.sh Resolved
… prints

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 25s —— View job


Re-review of PR #171 after 5fecae6c

I re-read scripts/release.sh, the publish-all.mjs and npm-trust-all.mjs diffs, and the release.yml inputs at the new head. I did not run the release flow against npm or GitHub, and no tests were run. No files were changed.

Earlier findings

Finding Status
Bootstrap publish couldn't take 2FA Fixed. --interactive inherits stdio.
Resume skipped the trust attach Fixed. The release-trust-pending marker is removed only after npm-trust-all.mjs --only succeeds.
Dispatched commit could differ from the tagged commit Fixed. origin/main is re-checked before each dispatch, and the run is matched by event and --commit.
Tail steps not resumable Fixed. The tag, push and GitHub release are each skipped when already done.
--help range and npm whoami Fixed.
Interactive publish loses the "accepted but not readable" skip Acknowledged. The new message tells the user to wait and re-run.
Lost failure message from --log-failed Fixed in 5fecae6c (2>&1 | tail -30 || true, release.sh:62). die now always runs.
Prerelease version under --tag latest Open, P3. It is optional, and I am not re-posting it inline. release.yml also defaults tag to latest, so publishing betas under latest may be intentional while no stable release exists. If it isn't, a guard that dies when $version contains - and TAG=latest (unless --yes) is a few lines.

New findings: none that I can support with a concrete failure scenario.

Overall risk: Low. This is release tooling only, and every earlier correctness issue is resolved.

Merge recommendation: Merge. The prerelease-under-latest guard is optional.

Most important remaining item

  1. The optional prerelease-under-latest guard in release.sh.

Positives

  • Preflight is thorough: branch, clean tree, origin/main match, CI on the exact commit, and the publishable gate.
  • The dispatch is pinned to the preflight commit and refused if main moves.
  • The marker file keeps the trust step resumable.
  • Every tail step is idempotent.
  • Registry waits tolerate the 202-then-readable delay.
  • --only on both helper scripts keeps the bootstrap narrow.

@krassx
krassx merged commit 063d040 into main Oct 30, 2026
8 checks passed
@krassx
krassx deleted the chore/release-script branch October 30, 2026 08:12
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