feat(release): add synchronized macOS arm64 CLI artifact - #3002
feat(release): add synchronized macOS arm64 CLI artifact#3002zupengwang wants to merge 4 commits into
Conversation
Generated-by: Codex Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for consolidating the release flow around the contract agreed in #1510. The overall direction looks right, and I think this can cleanly supersede #1823 once the following two gaps are addressed.
[P1] Include @maka/eval runtime assets in the packaged workspace closure
stageWorkspacePackages() currently copies only each workspace’s package.json and dist. That assumption does not hold for @maka/eval: its production executor resolves ../harbor relative to the compiled module and launches harbor/run_trial.py, which in turn loads Maka’s relay_agent.py.
Those files are therefore Maka runtime code, not something supplied by the user’s Harbor installation. They are absent from the resulting ZIP, so maka eval --help can pass while a real maka eval run exits before Harbor creates a Trial.
Please let @maka/eval explicitly declare its release files—dist plus the required Harbor runtime assets, excluding test_*.py—and have the packager consume that declaration. The complete runtime asset set is only about 80 KB uncompressed, so this has negligible artifact-size impact. The final extracted artifact should also run one deterministic maka eval smoke test rather than checking help alone.
[P2] Run the release contract tests in CI
npm run test:release adds 13 useful checks for release identity, launcher layout, dependency closure, Node provenance, Mach-O filtering, archive safety, and TUI readiness, but no CI workflow currently invokes it. The normal workspace test runner does not discover scripts/release.test.mjs, so these tests cannot prevent regressions.
I do not think this needs a new CI subsystem. Running it from an existing lightweight code-validation job should be enough. It would also be valuable to add one contract test asserting that every workspace’s declared runtime assets are present in the staged artifact; the current tests all pass while missing the @maka/eval/harbor files.
I would not restore the deleted manual checks for packaged Git license files or reinstalling ripgrep on every release. The license files are already checked deterministically by the packaged-app verifier, while repeated ripgrep installation mostly tests Homebrew, winget, and PATH behavior. Keeping the manual gate focused on quarantine/Gatekeeper, real UI behavior, and a real provider call is a useful simplification.
中文对照
感谢按照 #1510 确认的契约统一发布流程。整体方向是正确的；处理下面两个缺口后，我认为它可以干净地取代 #1823。
[P1] 将 @maka/eval 的运行资源纳入打包闭包
stageWorkspacePackages() 目前只复制各 workspace 的 package.json 和 dist，但这个假设不适用于 @maka/eval。它的生产 executor 会相对编译后模块解析 ../harbor，启动 harbor/run_trial.py，后者还会加载 Maka 自己的 relay_agent.py。
这些文件属于 Maka 的运行时代码，并不是用户安装 Harbor Python 包后就会自然提供的内容。当前 ZIP 中没有它们，因此 maka eval --help 可以通过，但真正的 maka eval run 会在 Harbor 创建 Trial 之前退出。
建议由 @maka/eval 明确声明发布文件，包括 dist 和必要的 Harbor 运行资源，但排除 test_*.py，然后让打包器消费这份声明。全部运行资源未压缩也只有约 80 KB，对包体积几乎没有影响。最终解压后的 artifact 还应该执行一次确定性的真实 maka eval，而不只是检查 help。
[P2] 在 CI 中执行 Release Contract Tests
npm run test:release 新增了 13 项有价值的检查，覆盖发布身份、launcher 布局、依赖闭包、Node 来源、Mach-O 过滤、归档安全和 TUI readiness，但目前没有任何 CI workflow 调用它。普通 workspace test runner 也不会发现 scripts/release.test.mjs，所以这些测试无法阻止回归。
这里不需要建立新的复杂 CI 系统，在现有的轻量代码检查 job 中执行即可。另外建议增加一项 contract test，确认每个 workspace 声明的运行资源都进入了 staged artifact；现有测试全部通过，却没有发现 @maka/eval/harbor 缺失。
我不建议恢复已经删除的 Git license 文件人工复查，也不建议每次发布都重新安装 ripgrep。许可文件已经由 packaged-app verifier 确定性检查；重复安装 ripgrep 主要是在测试 Homebrew、winget 和 PATH。让人工 gate 聚焦 quarantine/Gatekeeper、真实 UI 行为和真实 provider 调用，是有价值的简化。
Disclosure: This review was prepared with assistance from OpenAI Codex and Claude Opus. I reviewed the evidence, traced the relevant production paths, and made the final judgment on the findings and recommendations.
Generated-by: Codex Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
|
Addressed both review items:
Apple Silicon validation passed with Node 24.18.1/npm 11.12.1: format, lint, typecheck, build, release tests 15/15, The existing |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — the latest revision closes both issues from my previous review. @maka/eval now owns an explicit runtime-asset declaration, the packaged files are checked byte-for-byte with their executable bits, the extracted artifact exercises the staged Eval runner, and the release contract tests now run in CI.
The overall release boundary also matches the decision in #1510: one product version, source commit, tag, Draft Release, and required-artifact graph. Once the point below is addressed, I think this can supersede #1823 cleanly.
[P2] Make the pinned npm version the actual authority for every release path
The release identity declares the root packageManager value, npm@11.12.1, as the pinned npm toolchain. The workflow uses that version for ci and audit, but the Desktop packaging path then returns to bare npm run ...; the macOS and Windows packagers also invoke bare npm internally.
This is observable with the selected Node version: official Node 24.18.1 ships npm 11.16.0. Running:
npx --yes npm@11.12.1 --version → 11.12.1
npm --version → 11.16.0
therefore leaves the CLI path using the declared npm version while the Desktop build/check/package path is driven by a different one. That creates two release-toolchain authorities even though the metadata records only one.
The smallest fix is to route the remaining release npm run entry points—especially the Desktop package commands—through the same existing pinned invocation:
npx --yes "npm@${{ needs.release-identity.outputs.npm_version }}" run ...Alternatively, each release job can install and assert the pinned npm once before doing any release work. Either approach is fine as long as every nested packaging command sees the same npm version.
I found no other P0–P3 issues in the current head. The latest workflow runs are still awaiting maintainer approval, which is separate from this code finding.
中文对照
感谢更新——上一轮提出的两个问题都已经真正解决：@maka/eval 现在自己声明运行资源；打包测试逐字节检查文件内容和可执行位；解压后的制品会执行 staged Eval runner；Release Contract Tests 也已经进入 CI。
整体发布边界也符合 #1510 的决定：唯一产品版本、源码提交、Tag、Draft Release 和必需制品依赖图。处理下面这一点后，我认为它可以干净地取代 #1823。
[P2] 让声明的 npm 版本真正成为所有发布路径的唯一权威
Release identity 将根目录 packageManager 中的 npm@11.12.1 定义为固定 npm 工具链。workflow 在执行 ci 和 audit 时使用了这个版本，但随后 Desktop 打包又回到裸 npm run ...；macOS 和 Windows packager 内部也继续调用裸 npm。
这个差异可以直接复现：官方 Node 24.18.1 自带 npm 11.16.0，因此：
npx --yes npm@11.12.1 --version → 11.12.1
npm --version → 11.16.0
最终 CLI 路径使用声明的 npm 版本，而 Desktop 的 build/check/package 路径由另一个版本驱动，形成了两套 release toolchain authority，但发布元数据只记录其中一个。
最小修复是让剩余的 release npm run 入口——尤其是 Desktop package 命令——统一使用现有的 pinned invocation：
npx --yes "npm@${{ needs.release-identity.outputs.npm_version }}" run ...也可以在每个 release job 开始时安装并校验一次固定 npm。具体方式不重要，关键是所有嵌套打包命令必须看到同一个 npm 版本。
当前 head 没有发现其他 P0–P3。最新 workflow 尚待 maintainer 批准运行，这是独立于该代码 finding 的 CI 状态。
AI-assisted review disclosure: OpenAI Codex assisted with tracing the release toolchain and independently checking the packaging and verification paths. I verified the evidence against the current head and made the final review decision.
Generated-by: Codex Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
Generated-by: Codex Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
📝 WalkthroughWhat problem this solvesThis PR adds a standalone macOS arm64 CLI/TUI artifact to the same release as the Desktop artifact. The artifact provides:
The release workflow now verifies Desktop, CLI/TUI, and source artifacts before it creates a Draft GitHub Release. Source of truthThe PR extends the existing root package manifest as the authoritative source for:
It does not create a parallel versioning path. The CLI package version and release identity checks enforce consistency with the root version. Solution scope and complexityThe solution is coherent for the release requirements. The workflow, packager, verifier, metadata checks, notices, and smoke tests form one release contract. The added complexity is necessary for:
The removed Desktop-only workflow and checklist reduce duplicate release paths. Possible simplificationNo clear deletion or simplification is identified without reducing coverage. The release test suite is large, but it covers distinct packaging, security, provenance, runtime, and workflow contracts. The Risks and validationThe PR changes release governance and user-visible distribution behavior. It adds a new macOS arm64 artifact and changes the public CLI surface so that The packager and verifier validate:
The reported checks include build, release, CLI, evaluation, lint, format, typecheck, audit, and packaging validation. These results are not independently verified here. Signing, notarization, Gatekeeper, separate-machine acceptance, and real provider calls remain release-environment gates. Review-relevant risks
The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughThe release system now uses one main-branch workflow for Desktop, CLI/TUI, and bundled source artifacts. It adds pinned toolchain validation, macOS arm64 CLI packaging, artifact verification, release contract tests, unified checksums, and expanded acceptance criteria. ChangesUnified release pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🔵 Low · up to The PR adds a macOS arm64 CLI artifact and replaces the release workflow; users could encounter an unclear launcher contract, while release operators could see false verification failures or a competing release during the workflow gap. These are bounded follow-ups rather than high-impact correctness, security, or availability defects, so the PR is mergeable with explicit owner awareness. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant ReleaseIdentity
participant ArtifactBuilders
participant ArtifactVerifiers
participant GitHubRelease
ReleaseWorkflow->>ReleaseIdentity: Resolve version, commit, tag, and toolchain
ReleaseIdentity-->>ReleaseWorkflow: Return release outputs
ReleaseWorkflow->>ArtifactBuilders: Build Desktop, CLI/TUI, and source artifacts
ArtifactBuilders->>ArtifactVerifiers: Verify artifacts and checksums
ArtifactVerifiers-->>ReleaseWorkflow: Return verification results
ReleaseWorkflow->>GitHubRelease: Create draft release with verified assets
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Addressed the pinned npm authority finding:
While validating the merged result against current Apple Silicon validation passed for build, format, lint, typecheck, 18/18 release tests, CLI/Eval suites, production audit, packaging, and the independent artifact verifier. The exact current- Disclosure: This update was prepared with assistance from OpenAI Codex and reviewed by the contributor before posting. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a unified product-level release workflow that produces and verifies a standalone macOS arm64 CLI/TUI artifact alongside Desktop and source artifacts, all bound to a single authoritative root version and release identity.
Changes:
- Adds macOS arm64 CLI/TUI packaging + verification with a stable
bin/maka+RELEASE.jsoncontract and artifact safety checks. - Establishes a single “Release” GitHub Actions workflow that gates Draft Release creation on verified Desktop (macOS/Windows), CLI, and source artifacts.
- Makes release identity/toolchain (version, tag, pinned Node + npm) explicit and test-enforced, including workspace release-file declarations and third-party notice closure validation.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/verify-macos-arm64-cli.mjs | Verifies the extracted CLI artifact (metadata, signatures, symlinks, notices, Mach-O inventory, TUI + eval smoke). |
| scripts/release.test.mjs | Adds contract-level tests for release identity, pinned npm bootstrap, packaging invariants, and closure/notice behavior. |
| scripts/release-identity.mjs | Defines a single authoritative release identity (version/tag/artifact names/toolchain) rooted in the repo manifests and main. |
| scripts/release-eval-smoke-sitecustomize.py | Provides a minimal offline Harbor API shim used by the extracted-artifact eval smoke test. |
| scripts/prepare-release-npm.mjs | Installs and exposes a pinned npm version for release jobs (and verifies it matches the manifest). |
| scripts/package-macos-arm64-cli.mjs | Packages a relocatable macOS arm64 CLI/TUI ZIP with pinned official Node, dependency closure, signing/notarization, and checksums. |
| scripts/generate-third-party-notices.mjs | Generalizes notices generation to support multiple workspaces (Desktop vs CLI) and adds a targeted license override. |
| scripts/assert-release-npm.mjs | Ensures both parent and nested npm invocations match the pinned release npm version. |
| packages/eval/package.json | Declares explicit releaseFiles to include Harbor runtime assets needed by the packaged eval smoke. |
| packages/cli/src/cli.ts | Updates help output to reflect maka as the only public launcher. |
| packages/cli/src/tests/cli.test.ts | Adds a test to enforce documentation/contract: no maka-agent launcher in help output. |
| packages/cli/package.json | Aligns CLI workspace version and removes the public maka-agent bin entry (only maka). |
| package.json | Adds releaseToolchain and release-related scripts (identity checks, contract tests, CLI package/verify). |
| package-lock.json | Updates lock metadata for the CLI package version/bin change. |
| docs/cli-distribution.md | Documents the CLI/TUI distribution contract and RELEASE.json stable fields. |
| .github/workflows/release.yml | New product-level release workflow: identity → desktop (mac/win) + cli + source → publish draft release. |
| .github/workflows/release-desktop.yml | Removes the prior Desktop-oriented release workflow in favor of the product-level workflow. |
| .github/workflows/ci.yml | Runs the new release contract tests as part of CI. |
| .github/RELEASE_CHECKLIST.md | Updates checklist to the product-level workflow and adds CLI/TUI acceptance steps. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
scripts/release.test.mjs (1)
236-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe workflow test asserts YAML text layout, not workflow structure.
The invariant is valuable: each npm-using job prepares the pinned npm and verifies it first. The mechanism is fragile. It depends on exact two-space job indentation, on job order in the file, on the literal string
run: node scripts/prepare-release-npm.mjs, and onpublishappearing after every listed job. A reformat of.github/workflows/release.yml, or a reorder of jobs, breaks the test without changing behavior.Parse the workflow and assert on the step list of each job instead. Node 22 has no built-in YAML parser, so this needs a dependency that is already present in the repo.
As per path instructions: "Flag tests that duplicate existing coverage, assert implementation details, or do not protect observable behavior."
#!/bin/bash # Check whether a YAML parser is already available for the release tests. rg -n '"(yaml|js-yaml|yaml-eslint-parser)"' package.json fd -t f 'package.json' -d 3 --exec rg -l 'js-yaml|"yaml"' {}Source: Path instructions
.github/workflows/release.yml (2)
348-353: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
publishomits themainbranch guard used by every other job.Jobs
release-identity,desktop,cli-macos-arm64, andsourceall carryif: github.ref == 'refs/heads/main'.publishdoes not. Today this is safe, because a job whose dependency is skipped is also skipped. The guarantee is implicit, and it breaks if anyone later addsif: always()or a fallback condition topublish.Either add the same guard to
publish, or move the condition to a single place, so the rule is stated once.
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the release metadata for
release-identity’s Node.js version. The job hardcodes24.18.1, whilerelease-identity.mjsreadsreleaseToolchain.nodefrompackage.json. Add a Node.js version assertion or configureactions/setup-nodefrom the same source.Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e796d08-a463-415d-af65-c868e827c8c8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
.github/RELEASE_CHECKLIST.md.github/workflows/ci.yml.github/workflows/release-desktop.yml.github/workflows/release.ymldocs/cli-distribution.mdpackage.jsonpackages/cli/package.jsonpackages/cli/src/__tests__/cli.test.tspackages/cli/src/cli.tspackages/eval/package.jsonscripts/assert-release-npm.mjsscripts/generate-third-party-notices.mjsscripts/package-macos-arm64-cli.mjsscripts/prepare-release-npm.mjsscripts/release-eval-smoke-sitecustomize.pyscripts/release-identity.mjsscripts/release.test.mjsscripts/verify-macos-arm64-cli.mjs
💤 Files with no reviewable changes (2)
- packages/cli/src/cli.ts
- .github/workflows/release-desktop.yml
| 2. Confirm `apps/desktop/package.json` and `packages/cli/package.json` exactly match the root | ||
| version, and the CLI manifest exposes only the `maka` command. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the CLI bin declarations and any maka-agent references.
rg -n '"bin"' -A6 packages/cli/package.json
rg -n 'maka-agent' --glob '!node_modules' -C2Repository: maka-agent/maka-agent
Length of output: 432
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- packages/cli/package.json ---'
cat -n packages/cli/package.json | sed -n '1,45p'
printf '%s\n' '--- release checklist ---'
cat -n .github/RELEASE_CHECKLIST.md | sed -n '25,50p'
printf '%s\n' '--- issue and agent references ---'
rg -n '1510|maka-agent|maka-agent' .github README.md packages apps --glob '!node_modules' -C2 || true
printf '%s\n' '--- repository metadata ---'
git remote -vRepository: maka-agent/maka-agent
Length of output: 50379
🌐 Web query:
site:github.com/maka-agent/maka-agent/issues/1510 maka-agent TUI launches through maka or maka-agent
💡 Result:
Issue #1510 requests a standalone macOS arm64 CLI/TUI artifact containing the maka command. It should launch the TUI when invoked as either maka or maka-agent, while also supporting run, eval, and inspect. (github.com)
Citations:
Align the CLI contract with issue #1510
Issue #1510 requires the TUI to launch as both maka and maka-agent, but packages/cli/package.json exposes only maka, and CLI tests reject maka-agent. Update the manifest, tests, and checklist, or document the intentional removal in issue #1510 and the PR.
| 4. Extract the CLI ZIP without clearing quarantine. Run `bin/maka --version` and `bin/maka --help`. | ||
| 5. Create an external link, for example `ln -s "$PWD/bin/maka" /tmp/maka-release-acceptance`, and | ||
| confirm the linked command reports the same version and help output. | ||
| 6. Start `maka` with no arguments and confirm the TUI renders, accepts input, and exits cleanly. | ||
| 7. Exercise one non-interactive `maka run`, one deterministic `maka eval`, and one streaming | ||
| tool-call path against the packaged artifact. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the invocation path consistent in steps 4, 6, and 7.
Step 4 uses bin/maka --version. Step 6 says "Start maka", and step 7 says maka run and maka eval. The artifact is relocatable and is not on PATH after extraction. An operator following the text literally gets command not found.
Use bin/maka in steps 6 and 7, or add one explicit PATH line after step 4.
📝 Proposed wording fix
-6. Start `maka` with no arguments and confirm the TUI renders, accepts input, and exits cleanly.
-7. Exercise one non-interactive `maka run`, one deterministic `maka eval`, and one streaming
+6. Start `bin/maka` with no arguments and confirm the TUI renders, accepts input, and exits cleanly.
+7. Exercise one non-interactive `bin/maka run`, one deterministic `bin/maka eval run`, and one streaming
tool-call path against the packaged artifact.| - name: Release contract tests | ||
| run: npm run test:release |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the changes job and its path filters.
sed -n '1,60p' .github/workflows/ci.yml
fd -H -t f -g '*.y*ml' .github | xargs rg -n 'filters|paths|scripts/' -C2
rg -n '"test:release"' package.json -A2Repository: maka-agent/maka-agent
Length of output: 10624
🏁 Script executed:
#!/bin/bash
set -e
script="$(fd -H -t f 'ci-test-plan\.mjs$' . | head -n1)"
printf '%s\n' "== $script =="
ast-grep outline "$script"
printf '%s\n' '== filter definitions and release inputs =='
rg -n -C4 'code|scripts|package\.json|release|workflow|filter|paths' "$script"
printf '%s\n' '== relevant workflow and manifest references =='
rg -n -C2 'release|package\.json|scripts/' .github/workflows/release.yml .github/workflows/ci.yml package.jsonRepository: maka-agent/maka-agent
Length of output: 34652
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== planner implementation =='
sed -n '209,310p' scripts/ci-test-plan.mjs
printf '%s\n' '== planner tests and release-test coverage =='
rg -n -C3 'planTests|ci-test-plan|release\.yml|package\.json|scripts/' scripts --glob '*test*.mjs' --glob 'ci-test-plan.mjs'
printf '%s\n' '== deterministic path probe (source treated as data) =='
python3 - <<'PY'
from pathlib import Path
import json
source = Path("scripts/ci-test-plan.mjs").read_text()
root = json.loads(Path("package.json").read_text())
full_suite = set()
in_full = False
for line in source.splitlines():
if line.startswith("const FULL_SUITE_FILES = new Set(["):
in_full = True
continue
if in_full and line.strip() == "]);":
break
if in_full:
value = line.strip().rstrip(",")
if value.startswith("'") and value.endswith("'"):
full_suite.add(value[1:-1])
workspace_dirs = set(root.get("workspaces", []))
typecheck_only = set()
in_typecheck = False
for line in source.splitlines():
if line.startswith("const TYPECHECK_ONLY_FILES = new Set(["):
in_typecheck = True
continue
if in_typecheck and line.strip() == "]);":
break
if in_typecheck:
value = line.strip().rstrip(",")
if value.startswith("'") and value.endswith("'"):
typecheck_only.add(value[1:-1])
def code_for(path):
if path in full_suite:
return True
if path in workspace_dirs or any(path.startswith(d + "/") for d in workspace_dirs):
return True
if path.startswith("scripts/"):
return True
if path.startswith("skills/"):
return True
if path in typecheck_only:
return True
if path.startswith(".github/"):
return False
return True
cases = {
"scripts/release-identity.mjs": True,
"scripts/release.test.mjs": True,
"package.json": True,
".github/workflows/ci.yml": True,
".github/workflows/release.yml": False,
}
for path, expected in cases.items():
actual = code_for(path)
print(f"{path}: code={str(actual).lower()} expected={str(expected).lower()}")
assert actual == expected, (path, actual, expected)
assert "package.json" in full_suite
assert ".github/workflows/release.yml" not in full_suite
print("planner path assertions passed")
PYRepository: maka-agent/maka-agent
Length of output: 16198
Include .github/workflows/release.yml in the code selection.
Changes to release scripts and package.json run npm run test:release, but release-workflow-only changes do not.
| - name: Reserve tag and release identity | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| TAG: ${{ steps.identity.outputs.tag }} | ||
| run: | | ||
| if git ls-remote --exit-code --tags origin "refs/tags/${TAG}" >/dev/null 2>&1; then | ||
| echo "Tag ${TAG} already exists." >&2 | ||
| exit 1 | ||
| fi | ||
| if gh release view "$TAG" >/dev/null 2>&1; then | ||
| echo "Release ${TAG} already exists." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The step is named "Reserve" but reserves nothing.
It only checks that the tag and release are absent. No tag is pushed and no release is created here. The actual creation happens in publish, up to 45 minutes later, so a concurrent dispatch or manual tag push in that window is not blocked. concurrency: group: release limits concurrent runs of this workflow only.
The check still has value as an early fail. Rename it to reflect the check, for example "Assert the tag and release are unused", so operators do not rely on a reservation that does not exist.
| The CLI-specific `THIRD_PARTY_NOTICES.txt` must enumerate exactly the external production | ||
| dependencies recorded in `RELEASE.json`. The checksum is generated only after signing and | ||
| notarization complete. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the checksum timing contract.
Lines 41-43 describe the thirdPartyNoticesSha256 checksum. The packager calculates that digest before signing and notarization. State that the archive checksum is generated after signing and notarization, or remove the ordering claim for the notices checksum.
Proposed fix
-The checksum is generated only after signing and notarization complete.
+The archive checksum is generated only after signing and notarization complete.As per path instructions, review the diff adversarially against the problem it claims to solve.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The CLI-specific `THIRD_PARTY_NOTICES.txt` must enumerate exactly the external production | |
| dependencies recorded in `RELEASE.json`. The checksum is generated only after signing and | |
| notarization complete. | |
| The CLI-specific `THIRD_PARTY_NOTICES.txt` must enumerate exactly the external production | |
| dependencies recorded in `RELEASE.json`. The archive checksum is generated only after signing and | |
| notarization complete. |
Source: Path instructions
| test('documents maka as the only public launcher', () => { | ||
| const help = parseMakaCliArgs(['--help'], '0.1.0', 'maka'); | ||
| assert.equal(help.kind, 'help'); | ||
| if (help.kind === 'help') { | ||
| assert.match(help.text, /^Usage: maka$/m); | ||
| assert.doesNotMatch(help.text, /maka-agent/); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test the default public launcher.
Line 50 passes maka directly to parseMakaCliArgs. The test still passes if RELEASE_MAKA_CLI_LAUNCH_OPTIONS.cliCommand regresses to maka-agent. Omit the third argument so the test protects the public default behavior.
Proposed fix
- const help = parseMakaCliArgs(['--help'], '0.1.0', 'maka');
+ const help = parseMakaCliArgs(['--help'], '0.1.0');As per path instructions, flag tests that “do not protect observable behavior.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('documents maka as the only public launcher', () => { | |
| const help = parseMakaCliArgs(['--help'], '0.1.0', 'maka'); | |
| assert.equal(help.kind, 'help'); | |
| if (help.kind === 'help') { | |
| assert.match(help.text, /^Usage: maka$/m); | |
| assert.doesNotMatch(help.text, /maka-agent/); | |
| } | |
| test('documents maka as the only public launcher', () => { | |
| const help = parseMakaCliArgs(['--help'], '0.1.0'); | |
| assert.equal(help.kind, 'help'); | |
| if (help.kind === 'help') { | |
| assert.match(help.text, /^Usage: maka$/m); | |
| assert.doesNotMatch(help.text, /maka-agent/); | |
| } |
Source: Path instructions
| send( | ||
| { | ||
| "token": kwargs.relay_token, | ||
| "kind": "ready", | ||
| "instruction": "release smoke", | ||
| "cwd": "/tmp", | ||
| } | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the hardcoded /tmp to keep Ruff quiet.
Ruff reports S108 as an error on line 119. The value is only a protocol field, so the risk is a lint-gate failure, not insecure file creation. Use tempfile.gettempdir() instead of a literal path.
Proposed fix
+import tempfile "kind": "ready",
"instruction": "release smoke",
- "cwd": "/tmp",
+ "cwd": tempfile.gettempdir(),The use-jsonify hints on the json.dumps calls do not apply here. This fixture is not a web handler.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| send( | |
| { | |
| "token": kwargs.relay_token, | |
| "kind": "ready", | |
| "instruction": "release smoke", | |
| "cwd": "/tmp", | |
| } | |
| ) | |
| send( | |
| { | |
| "token": kwargs.relay_token, | |
| "kind": "ready", | |
| "instruction": "release smoke", | |
| "cwd": tempfile.gettempdir(), | |
| } | |
| ) |
🧰 Tools
🪛 ast-grep (0.45.1)
[info] 118-118: Do not hardcode temporary file or directory names
Context: "/tmp"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🪛 Ruff (0.16.1)
[error] 119-119: Probable insecure usage of temporary file or directory: "/tmp"
(S108)
Source: Linters/SAST tools
| const archiveEntries = await run('unzip', ['-Z1', resolvedArchivePath]); | ||
| assertSafeCliArchiveEntries( | ||
| archiveEntries.stdout.split('\n').filter(Boolean), | ||
| expectedPaths.archiveRootName, | ||
| ); | ||
|
|
||
| const extractionRoot = await mkdtemp(join(dirname(resolvedArchivePath), '.verify-cli-')); | ||
| try { | ||
| await run('ditto', ['-x', '-k', resolvedArchivePath, extractionRoot]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Give the archive commands their own timeout.
runCommand applies a 30 s default timeout. unzip -Z1 on line 527 and ditto -x on line 535 both process the full artifact, which contains the embedded Node runtime plus the production dependency closure. On a slow runner this can exceed 30 s and fail verification for a reason unrelated to artifact quality.
Proposed fix
- const archiveEntries = await run('unzip', ['-Z1', resolvedArchivePath]);
+ const archiveEntries = await run('unzip', ['-Z1', resolvedArchivePath], { timeout: 120_000 });
@@
- await run('ditto', ['-x', '-k', resolvedArchivePath, extractionRoot]);
+ await run('ditto', ['-x', '-k', resolvedArchivePath, extractionRoot], { timeout: 300_000 });Note that maxBuffer of 20 MB also caps the unzip -Z1 listing. That headroom looks sufficient for this entry count.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const archiveEntries = await run('unzip', ['-Z1', resolvedArchivePath]); | |
| assertSafeCliArchiveEntries( | |
| archiveEntries.stdout.split('\n').filter(Boolean), | |
| expectedPaths.archiveRootName, | |
| ); | |
| const extractionRoot = await mkdtemp(join(dirname(resolvedArchivePath), '.verify-cli-')); | |
| try { | |
| await run('ditto', ['-x', '-k', resolvedArchivePath, extractionRoot]); | |
| const archiveEntries = await run('unzip', ['-Z1', resolvedArchivePath], { timeout: 120_000 }); | |
| assertSafeCliArchiveEntries( | |
| archiveEntries.stdout.split('\n').filter(Boolean), | |
| expectedPaths.archiveRootName, | |
| ); | |
| const extractionRoot = await mkdtemp(join(dirname(resolvedArchivePath), '.verify-cli-')); | |
| try { | |
| await run('ditto', ['-x', '-k', resolvedArchivePath, extractionRoot], { timeout: 300_000 }); |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The release graph in this branch is internally thoughtful: identity is centralized, artifact validation is staged before signing, and the smoke checks try to exercise extracted output. The problem definition has since changed, however. Current main now owns CLI publication through the staged/finalized CLI release pipeline, a different version/channel contract, and a newer npm toolchain; GitHub reports this head as conflicting across the exact release-authority files.
This should not be resolved as a mechanical rebase. The first-principles/Occam path is to decide whether current main already satisfies this PR. If a concrete gap remains, port only that gap into the existing CLI release seam and derive its asset/manifest checks from the current release contract. Carrying this complete parallel workflow forward would create two authorities for the same release.
Review performed with Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the conclusion against the latest PR head, current main, the reproduced conflicts, and live CI.
中文评论
本分支内部的 release graph 设计较完整：身份集中管理，签名前验证产物，smoke tests 也尝试验证解包后的输出。但问题定义已经变化：当前 main 已通过 CLI stage/finalize pipeline、不同的版本/通道契约和更新后的 npm toolchain 管理发布；GitHub 也显示本 head 在这些发布权威文件上发生冲突。
这不应机械 rebase。更符合第一性原理和奥卡姆剃刀的方案是先确认当前 main 是否已满足本 PR；若仍有明确缺口，只把该缺口扩展到现有 CLI release seam，并从当前发布契约派生 asset/manifest 校验。继续携带整套并行 workflow 会形成两个发布权威。
本次审查使用了 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已依据最新 PR head、当前 main、实际冲突和实时 CI 复核结论。
| @@ -7,6 +7,11 @@ | |||
| "node": ">=22.19.0" | |||
| }, | |||
| "packageManager": "npm@11.12.1", | |||
There was a problem hiding this comment.
P1 — This branch now defines a stale, competing release authority. Current main pins npm@11.19.0 and has a newer staged/finalized CLI publication architecture, while this head pins npm@11.12.1 and introduces a separate product release workflow/version contract. The conflict spans package.json, the lockfile, CLI manifests/entrypoints/tests, and notice generation, so selecting either side mechanically would lose required release semantics. Re-evaluate the PR as potentially superseded; if a gap remains, rebuild only that gap on current main's release seam.
|
/agentic_review |
Code Review by Qodo
1. Tag target can race
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| gh release create "$TAG" release-assets/* \ | ||
| --draft \ | ||
| --target "$SOURCE_COMMIT" \ |
There was a problem hiding this comment.
1. Tag target can race 🐞 Bug ≡ Correctness
The initial existence check does not reserve the tag, so if v<version> is created or moved after release-identity runs but before gh release create --target, the command can use an existing tag that does not point to the validated SOURCE_COMMIT. The Draft can therefore advertise or publish Desktop, CLI, and source artifacts built from SOURCE_COMMIT under a tag resolving to different code.
Agent Prompt
## Issue description
The release workflow checks tag availability early but does not reserve the tag. If the tag is externally created or moved during the build, `gh release create --target` does not correct the existing tag target, allowing the Draft to publish artifacts built from the validated `SOURCE_COMMIT` under a tag that resolves to different code.
## Issue Context
The `release-identity` job derives and validates the intended source SHA, while `publish` creates the Draft only after the long-running artifact jobs. Create the tag ref explicitly and atomically for the validated source SHA, fail if it already exists, and create the GitHub Release using tag verification rather than relying on `--target`.
## Fix Focus Areas
- .github/workflows/release.yml[55-67]
- .github/workflows/release.yml[393-414]
- .github/workflows/release.yml[410-412]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Thank you for carrying the CLI artifact work onto the unified release boundary. #3222 explicitly rebuilds this goal from current main and supersedes #3002, while preserving the useful packaging and verification work under one authoritative product version, tag, release gate, and Draft GitHub Release. This PR is now superseded and can be closed so review can continue on the single current implementation in #3222. Thanks again for establishing much of the release identity, artifact verification, and toolchain groundwork that the replacement carries forward. 中文感谢你把 CLI artifact 工作推进到统一 release boundary。#3222 已明确基于 current main 重建该目标并 supersede #3002，同时把有价值的 packaging 与 verification 工作收敛到一个权威的 product version、tag、release gate 和 Draft GitHub Release 下。 此 PR 现在已被 supersede，可以关闭，让 review 集中在唯一的当前实现 #3222 上。再次感谢你打下的 release identity、artifact verification 和 toolchain 基础，这些工作已经被后续方案继承。 |
Summary
packageManagerpin the single npm authority for every release job and all nested packaging commandsmakalauncher, pinned official Node, manifest-derived production closure, CLI-specific notices, and explicitly declared@maka/evalHarbor runtime assetsmaka eval runFixes #1510
Verification
npmcallsnpm run buildnpm run test:release— 18 passednpm --workspace @maka/eval run test:dist— 31 Node tests passed; Python suites passed with environment-dependent skipsnpm --workspace maka-agent run test:dist— 260 passednpm run check:releasenpm run lintnpm run format:checknpm run typechecknpx knip --workspace apps/desktopnpx knip --workspace packages/uinpm audit --omit=dev --audit-level=moderate— 0 production vulnerabilitiesMaka-0.1.10-cli-mac-arm64.zipon Apple Silicon1fd600695b4d5230e3fc2a037c9f58f2df44877523805fff64c2e471da6dcf63RELEASE.json.sourceCommitmatched083d006c836206674f1da4976d134d19d79ecd5amaka eval runthrough its stagedrun_trial.pyandrelay_agent.pymain(62cded221b9b351fcc3ac97a923c33d7871b0cc9) was conflict-free and passed build, format, lint, typecheck, 18/18 release tests, 52/52 Eval tests plus Python suites, 261/261 CLI tests, production audit, Apple Silicon packaging, and the independent artifact verifierThe development archive was structurally and behaviorally verified. Protected Developer ID signing/notarization, browser-download Gatekeeper checks, separate-machine acceptance, Windows real-machine packaging, and a real provider call remain release-environment gates.
Review focus
Please review the release identity boundary, pinned npm bootstrap/assertion boundary, required-artifact dependency graph, public
bin/maka/RELEASE.jsoncontract, validated workspace release-file declarations, dependency-notice closure, extracted-artifact eval smoke, and signing → notarization → checksum ordering.Material implementation was prepared with Codex and reviewed by the human contributor before submission. The final commits contain the required
Generated-by: Codextrailer.Checklist
Does this PR entail a change in behavior?