install: name the path a workspaces glob fails on

[robobun]

Problem

• bun install fails when a workspaces glob reaches a directory that the user cannot open. The error does not name the directory, and a second line blames bun:

  error: Failed to run workspace pattern pkgs/* due to error EACCES
  error: An internal error occurred (GlobError)
  

• process_names_array (src/install/lockfile/Package/WorkspaceMap.rs) prints only the errno and returns Error::GlobError. The CLI has no case for that error, so handle_root_error adds the second line.
• The glob walker reports only package.json when the stat of a literal last component fails (src/glob/GlobWalker.rs). sync_lockfile (package_json_write_back.rs) drops the log of its parse, so bun add prints only the GlobError line.

Fix

• The install still fails, as with npm 11.16, yarn 1.22 and pnpm 12.4 (table in Notes). The message now gives the syscall and the absolute path: ... due to error EACCES (open "/repo/pkgs/secret").
• The glob error sites return InstallFailed, as the rest of process_names_array does after it logs. Nothing else used GlobError, so it is removed.
• The walker joins the directory onto the literal. sync_lockfile prints its log when its parse fails.
• Verified: bad-workspace.test.ts, scan.test.ts ("literal fast path"), and the bun-add, bun-update, bun-workspaces suites. Self-reviewed: 9 concerns, 4 addressed, 5 not caused by this change (Notes).

Background

• GlobWalker walks a workspaces glob with package.json appended, for example pkgs/*/package.json.
• Literal fast path: when only the last, literal component is left, the walker opens the matched directory and stats the literal in it.
• On InstallFailed the CLI prints the log and exits with code 1.
• Write-back: after bun add resolves, sync_lockfile parses the edited package.json files again, with its own log. That parse walks the globs a second time.

Notes

Origin. The review of #43404 (a workspace root at /) noted that a broad glob such as * meets /root and /lost+found as a user that is not root. The same failure happens in an ordinary directory, and on main without that PR. That review suggested to skip directories that fail with EACCES.

Why the install still fails. A glob library skips a directory that it cannot read. The package managers do not behave that way end to end. Tree: pkgs/a/package.json, pkgs/a/nested/b/package.json, and pkgs/secret owned by root with mode 000. The install runs as nobody.

workspaces entry	npm 11.16.0	yarn 1.22.22	yarn 4.18.0	pnpm 12.4.2	bun
pkgs/*	EACCES	EACCES	installs, skips secret	ERR_PNPM_PACKAGE_MANIFEST_IO_ERROR	EACCES
pkgs/*/nested/*	installs, skips secret	EACCES	EACCES	ERR_PNPM_WORKSPACE_WALK_ERROR	EACCES
pkgs/**	EACCES	EACCES	EACCES	ERR_PNPM_WORKSPACE_WALK_ERROR	EACCES

npm's glob ignores the directory it cannot read, but @npmcli/map-workspaces then reads pkgs/secret/package.json and stops on every error except ENOENT and ENOTDIR. A silent skip also changes resolution: a sibling that depends on the skipped package by a plain version range resolves it from the registry. All four tools name the path in their error. bun did not.

Output after the change, same tree:

error: Failed to run workspace pattern pkgs/* due to error EACCES (open "/tmp/ws/pkgs/secret")
    at /tmp/ws/package.json:1:49

With chmod 444 pkgs/secret: ... due to error EACCES (fstatat "/tmp/ws/pkgs/secret/package.json"). Before the walker change this case reported the path package.json, which reads as the root manifest. new Bun.Glob("*/package.json").scanSync({ cwd }) had the same path: "package.json" and now has path: "locked/package.json" (absolute with absolute: true). The shell printed bun: Permission denied: package.json and now prints bun: Permission denied: locked/package.json.

On Windows the missing-directory case prints (open "C:\workspace\ws\missing"). I ran bad-workspace.test.ts there with a debug build.

Commands. I ran 29 package manager commands (install, add, remove, update, patch, outdated, pm ls, why, publish --dry-run, install --frozen-lockfile and others) against a project whose glob fails, some with a bun.lock and some without, before and after the change. Every command that reports the glob error now prints it once, with the path, and exits with code 1. No command became silent.

Behavior that this change does not cause (raised in self-review, left as it is):

• The lockfile migration prints the error name in front of "failed to migrate lockfile". That name is now InstallFailed, the same as for every other error this function logs.
• With a package-lock.json or yarn.lock, the error prints two times: one for the migration attempt, one for the install. Every workspace error on that path does this.
• From a workspace member directory, the workspace root discovery in PackageManager.rs ignores every error from process_names_array, so the member installs as a project of its own.
• The shell glob has no test of its own for the new path. It uses the same walker branch as the Bun.Glob test.
• The three EACCES cases skip as root, as the other permission tests in test/cli/install do.

Write-back. Found in review of this PR: with InstallFailed alone, a glob that fails in the write-back parse exited with code 1 and no message, because sync_lockfile dropped its log. The test serves baz from a local registry that removes the glob's directory when bun asks for the manifest. That is after the first walk and before the write-back walk. Main prints only An internal error occurred (GlobError) there. This branch prints the Failed to run workspace pattern error with the path.

Tests. The missing-directory test runs everywhere and fails without the change (two error: lines, no path). The two EACCES cases and the Bun.Glob case need a user that is not root, and skip on Windows, where chmod cannot make a directory unreadable. I ran them as nobody: they fail with the released build and pass with the debug build. In scan.test.ts the six whole-repo recursive scans hit their 30 s timeout under the debug build in my container. They do not reach the changed branch.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/glob/scan.test.ts, test/cli/install/bad-workspace.test.ts

[coderabbitai]

Warning

Review paused — included plan limit reached

Keep your review moving with free on-demand reviews.

• Run this review for free

On-demand reviews are free for one more day.

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Promotion and pricing details

On-demand reviews are free for one more day. After that, they cost $0.25 per reviewed file.

Review limit details

Or wait 3 minutes for your next included review.

Check out review usage here.

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: b58e1214-6081-45e1-9ab8-7214c39ce82f

📥 Commits

Reviewing files that changed from the base of the PR and between 26e7a4b and eb7a0bb.

📒 Files selected for processing (6)

• src/glob/GlobWalker.rs
• src/install/PackageManager/package_json_write_back.rs
• src/install/error.rs
• src/install/lockfile/Package/WorkspaceMap.rs
• test/cli/install/bad-workspace.test.ts
• test/js/bun/glob/scan.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 2:56 PM PT - Sep 19th, 2026}

✅ @robobun, your commit eb7a0bbc6e50a6dee1cf30d3ecb33d0312f67584 passed in Build #118489! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43517

That installs a local version of the PR into your bun-43517 executable, so you can run:

bun-43517 --bun

[robobun]

Status

Reproduced on Linux x64 with canary 1.4.3-canary.1 (367d939). Run it as a user that is not root, because root ignores the mode bits:

d=$(mktemp -d) && cd $d
mkdir -p pkgs/a pkgs/secret
echo '{"name":"a","version":"1.0.0"}' > pkgs/a/package.json
echo '{"name":"root","workspaces":["pkgs/*"]}' > package.json
chmod 000 pkgs/secret
bun install

error: Failed to run workspace pattern pkgs/* due to error EACCES
    at /tmp/tmp.X/package.json:1:16
error: An internal error occurred (GlobError)

With this PR there is one error line, and it names the directory. The exit code is still 1:

error: Failed to run workspace pattern pkgs/* due to error EACCES (open "/tmp/tmp.X/pkgs/secret")
    at /tmp/tmp.X/package.json:1:16

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked two things: the new fallible self.walker.join(...)? at src/glob/GlobWalker.rs:676 runs after close_disallowing_cwd(fd), so an allocation failure there does not leak the directory fd, and it mirrors the existing e.with_path(matched_as_slice::<SENTINEL>(..)) shape at line 439; and no references to Error::GlobError remain anywhere under src/ after the variant's removal.

Extended reasoning...

The diff is small and mostly a refactor of three duplicated error blocks in src/install/lockfile/Package/WorkspaceMap.rs into add_glob_error, plus a one-site change in the glob walker's literal fast path so the stat error carries the joined directory + literal path. I confirmed the walker change is placed after the fd is closed (no fd leak on the ? path), follows the same pattern as the sibling open error site in the same file, and that the removed GlobError variant has no remaining consumers. The confirmed inline finding about sync_lockfile dropping the scratch log (silent exit 1 on bun add/bun update write-back) is the one substantive behavior change beyond the message text and is the reason a human should weigh in; the other ruled-out candidates (dead Err arm for walker construction, Windows separator mixing, path-buffer overflow fallback) are cosmetic or unreachable.

[claude]

LGTM — the follow-up commit resolves the write-back concern from my earlier review: sync_lockfile now prints the scratch log before propagating, and the new bun add test exercises exactly that path.

What was reviewed:

• add_glob_error: join_abs_string_buf_checked falls back to the raw err.path on overflow rather than unwrapping; InstallFailed is handled by handle_error in install_command.rs and pm_update_package_json.rs, so the message prints once and the "internal error" line is gone. No remaining GlobError references in src/ or test/.
• GlobWalker literal fast path: self.walker.join already goes through ? on the sibling prepare_matched_path call, and join_sep_maybe_z skips an empty dir_path, so a pattern with no directory prefix does not gain a leading separator.
• Write-back: the scratch Log is distinct from Cli::LOG_, so printing it in sync_lockfile does not double-print with handle_error.

Extended reasoning...

Overview

The PR touches error reporting in src/install/lockfile/Package/WorkspaceMap.rs (three duplicated add_error_pretty! sites collapsed into add_glob_error, which joins err.path onto cwd and returns InstallFailed), removes the now-unused Error::GlobError variant from src/install/error.rs, changes the literal fast path in src/glob/GlobWalker.rs to report dir_path + literal instead of the bare literal, and splits sync_lockfile in src/install/PackageManager/package_json_write_back.rs so the scratch log's errors are printed to Output::error_writer() before the error propagates. Tests in test/cli/install/bad-workspace.test.ts and test/js/bun/glob/scan.test.ts assert exact error lines including syscall and path, and a local Bun.serve registry test verifies the write-back failure is now reported.

Security risks

None identified. The change only affects diagnostic output. The joined path is derived from the walker's own dir_path and pattern component, and from the install cwd; nothing user-controlled is interpreted as a path in a new way. The join_abs_string_buf_checked call is bounds-checked and falls back to the unjoined path on overflow.

Level of scrutiny

Moderate. It is a small, self-contained error-reporting change. The points that warranted verification were: (1) the ? on self.walker.join in the walker fast path — the enclosing transition_to_dir_iter_state returns Result<Maybe<()>, Error> and the sibling prepare_matched_path(...)? at line 689 already relies on the same AllocError conversion; (2) an empty dir_path for a root-level literal — join_sep_vec skips empty parts so no leading separator is introduced; (3) the possibility of double-printing in the write-back path — the ScratchManifests log is its own Log instance, separate from Cli::LOG_ that handle_error prints, so there is no duplication; (4) the removed enum variant — no references remain in src/ or test/. No CODEOWNERS entry covers the changed files.

Other factors

The second commit (eb7a0bbc) directly addresses the one inline finding from my earlier review, and adds a targeted test for it that deletes the glob directory between the two walks via the registry handler, which is deterministic (the manifest request always precedes the write-back). The bug hunt ran to a dry streak with no findings. The test.concurrent.skipIf(...).each(...) chain is already used elsewhere in the suite (sql-connection-socket-uaf.test.ts). The chmod-based tests are gated on non-Windows and non-root, matching existing permission tests in test/cli/install.