mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-13 04:18:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address CodeRabbit review comments for PTY support

Browse Source 
- 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>
This commit is contained in:
Claude Bot
2025-12-03 11:10:10 +00:00
parent 6d4965c79b
commit 0245de5c78
6 changed files with 44 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
docs/runtime/child-process.mdx
View File

@@ -138,8 +138,7 @@ console.log(output); // includes ANSI color codes
### Checking isTTY in child process
```ts
const proc = Bun.spawn({
cmd: ["bun", "-e", "console.log('isTTY:', process.stdout.isTTY)"],
const proc = Bun.spawn(["bun", "-e", "console.log('isTTY:', process.stdout.isTTY)"], {
stdout: "pty",
});
@@ -152,16 +151,13 @@ console.log(output); // "isTTY: true"
You can use PTY for stdin, stdout, and/or stderr. When multiple streams use PTY, they share the same underlying pseudo-terminal:
```ts
const proc = Bun.spawn({
cmd: [
"bun",
"-e",
`
console.log("stdout.isTTY:", process.stdout.isTTY);
console.log("stdin.isTTY:", process.stdin.isTTY);
console.log("stderr.isTTY:", process.stderr.isTTY);
`,
],
const code = `
console.log("stdout.isTTY:", process.stdout.isTTY);
console.log("stdin.isTTY:", process.stdin.isTTY);
console.log("stderr.isTTY:", process.stderr.isTTY);
`;
const proc = Bun.spawn(["bun", "-e", code], {
stdin: "pty",
stdout: "pty",
stderr: "pty",
@@ -178,15 +174,12 @@ const output = await proc.stdout.text();
Specify terminal width and height using the object syntax:
```ts
const proc = Bun.spawn({
cmd: [
"bun",
"-e",
`
console.log("columns:", process.stdout.columns);
console.log("rows:", process.stdout.rows);
`,
],
const code = `
console.log("columns:", process.stdout.columns);
console.log("rows:", process.stdout.rows);
`;
const proc = Bun.spawn(["bun", "-e", code], {
stdout: {
type: "pty",
width: 120, // columns

10
packages/bun-types/bun.d.ts vendored
View File

@@ -5407,7 +5407,7 @@ declare module "bun" {
* - `"ignore"`, `null`, `undefined`: The process will have no standard input (default)
* - `"pipe"`: The process will have a new {@link FileSink} for standard input
* - `"inherit"`: The process will inherit the standard input of the current process
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdin.isTTY === true`. Falls back to `"pipe"` on Windows.
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdin.isTTY === true`. Falls back to `"pipe"` on Windows. Not supported with `spawnSync`.
* - `ArrayBufferView`, `Blob`, `Bun.file()`, `Response`, `Request`: The process will read from buffer/stream.
* - `number`: The process will read from the file descriptor
*
@@ -5416,7 +5416,7 @@ declare module "bun" {
* - `"pipe"`, `undefined`: The process will have a {@link ReadableStream} for standard output/error
* - `"ignore"`, `null`: The process will have no standard output/error
* - `"inherit"`: The process will inherit the standard output/error of the current process
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdout.isTTY === true` / `process.stderr.isTTY === true`. Falls back to `"pipe"` on Windows.
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdout.isTTY === true` / `process.stderr.isTTY === true`. Falls back to `"pipe"` on Windows. Not supported with `spawnSync`.
* - `ArrayBufferView`: The process write to the preallocated buffer. Not implemented.
* - `number`: The process will write to the file descriptor
*
@@ -5431,7 +5431,7 @@ declare module "bun" {
* - `"ignore"`, `null`, `undefined`: The process will have no standard input
* - `"pipe"`: The process will have a new {@link FileSink} for standard input
* - `"inherit"`: The process will inherit the standard input of the current process
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdin.isTTY === true`. Falls back to `"pipe"` on Windows.
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdin.isTTY === true`. Falls back to `"pipe"` on Windows. Not supported with `spawnSync`.
* - `ArrayBufferView`, `Blob`: The process will read from the buffer
* - `number`: The process will read from the file descriptor
*
@@ -5444,7 +5444,7 @@ declare module "bun" {
* - `"pipe"`, `undefined`: The process will have a {@link ReadableStream} for standard output/error
* - `"ignore"`, `null`: The process will have no standard output/error
* - `"inherit"`: The process will inherit the standard output/error of the current process
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdout.isTTY === true`. Falls back to `"pipe"` on Windows.
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stdout.isTTY === true`. Falls back to `"pipe"` on Windows. Not supported with `spawnSync`.
* - `ArrayBufferView`: The process write to the preallocated buffer. Not implemented.
* - `number`: The process will write to the file descriptor
*
@@ -5457,7 +5457,7 @@ declare module "bun" {
* - `"pipe"`, `undefined`: The process will have a {@link ReadableStream} for standard output/error
* - `"ignore"`, `null`: The process will have no standard output/error
* - `"inherit"`: The process will inherit the standard output/error of the current process
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stderr.isTTY === true`. Falls back to `"pipe"` on Windows.
* - `"pty"`: The process will use a pseudo-terminal (PTY). The child will see `process.stderr.isTTY === true`. Falls back to `"pipe"` on Windows. Not supported with `spawnSync`.
* - `ArrayBufferView`: The process write to the preallocated buffer. Not implemented.
* - `number`: The process will write to the file descriptor
*

14
src/bun.js/api/bun/js_bun_spawn_bindings.zig
View File

@@ -635,11 +635,15 @@ pub fn spawnMaybeSync(
.stdout_maxbuf = subprocess.stdout_maxbuf,
};
log("After subprocess init: stdout state={s}, stdin FD={?d}, stdout FD={?d}", .{
@tagName(subprocess.stdout),
if (spawned.stdin) |fd| fd.native() else null,
if (spawned.stdout) |fd| fd.native() else null,
});
if (comptime Environment.isPosix) {
log("After subprocess init: stdout state={s}, stdin FD={?d}, stdout FD={?d}", .{
@tagName(subprocess.stdout),
if (spawned.stdin) |fd| fd.native() else null,
if (spawned.stdout) |fd| fd.native() else null,
});
} else {
log("After subprocess init: stdout state={s}", .{@tagName(subprocess.stdout)});
}
subprocess.process.setExitHandler(subprocess);

8
src/bun.js/api/bun/process.zig
View File

@@ -1531,7 +1531,13 @@ pub fn spawnProcessPosix(
// Use existing PTY slave (should have been created from primary stdio)
if (pty_slave) |slave| {
try actions.dup2(slave, fileno);
try extra_fds.append(pty_master.?);
// dup() the master FD so each extra_fd has its own FD for epoll
const duped = try bun.sys.dup(pty_master.?).unwrap();
if (!options.sync) {
try bun.sys.setNonblocking(duped).unwrap();
}
try to_close_on_error.append(duped);
try extra_fds.append(duped);
}
},
}

8
src/bun.js/api/bun/spawn/stdio.zig
View File

@@ -476,6 +476,9 @@ pub const Stdio = union(enum) {
if (try value.get(globalThis, "width")) |width_val| {
if (!width_val.isUndefinedOrNull()) {
if (!width_val.isNumber()) {
return globalThis.throwInvalidArguments("PTY width must be a number", .{});
}
const width = width_val.toInt32();
if (width <= 0 or width > std.math.maxInt(u16)) {
return globalThis.throwInvalidArguments("PTY width must be a positive integer <= 65535", .{});
@@ -486,6 +489,9 @@ pub const Stdio = union(enum) {
if (try value.get(globalThis, "height")) |height_val| {
if (!height_val.isUndefinedOrNull()) {
if (!height_val.isNumber()) {
return globalThis.throwInvalidArguments("PTY height must be a number", .{});
}
const height = height_val.toInt32();
if (height <= 0 or height > std.math.maxInt(u16)) {
return globalThis.throwInvalidArguments("PTY height must be a positive integer <= 65535", .{});
@@ -501,7 +507,7 @@ pub const Stdio = union(enum) {
}
}
return globalThis.throwInvalidArguments("stdio must be an array of 'inherit', 'ignore', 'pty', or null", .{});
return globalThis.throwInvalidArguments("stdio must be an array of 'inherit', 'pipe', 'ignore', 'pty', Bun.file(pathOrFd), number, or null", .{});
}
pub fn extractBlob(stdio: *Stdio, globalThis: *jsc.JSGlobalObject, blob: jsc.WebCore.Blob.Any, i: i32) bun.JSError!void {

4
src/shell/subproc.zig
View File

@@ -178,8 +178,8 @@ pub const ShellSubprocess = struct {
.ipc, .capture => {
return Writable{ .ignore = {} };
},
.pty, .readable_stream, .pipe => {
// The shell never uses these for stdin in sync mode
.pty => {
// The shell never uses PTY directly for stdin
return Writable{ .ignore = {} };
},
}