fix: handle DT_UNKNOWN in dir_iterator for bind-mounted filesystems #25838
Conversation
Fixes oven-sh#24007 Some filesystems (bind mounts, FUSE, NFS) don't provide d_type in directory entries, returning DT_UNKNOWN. This caused glob and recursive readdir to skip entries entirely. Added fstatat() fallback when d_type is unknown to determine actual file type, matching Node.js/libuv behavior.
WalkthroughResolve DT_UNKNOWN directory-entry kinds by performing non-following lazy stats (lstatat/fstatat) during traversal; propagate the resolved kind into NodeFS readdir recursion, GlobWalker traversal, and emitted Dirent.kind. Add regression and FUSE tests covering bind-mounted and DT_UNKNOWN behavior. Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @test/regression/issue/24007.test.ts:
- Around line 20-130: The test suite uses a sequential describe("issue #24007 -
glob with recursive patterns") containing multiple file-writing tests; make them
run concurrently by changing the test grouping or tests to concurrent: either
replace describe(...) with describe.concurrent(...) or mark each test(...) as
test.concurrent(...) so tests like the ones creating tempDir and calling
fs.globSync, Bun.Glob, and fs.readdirSync run in parallel; update the block
containing the tests (the describe and/or individual test declarations)
accordingly to use describe.concurrent or test.concurrent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/bun.js/node/dir_iterator.zigtest/regression/issue/24007.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/regression/issue/24007.test.ts
test/regression/issue/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Regression tests for specific issues go in
/test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory
Files:
test/regression/issue/24007.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/regression/issue/24007.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for specific GitHub issues in
test/regression/issue/${issueNumber}.test.tswith real issue numbers only
Files:
test/regression/issue/24007.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/regression/issue/24007.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/bun.js/node/dir_iterator.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/node/dir_iterator.zig
🧠 Learnings (16)
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `tempDir` from `harness` to create temporary directories - do not use `tmpdirSync` or `fs.mkdtempSync`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Applied to files:
src/bun.js/node/dir_iterator.zig
📚 Learning: 2025-10-29T19:05:25.638Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-types/telemetry.d.ts:252-261
Timestamp: 2025-10-29T19:05:25.638Z
Learning: In packages/bun-types/telemetry.d.ts and src/telemetry/telemetry.zig, the naming inconsistency between `kind` (public API) and `type` (internal implementation) is intentional. The public API uses `InstrumentInfo.kind: InstrumentKind` with string values ("http", "fetch", etc.) for developer ergonomics, while the internal implementation uses `InstrumentRecord.type: InstrumentType` with numeric enum (0-6) for O(1) dispatch performance. This dual naming provides both ergonomic strings externally and efficient numeric dispatch internally without compromising either developer experience or runtime performance.
Applied to files:
src/bun.js/node/dir_iterator.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/node/dir_iterator.zig
🔇 Additional comments (9)
src/bun.js/node/dir_iterator.zig (1)
165-179: Excellent fallback implementation for DT_UNKNOWN handling.The fstatat() fallback correctly addresses the issue with bind-mounted and FUSE filesystems that don't populate d_type. The implementation is sound:
- Type annotation follows Zig best practices
- Comment clearly explains when and why the fallback is needed
- Graceful degradation to
Entry.Kind.unknownif fstatat fails- The pointer cast at line 175 is safe because
namepoints to the null-terminated data fromlinux_entry.nameThis matches Node.js/libuv behavior and resolves the glob recursion failures reported in issue #24007.
test/regression/issue/24007.test.ts (8)
1-14: LGTM! Excellent documentation.The header comment clearly explains the issue, root cause, fix, and testing limitations. The note about requiring Docker bind mounts for full DT_UNKNOWN verification is appropriate.
15-18: LGTM!Imports follow coding guidelines correctly: uses
bun:test,tempDirfrom harness, and appropriate Node.js standard library modules.
21-41: LGTM!Test correctly validates recursive glob pattern matching. Uses
tempDirwithusingfor proper cleanup, creates appropriate test structure, and verifies both presence and count of matched files.
43-57: LGTM!Test properly validates scoped recursive glob patterns with both positive (files in scope) and negative (files out of scope) assertions. Good test coverage.
59-73: LGTM!Test correctly validates non-recursive glob patterns by verifying direct children are matched while nested files are properly excluded.
75-90: LGTM!Test provides good coverage of Bun's native Glob API alongside fs.globSync tests. Correctly uses scanSync with cwd option.
92-107: LGTM!Test thoroughly validates recursive directory reading with multiple nesting levels. Good coverage of the fs.readdirSync recursive option.
109-129: LGTM with a minor observation.Test correctly validates directory entry types and the withFileTypes option. The logic at line 120 using
parentPath?.includes("subdir")works correctly for this test setup, though it's slightly fragile. A more precise approach would be to construct the expected path usingpath.join, but the current implementation is acceptable for this regression test.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
This is going to be a huge performance hit. Can you point to where Node.js does this in libuv? Can you paste the output from the getdents64 struct in one of these filesystem mounts.
test/regression/issue/24007.test.ts
Outdated
| * Note: This test verifies recursive glob patterns work correctly. Full verification | ||
| * of the DT_UNKNOWN fallback requires testing in a Docker environment with bind mounts | ||
| * as described in the original issue. |
There was a problem hiding this comment.
There is a test for similar behavior in #18172 that you might be able to adopt
Fixes oven-sh#24007 Some filesystems (Docker bind mounts via sshfs/FUSE, NFS, etc.) return DT_UNKNOWN for d_type in directory entries. This caused glob patterns like **/*.ts to return 0 results because unknown entries were being skipped. Changes: - dir_iterator.zig: Return .unknown for DT_UNKNOWN (unchanged behavior) - GlobWalker.zig: Handle .unknown with lazy stat - check pattern match first, then fstatat() only if needed (follows PR oven-sh#18172 pattern) - node_fs.zig: Handle .unknown in recursive readdir operations The lazy stat approach avoids performance regression on filesystems that do provide d_type by only calling fstatat() when: 1. The entry type is unknown AND 2. The filename matches the glob pattern (for GlobWalker) 3. We need to recurse into it (for readdir)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/bun.js/node/node_fs.zig:
- Around line 4649-4665: The async DT_UNKNOWN branch should set an effective
kind and use it when constructing the Dirent so callers see the resolved type:
introduce a local effective_kind initialized to current.kind, and inside the
.unknown switch when bun.sys.fstatat returns .result compute real_kind via
bun.sys.kindFromMode(st.mode), assign effective_kind = real_kind (while still
using real_kind or real_kind==.directory or .sym_link to decide
async_task.enqueue(name_to_copy)); then ensure the later Dirent construction
(bun.jsc.Node.Dirent) uses effective_kind instead of current.kind so
isDirectory()/isSymbolicLink() reflect the resolved value.
- Around line 4807-4822: The DT_UNKNOWN handling logic is duplicated between
sync and async paths; extract a small helper (e.g., shouldRecurseUnknown(fd,
current, name_to_copy) -> bool) that encapsulates the bun.MAX_PATH_BYTES check,
bun.sys.fstatat call, bun.sys.kindFromMode check for .directory or .sym_link,
and silent error handling, then replace the duplicated blocks in both branches
to call this helper and call stack.writeItem(basename_allocator.dupeZ(...)) only
when it returns true to preserve existing behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/bun.js/node/dir_iterator.zigsrc/bun.js/node/node_fs.zigsrc/glob/GlobWalker.zig
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/bun.js/node/dir_iterator.zigsrc/bun.js/node/node_fs.zigsrc/glob/GlobWalker.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/node/dir_iterator.zigsrc/bun.js/node/node_fs.zigsrc/glob/GlobWalker.zig
🧠 Learnings (2)
📚 Learning: 2025-10-29T19:05:25.638Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-types/telemetry.d.ts:252-261
Timestamp: 2025-10-29T19:05:25.638Z
Learning: In packages/bun-types/telemetry.d.ts and src/telemetry/telemetry.zig, the naming inconsistency between `kind` (public API) and `type` (internal implementation) is intentional. The public API uses `InstrumentInfo.kind: InstrumentKind` with string values ("http", "fetch", etc.) for developer ergonomics, while the internal implementation uses `InstrumentRecord.type: InstrumentType` with numeric enum (0-6) for O(1) dispatch performance. This dual naming provides both ergonomic strings externally and efficient numeric dispatch internally without compromising either developer experience or runtime performance.
Applied to files:
src/bun.js/node/dir_iterator.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/node/dir_iterator.zigsrc/bun.js/node/node_fs.zigsrc/glob/GlobWalker.zig
🔇 Additional comments (2)
src/glob/GlobWalker.zig (1)
905-988: LGTM! Clean implementation of lazy stat fallback for DT_UNKNOWN entries.The approach correctly:
- Pre-filters with
matchPatternImplbefore incurring stat overhead- Uses arena allocator for the null-terminated name (consistent with line 645's pattern)
- Silently skips unstat-able entries which is appropriate for unknown types
- Routes to the correct handling logic for each discovered type
The duplicated handling logic for file/directory/symlink is acceptable here for clarity, and the symlink logic correctly mirrors the original behavior through the if/else if structure.
src/bun.js/node/dir_iterator.zig (1)
165-177: Good documentation addition.The explicit type annotation and comment clarify the contract: this iterator reports
.unknownfor DT_UNKNOWN entries, and callers are responsible for the lazy stat fallback. This aligns well with the handling added inGlobWalker.zigandnode_fs.zig.
src/bun.js/node/node_fs.zig
Outdated
| // DT_UNKNOWN for d_type. Use fstatat to determine if it's a directory. | ||
| .unknown => { | ||
| if (current.name.len + 1 + name_to_copy.len > bun.MAX_PATH_BYTES) break :enqueue; | ||
|
|
||
| // Lazy stat to determine if this is a directory we need to recurse into | ||
| const stat_result = bun.sys.fstatat(fd, current.name.sliceAssumeZ()); | ||
| switch (stat_result) { | ||
| .result => |st| { | ||
| const real_kind = bun.sys.kindFromMode(st.mode); | ||
| if (real_kind == .directory or real_kind == .sym_link) { | ||
| stack.writeItem(basename_allocator.dupeZ(u8, name_to_copy) catch break :enqueue) catch break :enqueue; | ||
| } | ||
| }, | ||
| .err => {}, // Skip entries we can't stat | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Sync DT_UNKNOWN handling mirrors async; consider sharing helper to avoid duplication
The synchronous recursive readdir now mirrors the async path’s .unknown handling with bun.sys.fstatat and the same MAX_PATH guard, which keeps behavior consistent across flavors.
Given the near-identical logic between the async and sync branches (including the “directory or symlink” check and silent error skip), consider extracting a small helper (e.g., shouldRecurseUnknown(fd, current, name_to_copy) returning a bool) to reduce duplication and keep future changes to DT_UNKNOWN handling in one place.
🤖 Prompt for AI Agents
In @src/bun.js/node/node_fs.zig around lines 4807 - 4822, The DT_UNKNOWN
handling logic is duplicated between sync and async paths; extract a small
helper (e.g., shouldRecurseUnknown(fd, current, name_to_copy) -> bool) that
encapsulates the bun.MAX_PATH_BYTES check, bun.sys.fstatat call,
bun.sys.kindFromMode check for .directory or .sym_link, and silent error
handling, then replace the duplicated blocks in both branches to call this
helper and call stack.writeItem(basename_allocator.dupeZ(...)) only when it
returns true to preserve existing behavior.
Node.js handles UV_DIRENT_UNKNOWN with an lstat() fallback in This implementation follows the same pattern but is more efficient for glob because we:
All entries return d_type=DT_UNKNOWN on Docker bind mounts (via Colima/sshfs on macOS). |
Adds test/cli/run/glob-on-fuse.test.ts which uses the same Python FUSE filesystem from PR oven-sh#18172 to verify glob and readdir work correctly when d_type returns DT_UNKNOWN. Tests: - Bun.Glob.scanSync on FUSE mount - fs.globSync on FUSE mount - fs.readdirSync on FUSE mount - fs.readdirSync with withFileTypes on FUSE mount Thanks to @190n for the suggestion.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @test/cli/run/glob-on-fuse.test.ts:
- Line 15: Add a brief inline comment next to the tmpdirSync call explaining why
tmpdirSync is used instead of harness.tempDir: note that tempDir requires
supplying a file-tree structure but this test needs an empty directory to act as
a FUSE mountpoint, so tmpdirSync is intentionally used for that purpose.
- Around line 27-33: The polling loop that waits for join(mountpoint, "main.js")
should also detect if the spawned Python process has already exited to fail
faster; update the loop that currently checks fs.existsSync(join(mountpoint,
"main.js")) and tries < 250 to additionally await or inspect
pythonProcess.exited (or the process exit code/promise) each iteration and, if
it has resolved with a non-zero/failed exit, throw or fail the test immediately
instead of continuing to poll. Keep the same polling/backoff behavior but break
and surface the Python process error when pythonProcess.exited indicates the
process finished with an error.
- Around line 14-52: The finally block in withFuseMount currently can throw a
cleanup error (the catch inside finally rethrows e) and thus swallow any
original exception from the try block; change the pattern to capture the
original error (e.g., store it in a variable before entering cleanup) and after
performing the unmount/cleanup logic rethrow the original error if it existed,
or if cleanup should be non-fatal, log the cleanup failure instead of throwing;
focus changes around withFuseMount, the pythonProcess handling, the umount
spawn/exited wait and the inner catch so the original exception is preserved (or
the cleanup error only logged) rather than lost.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for specific GitHub issues in
test/regression/issue/${issueNumber}.test.tswith real issue numbers only
Files:
test/regression/issue/24007.test.ts
🧠 Learnings (19)
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `tempDir` from `harness` to create temporary directories - do not use `tmpdirSync` or `fs.mkdtempSync`
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Organize regression tests for specific issues in `/test/regression/issue/${issueNumber}.test.ts`. Do not place regression tests in the regression directory if there is no associated issue number.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer concurrent tests over sequential tests. Use `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it is very difficult to make them concurrent.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-07T17:28:51.204Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Applied to files:
test/regression/issue/24007.test.ts
🧬 Code graph analysis (2)
test/cli/run/glob-on-fuse.test.ts (2)
test/harness.ts (1)
tmpdirSync(1211-1213)scripts/glob-sources.mjs (1)
glob(11-11)
test/regression/issue/24007.test.ts (2)
test/harness.ts (1)
tempDir(277-284)scripts/glob-sources.mjs (1)
glob(11-11)
🪛 Biome (2.1.2)
test/cli/run/glob-on-fuse.test.ts
[error] 48-48: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
🔇 Additional comments (3)
test/regression/issue/24007.test.ts (1)
1-18: LGTM! Good documentation and imports.The header documentation clearly explains the issue, the fix, and references related tests. Imports correctly use
tempDirfromharnessas required by coding guidelines.test/cli/run/glob-on-fuse.test.ts (2)
1-12: LGTM! Clear documentation and appropriate imports.Good header documentation linking to the related issue and PR. The
skipIf(!isLinux)is appropriate since FUSE is Linux-specific.
54-112: LGTM! Comprehensive FUSE filesystem tests.The tests cover the key scenarios for DT_UNKNOWN handling:
Bun.Glob.scanSyncpattern matchingfs.globSynccompatibility- Basic
fs.readdirSyncfs.readdirSyncwithwithFileTypesvalidatingisFile()andisSymbolicLink()The 10-second timeouts are appropriate for FUSE operations that may be slower than native filesystem calls.
When d_type is unknown and we stat to determine the real kind, now use the resolved kind when constructing Dirent objects so callers see correct isDirectory()/isFile()/isSymbolicLink() values instead of 'unknown'.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/regression/issue/24007.test.ts:
- Around line 108-129: The parentPath check for nestedFile is too loose—replace
the `.parentPath?.includes("subdir")` check with a strict path-segment test:
import/require Node's path utilities and ensure parentPath is split by path.sep
(or use path.basename/endsWith on a constructed relative path) and assert that
one of the path segments equals "subdir" (apply this change to the `nestedFile`
find predicate where `results.find((d) => d.name === "file.txt" &&
d.parentPath?.includes("subdir"))` is used) so the test only matches the
intended subdirectory.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/regression/issue/24007.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/regression/issue/24007.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for specific GitHub issues in
test/regression/issue/${issueNumber}.test.tswith real issue numbers only
Files:
test/regression/issue/24007.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/regression/issue/24007.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/regression/issue/24007.test.ts
🧠 Learnings (17)
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Organize regression tests for specific issues in `/test/regression/issue/${issueNumber}.test.ts`. Do not place regression tests in the regression directory if there is no associated issue number.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `tempDir` from `harness` to create temporary directories - do not use `tmpdirSync` or `fs.mkdtempSync`
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-07T17:28:51.204Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer concurrent tests over sequential tests. Use `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it is very difficult to make them concurrent.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/regression/issue/24007.test.ts
🔇 Additional comments (6)
test/regression/issue/24007.test.ts (6)
1-18: LGTM! Well-documented test setup.The file header clearly explains the issue, fix approach, and references the related FUSE test. Imports are appropriate for the test scenarios.
19-40: LGTM! Comprehensive recursive glob test.The test correctly validates that recursive glob patterns find files at all nesting levels. Uses
tempDirproperly and validates both individual matches and the exact count.
42-56: LGTM! Validates subdirectory filtering.The test correctly verifies that glob patterns with directory prefixes filter appropriately, including both positive and negative cases.
58-72: LGTM! Validates non-recursive glob patterns.The test ensures that top-level patterns (without
**) correctly exclude nested files, confirming the fix preserves the distinction between recursive and non-recursive patterns.
74-89: LGTM! Tests Bun.Glob API directly.Good coverage of the
Bun.Globclass in addition tofs.globSync, ensuring the fix works across both glob APIs.
91-106: LGTM! Validates recursive readdir.The test ensures
fs.readdirSyncwithrecursive: truediscovers files at all nesting levels, which also relies on the DT_UNKNOWN fix in directory iteration.
- Add comment explaining tmpdirSync usage (tempDir requires file tree) - Check if Python process exited early in polling loop - Preserve original error instead of swallowing in finally block
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/cli/run/glob-on-fuse.test.ts:
- Line 9: The import line in glob-on-fuse.test.ts includes unused symbols bunEnv
and bunExe; remove those from the import so only the actually used identifiers
(isLinux and tmpdirSync) are imported from "harness", and run a quick search for
any remaining references to bunEnv or bunExe in this file to ensure no usages
remain.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/bun.js/node/node_fs.zigtest/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/bun.js/node/node_fs.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/node/node_fs.zig
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for specific GitHub issues in
test/regression/issue/${issueNumber}.test.tswith real issue numbers only
Files:
test/regression/issue/24007.test.ts
🧠 Learnings (32)
📚 Learning: 2025-10-29T19:05:25.638Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-types/telemetry.d.ts:252-261
Timestamp: 2025-10-29T19:05:25.638Z
Learning: In packages/bun-types/telemetry.d.ts and src/telemetry/telemetry.zig, the naming inconsistency between `kind` (public API) and `type` (internal implementation) is intentional. The public API uses `InstrumentInfo.kind: InstrumentKind` with string values ("http", "fetch", etc.) for developer ergonomics, while the internal implementation uses `InstrumentRecord.type: InstrumentType` with numeric enum (0-6) for O(1) dispatch performance. This dual naming provides both ergonomic strings externally and efficient numeric dispatch internally without compromising either developer experience or runtime performance.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-09-02T19:17:26.376Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `tempDir` from `harness` to create temporary directories - do not use `tmpdirSync` or `fs.mkdtempSync`
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: When multi-file tests are required, use `tempDir()` from `harness` to create temporary test fixtures.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `tempDir()` from `harness` to create temporary directories with files in tests instead of manually creating them.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/run/glob-on-fuse.test.tstest/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Organize regression tests for specific issues in `/test/regression/issue/${issueNumber}.test.ts`. Do not place regression tests in the regression directory if there is no associated issue number.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-11-07T17:28:51.204Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Do not write flaky tests - do not use `setTimeout` in tests; instead await the condition to be met
Applied to files:
test/regression/issue/24007.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Prefer concurrent tests over sequential tests. Use `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it is very difficult to make them concurrent.
Applied to files:
test/regression/issue/24007.test.ts
🧬 Code graph analysis (2)
test/cli/run/glob-on-fuse.test.ts (2)
test/harness.ts (1)
tmpdirSync(1211-1213)scripts/glob-sources.mjs (1)
glob(11-11)
test/regression/issue/24007.test.ts (2)
test/harness.ts (1)
tempDir(277-284)scripts/glob-sources.mjs (1)
glob(11-11)
🪛 Biome (2.1.2)
test/cli/run/glob-on-fuse.test.ts
[error] 58-58: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
🔇 Additional comments (4)
src/bun.js/node/node_fs.zig (1)
4630-4669: Async DT_UNKNOWN fallback andeffective_kindpropagation look correctThe
.unknownbranch’sbun.sys.fstatatfallback, guarded by the same MAX_PATH check as the known-kinds, correctly resolves real kind viabun.sys.kindFromMode(st.mode)and only enqueues directories/symlinks. Using a localeffective_kindand wiring it into thebun.jsc.Node.Direntconstruction meansisDirectory()/isSymbolicLink()will now reflect the resolved type instead of staying.unknown, which aligns the observable behavior with the traversal logic. I don’t see correctness or resource‑handling issues here.Also applies to: 4683-4687
test/regression/issue/24007.test.ts (1)
19-130: Regression coverage for bind‑mount / DT_UNKNOWN behavior looks solidThe tests exercise all relevant surfaces for this bug:
fs.globSyncwith recursive and non‑recursive patterns,Bun.Glob.scanSync, andfs.readdirSyncboth with plain strings andwithFileTypes(including validation ofparentPathand Dirent type methods). They usetempDircorrectly, run underdescribe.concurrent, and assert both positive and negative matches plus counts, so they should reliably catch regressions in the DT_UNKNOWN fallback logic without being flaky.test/cli/run/glob-on-fuse.test.ts (2)
14-63: LGTM! Error handling pattern is correct.The
originalErrortracking pattern correctly preserves original test failures while allowing cleanup errors to surface only when no test failure occurred. The static analysis warning at line 58 is a false positive—the conditional guardif (!originalError)ensures the cleanup error doesn't overwrite the original exception.Previous review concerns (tmpdirSync usage, polling loop, exception preservation) have been properly addressed.
23-23: Thefuse-fs.pyfixture file already exists in the repository attest/cli/run/fuse-fs.py. No action is needed.Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @test/cli/run/glob-on-fuse.test.ts:
- Line 77: Remove the explicit 10000 timeout arguments from the four test()
calls in test/cli/run/glob-on-fuse.test.ts (the tests at the commented positions
around lines 77, 90, 103, and 121); edit each test invocation that currently
passes 10000 as a third parameter so it only calls test(name, async () => { ...
}) (or the existing two-argument form) and rely on Bun's built-in timeout
handling instead.
- Around line 53-60: The catch/finally currently may throw inside the finally
block (the throw e when !originalError), which triggers static analysis; change
the flow to avoid throwing inside finally by capturing the caught error into a
local variable (e.g., let caughtError = e) and performing cleanup
(pythonProcess.kill and stderr logging) in the finally, then after the finally
re-throw: if (!originalError) throw caughtError; Ensure you reference the
existing variables pythonProcess, originalError, and the caught error variable
you introduce so behavior is identical but no throw occurs inside the finally.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/cli/run/glob-on-fuse.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/cli/run/glob-on-fuse.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/cli/run/glob-on-fuse.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/cli/run/glob-on-fuse.test.ts
🧠 Learnings (24)
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `tempDir` from `harness` to create temporary directories - do not use `tmpdirSync` or `fs.mkdtempSync`
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: When multi-file tests are required, use `tempDir()` from `harness` to create temporary test fixtures.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `tempDir()` from `harness` to create temporary directories with files in tests instead of manually creating them.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
test/cli/run/glob-on-fuse.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/run/glob-on-fuse.test.ts
🪛 Biome (2.1.2)
test/cli/run/glob-on-fuse.test.ts
[error] 58-58: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
| expect(results.length).toBeGreaterThanOrEqual(1); | ||
| }); | ||
| }, | ||
| 10000 |
There was a problem hiding this comment.
Remove explicit test timeouts.
The coding guidelines specify: "Do not set a timeout on tests. Bun already has timeouts built-in." Remove the 10000 timeout parameter from all four test cases and rely on Bun's default timeout handling.
As per coding guidelines, Bun's built-in timeout mechanism should handle even potentially slow FUSE mount operations.
🔧 Proposed fix
test(
"Bun.Glob.scanSync finds files on FUSE mount",
async () => {
await withFuseMount(async (mountpoint) => {
const glob = new Bun.Glob("*.js");
const results = Array.from(glob.scanSync({ cwd: mountpoint }));
// fuse-fs.py provides main.js and main-symlink.js
expect(results).toContain("main.js");
expect(results.length).toBeGreaterThanOrEqual(1);
});
- },
- 10000
+ }
);
test(
"fs.globSync finds files on FUSE mount",
async () => {
await withFuseMount(async (mountpoint) => {
const results = fs.globSync("*.js", { cwd: mountpoint });
expect(results).toContain("main.js");
expect(results.length).toBeGreaterThanOrEqual(1);
});
- },
- 10000
+ }
);
test(
"fs.readdirSync works on FUSE mount",
async () => {
await withFuseMount(async (mountpoint) => {
const results = fs.readdirSync(mountpoint);
expect(results).toContain("main.js");
expect(results).toContain("main-symlink.js");
});
- },
- 10000
+ }
);
test(
"fs.readdirSync with withFileTypes returns correct types on FUSE mount",
async () => {
await withFuseMount(async (mountpoint) => {
const results = fs.readdirSync(mountpoint, { withFileTypes: true });
const mainJs = results.find((d) => d.name === "main.js");
expect(mainJs).toBeDefined();
expect(mainJs!.isFile()).toBe(true);
const symlink = results.find((d) => d.name === "main-symlink.js");
expect(symlink).toBeDefined();
expect(symlink!.isSymbolicLink()).toBe(true);
});
- },
- 10000
+ }
);Also applies to: 90-90, 103-103, 121-121
🤖 Prompt for AI Agents
In @test/cli/run/glob-on-fuse.test.ts at line 77, Remove the explicit 10000
timeout arguments from the four test() calls in
test/cli/run/glob-on-fuse.test.ts (the tests at the commented positions around
lines 77, 90, 103, and 121); edit each test invocation that currently passes
10000 as a third parameter so it only calls test(name, async () => { ... }) (or
the existing two-argument form) and rely on Bun's built-in timeout handling
instead.
- Restructure error handling to throw outside finally - Keep 10000ms timeouts (matches run-file-on-fuse.test.ts pattern) - Add comment explaining why timeouts are needed for FUSE cleanup
Added lstatat() to sys.zig - fstatat with AT_SYMLINK_NOFOLLOW flag. When d_type is DT_UNKNOWN, we need lstatat (not fstatat) to correctly identify symlinks as symlinks rather than following them to their target. This preserves consistent behavior with normal filesystems where d_type is available - symlinks should be reported as symlinks regardless of whether the filesystem populates d_type.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/glob/GlobWalker.zig:
- Line 920: Replace the unconditional heap allocation for entry_name with a
stack-fallback: check the length of entry_name and if it fits a small fixed-size
buffer (e.g., 256 bytes) copy and null-terminate into that stack buffer and set
name_z to point at it; otherwise call this.walker.arena.allocator().dupeZ(u8,
entry_name) catch bun.outOfMemory() as before. Ensure any heap-allocated name_z
is freed appropriately after use and that all callers expecting a C-style
zero-terminated name still work; mirror the literal-ending optimization pattern
already used elsewhere (the conditional stack/heap allocation approach).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/bun.js/node/node_fs.zigsrc/glob/GlobWalker.zigsrc/sys.zig
🧰 Additional context used
📓 Path-based instructions (2)
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/node/node_fs.zigsrc/sys.zigsrc/glob/GlobWalker.zig
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct { #foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/bun.js/node/node_fs.zigsrc/sys.zigsrc/glob/GlobWalker.zig
🧠 Learnings (6)
📚 Learning: 2025-10-29T19:05:25.638Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-types/telemetry.d.ts:252-261
Timestamp: 2025-10-29T19:05:25.638Z
Learning: In packages/bun-types/telemetry.d.ts and src/telemetry/telemetry.zig, the naming inconsistency between `kind` (public API) and `type` (internal implementation) is intentional. The public API uses `InstrumentInfo.kind: InstrumentKind` with string values ("http", "fetch", etc.) for developer ergonomics, while the internal implementation uses `InstrumentRecord.type: InstrumentType` with numeric enum (0-6) for O(1) dispatch performance. This dual naming provides both ergonomic strings externally and efficient numeric dispatch internally without compromising either developer experience or runtime performance.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-09-02T19:17:26.376Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/node/node_fs.zigsrc/sys.zigsrc/glob/GlobWalker.zig
📚 Learning: 2025-10-19T23:27:43.240Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23840
File: src/bun.js/node/node_fs_stat_watcher.zig:400-411
Timestamp: 2025-10-19T23:27:43.240Z
Learning: In Zig code (e.g., src/bun.js/node/node_fs_stat_watcher.zig), taking the address of a function return value (e.g., &this.getLastStat()) is valid when passed as a const pointer parameter (e.g., *const bun.sys.PosixStat) to another function in the same expression. The compiler materializes the temporary with appropriate lifetime for the call scope.
Applied to files:
src/sys.zig
🔇 Additional comments (5)
src/sys.zig (1)
747-762: Confirm whether Windowslstatatshould preserve symlink-following semantics.On Windows,
lstatatdelegates tofstatat, which opens the file withflags=0(noO.NOFOLLOW). This means Windows will follow symlinks by default, while the POSIX implementation correctly usesstd.posix.AT.SYMLINK_NOFOLLOWto prevent following symlinks. The function's documentation states it does not follow symlinks, and all usages inGlobWalker.zigandnode_fs.zigexplicitly rely on this behavior.Since this PR targets Linux FUSE/bind-mount issues where this function is primarily used, the Windows inconsistency may be acceptable. However, if Windows support is required, consider either passing
O.NOFOLLOWto the file open call or using an alternative stat approach (similar to howlstatdelegates tosys_uv.lstaton Windows).src/bun.js/node/node_fs.zig (2)
4630-4687: DT_UNKNOWN lazy lstatat fallback in async recursive readdir looks correct and consistentThe new
effective_kindtracking plus.unknownbranch that callsbun.sys.lstatat(fd, current.name.sliceAssumeZ())is wired correctly:
- You reuse the existing path-length guard before enqueueing, so recursion limits remain unchanged.
lstatat(no-follow) preserves symlink identity while still letting you distinguish directories/symlinks from regular files.- On stat success, you update
effective_kindand only enqueue when the real kind is.directoryor.sym_link, which is exactly what the existing recursion logic expects.- On stat error, you skip recursion and leave
effective_kindas.unknown, so emittedDirent.kindcorrectly reflects that the type could not be resolved.- Using
effective_kindin theDirentconstruction ensures callers of recursivereaddirsee resolved kinds instead of.unknownwhere possible, without touching the non-recursive path.Overall, this aligns with the PR’s DT_UNKNOWN strategy and should restore correct behavior on bind/FUSE/NFS without regressions on normal filesystems.
4798-4850: Sync recursive readdir DT_UNKNOWN handling mirrors async path appropriatelyThe synchronous
readdirWithEntriesRecursiveSyncchanges mirror the async logic well:
effective_kindis initialized fromcurrent.kindand only overridden whenlstatatsucceeds, so type information is never uninitialized.- The
.unknownbranch applies the sameMAX_PATH_BYTESguard and only pushes onto the basename stack when the resolved kind is.directoryor.sym_link, reusing the existing recursion contract.- Updating
Dirent.kind = effective_kindensures recursive syncreaddirexposes resolved kinds consistently with the async implementation.The symmetry between async and sync paths here is good and reduces divergence in behavior across the two flavors.
src/glob/GlobWalker.zig (2)
157-161: LGTM: Clean non-following stat implementation.The
lstatatfunction correctly provides symlink-preserving stat semantics by delegating toSyscall.lstataton POSIX and falling back tostatatWindowson Windows, mirroring the pattern of the existingstatatfunction.
924-924: The silent skip of stat failures on unknown entries is intentional and appropriate.Line 924 silently continues when
lstatatfails on DT_UNKNOWN entries, which is intentional per the comments at lines 911-913. This is a performance optimization (PR #18172 lazy stat pattern) specific to filesystems like Docker bind mounts, FUSE, and NFS that report DT_UNKNOWN for d_type. When the d_type is already unknown and the lazy stat fails, skipping the entry is reasonable—there's no additional information to determine. This approach is consistent throughout the codebase, including identical patterns insrc/bun.js/node/node_fs.zig(lines 4659-4667 and 4820-4828), confirming it is the intended design for handling unknown directory entries.
| // Some filesystems (e.g., Docker bind mounts, FUSE, NFS) return | ||
| // DT_UNKNOWN for d_type. Use lazy stat to determine the real kind | ||
| // only when needed (PR #18172 pattern for performance). | ||
| .unknown => { | ||
| // First check if name might match pattern (avoid unnecessary stat) | ||
| const might_match = this.walker.matchPatternImpl(dir_iter_state.pattern, entry_name); | ||
| if (!might_match) continue; | ||
|
|
||
| // Need to stat to determine actual kind (lstatat to not follow symlinks) | ||
| const name_z = this.walker.arena.allocator().dupeZ(u8, entry_name) catch bun.outOfMemory(); | ||
| const stat_result = Accessor.lstatat(dir.fd, name_z); | ||
| const real_kind: std.fs.File.Kind = switch (stat_result) { | ||
| .result => |st| bun.sys.kindFromMode(st.mode), | ||
| .err => continue, // Skip entries we can't stat | ||
| }; | ||
|
|
||
| // Process based on actual kind | ||
| switch (real_kind) { | ||
| .file => { | ||
| const matches = this.walker.matchPatternFile(entry_name, dir_iter_state.component_idx, dir.is_last, dir_iter_state.pattern, dir_iter_state.next_pattern); | ||
| if (matches) { | ||
| const prepared = try this.walker.prepareMatchedPath(entry_name, dir.dir_path) orelse continue; | ||
| return .{ .result = prepared }; | ||
| } | ||
| }, | ||
| .directory => { | ||
| var add_dir: bool = false; | ||
| const recursion_idx_bump_ = this.walker.matchPatternDir(dir_iter_state.pattern, dir_iter_state.next_pattern, entry_name, dir_iter_state.component_idx, dir_iter_state.is_last, &add_dir); | ||
|
|
||
| if (recursion_idx_bump_) |recursion_idx_bump| { | ||
| const subdir_parts: []const []const u8 = &[_][]const u8{ | ||
| dir.dir_path[0..dir.dir_path.len], | ||
| entry_name, | ||
| }; | ||
|
|
||
| const subdir_entry_name = try this.walker.join(subdir_parts); | ||
|
|
||
| if (recursion_idx_bump == 2) { | ||
| try this.walker.workbuf.append( | ||
| this.walker.arena.allocator(), | ||
| WorkItem.new(subdir_entry_name, dir_iter_state.component_idx + recursion_idx_bump, .directory), | ||
| ); | ||
| try this.walker.workbuf.append( | ||
| this.walker.arena.allocator(), | ||
| WorkItem.new(subdir_entry_name, dir_iter_state.component_idx, .directory), | ||
| ); | ||
| } else { | ||
| try this.walker.workbuf.append( | ||
| this.walker.arena.allocator(), | ||
| WorkItem.new(subdir_entry_name, dir_iter_state.component_idx + recursion_idx_bump, .directory), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (add_dir and !this.walker.only_files) { | ||
| const prepared_path = try this.walker.prepareMatchedPath(entry_name, dir.dir_path) orelse continue; | ||
| return .{ .result = prepared_path }; | ||
| } | ||
| }, | ||
| .sym_link => { | ||
| if (this.walker.follow_symlinks) { | ||
| const subdir_parts: []const []const u8 = &[_][]const u8{ | ||
| dir.dir_path[0..dir.dir_path.len], | ||
| entry_name, | ||
| }; | ||
| const entry_start: u32 = @intCast(if (dir.dir_path.len == 0) 0 else dir.dir_path.len + 1); | ||
| const subdir_entry_name = try this.walker.join(subdir_parts); | ||
|
|
||
| try this.walker.workbuf.append( | ||
| this.walker.arena.allocator(), | ||
| WorkItem.newSymlink(subdir_entry_name, dir_iter_state.component_idx, entry_start), | ||
| ); | ||
| } else if (!this.walker.only_files) { | ||
| const matches = this.walker.matchPatternFile(entry_name, dir_iter_state.component_idx, dir_iter_state.is_last, dir_iter_state.pattern, dir_iter_state.next_pattern); | ||
| if (matches) { | ||
| const prepared_path = try this.walker.prepareMatchedPath(entry_name, dir.dir_path) orelse continue; | ||
| return .{ .result = prepared_path }; | ||
| } | ||
| } | ||
| }, | ||
| else => {}, // Skip other types (block devices, etc.) | ||
| } | ||
| continue; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication across file/directory/symlink handling.
The logic for handling .file (lines 929-935), .directory (lines 936-968), and .sym_link (lines 970-989) within the .unknown case duplicates the existing handlers for .file (lines 832-838), .directory (lines 840-874), and .sym_link (lines 876-909). This creates ~60 lines of duplicated logic that must be maintained in parallel.
♻️ Refactoring approach to eliminate duplication
Consider extracting the kind-specific processing logic into helper methods that can be called from both the direct-kind cases and the .unknown fallback case:
// Helper for file processing
fn processFileEntry(
this: *Iterator,
entry_name: []const u8,
dir_iter_state: *const IterState.Directory,
) !Maybe(?MatchedPath) {
const matches = this.walker.matchPatternFile(
entry_name,
dir_iter_state.component_idx,
dir_iter_state.is_last,
dir_iter_state.pattern,
dir_iter_state.next_pattern
);
if (matches) {
const prepared = try this.walker.prepareMatchedPath(
entry_name,
dir_iter_state.dir_path
) orelse return .{ .result = null };
return .{ .result = prepared };
}
return .{ .result = null };
}Then the .unknown case becomes:
.unknown => {
const might_match = this.walker.matchPatternImpl(dir_iter_state.pattern, entry_name);
if (!might_match) continue;
const name_z = this.walker.arena.allocator().dupeZ(u8, entry_name) catch bun.outOfMemory();
const real_kind: std.fs.File.Kind = switch (Accessor.lstatat(dir.fd, name_z)) {
.result => |st| bun.sys.kindFromMode(st.mode),
.err => continue,
};
switch (real_kind) {
.file => {
if (try this.processFileEntry(entry_name, dir_iter_state)) |result| {
return result;
}
},
.directory => {
if (try this.processDirectoryEntry(entry_name, dir_iter_state, dir.dir_path)) |result| {
return result;
}
},
.sym_link => {
if (try this.processSymlinkEntry(entry_name, dir_iter_state, dir.dir_path)) |result| {
return result;
}
},
else => {},
}
continue;
},This approach ensures that changes to file/directory/symlink processing logic only need to be made once.
Avoids arena allocation for short filenames (< 256 bytes) in the DT_UNKNOWN code path, matching the existing pattern used elsewhere in GlobWalker.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/sys.zig
🧰 Additional context used
📓 Path-based instructions (2)
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/sys.zig
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct { #foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/sys.zig
🧠 Learnings (1)
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/sys.zig
Use O.NOFOLLOW flag which maps to FILE_OPEN_REPARSE_POINT on Windows, making lstatat behave like POSIX lstat (doesn't follow symlinks). This addresses CodeRabbit feedback about Windows symlink handling parity.
3 participants
What does this PR do?
Fixes #24007
Possibly fixes #18902, #7412
Some filesystems (bind mounts, FUSE, NFS) don't provide
d_typein directory entries, returningDT_UNKNOWN. This caused glob and recursive readdir to skip entries entirely.Problem
On Linux filesystems that don't populate
d_typein directory entries (bind mounts, FUSE, NFS, some ext4 configurations),readdir()returnsDT_UNKNOWNinstead of the actual file type. This caused:Bun.Globto skip files/directories entirelyfs.readdirSync(..., {recursive: true})to not recurse into subdirectoriesfs.readdirSync(..., {withFileTypes: true})to report incorrect typesSolution
Implemented a lazy
lstatat()fallback whend_type == DT_UNKNOWN:sys.zig: Addedlstatat()function - same asfstatat()but withAT_SYMLINK_NOFOLLOWflag to correctly identify symlinksGlobWalker.zig: When encountering.unknownentries, first check if filename matches pattern, then calllstatat()only if needednode_fs.zig: Handle.unknownin both async and sync recursive readdir paths; propagate resolved kind to Dirent objectsdir_iterator.zig: Return.unknownforDT_UNKNOWNentries, letting callers handle lazy statWhy
lstatatinstead offstatat? We useAT_SYMLINK_NOFOLLOWto preserve consistent behavior with normal filesystems - symlinks should be reported as symlinks, not as their target type. This matches Node.js behavior which useslstat()for the DT_UNKNOWN fallback, and follows the lazy stat pattern established in PR #18172.How did you verify your code works?
Testing:
test/regression/issue/24007.test.tstest/cli/run/glob-on-fuse.test.ts(reusesfuse-fs.pyfrom PR Correctly handle unknown type in FileSystem.DirEntry.addEntry #18172, includes symlink verification)0 files3 filesPerformance: No impact on normal filesystems - the
.unknownbranch is only hit whend_type == DT_UNKNOWN. The lazy stat pattern avoids unnecessary syscalls by checking pattern match first.