node:fs: accept the readdir recursive/withFileTypes values node accepts

[robobun]

Problem

• fs.readdirSync(dir, { recursive: null }) throws ERR_INVALID_ARG_TYPE (The "recursive" property must be of type boolean, got object); node returns the flat listing. Same for fs.readdir (callback) and fs.promises.readdir.
• { withFileTypes: 1 }, { withFileTypes: "x" }, { withFileTypes: null }, { withFileTypes: 0 } throw the same way in all three forms; node returns Dirents for the truthy values and names for the falsy ones.
• fs.promises.readdir(dir, { recursive: "x" }) / { recursive: 1 } reject; node returns the recursive listing ({ recursive: 0 } the flat one).
• Cause: args::Readdir::from_js (src/runtime/node/node_fs.rs:3488-3493) reads both options with get_boolean_strict, which rejects anything present that is not undefined or a boolean. It is the one parser behind readdirSync, readdir and promises.readdir.
• Node (v26.3.0): readdir/readdirSync run validateBoolean only when options.recursive != null (lib/fs.js#L1546-L1548); no entry point validates withFileTypes, they all pass !!options.withFileTypes (lib/fs.js#L1557); fs.promises.readdir never validates recursive and just branches on it (lib/internal/fs/promises.js#L1601).
• Until bake: csr, streaming ssr, serve integration, safer jsvalue functions, &more #14900 replaced the old getOptional(..., bool) with getBooleanStrict, the parser treated null as absent and accepted numbers, so { recursive: null } and { withFileTypes: 1 } used to work in Bun too.

Fix

• Readdir::from_js: a null recursive is skipped (stays false); any other non-boolean still goes through validators::validate_boolean, so readdirSync/readdir keep throwing ERR_INVALID_ARG_TYPE for 0, "x", {}, ... with the message unchanged. withFileTypes is read with get_boolean_loose, i.e. JS truthiness, like !!options.withFileTypes.
• fs.promises.readdir (src/js/node/fs.promises.ts) gets a small wrapper in place of asyncWrap: when recursive is present, non-nullish and not a boolean, it passes the native binding { __proto__: options, recursive: !!recursive }, so the parser still reads encoding/withFileTypes off the caller's object (own or inherited, exactly as on the pass-through path) and the caller's object is never mutated. Booleans, null and undefined go through untouched, so the common case does no extra work. This is the only form whose node behavior differs from the shared parser, and the same file already adapts rm/rmdir this way. (Side effect: promises.readdir.length is now 2, as in node; it was 3.)
• Why this is right: node's observed behavior is the contract for node:*, and every loosened value here is one node accepts; the values node rejects (non-nullish, non-boolean recursive in the sync and callback forms) are still rejected, and mkdir's recursive, which node validates strictly, is untouched.
• The error text for the remaining rejection is deliberately left as is; node:fs: report mkdir/readdir/rm boolean option type errors like node #39328 changes that wording. Both PRs touch these lines: whichever lands second keeps this PR's acceptance rules with node:fs: report mkdir/readdir/rm boolean option type errors like node #39328's message, and node:fs: report mkdir/readdir/rm boolean option type errors like node #39328's readdir assertions that withFileTypes: "x" / withFileTypes: 1 / withFileTypes: null throw and that promises.readdir({ recursive: "x" }) rejects no longer hold once this lands.
• Tests: test/js/node/fs/fs.test.ts, describe("readdir accepts the recursive/withFileTypes values node accepts"). 12 accepted option bags checked through all three forms, 7 non-boolean recursive values checked to still throw synchronously in the sync and callback forms (callback never called) and to act as truthiness in the promise form, plus a no-mutation check and an inherited-withFileTypes check for the coercion path. 18 of the 28 cases fail on the unfixed build (USE_SYSTEM_BUN=1), all pass with the fix; the rest pin what must keep rejecting.
• Also run with the fix: the whole of fs.test.ts (539 pass), promises.test.js, dir.test.ts, and the ported test-fs-readdir*.js / test-fs-promises.js.

Background

• node:fs in Bun has one native argument parser per operation (args::*::from_js in node_fs.rs). The sync binding and the async binding both call it, and fs.readdir (callback) and fs.promises.readdir both call the same async binding, so a rule that only applies to the promise form has to live in fs.promises.ts.
• JSValue::get returns None for a missing or undefined property, so { recursive: undefined } was already treated as absent; null is the value that reaches the check. get_boolean_loose is the existing helper for "missing means default, otherwise JS truthiness"; get_boolean_strict is the one for "must be a boolean".
• validators::validate_boolean is the Rust port of node's validateBoolean; it throws the same ERR_INVALID_ARG_TYPE get_boolean_strict threw before, so only the accepted set changes in this PR.

node 26.3.0 vs bun 1.4.0 vs this branch

Each option was probed through readdirSync, readdir (callback) and promises.readdir on a directory containing a.txt and sub/b.txt.

                      node 26.3.0 (sync / cb / promises)     bun 1.4.0 (all three)    this branch
recursive: null       names x2 / names x2 / names x2         ERR_INVALID_ARG_TYPE     same as node
recursive: 0          THROW / THROW / names x2               ERR_INVALID_ARG_TYPE     same as node
recursive: 1          THROW / THROW / names x3               ERR_INVALID_ARG_TYPE     same as node
recursive: "x"        THROW / THROW / names x3               ERR_INVALID_ARG_TYPE     same as node
recursive: {}         THROW / THROW / names x3               ERR_INVALID_ARG_TYPE     same as node
recursive: 0n         THROW / THROW / names x2               ERR_INVALID_ARG_TYPE     same as node
withFileTypes: null   names x2 in all three                  ERR_INVALID_ARG_TYPE     same as node
withFileTypes: 0      names x2 in all three                  ERR_INVALID_ARG_TYPE     same as node
withFileTypes: ""     names x2 in all three                  ERR_INVALID_ARG_TYPE     same as node
withFileTypes: 1      dirents x2 in all three                ERR_INVALID_ARG_TYPE     same as node
withFileTypes: "x"    dirents x2 in all three                ERR_INVALID_ARG_TYPE     same as node
withFileTypes: {}     dirents x2 in all three                ERR_INVALID_ARG_TYPE     same as node

THROW is node's ERR_INVALID_ARG_TYPE, thrown synchronously in the callback form as well (the callback is not invoked); this branch does the same. undefined, true and false behave identically in all three runtimes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9c51252a-f71a-4f5c-b5eb-814097686290

📥 Commits

Reviewing files that changed from the base of the PR and between aec33f5 and 0949c4e.

📒 Files selected for processing (3)

• src/js/node/fs.promises.ts
• src/runtime/node/node_fs.rs
• test/js/node/fs/fs.test.ts

Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review.

---

Walkthrough

readdir now applies Node-compatible coercion and validation for recursive and withFileTypes across sync, callback, and promise APIs. Tests cover return values, errors, callback behavior, option immutability, and inherited properties.

Changes

readdir option compatibility

Layer / File(s)	Summary
Option handling src/js/node/fs.promises.ts, src/runtime/node/node_fs.rs	The promise API coerces defined non-boolean recursive values. Runtime parsing validates non-nullish recursive values and loosely converts withFileTypes.
Compatibility validation test/js/node/fs/fs.test.ts	Tests cover option values across all readdir APIs, API-specific errors, callback invocation, option immutability, and inherited properties.

Possibly related PRs

• oven-sh/bun#38961: Both PRs modify node_fs.rs readdir implementations.

Suggested reviewers: cirospaciari, jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: Node-compatible handling of readdir option values.
Description check	✅ Passed	The description explains the problem, implementation, compatibility behavior, and verification results, covering the template requirements despite different headings.

---

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

[robobun]

Reproduced on bun 1.4.0 and on main (aec33f5) by running each of readdirSync, readdir (callback) and promises.readdir with recursive / withFileTypes set to null, 0, 1, "", "x", {}, [], 0n and comparing against node v26.3.0 on the same directory (table in the PR body). Bun threw ERR_INVALID_ARG_TYPE for all of them; node accepts everything except a non-nullish, non-boolean recursive in the sync and callback forms.

With this branch the three forms match node for every value probed. The new cases in test/js/node/fs/fs.test.ts ("readdir accepts the recursive/withFileTypes values node accepts") fail 18/28 on the unfixed build and pass with the fix; the whole file passes (539 tests).

Related: #39328 changes the wording of the error that remains for a bad recursive; the readdir hunks of the two PRs overlap, see the Fix section of the PR body for how they combine.

[claude]

LGTM — small, well-scoped Node compat fix with a thorough test matrix. The inline nit (spread dropping inherited options) needs both an inherited withFileTypes/encoding and a non-boolean recursive to trigger, and that input threw before this PR anyway; fine to take or leave.

Extended reasoning...

Overview

Loosens fs.readdir option parsing to match Node v26.3.0: args::Readdir::from_js in src/runtime/node/node_fs.rs now skips a null recursive (was strict-boolean → ERR_INVALID_ARG_TYPE) and reads withFileTypes via get_boolean_loose (JS truthiness, matching Node's !!options.withFileTypes). src/js/node/fs.promises.ts replaces the asyncWrap wrapper for readdir with one that coerces a non-nullish, non-boolean recursive to !!recursive before calling the native binding — the promise form is the only one where Node accepts truthy non-booleans, and the shared native parser can't special-case one caller. New describe block in test/js/node/fs/fs.test.ts covers 12 accepted option bags across all three forms, 7 rejected recursive values in sync/callback that resolve as truthiness in the promise form, and a no-mutation check.

Security risks

None. This only relaxes type validation on two option fields to match Node; the resulting values are still bool on the Rust side. No new user-controlled data reaches a syscall path differently than before.

Level of scrutiny

Low-to-medium. ~15 lines of production code, all in the argument-parsing prelude before any I/O. validators::validate_boolean and get_boolean_loose are existing helpers already used throughout node_fs.rs / JSValue.rs. The JS wrapper follows the same shape as rm a few lines below. Each change is line-for-line traceable to a cited Node source line, and the PR body includes an empirical Node-vs-Bun comparison table.

Other factors

• validators:: is already in scope in node_fs.rs (used at 2682, 2756, etc.), so no import change needed.
• val.get returns None for missing/undefined, so the is_null() guard correctly leaves recursive = false for both nullish spellings while still routing 0/"x"/{} through validate_boolean — the sync/callback rejection set is unchanged.
• The promise wrapper only allocates a new options object on the exotic path (non-nullish, non-boolean recursive); booleans and null pass the caller's object straight through, and the caller's object is never mutated (test pins this).
• Tests use tempDir, it.each, jest.fn() for the callback-not-called assertion, and assert code: 'ERR_INVALID_ARG_TYPE' rather than bare toThrow(). 17/27 cases fail on the unfixed build per the PR body, so the suite is load-bearing.
• The one inline nit is real but requires inherited withFileTypes/encoding combined with a non-boolean recursive; that same input threw ERR_INVALID_ARG_TYPE before this PR, so it's not a regression from a working state. The suggested { __proto__: options, ... } fix is a one-liner if the author wants it.
• The PR body flags the overlap with #39328 (error-message wording) and how the two combine; nothing to do here beyond whichever lands second reconciling.

[robobun]

^{Updated 12:59 AM PT - Aug 16th, 2026}

@robobun, your commit 0949c4e is building: #99316

[claude]

LGTM — the earlier nit about { ...options } dropping inherited options is addressed (now { __proto__: options, recursive: !!recursive }) with a covering test, and the comment-cop feedback is resolved.

What was reviewed:

• Readdir::from_js: val.get returns None for missing/undefined so the added !r.is_null() guard exactly matches Node's options.recursive != null; get_boolean_loose → to_boolean() matches !!options.withFileTypes.
• Confirmed validators::validate_boolean is in scope (use super::util::validators at node_fs.rs:187) and accepts &str for the name arg.
• The __proto__-shadow wrapper: verified JSValue::get walks the prototype (via getIfPropertyExistsImpl), so encoding/withFileTypes on the original object are still seen and the caller's object is never mutated.
• Tests: all referenced imports (Dirent, inspect, promisify, jest, tempDir) are already in the file; the callback-form rejection test asserts the callback is never invoked.

Extended reasoning...

Overview

Loosens readdir option parsing to match Node v26.3.0: recursive is only validated when non-nullish (sync/callback forms), withFileTypes is truthiness-coerced everywhere, and fs.promises.readdir skips recursive validation entirely (truthiness only). Touches src/runtime/node/node_fs.rs (~8 lines in Readdir::from_js), src/js/node/fs.promises.ts (a 9-line wrapper replacing asyncWrap), and adds a ~100-line test block to test/js/node/fs/fs.test.ts.

Security risks

None. This only loosens type validation on two boolean-ish option flags to match Node's coercion rules; no paths, buffers, or privileged operations are affected.

Level of scrutiny

Low-to-medium. This is a focused Node-compat fix in a well-understood parser. The Rust change swaps get_boolean_strict for existing helpers (get + is_null + validators::validate_boolean, and get_boolean_loose) whose semantics I verified against source: JSValue::get returns None for missing/undefined and Some for everything else including null, so the !r.is_null() guard is exactly Node's != null check; get_boolean_loose calls to_boolean(), i.e. JS truthiness. The JS wrapper only fires on the exotic case (non-nullish, non-boolean recursive) and passes the common case straight through.

Other factors

• My prior review's nit (spread dropping inherited options) was addressed in 9f6275d with the __proto__-shadow approach and a regression test that fails on the spread version. All comment-cop flags were resolved in 0949c4e by collapsing the multi-line comments to single lines with node permalinks.
• Test coverage is thorough: 12 accepted option bags × 3 API forms, 7 non-boolean recursive values × (sync+callback rejection, promise truthiness), plus no-mutation and inherited-option checks. robobun confirmed 18/28 fail on the unfixed build and all pass with the fix.
• The PR body documents the interaction with #39328 (which changes the remaining error's wording); whichever lands second needs a small reconciliation but that's a merge-order concern, not a correctness issue here.
• validators is already imported at node_fs.rs:187 and validate_boolean's name: impl fmt::Display accepts the bare "recursive" string literal — the change compiles as written.