Skip to content

fix(coding-agent): handled <bunfs-root>/<binary> in __computeBunfsPackageRoot#3330

Merged
can1357 merged 5 commits into
mainfrom
farm/4a0fd941/fix-bunfs-root-homebrew-arm64
Jun 23, 2026
Merged

fix(coding-agent): handled <bunfs-root>/<binary> in __computeBunfsPackageRoot#3330
can1357 merged 5 commits into
mainfrom
farm/4a0fd941/fix-bunfs-root-homebrew-arm64

Conversation

@roboomp

@roboomp roboomp commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Repro

On the Homebrew-installed darwin-arm64 compiled binary (omp/16.1.15, Bun 1.3.14), Bun reports import.meta.dir as //root/omp-darwin-arm64 for the compiled entry — the bunfs mount root followed by the binary's basename, not the bare bunfs root previous Bun versions emitted. Driving the pre-fix __computeBunfsPackageRoot against that input produces /root/omp-darwin-arm64/packages instead of //root/packages, so the typebox / legacy-pi-ai / legacy-pi-coding-agent shim paths land on /root/omp-darwin-arm64/packages/coding-agent/src/extensibility/*.js (which does not exist in the bunfs). Every override is dropped by __validateLegacyPiPackageRootOverrides, resolveCanonicalPiSpecifier falls through to a bunfs Bun.resolveSync that also fails, and every @oh-my-pi/pi-* extension (pi-dynamic-workflows, pi-lens, pi-subagents, …) is silently rejected with Cannot find module '@oh-my-pi/pi-coding-agent' from '<plugin>'.

$ bun run repro-3329.ts
observed import.meta.dir: "//root/omp-darwin-arm64"
pre-fix BUNFS_PACKAGE_ROOT: "/root/omp-darwin-arm64/packages"
expected:                   "//root/packages"
typebox shim path (pre-fix): /root/omp-darwin-arm64/packages/coding-agent/src/extensibility/typebox.js
typebox shim path (expected): /root/packages/coding-agent/src/extensibility/typebox.js
exit=1

Cause

packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts __computeBunfsPackageRoot only knew two import.meta.dir shapes: the bunfs root (/$bunfs/root / <drive>:\~BUN\root) and the deep plugins source path. The new Bun 1.3.x shape <bunfs-root>/<binary-basename> fell through the suffix branch and into pathImpl.join(metaDir, "packages"), which appended "packages" after the binary segment.

A naive fix using path.join on path.dirname(metaDir) also fails on POSIX: Bun's bunfs identifier //root collapses to /root under path.posix.join, and Bun's bunfs lookup is keyed on the exact //root form.

Fix

  • Added a third branch to __computeBunfsPackageRoot in legacy-pi-compat.ts that recognises <bunfs-root>/<binary> by checking whether the normalized parent ends in the root segment (path.basename(path.normalize(path.dirname(metaDir))) === "root").
  • The branch slices the original metaDir via path.dirname(metaDir) + path.sep + "packages" rather than path.join, so the bunfs-native //root / B:\~BUN\root prefix survives verbatim.
  • The existing single-segment-root and deep-plugins branches are untouched; only the previously-uncovered third shape changes.
  • Module docstring updated to enumerate all three observed import.meta.dir shapes and the reason the strip path avoids path.join.
  • Regression coverage added in test/extensibility/legacy-pi-bunfs-root.test.ts for the POSIX //root/<bin>, POSIX /$bunfs/root/<bin>, and Win32 <drive>:\~BUN\root\<bin>.exe inputs, plus the original repro path //root/omp-darwin-arm64 → //root/packages from this issue.
  • Changelog entry under [Unreleased] → Fixed in packages/coding-agent/CHANGELOG.md.

Verification

bun test packages/coding-agent/test/extensibility/legacy-pi-bunfs-root.test.ts — 7 pass, 0 fail (the four pre-existing shapes plus the new <bunfs-root>/<binary> case). The pre-publish gate (bun run fix + bun check) ran clean as part of gh_push_branch. The neighbouring legacy-pi-ai-type-remap.test.ts / custom-commands/ci-green.test.ts failures observed locally are pre-existing on main (missing ./tool-views.generated.js build artifact in dev mode) and unrelated to this diff. Fixes #3329.

roboomp added 2 commits June 23, 2026 15:56
…ackageRoot

Bun 1.3.14 reports `import.meta.dir` as `<bunfs-mount>/<binary-basename>` for
the compiled entry on some hosts — e.g. the Homebrew darwin-arm64 build sees
`//root/omp-darwin-arm64` instead of the bunfs root alone. The pre-fix path
joined `metaDir` with `"packages"` and baked the binary basename into every
bunfs path, so the typebox / legacy-pi shim overrides failed `existsSync`
validation, `resolveCanonicalPiSpecifier` fell through to a bunfs
`Bun.resolveSync` that also could not find the module, and every third-party
`@oh-my-pi/pi-*` extension was silently dropped.

`__computeBunfsPackageRoot` now detects the trailing binary-basename segment
(`path.basename(path.dirname(metaDir)) === "root"`) and strips it off the
original `metaDir` via string slicing rather than `path.join`, so Bun's
bunfs-native `//root` and `B:\~BUN\root` prefixes survive verbatim
(`path.posix.join` would collapse `//root` to `/root`). The single-segment
`<bunfs-root>` and deep `<bunfs>/packages/coding-agent/src/extensibility/plugins`
paths keep their existing branches.

Regression test added in `legacy-pi-bunfs-root.test.ts` for the POSIX
`//root/<bin>`, POSIX `/$bunfs/root/<bin>`, and Win32 `<drive>:\~BUN\root\<bin>.exe`
shapes.

Fixes #3329

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5766952c20

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}
const parent = pathImpl.dirname(metaDir);
if (pathImpl.basename(pathImpl.normalize(parent)) === "root") {
return `${parent + pathImpl.sep}packages`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve double-slash bunfs paths beyond the root helper

For the //root/<binary> case this line returns //root/packages, but production immediately feeds BUNFS_PACKAGE_ROOT into bunfsPath(), which calls path.join(BUNFS_PACKAGE_ROOT, ...); on POSIX path.join("//root/packages", "coding-agent") normalizes back to /root/packages/coding-agent. That means the Homebrew/Bun 1.3.14 scenario this branch targets still builds shim and package-root override paths outside Bun's //root bunfs mount, so override validation drops them and extension loading continues to fail; the added test only checks the helper return value and misses the joined production paths.

Useful? React with 👍 / 👎.

roboomp added 3 commits June 23, 2026 16:00
…brew

The reporter clarified that the failing binary is the pre-built
`omp-darwin-arm64` release asset from GitHub Releases; Homebrew is only a
local-tap wrapper that downloads that asset. The fix already covers every
cross-compiled `<bunfs-root>/<binary>` shape, but the source/test docstrings
and changelog blurb framed it as a Homebrew-build-specific bug. Updated those
three call sites to name the release asset and note the Homebrew tap as a
downstream consumer of the same binary; no code change.
`__computeBunfsPackageRoot` now returns `//root/packages` for the Bun 1.3.14
`//root/<binary>` import.meta.dir shape, but production immediately joined that
root with shim and package segments through `path.join`, which collapses the
POSIX double-slash bunfs mount back to `/root`. That still made override
validation miss the embedded shim files.

Added a bunfs join helper that preserves the `//root` mount prefix after joining
production descendants, wired `bunfsPath` through it, and extended the #3329
regression test to assert the full typebox shim path stays under
`//root/packages/...`.

Fixes #3329
@roboomp

roboomp commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed in 57b4ee1eaf94:

  • Added __joinBunfsPath() and routed production bunfsPath() through it so descendant shim/override paths preserve Bun’s POSIX //root bunfs mount instead of collapsing through path.join.
  • Extended the bug: __computeBunfsPackageRoot fails on Homebrew arm64 binary (import.meta.dir = '//root/omp-darwin-arm64') #3329 regression test to assert the full production-style typebox shim path stays //root/packages/coding-agent/src/extensibility/typebox.js, not /root/packages/....
  • Updated the changelog entry to call out preserving the descendant join path, not only the computed package root.

@can1357 can1357 merged commit 93d730c into main Jun 23, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: __computeBunfsPackageRoot fails on Homebrew arm64 binary (import.meta.dir = '//root/omp-darwin-arm64')

2 participants