fix(ext/node): multiple readline improvements (#32538)

## Summary

Several fixes and feature additions to the Node.js readline polyfill:

- **Fix tab completion column formatting** —
`Math.max.apply(completionsWidth)` was missing the array argument,
causing all completions to display one-per-line instead of in columns
- **Fix `historySize` validation** — now throws `ERR_OUT_OF_RANGE` for
negative/NaN values and `ERR_INVALID_ARG_TYPE` for non-number values
(was incorrectly throwing `ERR_INVALID_ARG_VALUE` for all)
- **Implement kill ring** with yank (`Ctrl+Y`) and yank-pop (`Meta+Y`)
- **Implement undo/redo** (`Ctrl+_` / `Ctrl+^`)
- **Fix `write()` after `close()`** to throw `ERR_USE_AFTER_CLOSE`
- **Fix `question()` abort signal cleanup** — `removeEventListener` was
missing the event name arg, and the callback wrapper had an infinite
recursion bug

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: bartlomieju

ext/node/polyfills/_readline.mjs

@@ -133,12 +133,13 @@ Interface.prototype.question = function question(query, options, cb) {
     };
     options.signal.addEventListener("abort", onAbort, { once: true });
     const cleanup = () => {
-      options.signal.removeEventListener(onAbort);
+      options.signal.removeEventListener("abort", onAbort);
     };
-    cb = typeof cb === "function"
+    const originalCb = cb;
+    cb = typeof originalCb === "function"
       ? (answer) => {
         cleanup();
-        return cb(answer);
+        return originalCb(answer);
       }
       : cleanup;
   }

ext/node/polyfills/internal/readline/interface.mjs

@@ -27,7 +27,9 @@
 import { op_get_env_no_permission_check } from "ext:core/ops";
 
 import {
+  ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
+  ERR_OUT_OF_RANGE,
   ERR_USE_AFTER_CLOSE,
 } from "ext:deno_node/internal/errors.ts";
 import {
@@ -118,6 +120,15 @@ const lineEnding = new SafeRegExp(/\r?\n|\r(?!\n)|\u2028|\u2029/g);
 const kLineObjectStream = Symbol("line object stream");
 export const kQuestionCancel = Symbol("kQuestionCancel");
 export const kQuestion = Symbol("kQuestion");
+const kKillRing = Symbol("kKillRing");
+const kKillRingCursor = Symbol("kKillRingCursor");
+const kYank = Symbol("kYank");
+const kYankPop = Symbol("kYankPop");
+const kUndoStack = Symbol("kUndoStack");
+const kRedoStack = Symbol("kRedoStack");
+const kUndo = Symbol("kUndo");
+const kRedo = Symbol("kRedo");
+const kPushToUndoStack = Symbol("kPushToUndoStack");
 
 // GNU readline library - keyseq-timeout is 500ms (default)
 const ESCAPE_CODE_TIMEOUT = 500;
@@ -223,12 +234,12 @@ export function InterfaceConstructor(input, output, completer, terminal) {
     historySize = kHistorySize;
   }
 
-  if (
-    typeof historySize !== "number" ||
-    Number.isNaN(historySize) ||
-    historySize < 0
-  ) {
-    throw new ERR_INVALID_ARG_VALUE.RangeError("historySize", historySize);
+  if (typeof historySize !== "number") {
+    throw new ERR_INVALID_ARG_TYPE("historySize", "number", historySize);
+  }
+
+  if (Number.isNaN(historySize) || historySize < 0) {
+    throw new ERR_OUT_OF_RANGE("historySize", ">= 0", historySize);
   }
 
   // Backwards compat; check the isTTY prop of the output stream
@@ -250,6 +261,10 @@ export function InterfaceConstructor(input, output, completer, terminal) {
     ? Math.max(kMincrlfDelay, crlfDelay)
     : kMincrlfDelay;
   this.completer = completer;
+  this[kKillRing] = [];
+  this[kKillRingCursor] = 0;
+  this[kUndoStack] = [];
+  this[kRedoStack] = [];
 
   this.setPrompt(prompt);
 
@@ -581,6 +596,9 @@ export class Interface extends InterfaceConstructor {
    * @returns {void}
    */
   write(d, key) {
+    if (this.closed) {
+      throw new ERR_USE_AFTER_CLOSE("readline");
+    }
     if (this.paused) this.resume();
     if (this.terminal) {
       this[kTtyWrite](d, key);
@@ -709,7 +727,7 @@ export class Interface extends InterfaceConstructor {
     const completionsWidth = completions.map(
       (e) => getStringWidth(e),
     );
-    const width = Math.max.apply(completionsWidth) + 2; // 2 space padding
+    const width = Math.max(...completionsWidth) + 2; // 2 space padding
     let maxColumns = Math.floor(this.columns / width) || 1;
     if (maxColumns === Infinity) {
       maxColumns = 1;
@@ -762,6 +780,7 @@ export class Interface extends InterfaceConstructor {
 
   [kDeleteLeft]() {
     if (this.cursor > 0 && this.line.length > 0) {
+      this[kPushToUndoStack]();
       // The number of UTF-16 units comprising the character to the left
       const charSize = charLengthLeft(this.line, this.cursor);
       this.line = this.line.slice(0, this.cursor - charSize) +
@@ -774,6 +793,7 @@ export class Interface extends InterfaceConstructor {
 
   [kDeleteRight]() {
     if (this.cursor < this.line.length) {
+      this[kPushToUndoStack]();
       // The number of UTF-16 units comprising the character to the left
       const charSize = charLengthAt(this.line, this.cursor);
       this.line = this.line.slice(0, this.cursor) +
@@ -787,6 +807,7 @@ export class Interface extends InterfaceConstructor {
 
   [kDeleteWordLeft]() {
     if (this.cursor > 0) {
+      this[kPushToUndoStack]();
       // Reverse the string and match a word near beginning
       // to avoid quadratic time complexity
       let leading = this.line.slice(0, this.cursor);
@@ -796,34 +817,100 @@ export class Interface extends InterfaceConstructor {
         0,
         leading.length - match[0].length,
       );
+      const killed = this.line.slice(leading.length, this.cursor);
       this.line = leading +
         this.line.slice(this.cursor, this.line.length);
       this.cursor = leading.length;
+      this[kKillRing].push(killed);
+      this[kKillRingCursor] = this[kKillRing].length - 1;
       this[kRefreshLine]();
     }
   }
 
   [kDeleteWordRight]() {
     if (this.cursor < this.line.length) {
+      this[kPushToUndoStack]();
       const trailing = this.line.slice(this.cursor);
       const match = trailing.match(/^(?:\s+|\W+|\w+)\s*/);
+      const killed = trailing.slice(0, match[0].length);
       this.line = this.line.slice(0, this.cursor) +
         trailing.slice(match[0].length);
+      this[kKillRing].push(killed);
+      this[kKillRingCursor] = this[kKillRing].length - 1;
       this[kRefreshLine]();
     }
   }
 
   [kDeleteLineLeft]() {
+    this[kPushToUndoStack]();
+    const killed = this.line.slice(0, this.cursor);
     this.line = this.line.slice(this.cursor);
     this.cursor = 0;
+    this[kKillRing].push(killed);
+    this[kKillRingCursor] = this[kKillRing].length - 1;
     this[kRefreshLine]();
   }
 
   [kDeleteLineRight]() {
+    this[kPushToUndoStack]();
+    const killed = this.line.slice(this.cursor);
     this.line = this.line.slice(0, this.cursor);
+    this[kKillRing].push(killed);
+    this[kKillRingCursor] = this[kKillRing].length - 1;
     this[kRefreshLine]();
   }
 
+  [kYank]() {
+    if (this[kKillRing].length > 0) {
+      this[kKillRingCursor] = this[kKillRing].length - 1;
+      const killed = this[kKillRing][this[kKillRingCursor]];
+      this[kInsertString](killed);
+    }
+  }
+
+  [kYankPop]() {
+    if (this[kKillRing].length > 1) {
+      // Remove previously yanked text
+      const prev = this[kKillRing][this[kKillRingCursor]];
+      this.line = this.line.slice(0, this.cursor - prev.length) +
+        this.line.slice(this.cursor);
+      this.cursor -= prev.length;
+
+      // Cycle to previous entry in kill ring
+      this[kKillRingCursor]--;
+      if (this[kKillRingCursor] < 0) {
+        this[kKillRingCursor] = this[kKillRing].length - 1;
+      }
+      const killed = this[kKillRing][this[kKillRingCursor]];
+      this[kInsertString](killed);
+    }
+  }
+
+  [kPushToUndoStack]() {
+    this[kUndoStack].push({ line: this.line, cursor: this.cursor });
+    this[kRedoStack] = [];
+  }
+
+  [kUndo]() {
+    if (this[kUndoStack].length > 0) {
+      this[kRedoStack].push({ line: this.line, cursor: this.cursor });
+      const state = this[kUndoStack].pop();
+      this.line = state.line;
+      this.cursor = state.cursor;
+      this[kRefreshLine]();
+    }
+  }
+
+  [kRedo]() {
+    if (this[kRedoStack].length > 0) {
+      this[kUndoStack].push({ line: this.line, cursor: this.cursor });
+      const state = this[kRedoStack].pop();
+      this.line = state.line;
+      this.cursor = state.cursor;
+      this[kRefreshLine]();
+    }
+  }
+
   clearLine() {
     this[kMoveCursor](+Infinity);
     this[kWriteToOutput]("\r\n");
@@ -991,6 +1078,16 @@ export class Interface extends InterfaceConstructor {
       }
     }
 
+    // Undo (Ctrl+_) and Redo (Ctrl+^)
+    if (key.sequence === "\x1F") {
+      this[kUndo]();
+      return;
+    }
+    if (key.sequence === "\x1E") {
+      this[kRedo]();
+      return;
+    }
+
     // Ignore escape key, fixes
     // https://github.com/nodejs/node-v0.x-archive/issues/2876.
     if (key.name === "escape") return;
@@ -1072,6 +1169,10 @@ export class Interface extends InterfaceConstructor {
           this[kHistoryPrev]();
           break;
 
+        case "y": // Yank killed text
+          this[kYank]();
+          break;
+
         case "z":
           if (process.platform === "win32") break;
           if (this.listenerCount("SIGTSTP") > 0) {
@@ -1136,6 +1237,10 @@ export class Interface extends InterfaceConstructor {
         case "backspace": // Delete backwards to a word boundary
           this[kDeleteWordLeft]();
           break;
+
+        case "y": // Yank pop
+          this[kYankPop]();
+          break;
       }
     } else {
       /* No modifier keys used */