Skip to content

refactor(release): project manifests from shipped files - #4152

Merged
Astro-Han merged 1 commit into
apache:mainfrom
Astro-Han:refactor/release-prune-test-only-exports
Aug 29, 2026
Merged

refactor(release): project manifests from shipped files#4152
Astro-Han merged 1 commit into
apache:mainfrom
Astro-Han:refactor/release-prune-test-only-exports

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Keep the five ./test-only/* exports in source workspace manifests for tests and E2E, while deriving every shipped workspace manifest from one workspaceReleaseManifest authority tied to the existing Maka release file policy.

The same projection feeds npm CLI staging, standalone macOS CLI staging, and Electron's nested @maka manifests. It recursively projects the complete exports value leaf by leaf, preserving top-level strings/arrays and nested conditional objects/arrays: excluded type/test targets disappear, while shipped runtime/default branches remain importable. Source manifests are never modified.

This replaces two old production paths instead of layering beside them: the npm CLI's inline manifest allowlist is deleted, and standalone staging filters development artifacts during copy instead of recursively pruning the copied tree afterward. Electron-builder remains the only ASAR and integrity writer.

The rewrite moves this PR from its initial +387/-5 implementation to +178/-39. The 39 deletions include 36 lines of replaced production code; the net addition is the shared release boundary and contract coverage for all three consumers, including conditional exports.

Verification

  • TDD RED: standalone staging retained ./test-only/fixture and scripts.build before the shared manifest projection
  • TDD RED: conditional export projection returned {} for a root containing excluded types and shipped default leaves
  • TDD RED: the outer Object.fromEntries converted a legal top-level export fallback array into an invalid numeric-key object
  • TDD RED: Electron's normalized FileSet plus an appended negative string created a new matcher that re-included excluded files
  • node --test scripts/release-cli-file-policy.test.mjs scripts/product-release.test.mjs scripts/desktop-nightly.test.mjs (46 passed)
  • npm run build
  • npm run format:check
  • npm run lint
  • npm run check:stale
  • npm run release:cli:pack -- --development; inspected and installed maka-agent-0.2.0-dev-cdff917c56ff.tgz, then successfully executed import('@maka/eval')
  • Staged the actual standalone workspace tree, ran its production npm ci (190 packages), successfully executed import('@maka/eval'), and confirmed the source Eval manifest remained byte-identical
  • Resolved all five source workspace test/E2E specifiers with import.meta.resolve
  • Reproduced the failed Windows verifier locally: the old hook reintroduced 269 main tests and 260 renderer side-files; the fixed electron-builder ASAR contains zero of both while preserving all three projected nested manifests

Signed DMG/ZIP and Windows packaging were not run; the macOS directory build intentionally disabled signing while retaining the production ASAR path.

Adversarial review

Three independent collaboration reviewers examined the shared authority, all three packaging paths, tests, and PR #3946 integration. No P0/P1 remained. One valid exports-shape gap was reproduced with a failing test and fixed by projecting the complete exports target, which also removed the outer object-only projection path. Suggestions to duplicate the five-entry rule in the tarball verifier or reintroduce custom ASAR parsing were rejected because they would create parallel authorities.

A follow-up packaging review examined the Windows verifier regression and the final delta. It found no P0-P3 issue: the package-level exclusion now remains in Electron's primary FileSet filter, and the staged manifest overlay is the only matcher appended at beforePack.

Review focus

This branch is based on origin/main at afc1a750b. PR #3946 remains open at 76d4f20262 and overlaps scripts/release-cli-file-policy.mjs, scripts/release-cli-file-policy.test.mjs, and scripts/release-cli-package.mjs. The current three-way merge has content conflicts in the test and package script because both PRs change the CLI staging boundary. Whichever PR lands second must manually preserve whole-value leaf-by-leaf conditional export projection and rerun the release suite; this PR adds no compatibility branch for a future merge.

The Desktop ASAR still contains some fixture file bodies under the existing Desktop file set. This PR intentionally changes nested manifest advertising only; Desktop artifact slimming is a separate file-policy decision.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex traced the release ownership boundary, replaced duplicate npm CLI and standalone paths with one shared release-manifest projection, preserved conditional runtime export branches, staged Electron manifests before packaging, and added and ran contract and artifact checks. Collaboration reviewers performed independent adversarial checks.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 29, 2026
@Astro-Han
Astro-Han force-pushed the refactor/release-prune-test-only-exports branch 2 times, most recently from d848bdb to fc9624c Compare August 29, 2026 10:28
@Astro-Han
Astro-Han marked this pull request as ready for review August 29, 2026 10:33
@Astro-Han
Astro-Han marked this pull request as draft August 29, 2026 10:39
@Astro-Han
Astro-Han force-pushed the refactor/release-prune-test-only-exports branch 2 times, most recently from 603824e to e96352c Compare August 29, 2026 10:52
@Astro-Han
Astro-Han marked this pull request as ready for review August 29, 2026 10:55

@ARE404 ARE404 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving refactor(release): project manifests from shipped files at head e96352c3.

I reviewed the 6 changed files and the shared workspaceReleaseManifest / releaseExportTarget logic end to end, and verified the release-scope behavior:

  • The pruning correctly removes src, test-only, dev-directory, .d.ts/.js.map, and *.test(.js) targets from release exports, while preserving every runtime branch (import/require/default pointing at dist).
  • Reachability holds: I grepped the shipped (non-test) codebase for imports of @maka/{mcp,runtime,runtime-host}/test-only/* and other pruned targets; only e2e and test-only modules reference them, so no packaged runtime branch is made unreachable. This matches the stated intent in the PR.
  • The source manifests are not mutated (projection builds fresh objects/arrays), covered by a test; the filter-during-copy prunes the same set the old post-copy prune removed.
  • Tests added for leaf-by-leaf recursion, array targets, Electron staging, and standalone/npm consistency are behavior-accurate.

Two non-blocking [P3] notes are inline: Electron staging hardcodes only three packages for manifest pruning (npm/standalone prune all), and the .d.ts/.js.map pruning intentionally drops types and source maps for shipped dist (recorded as intended).

Note on CI: at review time the test and package checks were still settling on this head (the addon builds and label had passed); I'm approving on the code findings (no P0/P1) as requested, not on a claimed green test.

简体中文

批准 refactor(release): project manifests from shipped files,head e96352c3
核查了 6 个改动文件与 workspaceReleaseManifest/releaseExportTarget 全链路:release 导出裁剪正确移除 src/test-only/dev 目录/.d.ts/.js.map/.test 目标,保留所有指向 dist 的运行时分支(import/require/default)。
可达性成立:grep 全部非测试代码,只有 e2e 和 test-only 模块引用被裁掉的目标,打包产物没有运行时分支变不可达;源 manifest 不被修改(有测试覆盖);filter 拷贝与旧的拷贝后裁剪等价。
两条 P3 行内:Electron 打包仅硬编码三个包做 manifest 裁剪(npm/standalone 裁剪全部);.d.ts/.js.map 裁剪会去掉发布的类型与 source map(属预期,记录为刻意行为)。
CI 备注:approve 时 test/package 尚未终态(addon 构建与 label 已过);我基于代码结论(无 P0/P1)批准,并非声称 test 已绿。

const stage = await packager.info.tempDirManager.createTempDir({
prefix: 'maka-release-manifests',
});
for (const name of ['mcp', 'runtime', 'runtime-host']) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] The Electron staging path prunes release manifests only for mcp, runtime, and runtime-host. The npm-CLI and standalone paths prune every workspace, so the Electron asar keeps whatever exports each other @maka/* package ships as-is, including any ./test-only/* branches. If the desktop depends on a package other than these three that exposes test-only exports, they will ride into the archive. Consider deriving the full set of shipped @maka package names (e.g. from the desktop's dependencies) instead of a hardcoded triple, so staging stays consistent with the other two packaging paths.


function releaseExportTarget(target) {
if (typeof target === 'string') {
return isMakaDevelopmentArtifact(target) ? undefined : target;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Leaf-level pruning also drops *.d.ts(map) and *.js.map targets from the release exports and strips source maps from copied dist. For CLI/internal packages that don't ship types this is intended, but the reachability consequence is worth an explicit note: production stack traces will no longer be source-mapped, and any consumer expecting types from @maka/* will not find them in the release. I verified no shipped (non-test) module imports a pruned branch (only e2e/test-only code references test-only/*), so this is not a correctness break today — just record the intended surface.

Keep test-only exports in source workspaces while deriving one release manifest shape for npm CLI, standalone CLI, and Electron staging. Project conditional exports leaf by leaf so shipped runtime branches remain importable.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the refactor/release-prune-test-only-exports branch from e96352c to 60e03b7 Compare August 29, 2026 11:16

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head 60e03b76 (verified unchanged at review time; rebased onto current main since the previous push, with the exclusion moved from the beforePack push into the static files list — I re-verified the amended commit).

Single-authority manifest projection, verified rather than assumed:

  • One allowlist, three consumers: workspaceReleaseManifest in release-cli-file-policy.mjs replaces the npm CLI's inline allowlist (byte-identical field set — verified against the deleted code), becomes the standalone staging manifest writer (previously a verbatim copy), and feeds the new Desktop beforePack staging for the nested @maka/{mcp,runtime,runtime-host} manifests. No parallel authorities added.
  • Exports projection is leaf-by-leaf: strings, top-level arrays, and nested conditional objects are all handled; excluded types/test-only targets drop out while shipped runtime/default branches survive, and the array-before-object ordering means a legal top-level fallback array stays an array. The tests pin exactly the five real ./test-only/* subpaths across the three shipped packages and prove the source manifests stay byte-identical.
  • Dropped fields are unreachable in every consumer: I enumerated the staged closure (@maka/core, runtime, storage, runtime-host, eval, mcp, maka-agent) — the fields the projection drops are private, scripts (no lifecycle scripts exist in any of these packages), devDependencies, types, releaseFiles (repo-side policy input, read from the source manifest before projection), and maka. The maka.managedRuntimeHostUpdateCompatibility field is read at runtime only from npm-tarball deployments (writeReleaseManifest re-adds it and throws if missing), never from the standalone staging path; the CLI's own version read needs only version, which the allowlist keeps. No code reads the nested @maka/*/package.json files in the Desktop bundle.
  • Copy-time filter equals post-copy prune: the standalone staging cp filter computes the same isMakaDevelopmentArtifact predicate on the same relative paths as the deleted pruneMakaDevelopmentArtifacts walk — same result, one less tree walk, and nothing can slip through between copy and prune anymore.

Executed on a real Windows machine at this head: release-cli-file-policy + product-release + desktop-nightly suites, 46/46 pass (including the new pins for conditional-export projection, top-level array preservation, and Desktop manifest staging). CI checks were still running at approval time.

简体中文

单一权威清单投影,全部核实而非假设:workspaceReleaseManifest 一处定义、三个消费方(npm CLI / standalone / Desktop beforePack),allowlist 与被删的内联版本逐字段一致;exports 逐叶投影,字符串/顶层数组/嵌套条件对象都正确处理;被丢的字段(private/scripts/devDependencies/types/releaseFiles/maka)逐一追过消费面,确认三条发布路径上都不可达(maka.managedRuntimeHostUpdateCompatibility 只从 npm tarball 部署读取,writeReleaseManifest 单独保留)。standalone 的 copy 期 filter 与被删的事后 prune 谓词和相对路径完全一致。本机真 Windows 三个套件 46/46 过;批准时 CI 仍在跑。

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found no P0–P3 issues on exact head 60e03b76287724760cf3ea73e5b4df2522a73adf.

The five ./test-only/* export keys live only on @maka/mcp, @maka/runtime, and @maka/runtime-host. workspaceReleaseManifest projects each exports target leaf by leaf against the existing Maka artifact filter, so those keys disappear from a shipped manifest while default branches remain. npm CLI staging, standalone macOS CLI staging, and Electron beforePack all call that same function. Source package.json files are not rewritten. Top-level export arrays stay arrays. Electron excludes those three nested manifests in the static files list and overlays the projected copies from a temp dir; it does not push a glob onto electron-builder's FileSet at pack time.

I did not treat the PR body as evidence. The file-policy and product-release tests can show the claim false: the five keys are absent from the projected maps, a mixed types/default object keeps only default, a top-level fallback array does not become a numeric-key object, Electron stages those three nested manifests without ./test-only/, and standalone copy drops the matching files.

I am not merging. This is a release-packaging refactor; merge is a human call. Hosted test was not re-checked after the rebase onto current main. This review does not claim CI is green. PR #3946 overlaps the CLI staging scripts; whichever lands second has to keep this whole-value projection.


Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

精确 head 60e03b76287724760cf3ea73e5b4df2522a73adf 上我没有发现 P0–P3。

五个 ./test-only/* export 只出现在 @maka/mcp@maka/runtime@maka/runtime-hostworkspaceReleaseManifest 按现有 Maka 制品过滤规则逐叶投影 exports,这些 key 从发版 manifest 里消失,default 分支留下。npm CLI、独立 macOS CLI、Electron beforePack 都走同一个函数。源仓库的 package.json 不会被改写。顶层 export 数组仍是数组。Electron 在静态 files 里排除那三个嵌套 manifest,再用临时目录里的投影覆盖;打包时不会往 electron-builder 的 FileSet 上推 glob。

我没有把 PR 正文当证据。file-policy 和 product-release 测试能证伪:投影后五个 key 不在;types/default 混在一起时只留 default;顶层 fallback 数组不会变成数字 key 的对象;Electron 给那三个嵌套 manifest 去 ./test-only/;独立拷贝会丢掉对应文件。

我不合入。这是发版打包重构,合入由人类决定。rebase 到当前 main 之后我没有重读 hosted test,这次审查不表示 CI 已绿。#3946 和 CLI staging 脚本重叠;后合的那份必须保住这次的整值投影。

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@Astro-Han
Astro-Han merged commit fdef796 into apache:main Aug 29, 2026
13 checks passed
saltand pushed a commit to saltand/maka-agent that referenced this pull request Aug 31, 2026
Keep test-only exports in source workspaces while deriving one release manifest shape for npm CLI, standalone CLI, and Electron staging. Project conditional exports leaf by leaf so shipped runtime branches remain importable.

Generated-by: Codex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants