Make 0.5.0 a coordinated, verifiable package release#90
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (27)
📝 WalkthroughWalkthroughThe release process derives and validates a coordinated public package set, verifies packed CLI artifacts, publishes through ChangesCoordinated release
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseOperator
participant publish_local_mjs
participant npmRegistry
participant check_cli_package_mjs
ReleaseOperator->>publish_local_mjs: run release using computed package plan
publish_local_mjs->>npmRegistry: inspect versions and dist-tags
npmRegistry-->>publish_local_mjs: return package publication state
ReleaseOperator->>check_cli_package_mjs: verify packed packages in isolated install
check_cli_package_mjs->>check_cli_package_mjs: run opentag doctor validation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request prepares the OpenTag package family for the v0.5.0 release, which introduces Discord, Linear, and Microsoft Teams adapters, durable Attempt leases, and a dynamic, topologically sorted release package plan. The hardcoded package lists in the release scripts are replaced with automated discovery and validation logic, accompanied by new unit tests. A high-severity issue was identified in check-cli-package.mjs where calling process.exit() on command failures bypasses the finally block, preventing the cleanup of temporary directories; throwing an error instead is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (result.status !== 0 && !options.allowFailure) { | ||
| process.exit(result.status ?? 1); | ||
| } |
There was a problem hiding this comment.
In check-cli-package.mjs, the main execution block is wrapped in a try...finally block to ensure that the temporary directory tempRoot is cleaned up. However, the run helper function calls process.exit() directly when a command fails. Since process.exit() immediately terminates the Node.js process without unwinding the stack, the finally block is bypassed, leaving the temporary directory and its contents (including packed tarballs and installed node_modules) on the disk.
To ensure proper cleanup on failure, the run function should throw an Error instead of calling process.exit(). This allows the exception to propagate, triggering the finally block for cleanup before the process exits.
| if (result.status !== 0 && !options.allowFailure) { | |
| process.exit(result.status ?? 1); | |
| } | |
| if (result.status !== 0 && !options.allowFailure) { | |
| throw new Error("Command failed with exit code " + (result.status ?? 1) + ": " + command + " " + args.join(" ")); | |
| } |
There was a problem hiding this comment.
Fixed in fc00a91: command failures now throw, so the enclosing finally block always removes the temporary pack/install directory. The full release:check passed after the change.
7f70b1b to
f9f8cbc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@scripts/release/publish-local.mjs`:
- Around line 119-133: Update publishedVersionForTag() to return stdout only
when the npm command succeeds and produces a non-empty value, otherwise
normalize it to undefined; when result.ok is false, surface the raw npm error
instead of silently returning undefined. Preserve the existing tag lookup
behavior while ensuring unset dist-tags do not produce misleading output or
release decisions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ac9a137-2b88-4c91-afb8-e827a6f4bc05
📒 Files selected for processing (27)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdREADME.zh-CN.mddocs/npm-release.mddocs/versioning.mdpackage.jsonpackages/cli/package.jsonpackages/cli/test/release-package-plan.test.tspackages/client/package.jsonpackages/core/package.jsonpackages/discord/package.jsonpackages/dispatcher/package.jsonpackages/github/package.jsonpackages/gitlab/package.jsonpackages/lark/package.jsonpackages/linear/package.jsonpackages/local-runtime/package.jsonpackages/runner/package.jsonpackages/slack/package.jsonpackages/store/package.jsonpackages/teams/package.jsonpackages/telegram/package.jsonscripts/release/check-cli-package.mjsscripts/release/check-publication-set.mjsscripts/release/package-plan.mjsscripts/release/publish-local.mjs
f9f8cbc to
9636df0
Compare
OpenTag's CLI installation closes over every public workspace package, so the 0.5.0 family must be released atomically from one discovered dependency graph. The release gate now validates that graph, exercises the packed CLI in a clean npm project, and records the migration and staged rollout contract. Constraint: The CLI runtime dependency closure requires all 15 public packages at one lockstep version. Rejected: Maintain another hand-written package list | the existing check and publish lists had already drifted from the manifests. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Treat publishConfig.access=public as the sole release-set source and promote immutable versions with dist-tags only. Tested: build, lint, typecheck, 1510 unit/integration tests, governance matrix, privacy scan, 9 release-plan tests, publication-set checks, 15-package pack and clean npm install, installed CLI help and doctor. Not-tested: Public-registry install, foreground start, live provider callback, and npm dist-tag promotion remain post-publish gates.
9636df0 to
fc00a91
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/npm-release.md (1)
131-140: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFailed version assertion is silent — loop won't stop or report on a broken canary.
test "$(npm view "$package@0.5.0" version)" = "0.5.0"produces no output on failure and the loop keeps iterating over the remaining packages. An operator following this doc could miss a package that failed to publish correctly and proceed to promote a broken release tolatest.Proposed fix to fail loudly on mismatch
for manifest in packages/*/package.json; do [ "$(jq -r '.publishConfig.access // ""' "$manifest")" = "public" ] || continue package="$(jq -r '.name' "$manifest")" - test "$(npm view "$package@0.5.0" version)" = "0.5.0" + actual="$(npm view "$package@0.5.0" version)" + [ "$actual" = "0.5.0" ] || { echo "FAIL: $package resolved to '$actual', expected 0.5.0"; exit 1; } npm view "$package" dist-tags --json done🤖 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 `@docs/npm-release.md` around lines 131 - 140, Update the verification loop around the version assertion to fail loudly when a package’s canary version does not match 0.5.0, including the package name and expected/actual versions in the error output, and stop the loop with a nonzero status so promotion cannot continue unnoticed.packages/cli/test/release-package-plan.test.ts (1)
159-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding tests for malformed
publishConfigand runtime dependency field types.The existing malformed manifest test covers invalid JSON, but
readPackageManifestinpackage-plan.mjsalso validates thatpublishConfigand each runtime dependency field are objects, throwing structured errors. Adding test cases for non-objectpublishConfig(e.g.,publishConfig: "invalid") or non-objectdependencies(e.g.,dependencies: []) would cover these validation paths and prevent regressions.🤖 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 `@packages/cli/test/release-package-plan.test.ts` around lines 159 - 166, Add test coverage in the release-package-plan tests for readPackageManifest validation of non-object publishConfig and runtime dependency fields, such as string publishConfig and array dependencies. Create valid JSON manifests with these malformed values, assert buildPublicPackagePlan throws structured errors identifying the relevant field/package directory, and keep the existing invalid-JSON test unchanged.
🤖 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 `@docs/npm-release.md`:
- Around line 131-140: Update the verification loop around the version assertion
to fail loudly when a package’s canary version does not match 0.5.0, including
the package name and expected/actual versions in the error output, and stop the
loop with a nonzero status so promotion cannot continue unnoticed.
In `@packages/cli/test/release-package-plan.test.ts`:
- Around line 159-166: Add test coverage in the release-package-plan tests for
readPackageManifest validation of non-object publishConfig and runtime
dependency fields, such as string publishConfig and array dependencies. Create
valid JSON manifests with these malformed values, assert buildPublicPackagePlan
throws structured errors identifying the relevant field/package directory, and
keep the existing invalid-JSON test unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c66f0a1-2043-47ab-bd1a-37c37096ae3e
📒 Files selected for processing (27)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdREADME.zh-CN.mddocs/npm-release.mddocs/versioning.mdpackage.jsonpackages/cli/package.jsonpackages/cli/test/release-package-plan.test.tspackages/client/package.jsonpackages/core/package.jsonpackages/discord/package.jsonpackages/dispatcher/package.jsonpackages/github/package.jsonpackages/gitlab/package.jsonpackages/lark/package.jsonpackages/linear/package.jsonpackages/local-runtime/package.jsonpackages/runner/package.jsonpackages/slack/package.jsonpackages/store/package.jsonpackages/teams/package.jsonpackages/telegram/package.jsonscripts/release/check-cli-package.mjsscripts/release/check-publication-set.mjsscripts/release/package-plan.mjsscripts/release/publish-local.mjs
🚧 Files skipped from review as they are similar to previous changes (21)
- packages/gitlab/package.json
- packages/client/package.json
- packages/slack/package.json
- README.md
- packages/store/package.json
- packages/cli/package.json
- packages/telegram/package.json
- packages/discord/package.json
- packages/github/package.json
- package.json
- packages/dispatcher/package.json
- packages/linear/package.json
- packages/local-runtime/package.json
- scripts/release/check-publication-set.mjs
- packages/lark/package.json
- packages/core/package.json
- packages/runner/package.json
- .github/workflows/ci.yml
- scripts/release/package-plan.mjs
- scripts/release/check-cli-package.mjs
- scripts/release/publish-local.mjs
1 participant
Summary
@opentag/*packages to the coordinated0.5.0versionnextthe safe publish default, require real publication frommain, and fail closed on empty publication sets or mismatched recovery dist-tagsrelease:checkgates to CInext→ registry smoke →latest→ matching git tag/GitHub Release procedure and rollback pathVerification
pnpm install --frozen-lockfilepnpm release:publication-set— 15 packages, all 0.5.0pnpm buildpnpm lintpnpm typecheckpnpm test— 119 files, 1510 testspnpm smoke:governance -- --all— 7/7 cases passedpnpm release:check— packed all 15 packages, clean npm install, installed CLI help/doctor checksrelease:publish --dry-run --skip-check --tag next— all npm tarball previews passedRollout after merge
mainto npm dist-tagnext@opentag/cli@0.5.0from the public registry and run setup/doctor/start plus a credentialed Lark smokelatestv0.5.0and publish the matching GitHub Release from the exact release commitlatestremains untouched until the registry and live-platform gates pass.Summary by CodeRabbit
New Features
Migration / Breaking Changes
Documentation
latest.Quality Improvements