fix(build): drop --bytecode from compiled-binary build#1354
Merged
Conversation
Bun 1.3.11's bytecode generation produces broken output for our
module graph ("TypeError: Expected CommonJS module to have a function
wrapper" at runtime, reproduces natively on darwin-arm64 too). The
compile already fails with "Failed to generate bytecode for ./cli.js"
and proceeds without it, but the resulting binary still crashes at
module instantiation.
Only --minify remains. Binary size is unchanged in practice
(bytecode was being skipped after the error anyway), but now the
build succeeds cleanly and the binary runs.
Surfaced while building the v0.3.7 release.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe build script simplifies its bytecode handling by unconditionally removing the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 tasks
Wirasm
added a commit
that referenced
this pull request
Apr 22, 2026
…mport From the multi-agent review on #1355: - Add CHANGELOG.md entries under [Unreleased] ### Fixed for this PR's Pi lazy-load fix and the --bytecode removal from #1354 — both user-visible fixes (compiled binary unusable without them). - Rewrite the test-header docstring in provider-lazy-load.test.ts to describe counter-based detection instead of "mocks throw" (contradicted the actual code directly below it). - Tighten `lookupPiModel`'s first parameter from `unknown` to a local `GetModelFn` alias, moving the runtime-string cast to the single call site with a pointer to the docblock. - Update the class docblock on `PiProvider` — "v1 capabilities are all false" was stale; PI_CAPABILITIES has seven flags set to true. - `ui-context-stub.ts` imports `Theme` as a non-type value even though every usage is in a type position. Fold it into the existing `import type {…}` block so a future runtime-class `Theme` in Pi can't reintroduce an eager module load via this sibling. No behavior change. Type-check, lint, format, tests, and a local darwin-arm64 compile + version smoke all clean.
Wirasm
added a commit
that referenced
this pull request
Apr 22, 2026
…#1355) * fix(providers/pi): lazy-load Pi SDK to unbreak compiled archon binary Pi's @mariozechner/pi-coding-agent/dist/config.js runs `readFileSync(getPackageJsonPath(), 'utf-8')` at module top level. Inside a compiled archon binary `getPackageJsonPath()` resolves to `dirname(process.execPath) + '/package.json'`, which doesn't exist next to `/usr/local/bin/archon` — so archon crashes with ENOENT at startup before any command runs. v0.3.7's release binary build appeared to compile clean (CI fell over first on an unrelated --bytecode issue) but even the fixed-bytecode binary fails the same way locally. Convert all Pi SDK value imports and Pi-dependent helper imports to dynamic imports inside `PiProvider.sendQuery()`. Type-only imports stay static (erased by TS). Effect: registering the Pi provider and creating an instance no longer loads Pi's SDK — load happens only when a Pi workflow actually runs. Claude and Codex providers keep their static import style (their SDKs have no module-init side effects that fail in a binary); see the file header comment in provider.ts for why Pi is the deliberate outlier. Class constructors (AuthStorage, ModelRegistry, SettingsManager) are accessed via `piCodingAgent.X` rather than destructured to keep eslint's naming-convention rule happy without a disable. Add a regression test (provider-lazy-load.test.ts) that mocks @mariozechner/pi-coding-agent and @mariozechner/pi-ai, walks the same registerCommunityProviders() → getAgentProvider('pi') path the CLI and server take, and asserts neither SDK module was loaded. Runs in its own `bun test` invocation (mock.module is process-wide). Verified locally: `bun build --compile --minify --target=bun-darwin-arm64` produces a binary whose `archon version` runs cleanly and reports Build: binary, where previously every command crashed at boot. * address review: changelog, docstring, type tighten, Theme type-only import From the multi-agent review on #1355: - Add CHANGELOG.md entries under [Unreleased] ### Fixed for this PR's Pi lazy-load fix and the --bytecode removal from #1354 — both user-visible fixes (compiled binary unusable without them). - Rewrite the test-header docstring in provider-lazy-load.test.ts to describe counter-based detection instead of "mocks throw" (contradicted the actual code directly below it). - Tighten `lookupPiModel`'s first parameter from `unknown` to a local `GetModelFn` alias, moving the runtime-string cast to the single call site with a pointer to the docblock. - Update the class docblock on `PiProvider` — "v1 capabilities are all false" was stale; PI_CAPABILITIES has seven flags set to true. - `ui-context-stub.ts` imports `Theme` as a non-type value even though every usage is in a type position. Fold it into the existing `import type {…}` block so a future runtime-class `Theme` in Pi can't reintroduce an eager module load via this sibling. No behavior change. Type-check, lint, format, tests, and a local darwin-arm64 compile + version smoke all clean.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bun 1.3.11's bytecode generation produces broken output for our module graph. At build time, it logs
error: Failed to generate bytecode for ./cli.jsand proceeds without bytecode; the resulting binary then crashes at module instantiation with:```
TypeError: Expected CommonJS module to have a function wrapper.
If you weren't messing around with Bun's internals, this is a bug in Bun.
```
This reproduces on every target (darwin-arm64, darwin-x64, linux-x64, linux-arm64) — verified natively on darwin-arm64. Without the flag, the compile succeeds cleanly and the binary runs.
Windows was already excluded from
--bytecode; this change just aligns every other target with that behaviour.Why it wasn't caught before
The flag was only failing loudly once our module graph included
@mariozechner/pi-coding-agent(0.3.7 via #1270). Earlier releases happened to tree-shake to something Bun's bytecode step could handle.Surfaced by
Found while building the v0.3.7 release locally after the release CI failed with the same error. Still doesn't make v0.3.7 binaries viable on its own — the Pi SDK separately crashes at module init with an ENOENT on
package.json— but this change is correct independent of that and unblocks the compile step.Test plan
bun run validatepassesbash scripts/build-binaries.shcompletes without the "Failed to generate bytecode" error on all targets./dist/binaries/archon-darwin-arm64 versionruns (on macOS) — not blocked by bytecode, at least; downstream Pi issue tracked separatelySummary by CodeRabbit