feat(release): distribute standalone macOS arm64 CLI - #1823
Conversation
# Conflicts: # scripts/macos-arm64-release.test.mjs
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The shape here is right. Materializing the real dependency tree with npm ci instead of bundling sidesteps both the dynamic import() calls and require-addon's runtime resolution, which is where an esbuild/SEA/single-binary approach would have broken. I verified the happy path end to end on Apple Silicon with official Node 24: symlink-relative resolution survives the rename into libexec/, the version read via realpath lands on the rewritten manifest, node-pty spawns under the embedded Node, and spawn-helper keeps its +x bit despite --ignore-scripts.
Everything marked "reproduced" below I ran locally.
P1: the artifact crashes on the documented download path
All three shipped .node files are flags=0x20002(adhoc,linker-signed) with no Developer ID. Downloading through the GitHub UI sets com.apple.quarantine, Gatekeeper refuses the dlopen, and require-addon reports it as a missing file:
Error: Cannot find addon '.' imported from '.../fs-native-extensions/binding.js'
Candidates:
- .../prebuilds/darwin-arm64/fs-native-extensions.node <- this file exists
Reproduced: set the xattr on the tarball, extract, run maka --version, crash. Remove it with xattr -d -r com.apple.quarantine and you get 0.1.2. The quarantine attribute is the only variable.
This breaks step 3 of the acceptance section this PR adds to RELEASE_CHECKLIST.md. The fix belongs in the macOS release layer rather than in a manual checklist step: sign the embedded Node and all three .node files with the existing Developer ID, then notarize. notarytool doesn't accept .tar.gz, so that probably means switching the container to .zip (desktop already ships one), or submitting a temporary zip to obtain the ticket. Tickets are recorded per cdhash, so they still apply once the files move. The maka and maka-agent wrappers are shell scripts and don't need signing.
Signing needs repository secrets you can't exercise from a fork. If you'd rather not block on that, land the packaging and verification scripts but leave the CLI assets out of gh release create, then add the upload in a follow-up. That keeps this PR mergeable without publishing an artifact that can't run.
P2: the embedded Node is never proven self-contained
copyFile(process.execPath, ...) copies only bin/node. Reproduced with Homebrew's Node: the packager exits 0 and silently produces an archive whose Node links against @rpath/libnode.147.dylib plus a dozen /opt/homebrew/opt/* dylibs that aren't in the archive. The failure surfaces much later, as a raw dyld stack from the verifier.
codesign --verify --strict passes on that ad-hoc binary. The sibling script verify-macos-arm64-dmg.mjs already uses spctl --assess --type execute, which returns rejected for it. Official builds are identifiable by Authority=Developer ID Application: Node.js Foundation (HX7739G8FX), flags=0x10000(runtime), and an otool -L closure containing only system frameworks. Asserting that in packageMacosArm64Cli would fail fast with a message that says what's wrong.
P2: localPackageDirectories is a second copy of the dependency graph
The hardcoded five-package list has to be the CLI's workspace closure, but nothing keeps it in sync with the manifests. Reproduced by omitting @maka/headless from the staged tree: npm ci exits 0 reporting "added 136 packages" and creates packages/headless containing only node_modules, with no package.json and no dist. maka --version and --help still work because headless is lazily imported. Only eval fails.
The verifier's eval step happens to cover this particular package. A package reachable only from inspect, or from a runtime feature the smoke doesn't touch, would ship broken. Deriving the closure recursively from the maka-agent manifest and rejecting dangling symlinks after relocation would put the graph back under the manifests' ownership.
P2: npm ci --prefix relies on behavior that has shifted between npm minors
| staging inside repo | staging in /tmp | |
|---|---|---|
| npm 11.6.2 (what CI actually runs) | ok | ok |
npm 11.12.1 (packageManager) |
ok | no workspaces present |
CI passes today only because setup-node with node-version: '24' ships npm 11.6.2, and because mkdtemp happens to place staging inside the repo. The declared packageManager: npm@11.12.1 never takes effect, since the workflow has no corepack enable.
Dropping --prefix and running npm ci with cwd: installRoot through the existing runCommand succeeds on all four combinations above, and stops the packaging step from depending on prefix-resolution semantics.
P2: floating runtime input
node-version: '24' plus process.execPath means the same commit produces different artifacts depending on when it's built. Pinning the full Node version, and npm alongside it, makes the archive reproducible and gives you somewhere to attribute a future Node regression.
P2: the TUI smoke can pass on a crash
/Maka/i matches any output containing "Maka", and the archive root is named Maka-<version>-cli-mac-arm64, so every fatal stack trace contains it. onExit then resolves without checking exitCode. Verified with a stub that prints a crash trace containing the archive path and exits 1: PASS. The same stub without "Maka" in its output correctly fails.
This compounds the P1 above. Even if a quarantine check were added to the TUI step, this logic would let the failure through. Matching a stable UI marker, and separating the expected post-Ctrl-C exit from a startup crash, would close it.
P3: test artifacts ship in the archive
libexec/packages is 31MB and contains 456 .test.js files and 9 __tests__ directories. electron-builder.config.mjs already excludes these for the desktop build.
P3: the new unit tests assert shapes rather than invariants
The additions to macos-arm64-release.test.mjs mostly check path suffixes, argument arrays, and YAML text. The workflow regexes would pass even if the command appeared only in a comment or in an unreachable step, and the --prefix assertion locks in the shape flagged above. The verifier's end-to-end smoke is the part carrying real weight here.
Unrelated: the diff also removes a blank line at the top of the existing test.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Review findings (independent deepseek-v4-flash review, 2 passes)
The direction (standalone macOS arm64 CLI via esbuild bundle) is sound, but there are two blockers.
P1 — The bundled CLI omits the provider-utils patch: streaming tool calls crash in the published artifact
- Predicted failure: the release bundle is missing the
provider-utilspatch that makes streaming tool calls work, so the distributed CLI fails on the core path (verified: a local stagingnpm ci+ packaged run reproduces the crash). - Evidence: the bundle excludes/does not apply the patch that other packaging paths include; the prior comment thread raised this and the head still does not include it.
- Fix: include the patch in the bundle (same mechanism as the other package paths), and verify with a packaged-install run of a streaming tool call, not just a workspace run.
P1 — Merge conflict with current main: the release workflow has drifted
- Predicted failure:
git merge-treeshows conflicts with current main (the workflow file changed on main since this PR's base), so the PR cannot merge and CI results don't cover the merged state. - Fix: rebase onto latest main and re-run the release workflow validation.
P2 (from the earlier comment thread, still unaddressed)
- The earlier comment items have not been responded to; please address or explicitly defer each.
Gate: FAIL. Both P1s block merge.
…-cli-release # Conflicts: # scripts/measure-session-bundle.mjs
|
Thanks for working through the standalone CLI packaging and verification details. Before continuing with the implementation, I think the distribution contract needs to be settled in #1510. P1: The implementation targets a retired release boundary before the CLI distribution shape is decidedThis PR extends This is not only a rebase problem. The new release path changes several decisions that #1823 currently makes implicitly:
I suggest using the existing open issue #1510 as the design authority rather than opening another issue. It already describes the CLI distribution goal, but it predates the unified release workflow and currently has no design discussion. A likely structure is: The CLI packager and verifier should remain independent from the Electron packager. What should be shared is the source commit, version, artifact collection, and publication boundary. The workspace-closure derivation, isolated artifact verification, and separate packager/verifier in this PR are useful work and can be carried forward. The old workflow wiring and release layout should not be. My suggestion is to pause or supersede this PR, settle the artifact and release contract in #1510, then implement the agreed shape from current 简体中文感谢你处理独立 CLI 打包和验证中的大量细节。继续实现之前，我认为应该先在 #1510 中确定 CLI 的分发契约。 P1：CLI 分发方案尚未确定，实现却接在了已经退出的发布边界上本 PR 扩展的是 这不只是 rebase 问题。新的发布路径使 #1823 中一些隐含决定需要重新讨论：
建议直接使用现有 open issue #1510 作为设计 authority，不要再开一个重复 issue。它已经描述了 CLI 分发目标，但早于当前统一 release workflow，而且目前还没有方案讨论。 一种可能的结构是： CLI packager 和 verifier 应继续独立于 Electron packager。需要共享的是 source commit、版本、artifact 汇集和发布边界，而不是具体打包实现。 本 PR 中的 workspace closure 推导、隔离 artifact 验证、独立 packager/verifier 都可以保留并迁移。旧 workflow 接线和 release layout 不应继续沿用。 我的建议是暂停或 supersede 当前 PR，先在 #1510 中确定 artifact 与 release contract，再从 current |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Codex automated review for exact head 81e53ed079dde0f658de306893208e19bf83511d.
Disclosure: This is an automated review performed by Codex using delegated adversarial review passes and a final evidence check. It has not been independently verified by Astro-Han or another human reviewer, does not constitute human approval, and does not represent the final judgment of a human reviewer.
The independent packager/verifier work addresses most of the earlier artifact findings: the toolchain is pinned, the workspace closure and patches are derived, native binaries are signed/notarized, and the behavior-level verifier is materially stronger. Two issues remain:
- P1: the PR still wires publication into a release workflow that current main has retired, and the branch is now conflicting/dirty;
- P2: the public launchers are not relocatable through an external symlink, contrary to the distribution contract recorded in #1510 and the intended future Homebrew installation shape.
This is not a request to mechanically split a ~1.4k-line release change. The owner boundary has changed underneath the PR: please rebase/rebuild the integration around the current unified release workflow, carrying the independent packager, verifier, and focused tests as one coherent CLI artifact slice. There are no CI results on this exact head, and the merged release path cannot be evaluated until the conflicts are resolved.
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '24' | ||
| node-version: '24.18.1' |
There was a problem hiding this comment.
[P1] Move this integration to the current release-workflow owner. Current main has removed release-macos-arm64.yml in favor of the unified release-desktop.yml; this branch now conflicts in the workflow, RELEASE_CHECKLIST.md, and package.json, and GitHub reports the exact head as CONFLICTING/DIRTY. As written there is no merge result whose signing, verification, and publication order can be validated. Please port the CLI job/artifact into the current unified workflow (or land the independent packager/verifier before wiring publication there), then rerun the release checks on the rebased head.
| export function macosArm64CliWrapper() { | ||
| return `#!/bin/sh | ||
| set -eu | ||
| bin_dir=$(CDPATH= cd "$(dirname "$0")" && pwd) |
There was a problem hiding this comment.
[P2] Resolve the launcher itself before deriving the archive root. This uses dirname "$0", so invoking a normal external link such as /usr/local/bin/maka -> <archive>/bin/maka makes the wrapper look for /usr/local/libexec/node/bin/node and fail before --version can run. #1510's distribution contract explicitly requires both commands to work through symlinks outside the extracted tree for future Homebrew installation. Resolve the launcher's real path first and add verifier coverage that runs both aliases through external symlinks.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Codex automated review — final follow-up
Disclosure: This is an automated review performed by Codex using delegated adversarial review passes and a final evidence check. It has not been independently verified by Astro-Han or another human reviewer, does not constitute human approval, and does not represent the final judgment of a human reviewer.
One additional P2 surfaced while reconciling the exact head against the agreed #1510 artifact contract: the CLI-specific third-party notice is not produced or verified. This is independent of the retired-workflow P1 and symlink-launcher P2 in my preceding review.
| copyFile(execPath, join(embeddedNodeDirectory, 'bin', 'node')), | ||
| copyFile(nodeLicensePath, join(embeddedNodeDirectory, 'LICENSE')), | ||
| copyFile(join(repoRoot, 'LICENSE'), join(archiveRoot, 'LICENSE')), | ||
| copyFile(join(repoRoot, 'NOTICE'), join(archiveRoot, 'NOTICE')), |
There was a problem hiding this comment.
[P2] Include notices for the packaged CLI dependency closure. The agreed #1510 artifact contract requires THIRD_PARTY_NOTICES.txt generated from the exact maka-agent production dependency closure, but this staging path copies only Maka's LICENSE/NOTICE and the embedded Node license; the verifier likewise never requires a CLI notice. The existing Desktop notice cannot establish the CLI closure. Please generate the closure-scoped notice, include it in the ZIP, and verify it against the packaged dependency metadata before publication.
|
Thanks for the automated follow-up. I confirmed all three findings against exact head I am treating these as automated review findings rather than human approval or a final maintainer decision. #1510 currently contains my proposed distribution contract, but it has not yet received maintainer confirmation, so I do not consider that contract agreed yet. I will keep #1823 paused and will not mechanically rebase its retired |
|
/agentic_review |
Code Review by Qodo
1. Node JIT entitlements stripped
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| '--force', | ||
| '--options', | ||
| 'runtime', | ||
| '--timestamp', |
There was a problem hiding this comment.
1. Node jit entitlements stripped 🐞 Bug ≡ Correctness
signCliBinaries replaces the official Node signature with a hardened-runtime signature but does not preserve or reapply Node’s JIT entitlements. The resulting runtime can pass codesign --verify yet fail when V8 needs executable JIT memory.
Agent Prompt
## Issue description
Re-signing the embedded official Node executable with `codesign --force --options runtime` replaces its signature without preserving the entitlements required by V8 under the hardened runtime.
## Issue Context
Reuse the official runtime’s existing entitlement authority rather than defining a second hard-coded entitlement list. Preserve its entitlement metadata when replacing the signature, and extend release verification to assert that the signed Node runtime retains at least `com.apple.security.cs.allow-jit`; ordinary native addons should continue using their existing signing path.
## Fix Focus Areas
- scripts/package-macos-arm64-cli.mjs[439-461]
- scripts/verify-macos-arm64-cli.mjs[297-310]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| normalized.startsWith('/') || | ||
| segments.includes('..') || | ||
| segments.some((segment) => segment.startsWith('._')) || | ||
| segments[0] !== archiveRootName |
There was a problem hiding this comment.
2. Archive symlinks escape root 🐞 Bug ⛨ Security
assertSafeCliArchiveEntries validates only ZIP entry names, so a safe-looking path may be a symlink whose target is outside the extracted archive. Because the later dangling-link check only requires realpath to succeed, verification can inspect and execute host files through escaped node, CLI, or dependency paths.
Agent Prompt
## Issue description
ZIP entry-name checks do not validate symlink targets, allowing extracted artifact paths to resolve outside the archive before the verifier executes them.
## Issue Context
Reuse and strengthen the existing symlink-validation seam instead of adding a separate archive authority. Validate every symlink under the entire extracted archive root, require its resolved target to remain inside that root, and perform this check immediately after extraction and before reading metadata, inspecting binaries, importing modules, or running entrypoints.
## Fix Focus Areas
- scripts/package-macos-arm64-cli.mjs[298-312]
- scripts/verify-macos-arm64-cli.mjs[343-366]
- scripts/verify-macos-arm64-cli.mjs[385-404]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Thank you for the original standalone macOS CLI packaging work. The release-macos-arm64 workflow targeted by this branch has since been retired, and the discussion here already concluded that the useful packager/verifier pieces should move into a clean replacement on the unified release boundary. That replacement path progressed through #3002 and is now consolidated in #3222. This PR is now superseded and can be closed to keep the backlog aligned with the current release architecture. The workspace-closure, isolated verification, signing, and artifact-safety work from this branch was valuable and informed the current solution. Thank you again for the contribution. 中文感谢你最初完成 standalone macOS CLI packaging 工作。该分支依赖的 release-macos-arm64 workflow 后来已经退出；这里的讨论也已经确认，应把有价值的 packager/verifier 部分迁移到统一 release boundary 上的全新方案。这个 replacement 先演进为 #3002，现在进一步收敛到 #3222。 此 PR 现在已被 supersede，可以关闭，让 backlog 与当前 release architecture 保持一致。本分支中的 workspace closure、isolated verification、signing 与 artifact safety 工作很有价值，也为当前方案提供了重要基础。再次感谢贡献。 |
3 participants
Summary
makaandmaka-agententrypointsnode-ptyhelper with Developer ID + hardened runtime, then notarize the ZIP before publicationCloses #1510
Verification
npm run build— passednpm run typecheck— passednpm run check:release— passednpm run test:scripts:extended— 19 passed, 0 failednpm --workspace maka-agent test— 464 passed, 0 failednpm --workspace @maka/runtime test— 2,685 passed, 0 failed, 9 skippednpm audit --omit=dev --audit-level=high— passed the high-severity gate; two existing moderate advisories remainMaka-0.1.2-cli-mac-arm64.zipon Apple Silicon with official Node.js 24.18.1 and npm 11.12.1maka,maka-agent, help, deterministic fake evaluation, and PTY TUI startup/exitThe repository-wide
npm testrun passed the release-script, CLI, runtime, desktop, and other completed workspace suites, but did not terminate locally because the upstreamruntime-host/host-kernel.test.jsprocess remained idle with no test timeout. The PR CI run is the authoritative full-suite result.Release validation
.github/RELEASE_CHECKLIST.md