pack, publish: include entries on filesystems whose readdir reports DT_UNKNOWN

[robobun]

Problem

• On a filesystem whose readdir does not report entry types (FUSE such as sshfs, some NFS servers, XFS formatted with ftype=0), bun pm pack and bun publish produce a tarball containing only package.json plus the bins named in package.json; every other file and every bundledDependencies entry is silently dropped, and the command exits 0.
• bun publish with directories.bin additionally stops recursing into subdirectories of the bin directory on such a filesystem, so nested bins are missing from the published manifest.
• Cause: bun_sys's directory iterator maps DT_UNKNOWN to FileKind::Unknown and leaves resolving it to the caller (src/sys/lib.rs, kind_from_dt). Every readdir loop in src/runtime/cli/pack_command.rs (iterate_project_tree, iterate_included_project_tree, add_entire_tree, add_bundled_dep, and the node_modules scan in iterate_bundled_deps) discards anything that is not File or Directory before doing anything else, and publish_command.rs's directories.bin walk only recurses into Directory.

Fix

• bun_sys::dir_iterator::WrappedIterator gets resolve_unknown_entry_types (same name and shape as the existing flag on walker_skippable::Walker). When set, next() lstats an entry whose kind came back Unknown and stores the result; an entry that cannot be stat'ed stays Unknown. Off by default: the resolver and the glob walker deliberately defer the stat to the entries they end up using. The Windows iterator always knows the kind, so the flag is a no-op there (it still compiles; checked for x86_64-pc-windows-msvc and aarch64-apple-darwin).
• The seven pack and publish loops set the flag on the iterator they create. Setting it at construction means the later match entry.kind arms and is_excluded() (which checks the kind for directory-only ignore patterns such as build/) all see the resolved kind. pack: skip bins reached through symlinks; publish: do not read the readme through a symlink #38707 edits one of the same lines in find_workspace_readme; whichever lands second keeps both.
• Why lstat rather than stat: d_type describes the entry itself, so a symlink is DT_LNK on a normal filesystem and is never packed. lstat reports the same thing, so a project packs identically on both kinds of filesystem; stat would have started packing symlink targets only on DT_UNKNOWN filesystems.
• Filesystems that do report d_type never take the new path: the only added work is one boolean and one enum comparison per entry.
• Scope: this PR is the flag plus the subsystem whose failure is silent and exits 0. The same filter exists in other consumers of this iterator and they stay broken on such filesystems until Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961, which is stacked on this PR and converts them: install/bin.rs (linking and unlinking directories.bin bins, the install-side twin of the publish walk fixed here), PackageManager/updatePackageJSONAndInstall.rs (dangling .bin cleanup on bun remove), PackageManager/PackageManagerResolution.rs (versions installed in the cache), cli/create_command.rs (template copy and listing), cli/bunx_command.rs (directories.bin lookup), cli/init_command.rs, cli/pm_licenses_command.rs, node/path_watcher.rs (recursive fs.watch), shell/builtin/ls.rs (ls -R), node_fs.rs (fs.cp, via the node-tier iterator), and the two private copies of the fallback in walker_skippable.rs and install/prune.rs. Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961 currently targets an earlier per-entry shape of this PR and needs rebasing onto this one; once every consumer is either opted in or deliberately lazy, flipping the flag's default and keeping explicit opt-outs for the lazy ones is a small further step that PR can take, which would make the bug unreachable for new consumers. Not done here so that this PR stays reviewable on its own.
• Test: test/cli/install/dt-unknown-readdir.test.ts. Neither this container nor CI has /dev/fuse, so test/fixtures/dt-unknown-readdir-shim.c (exposed as dtUnknownReaddir in the harness so Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961 can reuse it) is an LD_PRELOAD shim that interposes libc syscall(), which bun issues getdents64 through, and zeroes d_type in every record. It writes a marker to stderr when it does, and every test asserts on the marker, so the tests cannot pass vacuously if bun stops going through the wrapper. Eleven existing tests use the same LD_PRELOAD technique; shell-pipe-read-fault.test.ts interposes syscall() the same way. The shim is compiled asynchronously from beforeAll with an explicit hook timeout: compiling it from the first test body blocked the runner for as long as the compiler took, which on a loaded machine timed out the other concurrently started tests, and the default 5 s hook timeout is too short for a C compiler on a loaded machine. Cases: the plain tree walk with a directory-only .npmignore pattern and a symlink, the files field walk with a directory-only exclude, bundledDependencies (scoped and unscoped), and bun publish against a local mock registry (attached tarball contents, nested directories.bin entries, readme). All four fail on the current release build (tarball has only package.json; manifest bin lacks the nested entry) and pass with this change; earlier revisions of the same cases ran (not skipped) and passed in CI on the glibc and musl lanes. Expected contents were checked against what the same projects produce on a normal filesystem.
• Also run with this change: bun-pack.test.ts (76) and the bin, readme and tarball tests of bun-publish.test.ts; cargo clippy on bun_sys and bun_runtime is clean.

Background

• d_type: the entry type a Linux/BSD readdir (getdents64) returns alongside each name. Filling it in is optional for the filesystem; those that do not return DT_UNKNOWN for every entry, and callers are expected to stat the entry themselves when they need the type.
• lstat vs stat: stat follows a symlink and reports its target's type; lstat reports the symlink itself, which is what d_type reports on filesystems that fill it in.
• bun_sys::dir_iterator::WrappedIterator: the low-level readdir loop shared by the installer, the glob walker and the CLI commands; it owns the directory fd and an 8 KiB getdents64 buffer, and next() yields one IteratorResult (a name borrowed from that buffer, NUL-terminated, plus a kind) at a time.
• Pack walkers: iterate_project_tree walks the whole project when package.json has no files field; iterate_included_project_tree plus add_entire_tree handle the files field; iterate_bundled_deps plus add_bundled_dep copy bundledDependencies out of node_modules. bun publish builds its tarball with the same walkers and separately walks directories.bin to fill in the manifest's bin map.

Reproduction on the current release build

Project with index.js, lib/a.js, lib/nested/b.js, a bin entry, and two bundled dependencies under node_modules, packed with the shim preloaded:

packed 117B package.json
packed 2B bin.js

Total files: 2

Same project without the shim packs all 8 files. bun publish under the shim sends bin: {"more": "bins/more", "a.js": "bins/a.js"} (missing bins/more/b.js) and a tarball containing only package/package.json.

Earlier shapes of this PR

• 4daa60b: a pub(crate) fn entry_kind in pack_command.rs that copied the entry name into a ZBox and lstated it, imported by publish_command.rs.
• c7c1c33: the same as a public per-entry IteratorResult::resolve_kind(dir) in bun_sys, called from each loop. Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961 was branched from this shape.
• a02e49a: the iterator flag, plus walker_skippable and prune.rs moved onto it and an install test case for the walker. Those parts were dropped again in 2c6552f: Resolve readdir entries of unknown type in the remaining consumers (create, bin links, bunx, fs.watch, cp, ls, init) #38961 is where the walker's own policy is being decided (it resolves unconditionally, since five of its six callers already opt in), so having both PRs rewrite the same function only created conflicts.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8cbd4e12-ae12-4c54-977e-240d6a81aa70

📥 Commits

Reviewing files that changed from the base of the PR and between d3f975b and 2c6552f.

📒 Files selected for processing (6)

• src/runtime/cli/pack_command.rs
• src/runtime/cli/publish_command.rs
• src/sys/lib.rs
• test/cli/install/dt-unknown-readdir.test.ts
• test/fixtures/dt-unknown-readdir-shim.c
• test/harness.ts

---

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

[robobun]

^{Updated 12:05 PM PT - Aug 15th, 2026}

❌ @robobun, your commit 2c6552f has some failures in Build #98146 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38716

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

bun-38716 --bun

[robobun]

Status: ready for review at 2c6552f (CI for it pending; the previous revision's build had no failures).

Reproduced on the current release build with the shim from the test (zeroes d_type in every getdents64 record): bun pm pack of a project with files, subdirectories and two bundled dependencies packed only package.json and the listed bin; bun publish additionally lost the nested directories.bin entry. test/cli/install/dt-unknown-readdir.test.ts fails 4/4 on the release build and passes with this branch; earlier revisions of the same cases ran (not skipped) and passed in CI on the glibc and musl lanes.

Final shape: an opt-in resolve_unknown_entry_types flag on the bun_sys directory iterator, set by the seven pack/publish loops, plus the shared test shim. The other readdir consumers with the same filter, and the two private copies of the fallback (walker_skippable, prune.rs), are converted in #38961, which is stacked on this PR and needs rebasing onto this shape; the PR description lists them and the earlier shapes.

[claude]

LGTM — targeted fix following the same lstatat fallback pattern already used in install/prune.rs, walker_skippable.rs, node_fs.rs and GlobWalker.rs.

What was reviewed:

• Confirmed all five pack walkers plus the two publish readdir loops are covered; the scoped-package inner loop in iterate_bundled_deps doesn't read sub_entry.kind, so it needed no change.
• lstat (not stat) preserves the existing symlinks-are-not-packed contract on DT_UNKNOWN filesystems; the test asserts this.
• Fast path unchanged when d_type is populated (single enum compare); lstat failure keeps the entry Unknown and skipped as before.
• Test shim asserts its own marker on stderr so it can't pass vacuously if bun's readdir stops going through libc syscall().

Extended reasoning...

Overview

Fixes bun pm pack / bun publish silently dropping every file except package.json on filesystems whose readdir returns DT_UNKNOWN for every entry (FUSE/sshfs, some NFS, XFS with ftype=0). Adds a small entry_kind() helper to pack_command.rs that falls back to lstatat when the iterator kind is Unknown, and applies it at the six readdir consumers in the pack walkers plus the two in publish_command.rs (readme lookup and directories.bin recursion). Ships a Linux-only test that simulates the condition via an LD_PRELOAD shim interposing getdents64.

Security risks

None. This is a CLI-local directory walk; the only new syscall is an lstatat on entries the iterator already yielded, relative to the already-open directory fd. No user-controlled path parsing, no network, no auth surface changed.

Level of scrutiny

Low-to-medium. The Rust change is ~15 net lines of logic, entirely additive on the Unknown branch (normal filesystems hit only one extra enum comparison per entry). The pattern is copied verbatim from four existing in-tree call sites (install/prune.rs:1334, sys/walker_skippable.rs:69, runtime/node/node_fs.rs:6372, glob/GlobWalker.rs:1119), so there is no novel design here. The choice of lstat over stat is correctly reasoned (matches d_type semantics, keeps symlink handling identical across filesystem types) and the test explicitly covers it.

Other factors

• I checked that every readdir loop in the two touched files that inspects entry.kind was updated. The one loop that wasn't (the @scope/ sub-iteration in iterate_bundled_deps) never reads sub_entry.kind, so it's correctly left alone.
• The test is well-constructed per the repo's review rules: hermetic (local mock registry via Bun.serve({port: 0})), asserts exact sorted tarball paths, covers all four walker variants plus a directory-only ignore pattern and a symlink, uses test.concurrent, skipIfs when cc is unavailable, and guards against becoming vacuous by asserting the shim's stderr marker.
• Cross-platform: the PR notes cargo check passes on Windows and macOS targets; the new branch is dead on Windows (its iterator never yields Unknown) but still compiles since lstatat/kind_from_mode are cross-platform.
• Existing bun-pack.test.ts (76 tests) passes per the description; no behavior change on the normal-filesystem path.
• No prior human or bot review comments to address; bug hunting system found nothing.

[claude]

I reviewed this PR and didn't find any bugs. Because it adds a new public method to bun_sys and introduces an LD_PRELOAD-based C shim as a test technique, a human sign-off on those two choices would still be worthwhile.

What was reviewed:

• resolve_kind matches the existing lstat-fallback pattern in walker_skippable.rs:69-70; as_zstr()/lstatat/kind_from_mode are all in scope and correctly typed on POSIX, and the Windows arm is a no-op returning self.kind.
• Checked every DirIterator::iterate loop in both files: the inner @scope loop in iterate_bundled_deps (line ~911) doesn't filter on kind, so it correctly needs no resolve_kind call.
• The test asserts on the shim's stderr marker before anything else, so it can't pass vacuously if bun stops routing getdents64 through libc syscall(); skipIf(!isLinux || !cc) gates it and the shim dir is disposed in afterAll.

Extended reasoning...

Overview

The PR fixes silent data loss in bun pm pack and bun publish on filesystems whose readdir returns DT_UNKNOWN for every entry (FUSE/sshfs, some NFS, XFS with ftype=0). It adds IteratorResult::resolve_kind(dir) to src/sys/lib.rs — an lstat fallback when kind == Unknown, no-op otherwise — and calls it at seven readdir loops across pack_command.rs (5 sites) and publish_command.rs (2 sites plus the readme lookup). A new Linux-only test compiles a small LD_PRELOAD shim in C that interposes libc syscall() and zeroes d_type in every getdents64 record, then asserts exact tarball contents for the plain tree walk, the files walk, bundledDependencies, and a bun publish against a local mock registry.

Security risks

None identified. The change only widens which entries the pack walkers consider from "File or Directory" to "File or Directory after lstat resolution"; symlinks remain excluded because lstat (not stat) is used, so packing behaves identically to a d_type-reporting filesystem. No new user-controlled input reaches a syscall path that wasn't already there.

Level of scrutiny

Medium. The call-site changes are mechanical and obviously correct — each is a one-line entry.resolve_kind(dir.fd) before the existing kind filter, and I confirmed the fallback matches the pattern already in src/sys/walker_skippable.rs. But two aspects warrant a maintainer's eyes: (1) resolve_kind is a new public method on bun_sys::dir_iterator::IteratorResult, a foundational crate — the placement and signature look right (the kind_from_dt comment already told callers to lstat, and this centralizes it), but adding shared API surface is a design decision; (2) the LD_PRELOAD C shim test pattern is novel for this repo and a maintainer should confirm it's acceptable long-term (it depends on bun continuing to route getdents64 through libc syscall(), which the test guards against with a stderr marker assertion, but the coupling is worth acknowledging).

Other factors

• The comment-cop bot left flags on the resolve_kind doc comment; the author trimmed it to three lines and responded that it's API documentation, not a workaround justification. I agree — the current three-line rustdoc is appropriate for a public method and the flag looks like a false positive, but two of those inline threads are still marked unresolved.
• The PR description notes overlap with #38707 on the readme comparison line in publish_command.rs; whichever lands second needs to keep resolve_kind in the merged line.
• CI for the final revision (486b072) was still building at last timeline update; the first revision's CI passed on glibc and musl with the new tests running (not skipped).
• I verified the one readdir loop in the touched files that was not changed (the inner @scope loop in iterate_bundled_deps) doesn't filter on kind, so leaving it alone is correct.

[robobun]

One correction to the automated review above, for whoever picks this up: the LD_PRELOAD shim is not a new technique in this test suite. Eleven existing test files compile a shim the same way (for example test/js/bun/http/serve-epoll-add-fail.test.ts, test/js/bun/shell/shell-write-fault.test.ts, test/cli/install/bun-install-hardlink-fallback.test.ts), and test/js/bun/shell/shell-pipe-read-fault.test.ts already interposes syscall() itself, which is what this test does for getdents64. The new part is only the stderr marker that makes the test fail loudly instead of passing vacuously if bun stops issuing getdents64 through the libc wrapper.

[claude]

I reviewed this PR and didn't find any bugs. Because it adds new public API surface to bun_sys::dir_iterator (the shared readdir iterator) and refactors walker_skippable's control flow, a human look at the API shape would still be worthwhile — this is the pattern the follow-up sweep is meant to copy.

Checked: the walker_skippable behavioral change (unstat'able entries now yield as Unknown instead of being skipped) — every consumer (PackageInstall, FileCopier, Hardlinker, build_command) matches only File/Directory and continues otherwise, so it's equivalent.
Checked: resolve_unknown_kind borrows self.name.as_zstr() from the getdents buffer while the iterator state is untouched, and lstatat matches the semantics of the lstatat call being replaced in both walker_skippable and prune.rs.
Checked: the flag defaults to false in both iterate() constructors, so existing iterator users are unaffected; the seven pack/publish loops set it immediately after construction.

The four unresolved comment-cop flags are firing on the 3-line field doc and on pre-existing comments that were re-indented by the let ... else rewrite — not new prose.

Extended reasoning...

Overview

This PR fixes bun pm pack and bun publish on filesystems whose getdents64 returns DT_UNKNOWN for every entry (FUSE/sshfs, some NFS, XFS with ftype=0). It adds an opt-in resolve_unknown_entry_types flag to bun_sys::dir_iterator::WrappedIterator that lstats entries whose kind came back Unknown, sets it at the seven pack/publish readdir loops, and migrates the two existing private copies of the same fallback (walker_skippable, install/prune.rs) onto the shared flag. It ships an LD_PRELOAD C shim that zeroes d_type in every getdents64 record, exposed as dtUnknownReaddir in the harness, plus a test file covering pack (tree walk, files field, bundledDependencies), publish (directories.bin recursion, readme, tarball contents against a mock registry), and install of a file: dependency through the walker.

Security risks

None identified. The change adds an lstatat on entries that were previously discarded; input is directory entry names from getdents64. The LD_PRELOAD shim is test-only, compiled in a temp dir, and gated to Linux with a C compiler.

Level of scrutiny

This warrants human review. src/sys/lib.rs is core shared infrastructure — the directory iterator is used by the resolver, glob walker, installer, and every CLI command that reads a directory. The PR introduces new public API surface (resolve_unknown_entry_types) that is explicitly intended as the pattern for a follow-up sweep across bun create, fs.watch, fs.cp, shell ls -R, and other consumers, so the shape here is a design decision worth maintainer sign-off. The walker_skippable refactor also changes error-path behavior (an entry whose lstat fails now yields as Unknown rather than being silently skipped) — I verified every current consumer handles that identically, but it's the kind of subtle invariant a maintainer should confirm.

Other factors

• The author iterated the design three times (private helper → per-entry resolve_kind → iterator flag), and the final shape lets walker_skippable and prune.rs drop their own copies — that's the right layering per REVIEW.md, but confirms this is a design choice rather than a mechanical fix.
• The walker_skippable.rs diff is large but is mostly reindentation from clippy's let ... else requirement after the match was flattened; the pre-existing name_buffer NUL-terminator comment and skip-hash logic are unchanged.
• Test coverage is thorough and asserts on a stderr marker so it fails loudly if bun stops going through the interposed syscall() wrapper; the shim technique matches eleven existing tests.
• CI on the latest revision (a02e49a → build #97656) was still building at the time of review.
• The four unresolved comment-cop inline flags are on the 3-line public-field doc comment and on pre-existing comments that moved due to reindentation; the author's earlier responses (that these are API docs, not workaround justifications) are accurate.

[claude]

I reviewed this PR and didn't find any bugs. Because it adds new public API surface to bun_sys::dir_iterator::WrappedIterator (a foundational iterator used across the codebase) and introduces a reusable harness helper plus C fixture, a human look at the API shape would still be worthwhile.

What was reviewed:

• resolve_unknown_kind mirrors the existing lstatat + kind_from_mode pattern already in walker_skippable.rs:92-93; the new field defaults false in both iterate() constructors so existing callers are unaffected.
• Checked that name.as_zstr() is cfg(not(windows))-gated to match the call site, and that the borrowed name (into the getdents64 buffer) stays valid across the lstatat call.
• The seven flag-setting sites each precede a kind != File && kind != Directory filter or a == Directory recursion check that would otherwise drop Unknown.
• Note: the PR description still references the walker_skippable/prune.rs consolidation and a fifth bun install test case, but those were backed out in 2c6552f — the diff now touches only the six listed files.

Extended reasoning...

Overview

The PR fixes bun pm pack and bun publish silently producing near-empty tarballs on filesystems whose readdir returns DT_UNKNOWN for every entry (FUSE, some NFS, XFS with ftype=0). It adds an opt-in resolve_unknown_entry_types: bool field to bun_sys::dir_iterator::WrappedIterator; when set, next() calls lstatat on any entry whose kind came back Unknown and stores the resolved kind. Seven readdir loops in pack_command.rs and publish_command.rs set the flag. A new LD_PRELOAD shim (test/fixtures/dt-unknown-readdir-shim.c) plus a harness helper (dtUnknownReaddir) simulate such a filesystem, and four test cases cover the tree walk, files field, bundledDependencies, and bun publish with directories.bin.

The final commit (2c6552f) backed out the walker_skippable.rs and install/prune.rs consolidation described in the PR body, so the diff is narrower than the description — only src/sys/lib.rs, the two CLI command files, the test, the fixture, and test/harness.ts.

Security risks

None identified. The change adds an lstatat on entries the process just read from a directory it already has open; no new trust boundary is crossed. The C shim is test-only, compiled from a checked-in fixture, and loaded only into spawned test subprocesses via LD_PRELOAD.

Level of scrutiny

Medium-high. The behavioural fix itself is small and defaults off, but src/sys/lib.rs is the foundational syscall wrapper layer and WrappedIterator is used by the resolver, glob walker, installer, and every CLI directory walk. Adding a public mutable field is a minor API-shape decision that the PR itself iterated on three times (per-site helper → per-entry method → iterator flag), which suggests it merits a maintainer's sign-off on the final shape. The new dtUnknownReaddir harness helper is also intended for reuse by a follow-up sweep, so its placement and interface are worth a human glance.

Other factors

• The resolve_unknown_kind implementation is byte-for-byte the same pattern already proven in walker_skippable.rs (lstatat + kind_from_mode with the st_mode as Mode cast), so correctness risk is low.
• Both iterate() constructors (POSIX and Windows) initialize the new field to false, and the Windows arm of resolve_unknown_kind is a no-op, so no cross-platform behaviour change.
• Tests assert on a stderr marker written by the shim, so they cannot pass vacuously if bun stops routing getdents64 through libc's syscall() wrapper; they use test.concurrent with isolated tempdirs and a local Bun.serve({port: 0}) mock registry.
• The comment-cop bot fired repeatedly on doc-comment length; all threads are resolved and the current diff's comments are short.
• The PR description lists other readdir consumers with the same bug class (bun create, fs.watch, fs.cp, shell ls -R, etc.) as an intentional follow-up; whether that scope split is acceptable is a maintainer call.