feat(spawn): add PTY support for stdin, stdout, stderr #25319
Conversation
Add support for `stdin: "pty"`, `stdout: "pty"`, and `stderr: "pty"` options in `Bun.spawn()`. This allows spawned processes to see `process.stdout.isTTY === true`. Key implementation details: - PTY creation via openpty/forkpty syscalls in sys.zig - When multiple stdios use PTY, they share the same master FD - Handle epoll EEXIST gracefully when FD is already registered - Timer-based polling fallback for shared PTY FDs - EIO treated as normal EOF when PTY slave closes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unused uws import from PipeReader.zig - Remove unused PTY helper functions (setWinSize, getWinSize, setControllingTerminal) - Remove unused ioctl constants (TIOCSWINSZ, TIOCSCTTY, TIOCGWINSZ) - Change spawn_bindings log visibility back to hidden - Add "pty" to TypeScript types in bun.d.ts - Add PTY documentation to docs/runtime/child-process.mdx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add tests for: - stderr: 'pty' only - stdin: 'pty' only (stdout/stderr not PTY) - PTY object syntax with custom width/height dimensions - ANSI color output detection - Multiple concurrent PTY spawns - Windows error handling (skipped on non-Windows) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add comprehensive PTY documentation with examples: - Basic usage - Multiple PTY streams (stdin/stdout/stderr) - Custom terminal dimensions (width/height) - Colored output from git, grep, etc. - Platform support table - Limitations section - Make PTY fall back to pipe on Windows (instead of error) - Add spawnSync error for PTY (not supported) - Add tests for spawnSync PTY error 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Updated 4:29 AM PT - Dec 3rd, 2025
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 25319That installs a local version of the PR into your bun-25319 --bun |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds PTY support across Bun's spawn stack: new Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)src/**/*.{cpp,zig}📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
src/**/*.zig📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
**/*.zig📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-10-26T01:32:04.844ZApplied to files:
📚 Learning: 2025-11-08T04:06:33.198ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-11-06T00:58:23.965ZApplied to files:
🔇 Additional comments (6)
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: 9
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/api/bun/subprocess.zig (1)
284-291: Additional stdout debug logging is safeThe new
log("getStdout: ...")call is purely diagnostic and doesn’t affect control flow or resource handling. It should be helpful when debugging PTY/stdout state without impacting behavior.If it proves too chatty, you can later gate it behind
Environment.isDebugor a dedicated debug flag.packages/bun-types/bun.d.ts (1)
5334-5362: Type safety gap: spawnSync allows "pty" despite runtime error.According to the PR summary, PTY is not supported with
spawnSyncand will throw an error at runtime. However, adding"pty"to theReadableandWritableunions means TypeScript will allow this code:Bun.spawnSync({ cmd: ["echo", "hello"], stdin: "pty" // TypeScript allows this but it throws at runtime! });To fix this type safety issue, consider one of these approaches:
Option 1: Separate type unions
+type SyncReadable = Exclude<Readable, "pty">; +type SyncWritable = Exclude<Writable, "pty">; interface SpawnSyncOptions< - In extends Writable, - Out extends Readable, - Err extends Readable + In extends SyncWritable, + Out extends SyncReadable, + Err extends SyncReadable > extends BaseOptions<In, Out, Err> {}Option 2: Conditional types with never
interface SpawnSyncOptions< In extends Writable, Out extends Readable, Err extends Readable -> extends BaseOptions<In, Out, Err> {} +> extends BaseOptions<In, Out, Err> { + stdin?: In extends "pty" ? never : In; + stdout?: Out extends "pty" ? never : Out; + stderr?: Err extends "pty" ? never : Err; + stdio?: In extends "pty" ? never : Out extends "pty" ? never : Err extends "pty" ? never : [In, Out, Err, ...Readable[]]; +}
📜 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.
📒 Files selected for processing (13)
docs/runtime/child-process.mdx(4 hunks)packages/bun-types/bun.d.ts(9 hunks)src/bun.js/api/bun/js_bun_spawn_bindings.zig(2 hunks)src/bun.js/api/bun/process.zig(5 hunks)src/bun.js/api/bun/spawn/stdio.zig(5 hunks)src/bun.js/api/bun/subprocess.zig(1 hunks)src/bun.js/api/bun/subprocess/Readable.zig(5 hunks)src/bun.js/api/bun/subprocess/SubprocessPipeReader.zig(4 hunks)src/bun.js/api/bun/subprocess/Writable.zig(1 hunks)src/io/PipeReader.zig(3 hunks)src/shell/subproc.zig(4 hunks)src/sys.zig(2 hunks)test/js/bun/spawn/spawn-pty.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/shell/subproc.zigsrc/sys.zigsrc/bun.js/api/bun/subprocess.zigsrc/io/PipeReader.zigsrc/bun.js/api/bun/subprocess/Readable.zigsrc/bun.js/api/bun/subprocess/SubprocessPipeReader.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/subprocess/Writable.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)Be careful with memory management in Zig code - use defer for cleanup with allocators
Files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/shell/subproc.zigsrc/sys.zigsrc/bun.js/api/bun/subprocess.zigsrc/io/PipeReader.zigsrc/bun.js/api/bun/subprocess/Readable.zigsrc/bun.js/api/bun/subprocess/SubprocessPipeReader.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/subprocess/Writable.zig
**/js_*.zig
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
**/js_*.zig: ImplementJSYourFeaturestruct in a file likejs_your_feature.zigto create JavaScript bindings for Zig functionality
Usebun.JSError!JSValuefor proper error propagation in JavaScript bindings
Implement proper memory management with reference counting usingref()/deref()in JavaScript bindings
Always implement proper cleanup indeinit()andfinalize()methods for JavaScript binding classes
Files:
src/bun.js/api/bun/js_bun_spawn_bindings.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objects
Files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/shell/subproc.zigsrc/sys.zigsrc/bun.js/api/bun/subprocess.zigsrc/io/PipeReader.zigsrc/bun.js/api/bun/subprocess/Readable.zigsrc/bun.js/api/bun/subprocess/SubprocessPipeReader.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/subprocess/Writable.zig
test/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test,describe,expect) and import frombun:test
Usetest.eachand data-driven tests to reduce boilerplate when testing multiple similar cases
Files:
test/js/bun/spawn/spawn-pty.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:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/bun/spawn/spawn-pty.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using-eflag overtempDir
For multi-file tests in Bun test suite, prefer usingtempDirandBun.spawn
Always useport: 0when spawning servers in tests - do not hardcode ports or use custom random port functions
UsenormalizeBunSnapshotto normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
UsetempDirfromharnessto create temporary directories in tests - do not usetmpdirSyncorfs.mkdtempSync
In tests, callexpect(stdout).toBe(...)beforeexpect(exitCode).toBe(0)when spawning processes for more useful error messages on failure
Do not write flaky tests - do not usesetTimeoutin tests; insteadawaitthe condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails withUSE_SYSTEM_BUN=1 bun test <file>and passes withbun bd test <file>- tests are not valid if they pass withUSE_SYSTEM_BUN=1
Avoid shell commands in tests - do not usefindorgrep; use Bun's Glob and built-in tools instead
Test files must end in.test.tsor.test.tsxand be created in the appropriate test folder structure
Files:
test/js/bun/spawn/spawn-pty.test.ts
🧠 Learnings (30)
📓 Common learnings
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: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
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.
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;`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigtest/js/bun/spawn/spawn-pty.test.tssrc/bun.js/api/bun/subprocess.zigdocs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigpackages/bun-types/bun.d.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:
src/bun.js/api/bun/js_bun_spawn_bindings.zigtest/js/bun/spawn/spawn-pty.test.tssrc/bun.js/api/bun/subprocess.zigdocs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigtest/js/bun/spawn/spawn-pty.test.tsdocs/runtime/child-process.mdxsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/process.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.zig : Use `bun.Output.scoped(.${SCOPE}, .hidden)` for creating debug logs in Zig code
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/bun.js/api/bun/subprocess.zig
📚 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:
src/bun.js/api/bun/js_bun_spawn_bindings.zigtest/js/bun/spawn/spawn-pty.test.tsdocs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigpackages/bun-types/bun.d.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:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/shell/subproc.zigtest/js/bun/spawn/spawn-pty.test.tssrc/bun.js/api/bun/subprocess.zigdocs/runtime/child-process.mdxsrc/bun.js/api/bun/subprocess/SubprocessPipeReader.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/subprocess/Writable.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Enable debug logs for specific scopes using `BUN_DEBUG_$(SCOPE)=1` environment variable
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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/api/bun/js_bun_spawn_bindings.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/bun.js/api/bun/subprocess/Readable.zigsrc/bun.js/api/bun/subprocess/Writable.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, call `expect(stdout).toBe(...)` before `expect(exitCode).toBe(0)` when spawning processes for more useful error messages on failure
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigtest/js/bun/spawn/spawn-pty.test.tsdocs/runtime/child-process.mdx
📚 Learning: 2025-10-18T02:06:31.606Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23796
File: src/bun.js/test/Execution.zig:624-625
Timestamp: 2025-10-18T02:06:31.606Z
Learning: In Zig files using bun.Output.scoped(), the visibility level `.visible` means logs are enabled by default in debug builds unless `BUN_DEBUG_QUIET_LOGS=1` is set; if that environment variable is set, the logs can be re-enabled with `BUN_DEBUG_<scope>=1`. Use `.visible` for logs that should appear by default in debug builds, and `.hidden` for logs that require explicit opt-in via `BUN_DEBUG_<scope>=1`.
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.zigsrc/bun.js/api/bun/subprocess/Readable.zig
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
test/js/bun/spawn/spawn-pty.test.tsdocs/runtime/child-process.mdxpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
Applied to files:
test/js/bun/spawn/spawn-pty.test.tsdocs/runtime/child-process.mdxpackages/bun-types/bun.d.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:
test/js/bun/spawn/spawn-pty.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
Applied to files:
test/js/bun/spawn/spawn-pty.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Applied to files:
test/js/bun/spawn/spawn-pty.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/js/bun/spawn/spawn-pty.test.ts
📚 Learning: 2025-10-04T21:17:53.040Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23253
File: test/js/valkey/valkey.failing-subscriber-no-ipc.ts:40-59
Timestamp: 2025-10-04T21:17:53.040Z
Learning: In Bun runtime, the global `console` object is an AsyncIterable that yields lines from stdin. The pattern `for await (const line of console)` is valid and documented in Bun for reading input line-by-line from the child process stdin.
Applied to files:
docs/runtime/child-process.mdx
📚 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:
docs/runtime/child-process.mdxpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-11T22:55:08.547Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24571
File: src/css/values/url.zig:97-116
Timestamp: 2025-11-11T22:55:08.547Z
Learning: In Zig 0.15, the standard library module `std.io` was renamed to `std.Io` (with capital I). Code using `std.Io.Writer.Allocating` and similar types is correct for Zig 0.15+.
Applied to files:
src/io/PipeReader.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings
Applied to files:
src/bun.js/api/bun/subprocess/Readable.zig
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.
Applied to files:
src/bun.js/api/bun/subprocess/Readable.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Use `bun.JSError!JSValue` return type for Zig methods and constructors to enable proper error handling and exception propagation
Applied to files:
src/bun.js/api/bun/subprocess/Readable.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/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/api/bun/subprocess/Readable.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-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-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode
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: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules
Applied to files:
packages/bun-types/bun.d.ts
🧬 Code graph analysis (1)
test/js/bun/spawn/spawn-pty.test.ts (2)
test/harness.ts (1)
bunExe(102-105)packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts (1)
Response(2793-2806)
🔇 Additional comments (17)
src/bun.js/api/bun/js_bun_spawn_bindings.zig (1)
1006-1006: Log scope.spawn_bindingswith.hiddenis appropriateDefining
const log = Output.scoped(.spawn_bindings, .hidden);gives this module its own opt‑in debug channel and aligns with the project logging guidelines for new scopes. No issues here.test/js/bun/spawn/spawn-pty.test.ts (1)
1-167: PTY test coverage is thorough and well-structuredThe new tests cover the key PTY behaviors end‑to‑end (isTTY flags across stdio, shared PTY, custom dimensions, color output, concurrency, and spawnSync rejection) using
bunExe/bunEnvand-escripts as recommended. Assertions read stdout before exit codes and use.trim()/.toContain()to stay robust against\r\ntranslation.One thing to keep in mind is that the spawnSync tests assert the exact error message
"PTY is not supported with spawnSync". If the implementation message changes in the future, these will need updating; otherwise the suite looks solid.src/bun.js/api/bun/subprocess/Readable.zig (2)
73-81: PTY shared-master handling looks correct.When multiple stdio streams use PTY, they share the same master FD. The logic correctly handles
result == nullfor the secondary stream (stderr) by ignoring it, since stdout handles the reading. The non-null path correctly creates a PTY-aware pipe reader.
87-90: LGTM!Naming the error parameter and adding the debug log improves observability for PTY lifecycle debugging.
src/bun.js/api/bun/subprocess/Writable.zig (1)
231-254: PTY stdin handling implementation looks correct.The implementation correctly mirrors the
.pipepath but omits the.socketflag since PTY masters are character devices that usewrite()rather thansend(). The subprocess lifecycle management (weak pointer, ref counting, destructor flags) is properly set up.docs/runtime/child-process.mdx (1)
118-231: Comprehensive and well-structured PTY documentation.The documentation covers all essential aspects: basic usage, isTTY verification, multiple streams sharing PTY, custom terminal dimensions, colored output examples, platform support matrix, and limitations. This will be helpful for users adopting the feature.
src/io/PipeReader.zig (2)
86-100: PTY flag and padding look correct.The
is_ptyflag addition with 5-bit padding maintains thepacked struct(u16)alignment (11 bools + 5 padding = 16 bits). The comment on line 97-98 clearly documents the PTY-specific EIO handling behavior.
275-281: Correct PTY EIO-as-EOF handling.When the PTY slave side closes (child exits), the master receives EIO. Treating this as EOF rather than an error is the standard PTY behavior. The
closeWithoutReporting()followed bydone()ensures proper cleanup without propagating a spurious error to the caller.src/bun.js/api/bun/spawn/stdio.zig (2)
17-26: LGTM!The
PtyOptionsstruct with sensible defaults (80x24) matches standard terminal dimensions. The documentation comments clearly explain the fields.
361-373: LGTM!The string
"pty"parsing correctly handles:
- spawnSync rejection with clear error message
- Windows fallback to pipe
- Default PTY options for POSIX
src/bun.js/api/bun/process.zig (3)
1007-1013: LGTM!The
PtyConfigstruct is well-designed with sensible 80x24 defaults matching standard terminal dimensions.
1314-1344: LGTM!The PTY creation logic correctly:
- Creates a single PTY pair shared by all PTY stdio streams
- Uses the first PTY config's dimensions (reasonable since all share the same terminal)
- Properly tracks FDs for cleanup (slave for post-spawn close, master for error-only close)
- Sets non-blocking mode only for async operations
1462-1483: LGTM!The PTY stdio handling correctly:
- Uses the slave side for the child process
- Merges stderr into stdout when both use PTY (single stream)
- Duplicates the master FD for independent epoll registration per stream
- Maintains non-blocking consistency with the sync mode
src/bun.js/api/bun/subprocess/SubprocessPipeReader.zig (4)
17-18: LGTM!Good documentation explaining the semantic difference between PTY (character device) and regular pipes (sockets).
38-62: LGTM!Clean refactoring with factory methods that maintain backward compatibility while adding PTY support.
77-98: LGTM!Correctly:
- Sets PTY flag before
start()so error handlers can check it during poll registration- Avoids marking PTY as socket since it's a character device requiring
read()instead ofrecv()
190-196: LGTM!Correctly handles the PTY-specific behavior where EIO from the master indicates the slave side closed (child process exited). Treating this as EOF rather than an error is the standard approach for PTY handling.
| mmap, | ||
| munmap, | ||
| open, | ||
| openpty, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
PTY scaffolding (Tag.openpty + openpty helper) looks sound
The new syscall tag and openpty wrapper are consistent with existing patterns: Tag.openpty integrates cleanly into the enum, WinSize matches the expected C struct winsize layout, and openpty() correctly treats non‑zero return values as errors and pulls errno via std.posix.errno(rc). I don’t see correctness or portability issues here; higher‑level PTY code can safely build on this helper.
If you want to keep syscall logging quieter in debug builds, you could optionally gate the log("openpty() = ...) behind Environment.allow_assert like other logging in this file, but that’s stylistic, not required.
Also applies to: 4281-4342
🤖 Prompt for AI Agents
In src/sys.zig around line 224 (and also the openpty implementation region at
4281-4342), the debug log for openpty() is unconditionally emitted; to make
syscall logging consistent with other entries, wrap the log call in the same
Environment.allow_assert guard used elsewhere so the message is only printed
when allowed (i.e., check Environment.allow_assert before calling log), leaving
the rest of the openpty implementation and error handling unchanged.
- Fix docs example consistency: use array form consistently - Add spawnSync warning to TypeScript PTY type definitions - Fix Windows compile error in js_bun_spawn_bindings.zig - Fix extra_fds PTY to use dup() for consistency - Add isNumber() type check for PTY width/height options - Fix error message consistency in stdio.zig - Fix switch case overlap in shell/subproc.zig (remove .pipe/.readable_stream that were already handled) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add .pty case to the Windows switch statement in Writable.zig. On Windows, PTY falls back to pipe behavior, so stdin with PTY returns ignore (same as other unsupported stdin types). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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)
packages/bun-types/bun.d.ts (1)
5334-5353:"pty"stdio mode not reflected inSubprocessIO types
Spawn.Readable/Spawn.Writablenow include"pty", but the helper mappings still only special‑case"pipe":
ReadableToIO<"pty">andReadableToSyncIO<"pty">currently resolve toundefined, sosubprocess.stdout/stderrare typed asundefinedwhen using"pty", even though the runtime exposes a readable stream/buffer.WritableToIO<"pty">resolves toundefined, sosubprocess.stdinis also incorrectly typed when using"pty".This makes the new PTY mode effectively untyped from the caller’s perspective and will surprise TypeScript users relying on
Subprocessgenerics.If
"pty"behaves like"pipe"from the parent’s point of view (stream/FileSinkinterface with TTY semantics in the child), the mapping helpers should treat"pty"the same as"pipe":- type ReadableToIO<X extends Readable> = X extends "pipe" | undefined + type ReadableToIO<X extends Readable> = X extends "pipe" | "pty" | undefined ? ReadableStream<Uint8Array<ArrayBuffer>> : X extends BunFile | ArrayBufferView | number ? number : undefined; - type ReadableToSyncIO<X extends Readable> = X extends "pipe" | undefined ? Buffer : undefined; + type ReadableToSyncIO<X extends Readable> = X extends "pipe" | "pty" | undefined ? Buffer : undefined; type WritableIO = FileSink | number | undefined; - type WritableToIO<X extends Writable> = X extends "pipe" + type WritableToIO<X extends Writable> = X extends "pipe" | "pty" ? FileSink : X extends BunFile | ArrayBufferView | Blob | Request | Response | number ? number : undefined;Also applies to: 5688-5703
♻️ Duplicate comments (1)
packages/bun-types/bun.d.ts (1)
5411-5425: PTY JSDoc and spawnSync limitation are clearly documentedThe new
"pty"bullets forstdio,stdin,stdout, andstderrcorrectly describe the TTY semantics, Windows fallback to"pipe", and the fact that"pty"is not supported withspawnSync. This addresses the earlier documentation gap and keeps behavior expectations clear.Also applies to: 5434-5434, 5447-5447, 5460-5460
📜 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.
📒 Files selected for processing (6)
docs/runtime/child-process.mdx(4 hunks)packages/bun-types/bun.d.ts(9 hunks)src/bun.js/api/bun/js_bun_spawn_bindings.zig(2 hunks)src/bun.js/api/bun/process.zig(5 hunks)src/bun.js/api/bun/spawn/stdio.zig(5 hunks)src/shell/subproc.zig(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/shell/subproc.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)Be careful with memory management in Zig code - use defer for cleanup with allocators
Files:
src/shell/subproc.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objects
Files:
src/shell/subproc.zigsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zig
**/js_*.zig
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
**/js_*.zig: ImplementJSYourFeaturestruct in a file likejs_your_feature.zigto create JavaScript bindings for Zig functionality
Usebun.JSError!JSValuefor proper error propagation in JavaScript bindings
Implement proper memory management with reference counting usingref()/deref()in JavaScript bindings
Always implement proper cleanup indeinit()andfinalize()methods for JavaScript binding classes
Files:
src/bun.js/api/bun/js_bun_spawn_bindings.zig
🧠 Learnings (37)
📓 Common learnings
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: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
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.
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;`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
📚 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:
src/shell/subproc.zigdocs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/bun-types/bun.d.ts
📚 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/shell/subproc.zig
📚 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:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/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:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/process.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
Applied to files:
docs/runtime/child-process.mdx
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
docs/runtime/child-process.mdxpackages/bun-types/bun.d.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:
docs/runtime/child-process.mdxpackages/bun-types/bun.d.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Applied to files:
docs/runtime/child-process.mdx
📚 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:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/bun-types/bun.d.ts
📚 Learning: 2025-10-04T21:17:53.040Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23253
File: test/js/valkey/valkey.failing-subscriber-no-ipc.ts:40-59
Timestamp: 2025-10-04T21:17:53.040Z
Learning: In Bun runtime, the global `console` object is an AsyncIterable that yields lines from stdin. The pattern `for await (const line of console)` is valid and documented in Bun for reading input line-by-line from the child process stdin.
Applied to files:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, call `expect(stdout).toBe(...)` before `expect(exitCode).toBe(0)` when spawning processes for more useful error messages on failure
Applied to files:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/spawn/stdio.zigsrc/bun.js/api/bun/js_bun_spawn_bindings.zigpackages/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:
docs/runtime/child-process.mdxpackages/bun-types/bun.d.ts
📚 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/api/bun/spawn/stdio.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/api/bun/spawn/stdio.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/api/bun/spawn/stdio.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Use `bun.JSError!JSValue` return type for Zig methods and constructors to enable proper error handling and exception propagation
Applied to files:
src/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings
Applied to files:
src/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-09-04T02:04:43.094Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22278
File: src/ast/E.zig:980-1003
Timestamp: 2025-09-04T02:04:43.094Z
Learning: In Bun's Zig codebase, `as(i32, u8_or_u16_value)` is sufficient for casting u8/u16 to i32 in comparison operations. `intCast` is not required in this context, and the current casting approach compiles successfully.
Applied to files:
src/bun.js/api/bun/spawn/stdio.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/api/bun/spawn/stdio.zig
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/api/bun/spawn/stdio.zig
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.zig : Use `bun.Output.scoped(.${SCOPE}, .hidden)` for creating debug logs in Zig code
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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/api/bun/js_bun_spawn_bindings.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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/api/bun/js_bun_spawn_bindings.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} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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/api/bun/js_bun_spawn_bindings.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/api/bun/js_bun_spawn_bindings.zig
📚 Learning: 2025-10-18T02:06:31.606Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23796
File: src/bun.js/test/Execution.zig:624-625
Timestamp: 2025-10-18T02:06:31.606Z
Learning: In Zig files using bun.Output.scoped(), the visibility level `.visible` means logs are enabled by default in debug builds unless `BUN_DEBUG_QUIET_LOGS=1` is set; if that environment variable is set, the logs can be re-enabled with `BUN_DEBUG_<scope>=1`. Use `.visible` for logs that should appear by default in debug builds, and `.hidden` for logs that require explicit opt-in via `BUN_DEBUG_<scope>=1`.
Applied to files:
src/bun.js/api/bun/js_bun_spawn_bindings.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-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.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode
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: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules
Applied to files:
packages/bun-types/bun.d.ts
🔇 Additional comments (5)
src/bun.js/api/bun/js_bun_spawn_bindings.zig (1)
638-646: POSIX‑gated FD logging + dedicated scope look correctThe new log uses POSIX‑only optional FDs and falls back to a simpler message on Windows, which fixes the earlier type mismatch, and the
.spawn_bindings.hiddenscope keeps this as opt‑in debug output. No functional issues here.Also applies to: 1010-1010
src/bun.js/api/bun/spawn/stdio.zig (1)
17-26: PTY stdio parsing and platform behavior look solid
- Adding
pty: PtyOptions { width, height }toStdiowith POSIX →.ptyand Windows →.buffer(pipe) matches the documented semantics.- The
"pty"string and{ type: "pty", width?, height? }object forms:
- Correctly forbid use in
spawnSync.- Validate
width/heighttypes withisNumber()beforetoInt32(), and enforce0 < value ≤ 65535before casting tou16.- Apply the same Windows fallback to
"pipe".The unified invalid‑value error message now lists
"pty"and all other supported forms, which makes the API surface clearer. No issues from a runtime or typing perspective.Also applies to: 205-206, 258-259, 361-373, 461-510
packages/bun-types/bun.d.ts (3)
1743-1746: Doc reflow aroundformatunion looks fineAligning the
"esm"case with its JSDoc is stylistic only and keeps the type readable; no issues here.
3319-3322: Helpful JSDoc addition for"ansi"output formatDocumenting that
"ansi"returns a true‑color SGR escape (with an explicit example) is accurate and makes the overload clearer; no further changes needed.
5660-5666: Interfaces forSpawnSyncOptions/SpawnOptionsare reasonableSwitching to
interface SpawnSyncOptionsandinterface SpawnOptionsthat extendBaseOptionskeeps the public surface unchanged while letting you hang spawn‑only docs likelazyoff the async options type; the signatures remain structurally compatible with existing code.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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.
📒 Files selected for processing (1)
src/bun.js/api/bun/subprocess/Writable.zig(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/bun.js/api/bun/subprocess/Writable.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)Be careful with memory management in Zig code - use defer for cleanup with allocators
Files:
src/bun.js/api/bun/subprocess/Writable.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objects
Files:
src/bun.js/api/bun/subprocess/Writable.zig
🧠 Learnings (3)
📓 Common learnings
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: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
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.
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;`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
📚 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:
src/bun.js/api/bun/subprocess/Writable.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:
src/bun.js/api/bun/subprocess/Writable.zig
🔇 Additional comments (1)
src/bun.js/api/bun/subprocess/Writable.zig (1)
235-258: LGTM - Correct PTY implementation for POSIX.The PTY stdin handling correctly:
- Creates a FileSink with the PTY master fd
- Starts the writer without the
.socketflag (PTY masters are character devices, not sockets)- Sets up subprocess references consistently with the regular pipe case
- Uses
write()syscalls rather thansend()The explicit comment about not setting the socket flag is excellent defensive documentation that prevents future bugs.
- Fix PTY master FD leak: close original master FD after spawn succeeds since parent uses dup()'d copies for stdio/extra_fds - Fix PosixSpawnResult.close: correct type signature and add PTY handling - Harmonize shell PTY behavior: change POSIX stdin panic to return ignore (consistent with Windows behavior) - Update docs: add "Not supported with spawnSync" to PTY table entries - Update docs: expand TS type comments with platform/spawnSync notes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update comment to clearly state PTY stdin is not supported on Windows rather than misleadingly mentioning "falls back to pipe". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/bun.js/api/bun/process.zig (1)
1577-1582: Original PTY master closed butspawned.pty_masterstill references it.After closing the original PTY master FD (line 1581),
spawned.pty_masterstill holds the closed FD value. WhilePosixSpawnResult.close()will try to close it again (line 1122-1124), this could lead to confusion or double-close ifclose()is called. Consider settingspawned.pty_master = nullafter closing.Apply this diff for clarity and safety:
// Parent uses dup()'d copies of the PTY master for stdio/extra_fds; // the original master FD is no longer needed and should be closed // to avoid leaking one FD per PTY spawn. if (pty_master) |fd| { fd.close(); + spawned.pty_master = null; }
📜 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.
📒 Files selected for processing (4)
docs/runtime/child-process.mdx(4 hunks)src/bun.js/api/bun/process.zig(6 hunks)src/bun.js/api/bun/subprocess/Writable.zig(2 hunks)src/shell/subproc.zig(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/bun.js/api/bun/process.zigsrc/shell/subproc.zigsrc/bun.js/api/bun/subprocess/Writable.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)Be careful with memory management in Zig code - use defer for cleanup with allocators
Files:
src/bun.js/api/bun/process.zigsrc/shell/subproc.zigsrc/bun.js/api/bun/subprocess/Writable.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objects
Files:
src/bun.js/api/bun/process.zigsrc/shell/subproc.zigsrc/bun.js/api/bun/subprocess/Writable.zig
🧠 Learnings (16)
📓 Common learnings
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: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
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;`.
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.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
📚 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:
src/bun.js/api/bun/process.zigsrc/shell/subproc.zigdocs/runtime/child-process.mdxsrc/bun.js/api/bun/subprocess/Writable.zig
📚 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:
src/bun.js/api/bun/process.zigdocs/runtime/child-process.mdx
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
src/bun.js/api/bun/process.zigdocs/runtime/child-process.mdx
📚 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:
src/bun.js/api/bun/process.zigdocs/runtime/child-process.mdx
📚 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/shell/subproc.zig
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
docs/runtime/child-process.mdx
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
Applied to files:
docs/runtime/child-process.mdx
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
docs/runtime/child-process.mdx
📚 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:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/subprocess/Writable.zig
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Applied to files:
docs/runtime/child-process.mdx
📚 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:
docs/runtime/child-process.mdxsrc/bun.js/api/bun/subprocess/Writable.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} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
docs/runtime/child-process.mdx
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, call `expect(stdout).toBe(...)` before `expect(exitCode).toBe(0)` when spawning processes for more useful error messages on failure
Applied to files:
docs/runtime/child-process.mdx
📚 Learning: 2025-10-04T21:17:53.040Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23253
File: test/js/valkey/valkey.failing-subscriber-no-ipc.ts:40-59
Timestamp: 2025-10-04T21:17:53.040Z
Learning: In Bun runtime, the global `console` object is an AsyncIterable that yields lines from stdin. The pattern `for await (const line of console)` is valid and documented in Bun for reading input line-by-line from the child process stdin.
Applied to files:
docs/runtime/child-process.mdx
📚 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/api/bun/subprocess/Writable.zig
🔇 Additional comments (12)
src/shell/subproc.zig (3)
181-185: LGTM - PTY handling for shell stdin on Windows.The shell intentionally ignores PTY for stdin, which is documented in the comment. This is consistent with the PR's approach that the shell layer doesn't consume PTYs directly.
223-230: LGTM - Graceful fallback for PTY and readable_stream.Replacing the previous panic with a graceful ignore fallback is the right approach. The comments clearly document that the shell never uses these paths directly.
380-381: LGTM - Consistent PTY handling in Readable.init.Both Windows and POSIX paths consistently ignore PTY for readable streams, with clear comments explaining the shell doesn't use PTY directly.
Also applies to: 402-404
src/bun.js/api/bun/subprocess/Writable.zig (2)
156-159: LGTM - Windows PTY handling with corrected comment.The comment now correctly states that PTY stdin is not supported on Windows (returns ignore), which aligns with the implementation.
235-258: LGTM - PTY handling correctly mirrors pipe path without socket flag.The PTY implementation correctly:
- Creates a
FileSinkwith the PTY master fd- Starts the writer with proper error handling
- Omits the
.socketflag (correct for PTY character devices which usewrite()notsend())- Sets all lifecycle flags identically to the pipe path
The comment at lines 247-248 clearly documents why the socket flag is intentionally not set.
docs/runtime/child-process.mdx (3)
42-55: LGTM - Input stream table updated with PTY option.The table clearly documents the PTY option with appropriate caveats about Windows fallback and spawnSync limitations.
118-223: LGTM - Comprehensive PTY documentation.The new PTY section is well-structured and covers:
- Basic usage with clear examples
- Multiple PTY streams with shared terminal explanation
- Custom terminal dimensions with object syntax
- Practical use cases (colored output from git, grep, etc.)
- Platform support table with clear Windows fallback behavior
- Limitations (spawnSync, line endings, no dynamic resize)
The examples use consistent array-form spawn syntax throughout.
521-537: LGTM - Reference types with PTY documentation.The type definitions correctly include
"pty"with inline comments explaining platform support (macOS/Linux only), Windows fallback behavior, and spawnSync limitations.src/bun.js/api/bun/process.zig (4)
1007-1013: LGTM - PtyConfig struct with sensible defaults.The
PtyConfigstruct with default dimensions of 80 columns × 24 rows matches standard terminal defaults. The width/height are appropriately typed asu16.
1114-1130: LGTM - PosixSpawnResult PTY master handling.The
pty_masterfield is properly documented, and theclose()method correctly:
- Closes the PTY master fd if present
- Sets it to null after closing
- Then closes extra_pipes
The signature is now correctly
*PosixSpawnResult(previously flagged in past reviews).
1317-1347: LGTM - PTY pair creation with proper resource management.The implementation correctly:
- Creates a single PTY pair shared across all PTY stdios
- Applies window size from the first PTY config encountered
- Adds slave to
to_close_at_end(closed after spawn regardless of success)- Adds master to
to_close_on_error(closed only on error path)- Sets master to non-blocking only for async operations (correct for spawnSync compatibility)
Note: If multiple stdios have different PTY dimensions, the first one wins. This is a reasonable design choice given PTYs are inherently shared.
1533-1545: LGTM - extra_fds PTY handling with proper error cleanup.The extra_fds path correctly adds duped master FDs to
to_close_on_error(line 1542), ensuring cleanup on spawn failure.Note: If no primary stdio used PTY (
pty_slaveis null), this silently does nothing. This is acceptable since PTY is primarily intended for stdin/stdout/stderr rather than extra file descriptors.
Add duped master FDs to to_close_on_error list so they are properly cleaned up if spawn fails after duping. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Adds support for
stdin: "pty",stdout: "pty", andstderr: "pty"options inBun.spawn(). This allows spawned processes to run in a pseudo-terminal, makingprocess.stdout.isTTY === truein the child process.This is useful for:
Usage
Basic usage
Custom terminal dimensions
Multiple PTY streams
Implementation Details
openpty()to create a PTY master/slave pairdup()'d master FD for independent epoll registrationPlatform Support
"pipe"(no TTY semantics)Limitations
spawnSync: PTY requires async I/O. Throws an error.\nto\r\non output (standard terminal behavior).Test Plan
stdout: "pty"makesprocess.stdout.isTTYtruestderr: "pty"makesprocess.stderr.isTTYtruestdin: "pty"only (stdout/stderr as pipe)Changes
src/sys.zig: Addopenpty()syscall wrappersrc/bun.js/api/bun/process.zig: PTY creation in spawnsrc/bun.js/api/bun/spawn/stdio.zig: Parse "pty" optionsrc/bun.js/api/bun/subprocess/: PTY handling for Readable/Writablesrc/io/PipeReader.zig: Handle EIO as EOF for PTYpackages/bun-types/bun.d.ts: Add "pty" to typesdocs/runtime/child-process.mdx: Comprehensive documentation🤖 Generated with Claude Code