Repository navigation
build(npm): declare libc on platform packages, drop linux-x64 AVX2 variants, add files allowlist - #231
Conversation
…riants, add files allowlist Bun 1.4.3 ships the same binary for its baseline and AVX2 x64 targets, and the 0.5.1 AVX2 and -baseline platform packages hold byte-identical binaries on linux, linux-musl, darwin and windows. Build and publish only the -baseline package on each x64 OS (7 platform packages instead of 11), and declare libc on the linux packages so npm installs one of them instead of all four linux-x64 packages (459 MB -> 116 MB glibc, 110 MB musl). The wrapper drops AVX2 detection and reads the libc from /usr/bin/ldd, falling back to Node's diagnostic report, instead of spawning `ldd --version`. With no matching binary it names the expected package instead of trying `bun run src/index.ts`. A files allowlist limits @aictrl/cli to bin/aictrl, README.md, LICENSE and package.json. The publish smoke job now waits until every published platform package is visible on npm, with bounded retries, before the install check. Target: 0.5.3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nwvfUHdM3bgMgD9Zpz7Mj
![aictrl-dev[bot]](https://avatars.githubusercontent.com/in/2817739?s=60&v=4){kind=small}
| "bin": { | ||
| "aictrl": "./bin/aictrl" | ||
| }, | ||
| "files": [ |
There was a problem hiding this comment.
🟡 files allowlist drops src/ but exports still maps to it.
🤖 Fix with your agent
Fix this code review finding (aictrl-dev/cli PR #231, packages/cli/package.json:33-39):
Problem: files allowlist drops src/ but exports still maps to it
Detail: The new `files` allowlist ships only bin/aictrl, README.md and LICENSE (plus the always-included package.json), but the manifest keeps `"exports": { "./*": "./src/*.ts" }`. In the published tarball every subpath export now resolves to a file that is no longer shipped, so the package advertises a public interface that 404s with ERR_MODULE_NOT_FOUND; up to 0.5.1 `src/` was published and the map worked. Either the exports map should be stripped from the published manifest, or the breaking drop of `@aictrl/cli/<subpath>` deep imports should be called out in the changelog entry about the wrapper-only package.
Suggested fix: The exports map is presumably still needed for workspace/dev imports, so strip it in the publish workflow's manifest step (it already rewrites packages/cli/package.json before `fs.writeFileSync`): `delete pkg.exports` (and any src-pointing `main`), so the published wrapper-only manifest no longer advertises unpublished paths. Alternatively, document the removal of `@aictrl/cli/*` subpath imports as a breaking change in the changelog.
Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters
The new files allowlist ships only bin/aictrl, README.md and LICENSE (plus the always-included package.json), but the manifest keeps "exports": { "./*": "./src/*.ts" }. In the published tarball every subpath export now resolves to a file that is no longer shipped, so the package advertises a public interface that 404s with ERR_MODULE_NOT_FOUND; up to 0.5.1 src/ was published and the map worked. Either the exports map should be stripped from the published manifest, or the breaking drop of @aictrl/cli/<subpath> deep imports should be called out in the changelog entry about the wrapper-only package.
"bin": {
"aictrl": "./bin/aictrl"
},
"files": [
"bin/aictrl",
"README.md",
"LICENSE"
],
"exports": {
"./*": "./src/*.ts"
},There was a problem hiding this comment.
FALSE · IGNORE (head 7e556c2). The published manifest has no exports. The publish workflow's manifest step already runs delete pkg.exports; before fs.writeFileSync (.github/workflows/publish.yml:137), as it has since before this PR. The exports map stays only in the workspace manifest, for dev imports. Evidence: the key list of package/package.json in the npm pack tarball produced by that step on this branch is $schema,version,name,description,license,publishConfig,homepage,repository,private,scripts,bin,files,optionalDependencies, with no exports, main or type. No published path points into src/.
|
|
||
| return true | ||
| }) | ||
| ? allTargets.filter( |
There was a problem hiding this comment.
🟡 --single on a musl host builds a glibc-labelled package.
| ? allTargets.filter( | |
| Detect the host libc (the same /usr/bin/ldd content read that bin/aictrl's linuxLibc uses) and keep the matching linux target for --single, or fail fast with a clear error ("--single is unsupported on musl hosts; build all targets") so a mislabelled glibc artifact is never produced. |
🤖 Fix with your agent
Fix this code review finding (aictrl-dev/cli PR #231, packages/cli/script/build.ts:43-47):
Problem: --single on a musl host builds a glibc-labelled package
Detail: The `--single` filter keeps only the target with `item.libc !== "musl"`, so on a musl host (e.g. Alpine) the single build puts the musl-linked Bun-compiled binary into the package named `@aictrl/cli-<os>-<arch>[-baseline]` whose generated manifest now declares `libc: ["glibc"]` — the new explicit libc field makes the artifact's name and metadata contradict its content. Before this PR the same package carried no libc field, so npm on musl hosts would still install it and it worked. CI publish builds all targets via cross-compilation on glibc runners, so only local `--single` builds on musl hosts are affected.
Suggested fix: Detect the host libc (the same /usr/bin/ldd content read that bin/aictrl's linuxLibc uses) and keep the matching linux target for --single, or fail fast with a clear error ("--single is unsupported on musl hosts; build all targets") so a mislabelled glibc artifact is never produced.
Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters
The --single filter keeps only the target with item.libc !== "musl", so on a musl host (e.g. Alpine) the single build puts the musl-linked Bun-compiled binary into the package named @aictrl/cli-<os>-<arch>[-baseline] whose generated manifest now declares libc: ["glibc"] — the new explicit libc field makes the artifact's name and metadata contradict its content. Before this PR the same package carried no libc field, so npm on musl hosts would still install it and it worked. CI publish builds all targets via cross-compilation on glibc runners, so only local --single builds on musl hosts are affected.
// A single build (CI, local) is the host's glibc package.
const targets = singleFlag
? allTargets.filter(
(item) => item.os === process.platform && item.arch === process.arch && item.libc !== "musl",
)
: allTargetsThere was a problem hiding this comment.
FALSE · IGNORE (head 7e556c2). The Bun runtime comes from the compile target, not from the host. script/build.ts:75 derives target from the package name, so @aictrl/cli-linux-x64-baseline always compiles against bun-linux-x64-baseline (the glibc runtime). Only a -musl name selects the musl runtime. On a musl host Bun downloads the glibc runtime for that target, so the binary matches its libc: ["glibc"] label. Evidence: the 0.5.1 packages were built on the same glibc runner, and their ELF interpreters follow the name: cli-linux-x64 → /lib64/ld-linux-x86-64.so.2, cli-linux-x64-musl → /lib/ld-musl-x86_64.so.1 (readelf -l). --single building the glibc package on a musl host is unchanged from before this PR, which also skipped the abi builds for --single. CI and publish run on glibc.
|
|
||
| const singleFlag = process.argv.includes("--single") || process.env.AICTRL_BUILD_SINGLE === "true" | ||
| const baselineFlag = process.argv.includes("--baseline") | ||
| const liteFlag = process.argv.includes("--lite") |
There was a problem hiding this comment.
⚪ Removed --baseline flag is still accepted silently.
| const liteFlag = process.argv.includes("--lite") | |
| Fail fast on removed flags: `if (process.argv.includes("--baseline")) { console.error("--baseline is no longer needed: every x64 build is the baseline build"); process.exit(1) }` — or at minimum print a warning. |
🤖 Fix with your agent
Fix this code review finding (aictrl-dev/cli PR #231, packages/cli/script/build.ts:18):
Problem: Removed --baseline flag is still accepted silently
Detail: `const baselineFlag = process.argv.includes("--baseline")` was removed but nothing rejects it: an existing invocation like `./packages/cli/script/build.ts --single --baseline` (valid up to 0.5.2) still exits 0 while the flag is silently ignored. It happens to produce the same artifact now that baseline is the only x64 build, but a stale flag being accepted without error hides that the script's contract changed.
Suggested fix: Fail fast on removed flags: `if (process.argv.includes("--baseline")) { console.error("--baseline is no longer needed: every x64 build is the baseline build"); process.exit(1) }` — or at minimum print a warning.
Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters
const baselineFlag = process.argv.includes("--baseline") was removed but nothing rejects it: an existing invocation like ./packages/cli/script/build.ts --single --baseline (valid up to 0.5.2) still exits 0 while the flag is silently ignored. It happens to produce the same artifact now that baseline is the only x64 build, but a stale flag being accepted without error hides that the script's contract changed.
const singleFlag = process.argv.includes("--single") || process.env.AICTRL_BUILD_SINGLE === "true"
const liteFlag = process.argv.includes("--lite")There was a problem hiding this comment.
FALSE (not a defect) · IGNORE (head 7e556c2). --single --baseline used to mean "build the host's x64 baseline package". That is now exactly what --single builds, so an old invocation still gets the artifact it asked for (@aictrl/cli-linux-x64-baseline). Failing the build would break working invocations without changing any output. The script also takes its other flags by process.argv.includes and does not reject unknown arguments in general. Adding a rejection only for this one flag would be a new contract, which is outside this packaging change.
Code reviewVerdict: Looks good — only minor / nit comments below. · 🔴 0 · 🟠 0 · 🟡 2 · ⚪ 1 · 0/3 resolved
🤖 Fix all 3 open findings with your agent📋 Out-of-diff findings (3)
Reviewed 9 files · 0 inline · view all 3 findings ↗ · AI-generated — verify findings before acting · AI guidance and feedback aictrl · AI code review for fast-moving teams · aictrl.dev |
|
Review round 1 (head
There are no code changes, so no re-review is requested. All checks are green: |
Target: 0.5.3
This is not part of the 0.5.2 release (#230). Merge it after #230, so the CHANGELOG entry stays under the next
Unreleasedand not under 0.5.2.Why
npm install -g @aictrl/clion linux-x64 installed all four linux-x64 platform packages (cli-linux-x64,-baseline,-musl,-baseline-musl), about 450 MB in total. All four declare the sameos/cpuand none declareslibc, yet the launcher runs only one of them. The wrapper package also shippedsrc/,test/, migrations and build scripts.The AVX2 split does not exist in the binaries. Bun 1.4.3 ships the same binary for its baseline and AVX2 targets. In the upstream release zips,
bun-{linux,linux-musl,darwin,windows}-x64and their-baselinebuilds have the same size and CRC. As a result, the published 0.5.1 AVX2 and-baselinepackages hold byte-identical binaries on all three x64 OSes (sha256):cli-linux-x64/-baselinea5ca26f4…cli-linux-x64-musl/-baseline-musl0386111b…cli-darwin-x64/-baseline50535347…cli-windows-x64/-baselined8e17f87…So dropping the AVX2 names changes nothing at runtime on any CPU, including darwin and windows. This also explains the earlier benchmark (425 vs 425 ms to first model request): it compared the same binary with itself.
What changes
This changes what is published.
cli-linux-x64,cli-linux-x64-baseline(nolibc)cli-linux-x64-baseline,libc: ["glibc"]cli-linux-x64-musl,cli-linux-x64-baseline-musl(nolibc)cli-linux-x64-baseline-musl,libc: ["musl"]cli-linux-arm64,cli-linux-arm64-musl(nolibc)libc: ["glibc"]/["musl"]cli-darwin-x64,cli-darwin-x64-baselinecli-darwin-x64-baselinecli-darwin-arm64cli-windows-x64,cli-windows-x64-baselinecli-windows-x64-baselineThat is 11 platform packages before and 7 after.
script/build.ts: builds 7 targets and writeslibc(pluslicenseandLICENSE) into each platformpackage.json. The--baselineflag is gone:--singlebuilds the host's glibc package, which on x64 is the baseline build.bin/aictrl(launcher):/proc/cpuinfo,sysctl, PowerShell) is removed./usr/bin/ldd(the same fast pathdetect-libcuses). The fallback isprocess.report(glibcVersionRuntime, or a musl loader insharedObjects), which is what npm itself uses forlibc. It no longer spawnsldd --version.libc.bun run src/index.tsfallback is removed. With no matching binary, the launcher exits 1 and names the expected package, for example@aictrl/cli-linux-x64-baseline@<version>, with a pinnednpm installcommand.required, so they can be unit-tested.packages/cli/package.json: addsfiles: ["bin/aictrl", "README.md", "LICENSE"]. The tarball goes from 369 files and 6.1 MB unpacked to 4 files and 18.4 kB.publish.yml:LICENSEand exports the list of platform packages as a job output.npm view <pkg>@<version> version --prefer-onlinefor every listed package. It makes 10 bounded attempts (30 s, then 60 s, 8.5 min in all, with a 15 min step timeout) and names the missing packages if any never appear. It runs before the install smoke.optionalDependenciesis still regenerated fromdist/, so it now has the 7 packages.Unreleasedhas a new Packaging section, which describes this as a change to what's published.--singleoutput path.Before / after
All runs are fresh
docker run --rm --cpus 2 --memory 2gcontainers on Node 20.20.2 with npm 10.8.2.npm install -g @aictrl/cli@0.5.1from npmjs.npm install -g @aictrl/cli@0.5.3-pkgtest.0from a local verdaccio, with every package published fromnpm packtarballs of this branch'sbun run build. The wrapper manifest was produced by running publish.yml's own manifest step.node:20-slim)node:20-alpine)-baseline)-baseline-musl)@aictrl/node -e 0startupaictrl --versionthrough the launcher vs bare binary²¹ The median of 100 interleaved runs, in two container runs per image. Each launcher runs a stub binary (
exit 0) from a full install tree, minus the cost of running the stub directly. Most of the launcher's cost is Node startup. Removing thelddspawn and the cpuinfo read saves about 2.5 ms on glibc. On Alpine the old launcher already stopped at/etc/alpine-release, so nothing changes there.² The median of 25 runs per install. The bare binary's own startup varies by more than 100 ms between containers, so this row is noise and doesn't measure the launcher.
The install time (40 to 54 s from npmjs before, 2 s from local verdaccio after) is not comparable, because the registries differ.
Container install evidence
aictrl runran against the scripted OpenAI-compatible stub (fixture/fixture, one text turn). The visibility gate also ran against the verdaccio registry:@aictrl/cli-linux-x64).Tests
test/cli/wrapper-binary-resolution.test.ts:lddscripts, the report fallback when there is noldd, andlibcFromReportcases.test/cli/publish-workflow.test.ts:optionalDependenciesand the job output, and copies LICENSE.Results:
bun run typecheckinpackages/clipassed.packages/clisuite passed: 1768 pass, 6 skip, 0 fail, 146 files.AICTRL_BUILD_SINGLE=true bun run buildbuilds onlycli-linux-x64-baseline.For the maintainer
npm deprecatefor the dropped names is not advisable now.@aictrl/cli≤ 0.5.2 still listscli-linux-x64,-linux-x64-musl,-darwin-x64and-windows-x64as optional dependencies. Deprecating them would print a warning on every install of those wrapper versions, including the pinned executor image. Old versions keep working. A deprecation could make sense later, once ≤ 0.5.2 is no longer installed. If so, use a message that points to the-baselinename. It has not been run.npm install -g. On glibc,node_modules/@aictrl/cli/node_modules/@aictrl/cli-linux-x64-baseline/bin/aictrlis still the path, and it is now the only linux-x64 package installed.installscript (it points atanomalyco/aictrland still probes AVX2), andpackages/cli/Dockerfile(dist/aictrl-*-litepaths).libcstill installs both libc packages. That is no regression from today.🤖 Generated with Claude Code
https://claude.ai/code/session_015nwvfUHdM3bgMgD9Zpz7Mj