Repository navigation
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Normalize filesystem overloads before native I/O so explicitly undefined and null arguments retain Node defaults, signed read-length coercion, bigint position validation and exact diagnostics. Preserve FileHandle defaults and native buffer ownership checks; consume every write argument independently. Adapts the independent write-slot proposal from oven-sh#37647. Thanks @robobun! Upstream proposal: oven-sh#44476. Node 24 oracle: all 1,217 non-crashing cases match. Both native fork CI lanes passed on the qualified head. OpenClaw's readSync capsule failure is fixed; the independent receipt-withdrawal race remains.
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
WalkthroughFilesystem read and write APIs now validate arguments and buffers, apply overload-specific defaults, and normalize string encodings. Tests cover read and write behavior, including zero-length reads, and the runtime compatibility notes document these cases. ChangesNode.js filesystem read and write compatibility
Suggested reviewers: Priority: Severity of issue fixed: Low Merge Risk:
 |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
No active directly linked issue remains. Issue #29017 is closed and provides historical context only. Therefore, no linked-issue coding requirements apply to this pull request. |
| Out of Scope Changes check | |
The changed source files implement filesystem argument normalization and validation. The tests cover the same read and write behaviors. The documentation records those behaviors. These changes remain … |
| Title check | |
The title clearly and concisely identifies the main change: matching Node.js read and write argument defaults in fs APIs. |
| Description check | |
The description explains the implementation scope, compatibility goals, validation approach, test results, and known unrelated race. It provides the required information for what the PR does and how i… |
Full details: Out of Scope Changes check
-
Explanation
The changed source files implement filesystem argument normalization and validation. The tests cover the same read and write behaviors. The documentation records those behaviors. These changes remain within the pull request scope described in the summary.
Full details: Description check
-
Explanation
The description explains the implementation scope, compatibility goals, validation approach, test results, and known unrelated race. It provides the required information for what the PR does and how it was verified, despite not using the template headings verbatim.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Autopilot is currently an internal CodeRabbit preview.
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 help to get the list of available commands.
Normalize overload defaults and validation before native I/O, including explicit undefined/null slots, zero-length position validation, bigint positions and exact errors. Adapt the independent write-slot behavior proposed in oven-sh#37647; thanks to robobun. Preserve native buffer ownership and bounds checks.
0f631a8 to
d3552fc
Compare
fs.readSync(fd, buffer, undefined, undefined, undefined)throws a NaN-lengthERR_OUT_OF_RANGEin Bun, while Node coerces the positional length to zero. Buffer writes also stop parsing at an undefined offset and ignore a later length or position.This normalizes the overloads in JavaScript before native I/O, matching Node's validation order and error text. Sync/callback reads use signed 32-bit length coercion;
FileHandle.readdefaults nullish lengths to the remaining buffer. Zero-byte reads still validate positions. Buffer writes consume all argument slots, and string writes preserve the UTF-8 fallback. Native buffer ownership and bounds checks remain in place.The independent write-slot behavior adapts the proposal in #37647, with thanks to @robobun. This extends that work to exact diagnostics and sibling read overloads. Closed #29017 only partially covers zero-length position validation; this change also fixes the undefined-length gap and enables the existing expected-failure test. The empty-buffer Node test again asserts the complete Node message.
Validation uses Node 24.21.0 as the oracle: 1,227 isolated argument shapes covering positional/options forms, undefined/null values, bigint positions, bounds/type errors, callback/FileHandle overloads, views and string writes. All 1,217 non-crashing Node outcomes match the patched fork exactly (byte effects, error class/code/message and delivery channel). Ten malformed FileHandle length cases abort Node; Bun retains recoverable behavior. The new regression groups fail on the unpatched fork and pass patched. All 25 filesystem files pass on the qualified fork through the repository runner, and 20 selected ported Node read/write files pass under Node 24 and patched Bun.
The same change removes an OpenClaw consumer failure caused by a preload forwarding explicit undefined values to readSync. Eleven additional consumer files pass on Node, unpatched Bun and patched Bun (216 tests per runtime). A separate receipt-withdrawal test race remains unchanged.
On upstream base
e29a7ca47cb31c58c9651287af567c705c036ca2, the Darwin release build passes all 13 regression groups (183 assertions), and its oracle also matches all 1,217 non-crashing Node results. A broader four-file run passed 659 tests and hit the existing fs.stat/setImmediate ordering assertion once; that assertion passed alone, and the same alternate order was independently reproduced on Node 24 (one of five runs). The complete four-file rerun passed 660 tests (21 skipped, zero failures). This PR does not change fs.stat. Scoped P2 review, lint, built-in TypeScript checking and formatting pass. The build-only macOS SDK compatibility patch is excluded from this PR.AOT-main rebase validation
Rebased onto upstream main
11c367163a4baa77d3627d14764c376c856a630a. Linux x64 main fails 13 read/write argument-default cases. The rebased release passes the full fs file: 642 passed, eight skipped, zero failed, and the ported Node empty-buffer test exits successfully. The rebase uses the current generated normalizeEncoding binding; formatting passes.The release build uses
bun run build:release --lto=off. Its executable revision matchesd3552fcce41a58e731bd4a2add03af1f11cdd89f. Independent P0–P2 review passes. Earlier platform results above are historical; this rebase's fresh validation is scoped to the results here.