terminal: throw when a termios flag assignment fails and clear its mordant baseline entry

mordant-baseline.toml

@@ -64,7 +64,6 @@
 "defaulted_failure:src/runtime/server/RequestContext.rs" = 1
 "defaulted_failure:src/runtime/test_runner/pretty_format.rs" = 4
 "derived_field:src/runtime/test_runner/bun_test.rs" = 1
-"error_collapsed_to_bool:src/runtime/api/bun/Terminal.rs" = 1
 "error_collapsed_to_bool:src/runtime/api/bun/h2_frame_parser.rs" = 32
 "error_collapsed_to_bool:src/runtime/ffi/ffi_body.rs" = 1
 "field_valid_only_when:src/runtime/cli/filter_run.rs" = 1

packages/bun-types/bun.d.ts

@@ -8296,31 +8296,31 @@ declare module "bun" {
      * Terminal input flags (c_iflag from termios).
      * Controls input processing behavior like ICRNL, IXON, etc.
      * Returns 0 if the terminal is closed.
-     * Setting returns true on success, false on failure.
+     * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags.
      */
     inputFlags: number;
 
     /**
      * Terminal output flags (c_oflag from termios).
      * Controls output processing behavior like OPOST, ONLCR, etc.
      * Returns 0 if the terminal is closed.
-     * Setting returns true on success, false on failure.
+     * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags.
      */
     outputFlags: number;
 
     /**
      * Terminal local flags (c_lflag from termios).
      * Controls local processing like ICANON, ECHO, ISIG, etc.
      * Returns 0 if the terminal is closed.
-     * Setting returns true on success, false on failure.
+     * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags.
      */
     localFlags: number;
 
     /**
      * Terminal control flags (c_cflag from termios).
      * Controls hardware characteristics like CSIZE, PARENB, etc.
      * Returns 0 if the terminal is closed.
-     * Setting returns true on success, false on failure.
+     * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags.
      */
     controlFlags: number;
   }

src/runtime/api/bun/Terminal.rs

@@ -1364,7 +1364,7 @@ impl Terminal {
             if self.flags.get().contains(Flags::CLOSED) || self.master_fd.get() == Fd::INVALID {
                 return JSValue::js_number(0.0);
             }
-            let Some(termios_data) = get_termios(self.master_fd.get()) else {
+            let Ok(termios_data) = get_termios(self.master_fd.get()) else {
                 return JSValue::js_number(0.0);
             };
             let raw: u64 = match FIELD {
@@ -1389,13 +1389,14 @@ impl Terminal {
         }
         #[cfg(unix)]
         {
-            if self.flags.get().contains(Flags::CLOSED) || self.master_fd.get() == Fd::INVALID {
-                return Ok(());
-            }
             let num = value.coerce_f64(global_object)?;
-            let Some(mut termios_data) = get_termios(self.master_fd.get()) else {
+            // Checked after coercion: a valueOf() may have closed the terminal.
+            let master_fd = self.master_fd.get();
+            if self.flags.get().contains(Flags::CLOSED) || master_fd == Fd::INVALID {
                 return Ok(());
-            };
+            }
+            let mut termios_data =
+                get_termios(master_fd).map_err(|err| err.throw(global_object))?;
             let max_val: f64 = libc::tcflag_t::MAX as f64;
             // Match Zig's `@max(0, @min(num, max_val))`: apply min first so NaN
             // resolves to max_val (f64::min returns the non-NaN operand), not 0.
@@ -1407,8 +1408,7 @@ impl Terminal {
                 TermiosField::Lflag => termios_data.c_lflag = bits,
                 TermiosField::Cflag => termios_data.c_cflag = bits,
             }
-            let _ = set_termios(self.master_fd.get(), &termios_data);
-            Ok(())
+            set_termios(master_fd, &termios_data).map_err(|err| err.throw(global_object))
         }
     }
 
@@ -1675,14 +1675,14 @@ type Termios = sys::posix::Termios;
 
 /// Get terminal attributes using tcgetattr
 #[cfg(unix)]
-fn get_termios(fd: Fd) -> Option<Termios> {
-    sys::posix::tcgetattr(fd.native()).ok()
+fn get_termios(fd: Fd) -> sys::Result<Termios> {
+    sys::posix::tcgetattr(fd.native())
 }
 
 /// Set terminal attributes using tcsetattr (TCSANOW = immediate)
 #[cfg(unix)]
-fn set_termios(fd: Fd, termios_p: &Termios) -> bool {
-    sys::posix::tcsetattr(fd.native(), sys::posix::TCSA::Now, termios_p).is_ok()
+fn set_termios(fd: Fd, termios_p: &Termios) -> sys::Result<()> {
+    sys::posix::tcsetattr(fd.native(), sys::posix::TCSA::Now, termios_p)
 }
 
 impl Terminal {

test/js/bun/terminal/terminal.test.ts

@@ -474,16 +474,88 @@ describe("Bun.Terminal", () => {
       expect(terminal.inputFlags).toBe(0);
     });
 
+    test.skipIf(isWindows)("setting flags on a terminal closed while coercing the value is a no-op", () => {
+      const terminal = new Bun.Terminal({});
+
+      terminal.localFlags = {
+        valueOf() {
+          terminal.close();
+          return 0;
+        },
+      } as any;
+
+      expect(terminal.closed).toBe(true);
+    });
+
+    test.skipIf(isWindows)("setting flags throws when the terminal's fd is not a PTY", async () => {
+      // No libc rejects a termios update on a healthy PTY portably, so swap
+      // the PTY master fd out for a regular file behind the terminal's back.
+      // openpty() hands out the lowest free fd, so the master lands on the fd
+      // number the probe just freed. A regular file fails termios ioctls with
+      // ENOTTY on every POSIX kernel; /dev/null would be ENODEV on macOS.
+      const script = `
+        const fs = require("node:fs");
+        // The first terminal may open unrelated fds on the side (openpty is
+        // loaded lazily); construct one up front so the probe below sees a
+        // settled fd table.
+        new Bun.Terminal({}).close();
+        const probe = fs.openSync(process.execPath, "r");
+        fs.closeSync(probe);
+
+        const terminal = new Bun.Terminal({});
+        const flags = terminal.localFlags;
+        fs.closeSync(probe);
+        const replacement = fs.openSync(process.execPath, "r");
+
+        let thrown = null;
+        try {
+          terminal.localFlags = flags;
+        } catch (e) {
+          thrown = { code: e.code, syscall: e.syscall };
+        }
+        console.log(JSON.stringify({
+          swapped: replacement === probe,
+          hadFlagsBeforeSwap: flags !== 0,
+          flagsAfterSwap: terminal.localFlags,
+          closed: terminal.closed,
+          thrown,
+        }));
+        terminal.close();
+      `;
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "-e", script],
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+      expect(stderr).toBe("");
+      expect(JSON.parse(stdout)).toEqual({
+        swapped: true,
+        hadFlagsBeforeSwap: true,
+        flagsAfterSwap: 0,
+        closed: false,
+        thrown: { code: "ENOTTY", syscall: "ioctl" },
+      });
+      expect(exitCode).toBe(0);
+    });
+
     test.skipIf(isWindows)("NaN clamps to tcflag_t max, same as Infinity", async () => {
       // Assigning NaN must clamp to the same value as Infinity (tcflag_t::MAX),
       // matching the reference `@max(0, @min(num, max))` semantics. The kernel
       // may mask some bits per field, so compare NaN against Infinity rather
-      // than a hard-coded constant.
+      // than a hard-coded constant. Both assignments start from the original
+      // flags: re-requesting only the already-masked c_cflag bits is a
+      // tcsetattr that changes nothing, which Debian's and Ubuntu's patched
+      // glibc reports as EINVAL (upstream glibc and other libcs return 0).
       await using terminal = new Bun.Terminal({ cols: 80, rows: 24 });
 
       for (const prop of ["inputFlags", "outputFlags", "localFlags", "controlFlags"] as const) {
+        const original = terminal[prop];
         terminal[prop] = Infinity;
         const fromInfinity = terminal[prop];
+        terminal[prop] = original;
         terminal[prop] = NaN;
         const fromNaN = terminal[prop];
         expect({ prop, fromNaN }).toEqual({ prop, fromNaN: fromInfinity });