[robobun]

Problem

• fs.writeFileSync(p, "ab", { flush: "yes" }) throws ERR_INVALID_ARG_TYPE with the message The "flush" argument must be of type boolean. Received type string ('yes'). Node v26.3.0 throws the same code with The "options.flush" property must be of type boolean. Received type string ('yes').
• The same text comes out of every entry point that takes string or Buffer data, because they share one native options parser (args::WriteFile::from_js_with_default_flag): fs.writeFile, writeFileSync, appendFile, appendFileSync, fs.promises.writeFile, fs.promises.appendFile, FileHandle#appendFile.
• Two causes:
  • src/runtime/node/node_fs.rs:4203 passes the bare name flush. Node validates the option as options.flush (validateBoolean(flush, 'options.flush') in lib/fs.js and lib/internal/fs/promises.js).
  • JSGlobalObject::throw_invalid_argument_type_value (src/jsc/JSGlobalObject.rs:559) hardcodes The "<name>" argument, so even a dotted name renders as an argument. Other callers already pass dotted names and hit the same thing: fs.rmSync(p, { maxRetries: "a" }) and new net.SocketAddress({ address: 1 }) say "options.maxRetries" argument / "options.address" argument where node says property.
• The vendored node tests for this (test-fs-write-file-flush.js, test-fs-append-file-flush.js) only check the code, which was already right, so they did not catch it.

Fix

• node_fs.rs: the flush error names the option options.flush.
• JSGlobalObject.rs: the three Rust ERR_INVALID_ARG_TYPE renderers (throw_invalid_argument_type_value, ..._value2, ..._one_of) format the name through InvalidArgTypeName, which applies node's subject rule: "x" argument, "a.b" property, or the name verbatim when it already ends in argument. This is the rule Bun::Message::addParameter in ErrorCode.cpp already applies to every C++ caller, so the Rust and C++ renderers now agree.
• Why this is right: for node:fs errors node's text is the contract (jsc: use 'property' wording in get_optional_slice type error #32355 landed the same argument-to-property correction for another Rust helper), and these messages are byte-identical to node v26.3.0. A script exercising the 7 flush entry points (string and Buffer data, path and fd targets), rmSync retry options and SocketAddress options diffs clean between this build and a local node, apart from things this PR leaves alone: FileHandle#writeFile does not forward flush on main (fs.promises: validate options.flush for iterable data and FileHandles, sync like node #39192 adds it), the signal option error (node:fs: validate options.signal in readFile, writeFile and watch like node #39274), and the mkdir/readdir recursive and rm recursive/force errors, which go through two other helpers and are tracked separately.
• Blast radius of the renderer change: only names containing a . change output. The dotted names currently passed to these renderers are options.flush (this PR), options.retryDelay / options.maxRetries (fs.rm), options.address / options.flowlabel (net.SocketAddress), options.silent / endStream / exclusive / parent / weight / signal (http2 priority and request options), handle.fd (cluster), and Bun's own compress.encoding (fetch) and rule.syscall / rule.action (socket fault injection). All of the node ones render as property in node; no test in the repo pinned the old wording for any of them. No Rust caller passes a name ending in argument. Names without a dot render exactly as before.
• Overlap: the JSGlobalObject.rs hunk is the one from node: render the Rust validators' ERR_INVALID_ARG_TYPE messages like Node #38663 (validators message parity), and node:fs: validate options.signal in readFile, writeFile and watch like node #39274 (options.signal, which edits the same WriteFile parser a few lines above this hunk) carries it verbatim too. Trial merges of this PR with either of them, and of all three together, are conflict-free and end up with a single InvalidArgTypeName; whichever lands first carries the renderer change and the others reduce to their own call sites. Neither of them touches the flush name. This stays a separate PR because it only changes text, while node:fs: validate options.signal in readFile, writeFile and watch like node #39274 also changes which values are accepted.
• Verified:
  • test/js/node/fs/fs.test.ts: new writeFile/appendFile flush option block covers all 7 entry points (path and fd, string and Buffer) for three Received shapes, plus the accepted values; a second block pins a dotted name reached through the validators (rmSync options.maxRetries) and a plain name (fsyncSync fd) still rendering as an argument. 7 of the 9 new tests fail on the released build, all pass with this change; the whole file passes (521 pass).
  • Existing tests that pin plain-name output from each renderer still pass: test-crypto-random.js and test/js/web/fetch/wasm-streaming.test.ts (..._value2), test/js/node/child_process/child_process-node.test.js (..._one_of).
  • Vendored test-fs-write-file-flush.js, test-fs-append-file-flush.js, test-fs-rm.js, test-socketaddress.js, and test/js/node/net/socketaddress.spec.ts, test/js/web/fetch/fetch-compress.test.ts pass.

Background

• ERR_INVALID_ARG_TYPE(name, expected, actual) in node's lib/internal/errors.js renders The <subject> must be <expected>. Received <description of actual>. The subject is "name" argument, unless the name contains a ., in which case it is an option path such as options.flush and renders as "name" property; a name that already ends in argument (such as first argument) is used verbatim.
• Bun renders these messages in two places: C++ (ErrorCode.cpp, used by the JS builtins through $ERR_INVALID_ARG_TYPE and by C++ bindings) and the Rust helpers on JSGlobalObject used by natively implemented modules. The Received ... half is shared (the Rust helpers call the C++ determineSpecificType), which is why only the subject differed.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 28 seconds

Limit details: You’ve used all 5 included reviews currently available under your plan.

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46c22e26-9db8-4138-94cf-a71a11c08e91

📥 Commits

Reviewing files that changed from the base of the PR and between a42889a and 1650c88.

📒 Files selected for processing (3)

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

---

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

[robobun]

^{Updated 6:48 PM PT - Aug 15th, 2026}

@robobun, your commit 1650c88 is building: #98973

[robobun]

Status: reproduced on the released build (1.4.0) with fs.writeFileSync(p, "ab", { flush: "yes" }) and the other six string/Buffer entry points; all report The "flush" argument ... where node v26.3.0 reports The "options.flush" property .... With this branch the comparison script matches node line for line. Tests: test/js/node/fs/fs.test.ts (writeFile/appendFile flush option), 7 of the 9 new cases fail before the change and pass after.

Self-review outcome: the only concern was packaging. The JSGlobalObject.rs hunk is the same one carried by #38663 and #39274; trial merges of any combination of the three are conflict-free, so this PR is kept as the text-only fix for flush and needs no particular merge order. The related mkdir/readdir recursive and rm recursive/force messages (different helpers) are tracked separately. Waiting on CI.

[claude]

LGTM — small Node-compat fix that names options.flush correctly and makes the Rust ERR_INVALID_ARG_TYPE renderers apply the same argument/property rule the C++ side already uses.

What was reviewed:

• InvalidArgTypeName::fmt matches Bun::Message::addParameter in ErrorCode.cpp exactly ( argument suffix → verbatim; contains . → property; else → argument); undotted names render byte-identical to before.
• strings::contains_char exists in bun_core::string::immutable and is already in scope.
• New tests cover all 7 flush entry points (sync/cb/promise, path/fd, string/Buffer), the accepted-values path, a dotted validator name (rmSync options.maxRetries), and a plain name (fsyncSync fd) — all needed imports already present in the file.

Extended reasoning...

Overview

Two source changes and one test file. src/runtime/node/node_fs.rs renames the flush option error subject from "flush" to "options.flush" at the single shared parse site. src/jsc/JSGlobalObject.rs adds a private InvalidArgTypeName Display wrapper and routes the three Rust ERR_INVALID_ARG_TYPE renderers (throw_invalid_argument_type_value, ..._value2, ..._one_of) through it so a dotted name renders as "x.y" property and a name already ending in argument renders verbatim, matching Node and matching what Bun::Message::addParameter in ErrorCode.cpp already does for the C++/JS-builtin path. test/js/node/fs/fs.test.ts gains two describe blocks pinning the message text across every writeFile/appendFile entry point plus a dotted and a plain name through other renderers.

Security risks

None. Pure error-message formatting; no parsing of untrusted input, no allocation changes, no FFI signature changes, no new syscalls.

Level of scrutiny

Low-to-moderate. The node_fs.rs hunk is a one-string change. The JSGlobalObject.rs hunk changes shared renderers, so the blast-radius question is whether any existing caller depended on the old wording. I confirmed the new Display impl is byte-identical to the old output for every name that lacks a . (the else branch emits exactly "\"{name}\" argument"), so only dotted names change — and for those the new output is what Node produces. The PR description enumerates every dotted caller and states no test pinned the old text; the author also re-ran the plain-name pinning tests. The logic mirrors the existing C++ addParameter I read in ErrorCode.cpp:632-642 line for line.

Other factors

Tests are thorough and follow harness conventions (tempDir with using, fd closed in finally, it.each matrix, awaited .rejects, negative assertion that the file was not written). All identifiers used in the new test code (tempDir, writeFileSync, openSync, closeSync, readFileSync, promises, rmSync, join, fs) are already imported at the top of the file. strings::contains_char is a real helper in bun_core::string::immutable.rs and strings is already in the use list. No prior human or bot review comments to address; CI is building. This is a focused, well-verified compat fix and I am comfortable approving.