fix(io): respect mode option when copying files with Bun.write()#25906
Conversation
|
Updated 4:21 PM PT - Jan 8th, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 25906That installs a local version of the PR into your bun-25906 --bun |
| // Apply destination mode if specified | ||
| if (this.destination_mode) |mode| { | ||
| if (this.destination_file_store.data.file.pathlike == .path) { | ||
| var node_fs: jsc.Node.fs.NodeFS = .{}; |
There was a problem hiding this comment.
This should definitely be using fchmod and only at the desired call site. Not synchronously on the main thread.
WalkthroughAdds an optional numeric 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. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📚 Learning: 2025-11-14T16:07:01.064ZApplied to files:
📚 Learning: 2025-10-19T02:52:37.412ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2026-01-05T23:04:01.518ZApplied to files:
📚 Learning: 2026-01-05T23:04:01.518ZApplied to files:
📚 Learning: 2025-10-17T20:50:58.644ZApplied to files:
🔇 Additional comments (2)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bun-types/bun.d.ts (1)
814-821: ClarifymodeJSDoc for BunFile copy overloadsThe new
modeoption is exposed correctly on both copy overloads, but the doc string:/** * If writing to a PathLike, set the permissions of the file. */ mode?: number;is misleading for the
write(destination: BunFile, input: BunFile, ...)overload — that overload doesn’t take aPathLikedestination.Consider rephrasing this comment so it’s accurate for both overloads, e.g.:
/** * Set the permissions of the destination file when copying. */ mode?: number;and reuse that wording in both options blocks.
Also applies to: 851-858
🤖 Fix all issues with AI agents
In @src/bun.js/webcore/Blob.zig:
- Around line 1545-1550: Validate the numeric mode before casting: after
obtaining mode_value from options_object.getTruthy and confirming isNumber(),
read its integer (e.g., using mode_value.toInt64()), check that it is within the
allowed unsigned file-mode range 0..0o777 (0..511) and not negative, and if it
is out of range return an appropriate error via globalThis (similar to other
argument errors); only then cast to the bun.Mode type (instead of
unconditionally using @intCast) to avoid runtime panics from negative or
out-of-range values.
In @src/bun.js/webcore/blob/copy_file.zig:
- Around line 453-456: The code ignores errors from bun.sys.chmod after a
successful clonefile, leaving destination permissions unchanged; modify the
block in copy_file.zig where this.destination_mode is applied to check the
return/error from bun.sys.chmod (invoke on the path from
this.destination_file_store.pathlike.path.sliceZ(&path_buf)) and handle
failures: on chmod error, either abort the copy by removing/cleaning the
destination file and return/propagate an error from the enclosing function (so
the overall operation fails), or at minimum log the failure with details and
propagate an error; ensure the surrounding function (copy logic using clonefile
and this.destination_mode) propagates the chmod error to callers rather than
discarding it.
- Around line 1070-1079: The code silences errors from node_fs.chmod when
applying this.destination_mode, which can leave the destination file with
incorrect permissions; change the block in copy_file.zig where
this.destination_mode is handled (the this.destination_mode conditional that
calls node_fs.chmod on this.destination_file_store.data.file.pathlike) to
capture the result, check for an error, and propagate or surface it (e.g.,
return the error or call the existing error handling path) instead of assigning
to `_`; apply the same explicit error handling pattern to the macOS path variant
that also calls chmod so failures are not ignored.
In @test/regression/issue/25903.test.ts:
- Around line 41-57: The test "Bun.write() uses default permissions when mode is
not specified" is flaky because it asserts a hardcoded 0o644; instead, create a
baseline file (e.g., baselinePath) written with Bun.write(baselinePath, "x")
without specifying mode and stat it to obtain expected permissions under the
current umask, then assert that the destination file (destPath) does not equal
the source restrictive mode (0o600) and that (destStat.mode & 0o777) equals
(baselineStat.mode & 0o777); update the assertions to use Bun.write, Bun.file,
stat, baselinePath, baselineStat and destStat comparisons so the test is
umask-agnostic while still verifying no inheritance of 0o600.
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 65d006a and 956536ce4a568e4d84288c47d399255517bb9318.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
🧠 Learnings (22)
📓 Common learnings
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.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
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.
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
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.
📚 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/25903.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/25903.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/25903.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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} : Do not set a timeout on tests. Bun already has timeouts built-in.
Applied to files:
test/regression/issue/25903.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/regression/issue/25903.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 unit tests by module in directories like `/test/js/bun/` and `/test/js/node/`.
Applied to files:
test/regression/issue/25903.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/regression/issue/25903.test.ts
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/bun.js/webcore/Blob.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
src/bun.js/webcore/Blob.zigpackages/bun-types/bun.d.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/webcore/Blob.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.ts
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (1)
test/regression/issue/25903.test.ts (1)
5-23: Regression coverage for explicitmodeandcreatePathlooks solidThese three tests cleanly exercise:
- copy with explicit restrictive perms (Line 18)
- copy with
createPath: trueinto a subdirectory (Line 34)- copy from
Bun.fileto PathLike with an explicit execute bit (Line 68)They match the PR objective (ensuring the destination honors
modein file-copy paths) and are deterministic given the implementation applieschmodafter copy. No changes needed here.Also applies to: 25-39, 59-72
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
2 times, most recently
from
3da5a56 to
929ddd2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bun-types/bun.d.ts (1)
724-741:modeoption typing looks correct; consider tightening doc wording around where it appliesThe added
mode?: numberoptions on thewrite(...)overloads match the implementation and unblock specifying permissions when copying to filesystem paths.The JSDoc text “If writing to a PathLike, set the permissions of the file.” is a bit ambiguous on the BunFile-only overload, since
destinationthere isBunFile. In practice the runtime only usesmodewhen the destination ultimately resolves to a real filesystem path (e.g. stringPathLikeor aBun.file("path")backing a path, not a raw fd or S3).Suggest rephrasing along the lines of:
“When the destination refers to a filesystem path (e.g. a string path or a
BunFilecreated from a path), set the permissions of the file. Ignored for non-path destinations such as file descriptors or S3.”to better set expectations without changing behavior.
Also applies to: 814-831, 852-868
🤖 Fix all issues with AI agents
In @src/bun.js/webcore/Blob.zig:
- Around line 1545-1550: The code accepts options.mode as a number but doesn't
validate its value range before casting; after obtaining mode_value (from
options_object.getTruthy) and before calling toInt64()/@intCast, convert to an
integer, verify it is within the valid Unix permission range (e.g., >= 0 and <=
0o7777) and reject out-of-range or negative values by returning an appropriate
JS error (e.g., use globalThis.throwRangeError or similar), then safely cast
using @intCast to the intended unsigned type (e.g., u32/usize) and assign to
mode; reference symbols: options_object.getTruthy, mode_value, mode, toInt64(),
@intCast(), and globalThis.throwInvalidArgumentType/throwRangeError.
In @test/regression/issue/25903.test.ts:
- Around line 41-57: The test "Bun.write() uses default permissions when mode is
not specified" currently hard-codes 0o644; instead derive the expected default
at runtime by creating a new temp file via Bun.write without a mode (or by using
process.umask to compute the default) and reading its mode (e.g., write a temp
file, stat it, use that mode & 0o777 as expectedDefault), then assert
(destStat.mode & 0o777) === expectedDefault and additionally assert it does not
equal the source's restrictive mode (0o600) to ensure permissions weren't
inherited; update the assertions in the test accordingly (references: Bun.write,
stat, destStat).
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 956536ce4a568e4d84288c47d399255517bb9318 and 3da5a56500d9def914e25b169c7b19e73f117b76.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.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/regression/issue/25903.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/25903.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/25903.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/25903.test.ts
🧠 Learnings (7)
📚 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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-12-11T02:11:47.024Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25462
File: src/ast/visitExpr.zig:1644-1695
Timestamp: 2025-12-11T02:11:47.024Z
Learning: In Bun's bundler feature flag implementation (src/ast/visitExpr.zig), the validation for feature() flag names intentionally only rejects UTF-16 strings (checking `is_utf16`) while allowing UTF-8 strings, even though the error message says "must be an ASCII string". This is the intended behavior and should not be changed to enforce strict ASCII validation.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (4)
src/bun.js/webcore/blob/copy_file.zig (1)
20-21: Destination mode is now correctly threaded through copy paths; verify behavior across platforms and destination shapesThe new
destination_mode: ?bun.Modefield is wired through cleanly:
CopyFile.create(...)plumbsdestination_modeinto the struct, anddoOpenFilenow usesthis.destination_mode orelse jsc.Node.fs.default_permissionas theopenmode for destination paths on POSIX.- On macOS, the
clonefilefast path now callsbun.sys.chmodwithdestination_modeafter cloning, ensuring the destination no longer inherits source perms when a mode is provided.- On Windows,
CopyFileWindows.init(...)storesdestination_modeandonCompleteapplies it via a synchronousNodeFS.chmodcall when the destination is a path, so bothuv_fs_copyfileand the read/write loop fallback end with the requested permissions.This matches the public API intent: explicit
modeis honored for path-based destinations while leaving fd destinations alone.Two minor follow-ups to consider:
- You silently ignore
chmodfailures on macOS and Windows. That’s reasonable for avoiding hard failures, but it means callers can’t distinguish “copy succeeded but mode tweak failed”. If this matters, we might eventually want a best-effort warning or a way to surface that error.- Please double-check on Windows that
NodeFS.chmod+stat().modebehave as expected for the tested masks (e.g.0o600,0o700,0o755) so the regression test passes consistently there.Also applies to: 27-51, 161-166, 442-457, 589-590, 797-816, 1070-1079
src/bun.js/webcore/Blob.zig (3)
905-920: LGTM: Correct mode handling for file creation.The mode derivation and usage looks correct. The fallback to
default_permissionwhen mode is not specified is appropriate, and the mode is properly passed to the system open call with the O_CREAT flag.
1050-1070: LGTM: Mode properly threaded to copy operations.The mode option is correctly passed to both the Windows (
CopyFileWindows.init) and non-Windows (CopyFile.create) code paths for file-to-file copies.
1208-1208: LGTM: Appropriate optional field addition.Adding
mode: ?bun.Mode = nulltoWriteFileOptionsis the correct approach for supporting an optional mode parameter.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/webcore/Blob.zig (1)
454-456: Consider whether chmod failures should be logged or ignored.After a successful
clonefile, if the requested mode cannot be applied viachmod, the operation silently succeeds with potentially incorrect permissions. While this matches the behavior of thecpcommand (copy first, chmod best-effort), it may be unexpected for security-sensitive use cases.Consider whether chmod failures should:
- Be silently ignored (current behavior)
- Be logged for debugging
- Cause the operation to fail
🤖 Fix all issues with AI agents
In @packages/bun-types/bun.d.ts:
- Around line 818-822: Update the JSDoc for the `mode` option on the BunFile
copy overloads to refer generically to the destination file instead of saying
“If writing to a PathLike”; specifically, change the description to something
like “If writing to the destination file, set the permissions of the file” (or
similar) for the overloads where `destination` is a `BunFile`, and retain any
PathLike-specific wording only in overloads whose parameter type is `PathLike`;
apply the same wording fix to the other occurrence of the `mode` comment in the
same file.
In @src/bun.js/webcore/Blob.zig:
- Around line 1545-1550: The code currently casts mode_value.toInt64() with
@intCast without validating range; first extract the int64 (e.g., let v =
mode_value.toInt64()), check that v is non-negative and within the allowed
permission range (e.g., 0 .. 0o777 (0..511)), and only then perform the @intCast
into mode; if the value is out of range return an appropriate JS error (e.g.,
via globalThis.throwRangeError or a descriptive error using
throwInvalidArgumentType) so debug/release builds don’t panic or silently
truncate.
In @test/regression/issue/25903.test.ts:
- Around line 41-57: The test currently hardcodes 0o644; instead derive the
default mode at runtime: create an additional file (e.g., defaultPath) using
Bun.write(defaultPath, "x") with no mode, stat it to get observedDefault =
(stat(mode) & 0o777), then create source with mode 0o600, write dest from
Bun.file(source) with no mode, stat dest and assert (destStat.mode & 0o777) ===
observedDefault and also !== (sourceStat.mode & 0o777); update the test named
"Bun.write() uses default permissions when mode is not specified" to use these
symbols (defaultPath, sourcePath, destPath, observedDefault, Bun.write, stat,
expect) so the check no longer relies on hard-coded 0o644.
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 3da5a56500d9def914e25b169c7b19e73f117b76 and 929ddd2390960dbce367a0213eeae3b47bb268e4.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
🧠 Learnings (32)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
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.
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.
📚 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/25903.test.tspackages/bun-types/bun.d.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/25903.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/25903.test.tspackages/bun-types/bun.d.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/25903.test.tspackages/bun-types/bun.d.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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/25903.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/**/*.test.ts?(x) : Always use `port: 0` in tests - do not hardcode ports or use custom random port number functions
Applied to files:
test/regression/issue/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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 unit tests by module in directories like `/test/js/bun/` and `/test/js/node/`.
Applied to files:
test/regression/issue/25903.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/regression/issue/25903.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:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
packages/bun-types/bun.d.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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-18T01:49:31.037Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).
Applied to files:
packages/bun-types/bun.d.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/webcore/Blob.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (10)
test/regression/issue/25903.test.ts (1)
5-23: Good coverage of Bun.write mode semantics across key scenariosThe tests around explicit
mode,createPath, and BunFile→PathLike behavior are well-structured, usetempDircorrectly, and exercise the regression in realistic flows. No functional issues spotted here.Also applies to: 25-39, 59-72
src/bun.js/webcore/Blob.zig (3)
904-920: LGTM: Mode correctly applied during file creation.The mode derivation and application looks correct. When creating/truncating the destination file, the mode is properly derived from options with a fallback to default_permission and applied via the open() syscall.
1056-1056: LGTM: Mode correctly propagated to copy operations.The mode option is consistently passed through to both Windows and non-Windows copy file implementations.
Also applies to: 1067-1067
1208-1208: LGTM: Mode field correctly added to options struct.The optional mode field with null default is appropriate and consistent with the struct's design.
src/bun.js/webcore/blob/copy_file.zig (6)
21-21: LGTM: Mode field correctly added to CopyFile.The destination_mode field is properly added to the struct, passed through the create() function, and initialized correctly.
Also applies to: 35-35, 46-46
161-165: LGTM: Mode correctly applied during destination file open.The mode is properly derived with fallback and passed to the open syscall. The mode parameter is used when O_CREAT is present, which correctly applies the permissions at file creation time.
454-456: LGTM: Mode applied after clonefile (best-effort).The chmod after clonefile is necessary because clonefile copies the source file's permissions. The error handling (silently ignoring failures) is consistent with the pattern in Blob.zig and matches standard
cpcommand behavior.
589-589: LGTM: Mode and state fields correctly added to CopyFileWindows.The destination_mode field and supporting written_bytes field are properly added. The comment on line 595 clearly explains why written_bytes is needed for the async chmod flow.
Also applies to: 595-595, 806-806, 817-817
1074-1102: LGTM: Async chmod logic correctly implemented.The async chmod flow after copyfile completion is well-structured:
- Properly reuses io_request after deinit
- Stores written_bytes across the async boundary
- Handles chmod startup failures gracefully (copy succeeded, chmod failed)
- Correctly manages event loop refcounting
- Falls back to immediate resolution when chmod is not needed
1105-1126: LGTM: Async chmod completion and promise resolution correctly implemented.The onChmod callback and extracted resolvePromise helper are well-implemented:
- Chmod errors are intentionally ignored (copy succeeded, chmod is best-effort)
- Event loop is properly unrefed
- Promise resolution is cleanly extracted
- Error handling philosophy is consistent across platforms
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
2 times, most recently
from
8e20905 to
df0a5ff
Compare
Review Feedback AddressedAll review comments have been addressed in the latest commit: 1. Mode validation in Blob.zig (lines 1549-1552)
2. chmod error handling on macOS clonefile path (copy_file.zig lines 454-461)
3. chmod error handling on Windows async path (copy_file.zig lines 1118-1127)
4. JSDoc wording in bun.d.ts (lines 819, 856)
5. Flaky test fix (25903.test.ts lines 45-53)
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/bun.js/webcore/Blob.zig (1)
1048-1071:modeonly affects file→file copies; other Bun.write paths still ignore itRight now
WriteFileOptions.modeis parsed and threaded into:
CopyFileWindows.init(..., destination_mode: ?bun.Mode)andcopy_file.CopyFile.create(..., destination_mode: ?bun.Mode),and used in
CopyFile.doOpenFile/CopyFileWindows.onCompleteto enforce permissions on destination files created via Bun.write when both source and destination are file-backed blobs.However, several other
Bun.writecode paths still completely ignoreoptions.mode:
- Fast-path writes to a path (
writeStringToFileFast/writeBytesToFileFast) always usewrite_permissionswhen callingbun.sys.open, regardless ofoptions.mode.- The async path for
destination_type == .fileandsource_type == .bytesgoes throughwrite_file.WriteFile/WriteFileWindows.createwithout anymodeargument.Blob.doWriteconstructsWriteFileOptionswithout amodefield, soblob.write(..., { mode })(if exposed in JS) cannot influence permissions at all.This means
modeis currently effective only for file→file copies, but a user calling:
Bun.write("file", "data", { mode: 0o600 }), orBun.write(Bun.file("dest"), someBlob, { mode: 0o600 })wheresomeBlobis bytes-backed,won’t get the requested permissions, even though the option is accepted and validated.
Given this is a security-sensitive option, I’d strongly recommend either:
- Threading
modethrough all file-creation/truncation paths (fast-path writers, WriteFile/WriteFileWindows, stream sinks), or- At minimum, rejecting
options.modefor unsupported input types to avoid a silent no-op.As written, callers can reasonably assume
modeis honored for allBun.writeusages, but it is not.Also applies to: 1205-1209, 1535-1554, 1559-1563
src/bun.js/webcore/blob/copy_file.zig (1)
588-596: Async chmod design is sound; confirm whetherdestination_modeis intentionally ignored for fd destinationsThe Windows copy path implementation is structurally correct:
destination_modeis properly threaded throughinit()via Decl literalio_requestcleanup is safe:deinit()is called once before starting the chmod request, then again during cleanup—no double-free- When
uv_fs_chmodfails to start synchronously, the promise resolves with written byte count instead of failing hard- Error handling in
onChmodproperly surfaces JS errors viathrow(err)However,
destination_modeis only applied when the destination is a path; for file descriptor destinations (pathlike == .fd), the mode is silently ignored with nofchmod-style fallback. Confirm whether this matches the intendedBun.writeAPI behavior, as it creates a divergence between path-based and fd-based destinations.
🤖 Fix all issues with AI agents
In @packages/bun-types/bun.d.ts:
- Around line 818-821: Update the JSDoc for the mode?: number option to clarify
behavior: state that mode is only applied when destination is a PathLike and is
ignored when destination is a BunFile or S3File (referencing Bun.write and the
destination parameter), document that omitting mode falls back to the system
default/umask-based permissions, and specify the expected numeric range/format
for mode (Unix-style permissions, e.g., 0–0o777 / 0 to 511) so callers know
valid values.
- Around line 855-858: Update the JSDoc for the mode property on the write
options to remove the redundant conditional phrasing and make it direct; replace
"If writing to a PathLike, set the permissions of the file." with a concise
description like "Set the file permissions of the destination." and add the same
usage example present in the other overloads (e.g., showing await
Bun.write("./secret.txt", data, { mode: 0o600 });) adjacent to the mode?: number
declaration so the property documentation matches the other overload's style.
In @test/regression/issue/25903.test.ts:
- Around line 5-77: Tests currently overstate mode support: the suite exercises
only the Bun.file copy path where {mode} is honored, but direct writes via
Bun.write(..., "string" | ArrayBuffer) still ignore mode and the initial
Bun.write(sourcePath, ..., { mode: 0o600 }) comment is misleading. Update the
in-test comments in the three affected cases (tests named "Bun.write() uses
default permissions when mode is not specified" and the other copy-path tests)
to explicitly state that {mode} is honored for Bun.file copy paths only, and
that mode is currently ignored for direct bytes-backed writes; additionally add
a small regression test (or TODO comment) referencing Bun.write and Bun.file
that asserts or documents the current behavior for direct string/ArrayBuffer
writes (i.e., that the mode option is ignored) so future changes are covered.
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 929ddd2390960dbce367a0213eeae3b47bb268e4 and 8e209052d30c73e3f5fc22d19ae4ef6084131504.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
🧠 Learnings (27)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
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.
📚 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/25903.test.tssrc/bun.js/webcore/Blob.zig
📚 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/25903.test.tssrc/bun.js/webcore/Blob.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) : 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/25903.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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 unit tests by module in directories like `/test/js/bun/` and `/test/js/node/`.
Applied to files:
test/regression/issue/25903.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/regression/issue/25903.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.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:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 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:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.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/webcore/Blob.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (2)
src/bun.js/webcore/Blob.zig (1)
902-921: Empty-source mkdirp fallback correctly appliesoptions.modeThe new
modehandling in the mkdirp fallback (const mode: bun.Mode = options.mode orelse jsc.Node.fs.default_permission;passed intobun.sys.open) ensures that when we have to create parent directories and then the file, the created destination file respects the explicitmodeinstead of always using the default. This matches the PR’s objective for theBun.write(dest, Bun.file(source), { mode })case where the source is effectively empty or missing.src/bun.js/webcore/blob/copy_file.zig (1)
20-47: POSIX copy path now correctly honors destinationmodeThe additions of
destination_mode: ?bun.ModeinCopyFile, the extra parameter inCopyFile.create, and the usage in:
doOpenFileviaconst mode = this.destination_mode orelse jsc.Node.fs.default_permission;passed tobun.sys.open, and- the macOS
clonefilepath’s post-copybun.sys.chmod(...)block,together ensure that when
Bun.write(dest, Bun.file(source), { mode })runs on Linux/macOS:
- Newly created destination files are opened with the requested mode instead of always using the default, and
- On macOS, clonefile no longer forces destination permissions to mirror the source when an explicit
modeis provided.This matches the intended behavior without changing semantics for calls that omit
mode.Also applies to: 134-166, 370-395, 417-463
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/bun.js/webcore/blob/copy_file.zig:
- Around line 1098-1102: The current Windows async path resolves the copy when
chmod "failed to start" (rc.errno() branch) while the onChmod callback treats
chmod errors as hard failures; change the rc.errno() branch in the async start
path to mirror the onChmod error handling instead of calling
this.resolvePromise(written): detect the chmod start error (rc.errno()),
create/normalize an Error (or use the same error creation used in onChmod), and
call this.rejectPromise(...) (or otherwise propagate the error) so chmod
failures fail the overall operation consistently when a destination mode was
requested, matching the onChmod behavior.
In @test/regression/issue/25903.test.ts:
- Around line 64-77: Add a parallel regression that calls Bun.write with a
BunFile destination (use Bun.file(destPath) as the first arg) and
Bun.file(sourcePath) as the second arg, and assert the mode via stat and expect
like the existing test; update the test name to reflect the BunFile-destination
case or add a new test block using tempDir, Bun.write, Bun.file, stat and expect
to verify mode 0o700, and then run the file locally to ensure it fails under
USE_SYSTEM_BUN=1 and passes under bun bd (i.e., confirm the test exercises the
system vs bundled Bun behavior per the regression guidelines).
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 8e209052d30c73e3f5fc22d19ae4ef6084131504 and df0a5ffbc8cd602ca912f410127f67db11722dd7.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
🧠 Learnings (25)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
📚 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/25903.test.tspackages/bun-types/bun.d.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/25903.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/25903.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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/25903.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/25903.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/regression/issue/25903.test.tspackages/bun-types/bun.d.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/regression/issue/25903.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 unit tests by module in directories like `/test/js/bun/` and `/test/js/node/`.
Applied to files:
test/regression/issue/25903.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/regression/issue/25903.test.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
packages/bun-types/bun.d.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:
packages/bun-types/bun.d.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:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.zigsrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (2)
packages/bun-types/bun.d.ts (1)
818-822: Mode option on BunFile copy overloads matches intended behaviorThe added
mode?: numberoptions and JSDoc for BunFile→BunFile and BunFile→PathLike overloads are consistent with existingwritesignatures and clearly describe the permissions behavior on create/overwrite. No issues from the types surface here.Also applies to: 855-858
test/regression/issue/25903.test.ts (1)
5-62: Regression coverage for mode handling and defaults looks solidThe first three tests correctly exercise:
- Explicit
modeon copy, including stricter perms than the source.modecombined withcreatePathfor nested destinations.- Behavior when
modeis omitted, using a baseline file to derive the environment’s default rather than assuming a constant, which makes the test robust to different umasks.Use of
tempDir,stat(...).mode & 0o777, andusingaligns with existing harness/test patterns. This should reliably catch regressions in how Bun.write applies and omitsmode.
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
2 times, most recently
from
be02b44 to
6812b16
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/webcore/Blob.zig (1)
1535-1563:options.modeparsing ignores explicit0, making fully restrictive permissions impossible
options.modeis fetched withgetTruthy("mode"), which means a caller passing{ mode: 0 }will be treated as ifmodewere absent. That prevents setting a file’s permissions to0o000, which is a valid and sometimes security-sensitive configuration, despite the subsequent range check explicitly allowing0.Concretely, this:
if (try options_object.getTruthy(globalThis, "mode")) |mode_value| { // ... const mode_int = mode_value.toInt64(); if (mode_int < 0 or mode_int > 0o777) { ... } mode = @intCast(mode_int); }will skip the block entirely for
mode: 0.Consider switching to a non-truthiness-based lookup so
0is respected, e.g.:Suggested change to allow `mode: 0`
- if (try options_object.getTruthy(globalThis, "mode")) |mode_value| { - if (!mode_value.isNumber()) { - return globalThis.throwInvalidArgumentType("write", "options.mode", "number"); - } - const mode_int = mode_value.toInt64(); - if (mode_int < 0 or mode_int > 0o777) { - return globalThis.throwRangeError(mode_int, .{ .field_name = "mode", .min = 0, .max = 0o777 }); - } - mode = @intCast(mode_int); - } + if (try options_object.get(globalThis, "mode")) |mode_value| { + if (!mode_value.isEmptyOrUndefinedOrNull()) { + if (!mode_value.isNumber()) { + return globalThis.throwInvalidArgumentType("write", "options.mode", "number"); + } + const mode_int = mode_value.toInt64(); + if (mode_int < 0 or mode_int > 0o777) { + return globalThis.throwRangeError(mode_int, .{ .field_name = "mode", .min = 0, .max = 0o777 }); + } + mode = @intCast(mode_int); + } + }This keeps the same validation while making
{ mode: 0 }behave as expected.
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between df0a5ffbc8cd602ca912f410127f67db11722dd7 and be02b44c3613687a2b9306117eb1d76745f4fd16.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
🧠 Learnings (26)
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
packages/bun-types/bun.d.tstest/regression/issue/25903.test.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
packages/bun-types/bun.d.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:
packages/bun-types/bun.d.tstest/regression/issue/25903.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:
packages/bun-types/bun.d.tstest/regression/issue/25903.test.ts
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
packages/bun-types/bun.d.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:
packages/bun-types/bun.d.tstest/regression/issue/25903.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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 `-e` flag for single-file tests when spawning Bun processes with `Bun.spawn()`.
Applied to files:
test/regression/issue/25903.test.ts
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
📚 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:
src/bun.js/webcore/blob/copy_file.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (8)
packages/bun-types/bun.d.ts (2)
818-829: LGTM! Well-documented mode optionThe documentation is comprehensive and clear:
- Specifies the valid range (0 to 0o777)
- Documents the default behavior (umask-based)
- Provides a practical example for secure file operations
This aligns perfectly with the PR objective to respect the mode option when copying files.
863-874: LGTM! Consistent documentation across overloadsExcellent consistency between the two file-copy overloads. The documentation is identical except for the example, which appropriately demonstrates the different destination type (string path vs BunFile).
Both overloads now properly support setting file permissions during copy operations.
src/bun.js/webcore/Blob.zig (2)
905-920: Empty-source destination creation now correctly appliesoptions.modeUsing
const mode: bun.Mode = options.mode orelse jsc.Node.fs.default_permission;when opening the destination for theNOENT+mkdirppath ensures the explicit mode is honored when creating/truncating an empty file instead of always falling back to defaults. This change is localized and looks correct.
1049-1068: Mode is correctly forwarded into file→file copy pathsForwarding
options.modeintoCopyFileWindows.init(...)andcopy_file.CopyFile.create(..., options.mode)ensures the publicBun.write(..., Bun.file(...), { mode })surfaces all the way down to the copy implementations on all platforms. The wiring is straightforward and consistent with the newWriteFileOptions.modefield.src/bun.js/webcore/blob/copy_file.zig (3)
20-36: Destination mode is correctly stored and propagated inCopyFileAdding
destination_mode: ?bun.ModetoCopyFileand threading it throughCopyFile.create(...)gives the copy machinery a clear, explicit source of the requested destination permissions. This is a clean extension of the existing struct without changing other semantics.
454-462: macOS clonefile path now correctly fixes up destination permissionsAfter
clonefilesucceeds, adding:if (this.destination_mode) |mode| { switch (bun.sys.chmod(this.destination_file_store.pathlike.path.sliceZ(&path_buf), mode)) { .err => |err| { this.system_error = err.toSystemError(); return; }, .result => {}, } }ensures the destination’s permissions honor the requested
modeinstead of inheriting from the source. Error propagation viasystem_erroris consistent with the surrounding error-handling style.
595-602: Windows async chmod and promise resolution flow looks correct and robustThe Windows path:
- Stores the requested
destination_modeonCopyFileWindows.- On completion (
onComplete), adjusts for truncation, then:
- For path destinations with a non-null
destination_mode, reusesio_requestto scheduleuv_fs_chmodand trackswritten_bytes.- If
uv_fs_chmodcannot start (rc.errno() set), immediately resolves the promise withwrittento avoid hanging.onChmod:
- Unrefs the event loop.
- Throws a JS error via
this.throw(err)on chmod failure.- Otherwise calls
resolvePromise(this.written_bytes)to resolve with the correct byte count.resolvePromisewraps the usualenter/exitanddeinit()flow.This design cleanly separates the copy and chmod phases and ensures the promise always resolves or rejects exactly once, with event-loop refs/unrefs balanced.
Also applies to: 806-825, 1072-1133
test/regression/issue/25903.test.ts (1)
1-90: Well-scoped regression coverage for Bun.write mode behaviorThese tests nicely capture the intended semantics around
modewhen copying fromBun.file():
- They validate explicit modes (0o600/0o755/0o700) on fresh destinations and with
createPath.- They use a baseline file to infer the default permissions when
modeis omitted, making the check robust against environment umask differences.- They rely on
tempDirandbun:testidioms as per the test harness guidelines.No issues found here.
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
from
6812b16 to
ab446e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/webcore/blob/copy_file.zig (1)
606-620: Windows async chmod flow is well-integrated; note FD destinations skip modeThe
CopyFileWindowschanges (storingdestination_mode/written_bytes, reusingio_requestforuv_fs_chmod, and resolving the promise only after chmod completes) correctly ensure that mode is applied—and chmod failures are surfaced—without leaking libuv requests or dangling event-loop refs. On Windows this logic only runs when the destination is path-backed; if the destination is a BunFile backed by a file descriptor, themodeoption is effectively ignored. If you needmodeto affect FD-backed destinations as well, consider adding anfchmod-style path there; otherwise the current behavior is consistent with the issue’s focus on path/Bun.file use cases.Also applies to: 824-843, 1090-1155
🤖 Fix all issues with AI agents
In @src/bun.js/webcore/Blob.zig:
- Around line 1205-1209: WriteFileOptions currently defines mode as ?bun.Mode
and the code reads options.mode via getTruthy, which treats 0 as "not specified"
and drops an explicit 0 mode. Update the parsing in the code paths that handle
WriteFileOptions (including where options.mode is validated and passed to
writeFileInternal) to detect presence rather than truthiness: read the raw
property (e.g., get or hasOwnProperty-style check) so that a numeric value 0 is
accepted, then validate it's a number in [0, 0o777] before passing it through;
ensure the same change is applied in the other occurrences noted (around the
sections referenced at 1535-1554 and 1559-1563).
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between be02b44c3613687a2b9306117eb1d76745f4fd16 and ab446e37abd4fb1fd3cb51a5ae276aef557e9c45.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
🧠 Learnings (26)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
packages/bun-types/bun.d.tstest/regression/issue/25903.test.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.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:
packages/bun-types/bun.d.tstest/regression/issue/25903.test.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 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:
packages/bun-types/bun.d.tstest/regression/issue/25903.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:
packages/bun-types/bun.d.tstest/regression/issue/25903.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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zig
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-10-18T01:49:31.037Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).
Applied to files:
packages/bun-types/bun.d.ts
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
packages/bun-types/bun.d.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/25903.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/25903.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/25903.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/25903.test.tssrc/bun.js/webcore/blob/copy_file.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) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/regression/issue/25903.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/25903.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/25903.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/25903.test.ts
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
📚 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:
src/bun.js/webcore/blob/copy_file.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.zig
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (5)
packages/bun-types/bun.d.ts (2)
818-829: LGTM! Well-documentedmodeoption for file copy operations.The JSDoc clearly explains the purpose, valid range (0 to 0o777), default behavior, and provides a practical example. The type definition is correct, and this aligns with the PR objective to fix issue #25903 where the mode option was previously ignored during file copying operations.
863-874: LGTM! Consistent documentation across both file copy overloads.The
modeoption documentation for the PathLike destination overload matches the BunFile destination overload, ensuring consistency. Both file copy operations now properly support explicit permission control, completing the fix for issue #25903.src/bun.js/webcore/blob/copy_file.zig (1)
20-21: Mode propagation and POSIX chmod behavior look correct
destination_modeis cleanly threaded throughCopyFile.create, used when opening the destination (falling back todefault_permission), and applied viafchmodindoClosepluschmodafterclonefileon macOS. Errors from these chmod calls now correctly populatesystem_errorso the promise rejects instead of silently succeeding. This matches the stated goal of honoring the explicit mode for both new files and overwrites while keeping resource handling unchanged.Also applies to: 27-37, 45-47, 100-121, 176-184, 388-413, 471-481
src/bun.js/webcore/Blob.zig (1)
850-989: Mode now correctly influences empty-file creation and file↔file copiesUsing
options.modewhen opening the destination inwriteFileWithEmptySourceToDestinationand threading it throughwriteFileWithSourceDestinationintoCopyFileWindows.init/CopyFile.createensures the explicit mode is honored for both the “truncate/create empty file” path and the file↔file copy path (including Bun.file() copies). This aligns with the regression’s requirements without changing other write behaviors.Also applies to: 991-1006, 1048-1071
test/regression/issue/25903.test.ts (1)
1-128: Regression tests thoroughly cover Bun.file() + mode behavior without assuming platform defaultsThe new tests hit the key scenarios (plain copy, createPath, default-permissions baseline, BunFile destination, overwrite) and correctly compare
stat().mode & 0o777, deriving the default mode from a baseline file instead of hardcoding 0o644. This should reliably catch regressions in mode handling for Bun.write with Bun.file() sources across platforms.
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
from
ab446e3 to
167968c
Compare
When using Bun.write() to copy a file with Bun.file() as the source, the mode option was being ignored. The destination file inherited permissions from the source instead of using the specified mode. Changes: - Add mode field to WriteFileOptions and CopyFile structs - Parse mode option from JavaScript options object with validation - Use destination_mode when opening file (Linux) or chmod after clonefile (macOS) - Use async libuv chmod on Windows to avoid blocking main thread - Add regression test to verify mode is respected Fixes #25903 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
robobun
force-pushed
the
claude/fix-bun-write-mode-option
branch
from
167968c to
baff4cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/bun-types/bun.d.ts:
- Around line 818-829: Update the JSDoc for the `mode?: number` option to
document that invalid values will throw a RangeError: state that values must be
between 0 and 0o777 (0 to 511 decimal) and that passing an out-of-range number
results in a RangeError being thrown; edit the comment block immediately above
the `mode?: number;` declaration so consumers see the validation error behavior
alongside the existing default and example.
📜 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.
📥 Commits
Reviewing files that changed from the base of the PR and between 167968c6cb028ab77bdf4972cad61229fc9406ea and baff4cd.
📒 Files selected for processing (4)
packages/bun-types/bun.d.tssrc/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zigtest/regression/issue/25903.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/25903.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/25903.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/25903.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/25903.test.ts
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
🧠 Learnings (26)
📚 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/25903.test.tspackages/bun-types/bun.d.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/25903.test.tspackages/bun-types/bun.d.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/25903.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/25903.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/25903.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/25903.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/regression/issue/25903.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/25903.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/25903.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/25903.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/25903.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/regression/issue/25903.test.tspackages/bun-types/bun.d.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} : Do not set a timeout on tests. Bun already has timeouts built-in.
Applied to files:
test/regression/issue/25903.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 `-e` flag for single-file tests when spawning Bun processes with `Bun.spawn()`.
Applied to files:
test/regression/issue/25903.test.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
src/bun.js/webcore/Blob.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-09-07T08:20:47.215Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22258
File: src/cli/test_command.zig:1258-1281
Timestamp: 2025-09-07T08:20:47.215Z
Learning: For Bun's test line filtering feature, the parseFileLineArg function should only handle the specific cases of "file:line" and "file:line:col" formats. It should not try to be overly tolerant of other patterns, as components like ":col" or other non-numeric segments could legitimately be part of filenames. The current conservative approach that checks for numeric segments in expected positions is appropriate.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 Learning: 2025-12-11T02:11:47.024Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25462
File: src/ast/visitExpr.zig:1644-1695
Timestamp: 2025-12-11T02:11:47.024Z
Learning: In Bun's bundler feature flag implementation (src/ast/visitExpr.zig), the validation for feature() flag names intentionally only rejects UTF-16 strings (checking `is_utf16`) while allowing UTF-8 strings, even though the error message says "must be an ASCII string". This is the intended behavior and should not be changed to enforce strict ASCII validation.
Applied to files:
src/bun.js/webcore/Blob.zig
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/webcore/Blob.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/webcore/Blob.zigsrc/bun.js/webcore/blob/copy_file.zig
📚 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:
src/bun.js/webcore/blob/copy_file.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/bun.js/webcore/blob/copy_file.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/webcore/blob/copy_file.zig
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
packages/bun-types/bun.d.ts
🧬 Code graph analysis (1)
test/regression/issue/25903.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (4)
src/bun.js/webcore/Blob.zig (1)
905-907: Mode option is parsed, validated, and plumbed through correctly to file copy paths
WriteFileOptionsgainingmode: ?bun.Mode = nullplus the parsing block inwriteFile()ensures:
- Only numeric
options.modeare accepted (throwsInvalidArgumentTypeotherwise).- Values must be in
[0, 0o777]with aRangeErrorfor out-of-range, and0is explicitly allowed.writeFileInternal()forwardsmodeto all internal write paths:
Blob.copy_file.CopyFileWindows.init(..., destination_blob.size, options.mode)on Windows.copy_file.CopyFile.create(..., ctx, mkdirp_if_not_exists orelse true, options.mode)on POSIX.- The mkdirp slow path for empty-source writes correctly uses
options.mode orelse jsc.Node.fs.default_permissionas theopen()mode argument.This wiring matches the intent of #25903—mode is only guaranteed for Bun.file() copy operations—while keeping other write paths' semantics unchanged.
test/regression/issue/25903.test.ts (1)
1-147: Regression test structure is solid but requires manual verificationThe test file follows repository guidelines (bun:test, harness.tempDir, describe.skipIf on Windows, no timeouts, proper .test.ts naming). The suite exercises the key scenarios for issue #25903: explicit mode on Bun.file() copy, createPath with mode, default permissions, PathLike/BunFile variants, overwriting existing files, and edge cases like mode: 0. The umask-safe approach using a baseline file to determine defaults is appropriate.
Run the tests before landing to confirm they fail with system Bun and pass with the PR build:
USE_SYSTEM_BUN=1 bun test test/regression/issue/25903.test.ts # should fail (reproduces #25903) bun bd test test/regression/issue/25903.test.ts # should pass (fix works)src/bun.js/webcore/blob/copy_file.zig (1)
21-21: Destination mode handling is correctly propagated and applied across POSIX and Windows copy paths
POSIX
CopyFile:
destination_mode: ?bun.Modeis threaded fromCopyFile.createinto:
doOpenFile()viamode = this.destination_mode orelse jsc.Node.fs.default_permissionfor the create case.doClose()where a finalfchmod()is performed (non-Windows only) before closing, guarded bysystem_error == nullso chmod errors are recorded and the promise rejects instead of silently ignoring mode failures.- macOS
clonefilepath now performs achmod(dest_path, mode)whendestination_modeis set, fixing the prior behavior where source permissions were preserved regardless of options.mode.Windows
CopyFileWindows:
- Adds
destination_modeandwritten_bytesscratch field, withinit()wiring matching the call sites.onComplete()runs the existing truncate/size normalization, then:
- For path destinations with a specified mode, starts an async
uv_fs_chmodand only resolves the promise fromonChmod, ensuring the mode is actually applied.- For other cases, resolves immediately without extra I/O.
onChmod()correctly handles errno by constructing a.chmodbun.sys.Error (with path attached when available) and rejecting viathis.throw(err), or resolving with the storedwritten_byteson success.- Event loop ref/unref and
io_requestreuse (deinit + zero before reuse) are consistent with other fs operations in this struct.This achieves the intended behavior: explicit
modeis honored when copying via Bun.file() for both new and existing destinations, and mode application failures are surfaced to the caller instead of being silently swallowed. The regression test attest/regression/issue/25903.test.tsprovides comprehensive coverage of this functionality across multiple scenarios (mode application, createPath option, default permissions, and both PathLike and BunFile destinations).packages/bun-types/bun.d.ts (1)
863-874: LGTM! Excellent consistency with the BunFile destination overload.The documentation here matches the earlier overload at lines 818-829, with the example appropriately updated to show a string path instead of
Bun.file(). This consistency makes the API easy to understand and use.
Document that passing an out-of-range mode value (outside 0-0o777) will throw a RangeError in the type definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
Summary
Bun.write()mode option ignored when copying fromBun.file()modeoption instead of default permissionsTest plan
test/regression/issue/25903.test.tsbun bd test test/regression/issue/25903.test.tsUSE_SYSTEM_BUN=1 bun test test/regression/issue/25903.test.ts(verifies the bug exists)Changes
src/bun.js/webcore/Blob.zig: Addmodefield toWriteFileOptionsand parse from optionssrc/bun.js/webcore/blob/copy_file.zig: Usedestination_modeinCopyFilestruct anddoOpenFilepackages/bun-types/bun.d.ts: Addmodeoption to BunFile copy overloads🤖 Generated with Claude Code