node:fs: implement mkdtempDisposable / mkdtempDisposableSync by sam-shridhar1950f · Pull Request #31019 · oven-sh/bun

sam-shridhar1950f · 2026-05-18T23:11:00Z

What does this PR do?

Adds the Node 24 disposable mkdtemp APIs that pair with TC39's explicit resource management proposal:

fs.mkdtempDisposableSync(prefix[, options]) — returns { path, remove(), [Symbol.dispose]() } for use with using.
fsPromises.mkdtempDisposable(prefix[, options]) — returns a Promise<{ path, remove(), [Symbol.asyncDispose]() }> for use with await using.

The callback form fs.mkdtempDisposable(prefix, options, callback) is intentionally not added, matching Node's documented surface (nodejs/node docs).

Was wrong

Per the issue, the API doesn't exist:

import { mkdtempDisposable } from "node:fs/promises";
await using temp = await mkdtempDisposable("/tmp/bun-");
// SyntaxError: Export named 'mkdtempDisposable' not found in module 'node:fs/promises'.

Bun's docs already reference mkdtempDisposableSync, so this also resolves an existing docs/runtime gap.

Fix

Implementations live in the existing JS shims:

src/js/node/fs.ts — adds mkdtempDisposableSync, exports it, and setNames it.
src/js/node/fs.promises.ts — adds mkdtempDisposable to the promises exports.

Both wrap the existing fs.mkdtemp / fs.mkdtempSync and dispose via the existing rm/rmSync with { recursive: true, force: true }. They capture process.cwd() at creation so a process.chdir() between creation and disposal cannot misdirect the recursive removal. force: true makes repeat disposal a no-op (matches Node).

How did you verify your code works?

Added 10 tests in test/js/node/fs/fs.test.ts covering:

mkdtempDisposableSync

Removes via Symbol.dispose
result.remove() as a manual alternative
Calling dispose twice is safe
Works with using syntax
Removes a non-empty directory recursively
Survives process.chdir() between create and dispose

fs.promises.mkdtempDisposable

Removes via Symbol.asyncDispose
Works with await using syntax
Calling asyncDispose twice is safe
result.remove() returns a Promise

Local results on macOS arm64 (1.3.14-debug+80a06a8c0):

$ ./build/debug/bun-debug test test/js/node/fs/fs.test.ts -t "mkdtempDisposable"
 10 pass
 0 fail

$ ./build/debug/bun-debug test test/js/node/fs/fs.test.ts
 258 pass
 7 skip
 0 fail

No new failures in the broader test/js/node/fs/ suite. Lint and format are clean (bun run lint, bun run fmt).

This PR effectively replaces the stalled #22068, with a tighter diff (3 files / +125 lines), Node-spec-faithful API surface (no speculative callback variant), and focused tests added to the existing module's test file rather than a separate Node-port file.

claude

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

coderabbitai · 2026-05-18T23:15:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 22d4b2c7-87cb-405a-b5fd-4a6bf458761e

📥 Commits

Reviewing files that changed from the base of the PR and between 92d4c6f and 8fad946.

📒 Files selected for processing (2)

src/js/node/fs.promises.ts
src/js/node/fs.ts

Walkthrough

Added mkdtempDisposable and mkdtempDisposableSync exports to Bun's Node.js fs module. Both functions create temporary directories and return objects with path, remove() method, and automatic cleanup via Symbol.asyncDispose/Symbol.dispose for use with await using/using syntax. Comprehensive tests validate cleanup, repeated disposal, and behavior across working directory changes.

Changes

Temporary Directory Disposable Helpers

Layer / File(s)	Summary
Async mkdtempDisposable implementation `src/js/node/fs.promises.ts`	New async function captures `process.cwd()` at creation, creates a temp directory via `mkdtemp`, and returns an object with `path`, `remove()` method, and `Symbol.asyncDispose` handler that recursively removes the directory when disposed.
Sync mkdtempDisposableSync implementation and wiring `src/js/node/fs.ts`	New sync function captures `process.cwd()` at creation, creates a temp directory via `mkdtempSync`, and returns an object with `path`, `remove()` method, and `Symbol.dispose` handler. Function is registered in `exports` and function name is preserved via `setName`.
Test coverage `test/js/node/fs/fs.test.ts`	Added mkdtempDisposableSync to imports and comprehensive test suite validating both functions: Symbol.dispose/Symbol.asyncDispose cleanup, repeated disposal idempotency, manual `remove()` calls, `using`/`await using` syntax compatibility, recursive removal of non-empty directories, and cleanup correctness when `process.chdir()` occurs between creation and disposal.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: implementing mkdtempDisposable and mkdtempDisposableSync APIs for node:fs, which is exactly what the PR does.
Description check	✅ Passed	The PR description follows the template with complete 'What does this PR do?' and 'How did you verify your code works?' sections, providing detailed explanations and test results.
Linked Issues check	✅ Passed	The PR successfully implements both mkdtempDisposable (promises) and mkdtempDisposableSync (sync) APIs with Symbol.dispose/asyncDispose and remove() methods, directly addressing issue `#24499`'s requirement for Node v24 parity.
Out of Scope Changes check	✅ Passed	All changes are directly in scope: two new functions added to fs.promises and fs shims, tests added to existing test file, and no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/fs/fs.test.ts`:
- Line 4038: Replace uses of tmpdirSync() with the harness helper tempDir(...)
in these tests so the created temp roots are tracked and auto-cleaned;
specifically update calls like mkdtempDisposableSync(join(tmpdirSync(),
"disposable-")) to instead call tempDir("disposable-") (or wrap tempDir().path
as needed) and ensure you import tempDir from the test harness if not present.
Apply the same replacement for all similar occurrences (references to tmpdirSync
in the block around mkdtempDisposableSync and other mkdtemp* usages) so temp
directories are created via tempDir and the disposable cleanup semantics are
preserved.
- Around line 4094-4125: The tests only exercise the internal
_promises.mkdtempDisposable export and miss exercising the public entry points;
add at least one equivalent test case that calls mkdtempDisposable via the
public promises API (e.g., fs.promises.mkdtempDisposable or the imported
promises object) to ensure export wiring is correct; locate the existing tests
around mkdtempDisposable (symbols: _promises.mkdtempDisposable, result.remove,
Symbol.asyncDispose) and duplicate one of the existing cases (for example the
"remove() returns a Promise that resolves" or "creates a directory and removes
it via Symbol.asyncDispose") but invoke it through fs.promises.mkdtempDisposable
(or promises.mkdtempDisposable) and assert the same behaviors (path is string,
directory exists, removal results in non-existence).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 16918969-5ad6-4eab-9c55-2d10a1a9d5fc

📥 Commits

Reviewing files that changed from the base of the PR and between 31028ec and ddaa2e2.

📒 Files selected for processing (3)

src/js/node/fs.promises.ts
src/js/node/fs.ts
test/js/node/fs/fs.test.ts

coderabbitai · 2026-05-18T23:23:01Z

+
+describe("mkdtempDisposableSync", () => {
+  it("creates a directory and removes it via Symbol.dispose", () => {
+    const result = mkdtempDisposableSync(join(tmpdirSync(), "disposable-"));


🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Replace tmpdirSync() with tempDir(...) in these new tests.

These new cases create temp roots with tmpdirSync(), which conflicts with the test harness rule and loses disposable cleanup ergonomics.

Suggested pattern

- const result = mkdtempDisposableSync(join(tmpdirSync(), "disposable-")); + using base = tempDir("mkdtemp-disposable-sync", {}); + const result = mkdtempDisposableSync(join(String(base), "disposable-"));

- const result = await _promises.mkdtempDisposable(join(tmpdirSync(), "disposable-")); + using base = tempDir("mkdtemp-disposable-async", {}); + const result = await _promises.mkdtempDisposable(join(String(base), "disposable-"));

As per coding guidelines: "Use tempDir from 'harness' to create temporary directories - do not use tmpdirSync or fs.mkdtempSync."

Also applies to: 4046-4046, 4053-4053, 4061-4061, 4069-4069, 4079-4079, 4096-4096, 4106-4106, 4114-4114, 4121-4121

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/node/fs/fs.test.ts` at line 4038, Replace uses of tmpdirSync() with the harness helper tempDir(...) in these tests so the created temp roots are tracked and auto-cleaned; specifically update calls like mkdtempDisposableSync(join(tmpdirSync(), "disposable-")) to instead call tempDir("disposable-") (or wrap tempDir().path as needed) and ensure you import tempDir from the test harness if not present. Apply the same replacement for all similar occurrences (references to tmpdirSync in the block around mkdtempDisposableSync and other mkdtemp* usages) so temp directories are created via tempDir and the disposable cleanup semantics are preserved.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/fs/fs.test.ts`:
- Around line 4094-4132: Add an async regression test to ensure
mkdtempDisposable does not suffer cwd drift by using process.chdir around the
async disposable - inside the "fs.promises.mkdtempDisposable" describe add a
test similar to the sync case that calls process.chdir(...) to a different
directory, then await _promises.mkdtempDisposable(...) (or
promises.mkdtempDisposable(...)), capture the returned result.path, change the
cwd back, call await result[Symbol.asyncDispose]() (or result.remove()), and
assert that the process.cwd() is unchanged and the temporary directory has been
removed (existsSync(result.path) is false); reference
_promises.mkdtempDisposable, promises.mkdtempDisposable, Symbol.asyncDispose and
remove() to locate the code.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 197ced2f-088c-42af-9aa4-fa7862c39a38

📥 Commits

Reviewing files that changed from the base of the PR and between ddaa2e2 and 92d4c6f.

📒 Files selected for processing (3)

src/js/node/fs.promises.ts
src/js/node/fs.ts
test/js/node/fs/fs.test.ts

coderabbitai · 2026-05-18T23:41:11Z

+describe("fs.promises.mkdtempDisposable", () => {
+  it("creates a directory and removes it via Symbol.asyncDispose", async () => {
+    const result = await _promises.mkdtempDisposable(join(tmpdirSync(), "disposable-"));
+    expect(typeof result.path).toBe("string");
+    expect(existsSync(result.path)).toBe(true);
+    await result[Symbol.asyncDispose]();
+    expect(existsSync(result.path)).toBe(false);
+  });
+
+  it("works with `await using` syntax", async () => {
+    let savedPath: string;
+    {
+      await using temp = await _promises.mkdtempDisposable(join(tmpdirSync(), "disposable-"));
+      savedPath = temp.path;
+      expect(existsSync(temp.path)).toBe(true);
+    }
+    expect(existsSync(savedPath!)).toBe(false);
+  });
+
+  it("calling asyncDispose twice is safe", async () => {
+    const result = await _promises.mkdtempDisposable(join(tmpdirSync(), "disposable-"));
+    await result[Symbol.asyncDispose]();
+    await result[Symbol.asyncDispose]();
+    expect(existsSync(result.path)).toBe(false);
+  });
+
+  it("remove() returns a Promise that resolves", async () => {
+    const result = await _promises.mkdtempDisposable(join(tmpdirSync(), "disposable-"));
+    await result.remove();
+    expect(existsSync(result.path)).toBe(false);
+  });
+
+  it("is also reachable via the node:fs `promises` namespace", async () => {
+    const result = await promises.mkdtempDisposable(join(tmpdirSync(), "disposable-"));
+    expect(existsSync(result.path)).toBe(true);
+    await result[Symbol.asyncDispose]();
+    expect(existsSync(result.path)).toBe(false);
+  });
+});


🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a process.chdir() regression test for the async variant too.

The sync suite verifies cwd drift safety, but the async suite doesn’t. Add one equivalent async case to lock in the captured-cwd cleanup behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/node/fs/fs.test.ts` around lines 4094 - 4132, Add an async regression test to ensure mkdtempDisposable does not suffer cwd drift by using process.chdir around the async disposable - inside the "fs.promises.mkdtempDisposable" describe add a test similar to the sync case that calls process.chdir(...) to a different directory, then await _promises.mkdtempDisposable(...) (or promises.mkdtempDisposable(...)), capture the returned result.path, change the cwd back, call await result[Symbol.asyncDispose]() (or result.remove()), and assert that the process.cwd() is unchanged and the temporary directory has been removed (existsSync(result.path) is false); reference _promises.mkdtempDisposable, promises.mkdtempDisposable, Symbol.asyncDispose and remove() to locate the code.

Adds the Node 24 disposable mkdtemp APIs that pair with the explicit resource management proposal: - fs.mkdtempDisposableSync(prefix[, options]) returns { path, remove(), [Symbol.dispose]() } for use with `using`. - fsPromises.mkdtempDisposable(prefix[, options]) returns a Promise resolving to { path, remove(), [Symbol.asyncDispose]() } for use with `await using`. Both capture process.cwd() at creation so a process.chdir() between creation and disposal cannot misdirect the recursive rm. Disposing twice is safe (force: true). Closes oven-sh#24499 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

robobun · 2026-05-25T20:39:56Z

I independently reimplemented this while looking at #31400 (a fresh report of the same gap as #24499) and arrived at the same design, so I checked this PR against Node's upstream implementation — it matches their semantics exactly:

No callback form. Node deliberately ships only fs.mkdtempDisposableSync + fsPromises.mkdtempDisposable; lib/fs.js has no fs.mkdtempDisposable(…, cb). (Node's docs: "There is no callback-based version of this API because it is designed for use with the using syntax.") This PR correctly omits it.
.path is the raw mkdtemp result, while cleanup uses path.resolve(process.cwd(), path) captured at creation — matching Node's fullPath stash so a later process.chdir() can't misdirect the rm. The chdir test covers this.
Object shapes match: sync → { path, remove, [Symbol.dispose] } with remove() returning undefined; async → { path, remove, [Symbol.asyncDispose] } with remove() returning a Promise.
{ recursive: true, force: true } with no one-shot "disposed" guard is the right call: it makes a second disposal a no-op (ENOENT swallowed) while still propagating real errors like EACCES, and it allows a retry after the error is cleared — which is exactly what Node's test-fs-mkdtempDisposableSync.js asserts (dispose under a read-only parent throws EACCES, then succeeds once the mode is restored).

One optional nit for even tighter Node parity: Node's async variant returns the object with __proto__: null. Not behaviorally important.

LGTM from my read. Deferring to this PR rather than opening a duplicate — it just needs a maintainer review/merge.

robobun · 2026-08-12T23:25:56Z

Thanks for the PR, and sorry it sat for so long. fs.mkdtempDisposableSync and fsPromises.mkdtempDisposable landed on main in #31830 with the same design as this PR, and #24499 is closed, so this is no longer needed. Closing.

claude Bot reviewed May 18, 2026

View reviewed changes

sam-shridhar1950f force-pushed the node-fs-mkdtemp-disposable branch from 31028ec to ddaa2e2 Compare May 18, 2026 23:19

coderabbitai Bot reviewed May 18, 2026

View reviewed changes

sam-shridhar1950f force-pushed the node-fs-mkdtemp-disposable branch from ddaa2e2 to 92d4c6f Compare May 18, 2026 23:36

coderabbitai Bot reviewed May 18, 2026

View reviewed changes

sam-shridhar1950f force-pushed the node-fs-mkdtemp-disposable branch from 92d4c6f to 8fad946 Compare May 19, 2026 18:48

robobun mentioned this pull request May 25, 2026

node:fs/promises missing mkdtempDisposable export (Bun 1.3.13) — blocks production CLI startup #31400

Closed

robobun closed this Aug 12, 2026

Navigation Menu

Uh oh!

node:fs: implement mkdtempDisposable / mkdtempDisposableSync - #31019

node:fs: implement mkdtempDisposable / mkdtempDisposableSync#31019
sam-shridhar1950f wants to merge 1 commit into
oven-sh:mainfrom
sam-shridhar1950f:node-fs-mkdtemp-disposable

sam-shridhar1950f commented May 18, 2026

Uh oh!

claude Bot left a comment

Uh oh!

coderabbitai Bot commented May 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot May 18, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot May 18, 2026

Uh oh!

robobun commented May 25, 2026

Uh oh!

robobun commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sam-shridhar1950f commented May 18, 2026

What does this PR do?

Was wrong

Fix

How did you verify your code works?

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

Uh oh!

robobun commented May 25, 2026

Uh oh!

robobun commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 18, 2026 •

edited

Loading