feat(release): unify Desktop and CLI product releases - #3222
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review. 📝 WalkthroughWhat this PR solvesThis PR creates one release path for Desktop and standalone macOS arm64 CLI/TUI products. It uses the root npm remains an optional staged channel. It does not create another tag or GitHub Release. Source of truth and solution scopeThe PR extends the existing product release source of truth. It does not create a parallel product release path. The root product version, product tag, source commit, release identity utility, manifest-based packaging policies, and single Draft GitHub Release now control Desktop, CLI/TUI, source, and npm artifacts. The separate Desktop workflow and CLI-specific tag and release behavior were removed. The solution is the smallest coherent path shown by the current diff. The added workflow, packaging, validation, signing, notarization, recovery, and test logic supports immutable source selection, workspace dependency closure, native artifact filtering, npm staging, and Draft Release safety. Simplification opportunitiesNo safe deletion is evident from the supplied change summary. The PR removes duplicated behavior by deleting the Desktop-only workflow, CLI-specific release metadata, the The packaging and verification scripts are large. The current evidence does not show that further deletion or simplification would preserve behavior and regression coverage. Concrete risks and validationKey risks include:
Reported validation covered release checks, formatting, workflow linting, CLI packaging, CLI smoke tests, workspace tests, the Desktop build, and a packaged macOS arm64 CLI/TUI ZIP. Developer ID signing, notarization, protected-environment approval, npm staging, and browser-download acceptance were not run locally because they require release credentials or human operators. Required-check status remains unverified without direct check results. Complexity deltaAdded
Removed
The PR adds implementation and test-maintenance complexity. It removes duplicated release authorities and parallel release states. Total maintenance complexity decreases at the release-model level. The added complexity is justified by the standalone artifact, security, provenance, and recovery requirements. Optional follow-up simplifications are non-blocking. Review-relevant risksThe diff affects protected areas:
Material changes in these areas require independent human review under repository policy. The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughThe release system now uses one product identity and source commit for desktop, CLI/TUI, npm, and source artifacts. It adds macOS arm64 packaging and verification, unified draft-release creation, product-tagged npm staging, and updated CLI distribution contracts. ChangesProduct release pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to The unified release changes the publication and acceptance contract, but the current checklist still omits verification of the required Windows Desktop ZIP. Merge should wait for that checklist correction or explicit owner acceptance because a required download artifact could otherwise go unverified. Sequence Diagram(s)sequenceDiagram
participant ReleaseDispatcher
participant ReleaseIdentity
participant ArtifactJobs
participant ArtifactVerifier
participant GitHubDraftRelease
ReleaseDispatcher->>ReleaseIdentity: resolve product version and source commit
ReleaseIdentity->>ArtifactJobs: provide tag, commit, and toolchain metadata
ArtifactJobs->>ArtifactVerifier: provide desktop, CLI/TUI, and source artifacts
ArtifactVerifier->>GitHubDraftRelease: upload verified assets and checksums
GitHubDraftRelease-->>ReleaseDispatcher: create or update draft release
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
5789994 to
55e7055
Compare
PR Summary by QodoUnify Desktop, standalone CLI, and npm release identity
AI Description
Diagram
High-Level Assessment
Files changed (35)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
scripts/verify-macos-arm64-cli.mjs (1)
207-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
realpathat the top of the file and delete the inline dynamic imports.Lines 216-219 and Lines 342-347 call
import('node:fs/promises')four times only to obtainrealpath. The file already imports named members fromnode:fs/promisesat Line 4-14. Addingrealpaththere removes four dynamic imports with no behavior change.Disposition: optional.
♻️ Proposed simplification
readFile, readdir, + realpath, rm,- const [resolvedLink, resolvedPackage] = await Promise.all([ - import('node:fs/promises').then(({ realpath }) => realpath(linkPath)), - import('node:fs/promises').then(({ realpath }) => realpath(packagePath)), - ]); + const [resolvedLink, resolvedPackage] = await Promise.all([ + realpath(linkPath), + realpath(packagePath), + ]);As per path instructions: "Flag concrete cases where code can be deleted or simplified."
Source: Path instructions
scripts/product-release.test.mjs (2)
206-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis assertion is a change detector for the Eval manifest.
workspaceReleaseFilesreads the declaration frompackages/eval/package.json. The test then compares it against a hand-copied duplicate of that same list. Any harbor asset added or renamed breaks this test even though no behavior changed, and the fix is always to re-copy the manifest.The stated contract is "the Eval workspace owns the complete runtime asset declaration". Assert that property instead:
distis present, every entry is a real path under the workspace, and no development directory leaks in. That still fails when the declaration is wrong and stops failing when the declaration merely grows.As per path instructions: "Flag tests that duplicate existing coverage, assert implementation details, or do not protect observable behavior."
Source: Path instructions
200-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLine 201 pins shell syntax, not the relocatability contract.
/while \[ -L "\$launcher" \]/ufails if the wrapper resolves symlinks with any other correct construct. The two following assertions are different: they pin the embedded runtime path and the CLI entry, which are contract. The symlink behavior itself is already covered by the acceptance step in.github/RELEASE_CHECKLIST.mdline 72 and byverify-macos-arm64-cli.mjs.Consider dropping Line 201 or replacing it with an assertion that the wrapper never references an absolute build-time path.
As per path instructions: "Flag tests that duplicate existing coverage, assert implementation details, or do not protect observable behavior."
Source: Path instructions
.github/workflows/release-cli-stage.yml (1)
40-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe commit assertion at Line 52 cannot fail.
git rev-list -n 1 FETCH_HEADandgit rev-parse "FETCH_HEAD^{commit}"both peel to the same commit. The comparison is always true, so it adds no protection against an annotated tag or a non-commit target.Delete the redundant line, or replace it with a check that actually constrains the target.
As per path instructions: "Flag concrete cases where code can be deleted or simplified."
♻️ Proposed simplification
git fetch --no-tags origin "refs/tags/$PRODUCT_TAG" source_commit="$(git rev-list -n 1 FETCH_HEAD)" - test "$(git rev-parse "FETCH_HEAD^{commit}")" = "$source_commit" echo "source_commit=$source_commit" >> "$GITHUB_OUTPUT"Source: Path instructions
scripts/release-cli-publication.test.mjs (1)
82-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative coverage for the new
productTagguard.
prepareStageReleasenow rejects aproductTagthat does not equalv<version>. The tests only exercise the matching value. The drift tests cover version confirmation and checksum drift, but not tag drift.Add one assertion that a mismatched
productTagthrows.scripts/release-cli-workflow-policy.test.mjs (1)
62-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLine 63 duplicates the invariant already covered by Line 62.
Line 62 asserts the workflow delegates version validation to
scripts/release-version.mjs. Line 63 additionally asserts that no inline bash regex comparesEXPECTED_VERSION. That second assertion matches on incidental text shape and breaks if the step is reformatted, without protecting extra observable behavior.Consider removing Line 63.
As per path instructions: "Flag tests that duplicate existing coverage, assert implementation details, or do not protect observable behavior."
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 53dae729-1ce9-4711-9e7c-6a49a45fa7e9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
.github/RELEASE_CHECKLIST.md.github/workflows/ci.yml.github/workflows/cli-package-validation.yml.github/workflows/release-cli-finalize.yml.github/workflows/release-cli-stage.yml.github/workflows/release-desktop.yml.github/workflows/release.ymldocs/README.mddocs/cli-distribution.mddocs/cli-npm-release.mddocs/cli-npm-release.zh-CN.mddocs/runtime-host-remote-access.mddocs/runtime-host-remote-access.zh-CN.mdpackage.jsonpackages/cli/README.mdpackages/cli/README.zh-CN.mdpackages/cli/package.jsonpackages/cli/src/__tests__/cli.test.tspackages/cli/src/cli-core.tspackages/eval/package.jsonscripts/ci-test-plan.mjsscripts/package-macos-arm64-cli.mjsscripts/product-release-identity.mjsscripts/product-release-tag.mjsscripts/product-release.test.mjsscripts/release-cli-file-policy.mjsscripts/release-cli-file-policy.test.mjsscripts/release-cli-package.mjsscripts/release-cli-publication.mjsscripts/release-cli-publication.test.mjsscripts/release-cli-workflow-policy.test.mjsscripts/release-eval-smoke-sitecustomize.pyscripts/release-version.mjsscripts/smoke-release-cli-package.mjsscripts/verify-macos-arm64-cli.mjs
💤 Files with no reviewable changes (3)
- scripts/smoke-release-cli-package.mjs
- packages/cli/src/cli-core.ts
- .github/workflows/release-desktop.yml
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
f7e8ac9 to
caa339a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 28261e16-0e19-4033-9efb-a3ccd733d89c
📒 Files selected for processing (18)
.github/RELEASE_CHECKLIST.md.github/workflows/release-cli-finalize.yml.github/workflows/release-cli-stage.yml.github/workflows/release.ymldocs/cli-distribution.mddocs/cli-npm-release.mddocs/cli-npm-release.zh-CN.mdscripts/package-macos-arm64-cli.mjsscripts/product-release-identity.mjsscripts/product-release.test.mjsscripts/release-checksum.mjsscripts/release-cli-file-policy.mjsscripts/release-cli-file-policy.test.mjsscripts/release-cli-package.mjsscripts/release-cli-publication.mjsscripts/release-cli-publication.test.mjsscripts/release-cli-workflow-policy.test.mjsscripts/verify-macos-arm64-cli.mjs
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review.
3ff0afb to
a395694
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
package.json (1)
55-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
test:product-releaseincheck:release.Line 55 and Line 58 both define the
scripts/product-release.test.mjsinvocation. Usenpm run test:product-releaseincheck:releaseso the test command has one source of truth.Disposition: optional.
As per path instructions, consolidate duplicated authority and prefer the closest existing seam.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3468f886-510c-49cc-b6e1-d05de8143930
📒 Files selected for processing (5)
.github/RELEASE_CHECKLIST.mddocs/cli-npm-release.mddocs/cli-npm-release.zh-CN.mdpackage.jsonscripts/smoke-release-cli-package.mjs
Included review availability: Your plan provides up to 3 included reviews per hour; 1 remains after this review.
ba1f50d to
0275809
Compare
22bc643 to
0c0e939
Compare
|
@coderabbitai review |
|
/agentic_review |
0c0e939 to
929f2a1
Compare
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Require product artifacts to come from the exact approved ASF source tag, and keep canonical repository identity across release metadata. Preserve Node's reviewed runtime entitlements during signing, pin the Apple signing team, and route every release authority change through its dedicated CI contract gate. Generated-by: Codex
Derive Desktop Runtime Host setup metadata from the shared product manifests and verify it in packaged applications. Re-sign the standalone Node runtime without its development-only entitlement while retaining exact upstream validation. Route direct release inputs through the contract gate and document the npm-before-publication acceptance order.
Keep packaged Desktop updates bound to apache/maka and preserve electron-builder metadata as the production feed authority.\n\nShare the product SemVer contract across npm and Windows release verification, including prerelease candidates, while removing duplicate artifact and toolchain identities.
Derive GitHub release classification from the shared product version and preserve electron-updater's version-derived channel policy. Reuse the platform verifiers as checksum owners and remove the stale macOS toolchain reference so packaging cannot diverge from the resolved release identity.
Keep every GitHub Release non-Latest while it remains a Draft, and require the exact live Draft state before npm staging or finalization. Centralize tag, ancestry, and release-classification checks while removing the duplicate Stage-run precheck.
Keep behavior coverage at its owning release boundary while removing duplicate channel and CI-routing checks. Drop historical negative assertions that only preserved superseded implementation shapes.
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
0404d02 to
5d77994
Compare
The immutable CLI package must point users at the canonical ASF repository. The Stage workflow already binds the product tag and commit through the live Draft authority, so retain only the independent version confirmation and remove the unused checksum output contract.
2 participants
Summary
main: rootpackage.jsonis the sole version authority, while Desktop, the standalone macOS arm64 CLI/TUI, and bundled source are built from the exact ASF source candidate tag approved by both the podling and Incubator PMC votes. One conveniencev<version>tag, release gate, and Draft GitHub Release cover those product artifacts.v<version>-incubating-rc<rc>tag inapache/maka, verifies its commit andmainancestry before building, and revalidates the live tag immediately before creating the convenience tag or Draft. An exact 15-file manifest is now shared by producers and publication: missing, extra, non-regular, conflicting, or byte-different assets fail closed. Interrupted Draft uploads keep byte-identical assets and add only missing assets.releaseFiles, development-file pruning, notices, current Eval assets, dependency patches, and caller-independent npm configuration. npm retains its OIDC staged-publishing and 2FA boundaries as an optional installation channel, without another tag or Release.makacommand, Developer ID/notarization hooks,DISCLAIMER-WIP, checksums, and end-to-end verification. The wrapper resolves one canonicallibexecroot for runtime and Eval assets. Signing failures redact credentials and clean temporary material even when keychain deletion fails..asf.yaml: immutablev*tag rules, RC-onlyreleaseenvironment tags, product-tag-onlynpm-releaseenvironment tags, required reviewerM4n5ter, and prevention of self-review. This supersedes feat(release): add synchronized macOS arm64 CLI artifact #3002 and incorporates or explicitly adjudicates every unresolved review thread there.Fixes #1510
Verification
npm cinpm run buildnpm run check:release— 70 focused release/publication contracts plus stale-dist and notice checksnode --test scripts/ci-test-plan.test.mjs scripts/release-cli-workflow-policy.test.mjs— 30 passedactionlint .github/workflows/ci.yml .github/workflows/release.yml .github/workflows/release-cli-stage.yml .github/workflows/release-cli-finalize.yml .github/workflows/cli-package-validation.ymlgit diff --checkRelease review required
.asf.yamlis the declarative authority for the tag ruleset and environment policies, but those controls can only be reconciled after merge. Verify the livereleaseandnpm-releaseenvironments and thev*tag ruleset before the first release.v0.1.11already exists. A future release preparation must bump root, Desktop, and CLI manifests together to one unused product version before preparing and voting on the ASF source candidate.AI use
Select exactly one:
Tool(s) and scope: Codex reconstructed the release architecture from current
mainand public decisions, implemented code/workflows/tests/docs, ran local verification, and prepared this PR. Claude provided an adversarial review; Codex independently adjudicated its findings. Material commits carryGenerated-by: Codextrailers.Checklist
Does this PR entail a change in behavior?