feat(pty): canonical-mode line editing (VKILL/VWERASE) + ECHOCTL caret echo (#166)

Ports the recovered kernel line-discipline work onto the converged terminal:
- pty.rs: VKILL (Ctrl-U kill-line) + VWERASE (Ctrl-W word-erase) control chars
  and process_input branches; ECHOCTL caret-form control echo; column-accurate
  erase_sequence helper.
- guest_pty.rs: wire the two control chars over tcsetattr/tcgetattr.
- 8 new kernel pty unit tests.
Double-print: investigated; the convergence model surfaces child stdout once
(no PTY-master drain), so no guard is warranted — documented as a TODO.

Author: NathanFlurry

crates/kernel/src/pty.rs

@@ -98,6 +98,8 @@ pub struct TermiosControlChars {
     pub vsusp: u8,
     pub veof: u8,
     pub verase: u8,
+    pub vkill: u8,
+    pub vwerase: u8,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
@@ -107,6 +109,8 @@ pub struct PartialTermiosControlChars {
     pub vsusp: Option<u8>,
     pub veof: Option<u8>,
     pub verase: Option<u8>,
+    pub vkill: Option<u8>,
+    pub vwerase: Option<u8>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -139,6 +143,8 @@ impl Default for Termios {
                 vsusp: 0x1a,
                 veof: 0x04,
                 verase: 0x7f,
+                vkill: 0x15,
+                vwerase: 0x17,
             },
         }
     }
@@ -187,6 +193,12 @@ impl TermiosControlChars {
         if let Some(verase) = update.verase {
             self.verase = verase;
         }
+        if let Some(vkill) = update.vkill {
+            self.vkill = vkill;
+        }
+        if let Some(vwerase) = update.vwerase {
+            self.vwerase = vwerase;
+        }
     }
 }
 
@@ -972,11 +984,48 @@ fn process_input(
             }
 
             if byte == pty.termios.cc.verase || byte == 0x08 {
+                if let Some(&erased) = pty.line_buffer.last() {
+                    if pty.termios.echo {
+                        deliver_output(pty, waiters, &erase_sequence(erased), true)?;
+                    }
+                    pty.line_buffer.pop();
+                }
+                continue;
+            }
+
+            if byte == pty.termios.cc.vkill {
                 if !pty.line_buffer.is_empty() {
                     if pty.termios.echo {
-                        deliver_output(pty, waiters, &[0x08, 0x20, 0x08], true)?;
+                        let erase: Vec<u8> = pty
+                            .line_buffer
+                            .iter()
+                            .flat_map(|b| erase_sequence(*b))
+                            .collect();
+                        deliver_output(pty, waiters, &erase, true)?;
+                    }
+                    pty.line_buffer.clear();
+                }
+                continue;
+            }
+
+            if byte == pty.termios.cc.vwerase {
+                let mut erased: Vec<u8> = Vec::new();
+                while matches!(pty.line_buffer.last(), Some(b' ') | Some(b'\t')) {
+                    if let Some(b) = pty.line_buffer.pop() {
+                        erased.push(b);
+                    }
+                }
+                while let Some(&b) = pty.line_buffer.last() {
+                    if b == b' ' || b == b'\t' {
+                        break;
                     }
                     pty.line_buffer.pop();
+                    erased.push(b);
+                }
+                if pty.termios.echo && !erased.is_empty() {
+                    let sequence: Vec<u8> =
+                        erased.iter().flat_map(|b| erase_sequence(*b)).collect();
+                    deliver_output(pty, waiters, &sequence, true)?;
                 }
                 continue;
             }
@@ -996,7 +1045,9 @@ fn process_input(
                 continue;
             }
             if pty.termios.echo {
-                deliver_output(pty, waiters, &[byte], true)?;
+                // ECHOCTL: echo control chars in caret form (e.g. 0x01 -> "^A")
+                // so they are visible; printable bytes echo verbatim.
+                deliver_output(pty, waiters, &echo_control_byte(byte), true)?;
             }
             pty.line_buffer.push(byte);
         } else {
@@ -1132,6 +1183,16 @@ fn echo_control_byte(byte: u8) -> Vec<u8> {
     }
 }
 
+/// Backspace-erase sequence for a single buffered input byte, accounting for how
+/// wide it was echoed: a control char echoed in caret form (`^X`, ECHOCTL)
+/// occupies two columns and needs two `BS SP BS` triples, while a printable byte
+/// occupies one. Used by VERASE / VKILL / VWERASE erase echo so the erased echo
+/// width matches the displayed width.
+fn erase_sequence(byte: u8) -> Vec<u8> {
+    let columns = echo_control_byte(byte).len();
+    (0..columns).flat_map(|_| [0x08, 0x20, 0x08]).collect()
+}
+
 fn buffer_size(buffer: &VecDeque<Vec<u8>>) -> usize {
     buffer.iter().map(Vec::len).sum()
 }

crates/kernel/tests/pty.rs

@@ -561,3 +561,186 @@ fn set_termios_only_updates_requested_fields() {
     assert_eq!(termios.cc.verase, 0x08);
     assert_eq!(termios.cc.vintr, 0x03);
 }
+
+#[test]
+fn canonical_mode_kill_erases_the_whole_line() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    // Type "abc", then VKILL (Ctrl-U, 0x15) to wipe the line, then "xy\n".
+    manager
+        .write(pty.master.description.id(), b"abc\x15xy\n")
+        .expect("write canonical input with kill");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    assert_eq!(String::from_utf8(line).expect("valid utf8"), "xy\n");
+
+    let echo = manager
+        .read(pty.master.description.id(), 64)
+        .expect("read echo")
+        .expect("echo should be available");
+    // "abc" echoed, then three BS-SP-BS to erase them, then "xy", then CRLF.
+    assert_eq!(
+        String::from_utf8(echo).expect("valid utf8"),
+        "abc\x08 \x08\x08 \x08\x08 \x08xy\r\n"
+    );
+}
+
+#[test]
+fn canonical_mode_kill_on_empty_line_is_noop() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    manager
+        .write(pty.master.description.id(), b"\x15hi\n")
+        .expect("write kill on empty line");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    assert_eq!(String::from_utf8(line).expect("valid utf8"), "hi\n");
+
+    let echo = manager
+        .read(pty.master.description.id(), 64)
+        .expect("read echo")
+        .expect("echo should be available");
+    // No erase output for the empty-line kill; just the "hi" echo + CRLF.
+    assert_eq!(String::from_utf8(echo).expect("valid utf8"), "hi\r\n");
+}
+
+#[test]
+fn canonical_mode_werase_erases_preceding_word_and_trailing_space() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    // "foo bar " then VWERASE (Ctrl-W, 0x17) erases the trailing space and
+    // "bar", leaving "foo ", then "baz\n".
+    manager
+        .write(pty.master.description.id(), b"foo bar \x17baz\n")
+        .expect("write canonical input with werase");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    assert_eq!(String::from_utf8(line).expect("valid utf8"), "foo baz\n");
+
+    let echo = manager
+        .read(pty.master.description.id(), 64)
+        .expect("read echo")
+        .expect("echo should be available");
+    // "foo bar " echoed (8 chars), then 4 BS-SP-BS erases (space + "bar"),
+    // then "baz", then CRLF.
+    let mut expected = String::from("foo bar ");
+    expected.push_str(&"\x08 \x08".repeat(4));
+    expected.push_str("baz\r\n");
+    assert_eq!(String::from_utf8(echo).expect("valid utf8"), expected);
+}
+
+#[test]
+fn canonical_mode_werase_on_leading_whitespace_only_erases_it() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    // Only whitespace before VWERASE: erase all of it, nothing else.
+    manager
+        .write(pty.master.description.id(), b"   \x17done\n")
+        .expect("write whitespace then werase");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    assert_eq!(String::from_utf8(line).expect("valid utf8"), "done\n");
+}
+
+#[test]
+fn canonical_mode_echoctl_echoes_control_char_in_caret_form() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    // A control char that is neither a signal, VEOF, VERASE, VKILL, VWERASE,
+    // nor newline (0x01 = Ctrl-A) is buffered and echoed as caret form "^A".
+    manager
+        .write(pty.master.description.id(), b"a\x01b\n")
+        .expect("write control char in canonical mode");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    // The raw control byte is delivered to the slave verbatim.
+    assert_eq!(line, b"a\x01b\n");
+
+    let echo = manager
+        .read(pty.master.description.id(), 64)
+        .expect("read echo")
+        .expect("echo should be available");
+    assert_eq!(String::from_utf8(echo).expect("valid utf8"), "a^Ab\r\n");
+}
+
+#[test]
+fn canonical_mode_erase_after_echoctl_removes_both_caret_columns() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    // Type Ctrl-A (echoed "^A", two columns), then VERASE (0x7f): the erase
+    // must remove both caret columns, then "z\n".
+    manager
+        .write(pty.master.description.id(), b"\x01\x7fz\n")
+        .expect("write control char then erase");
+
+    let line = manager
+        .read(pty.slave.description.id(), 64)
+        .expect("read canonical line")
+        .expect("line should be available");
+    assert_eq!(line, b"z\n");
+
+    let echo = manager
+        .read(pty.master.description.id(), 64)
+        .expect("read echo")
+        .expect("echo should be available");
+    // "^A" echoed, then two BS-SP-BS to erase both columns, then "z", then CRLF.
+    let mut expected = String::from("^A");
+    expected.push_str(&"\x08 \x08".repeat(2));
+    expected.push_str("z\r\n");
+    assert_eq!(String::from_utf8(echo).expect("valid utf8"), expected);
+}
+
+#[test]
+fn set_termios_updates_kill_and_werase_control_chars() {
+    let manager = PtyManager::new();
+    let pty = manager.create_pty();
+
+    let termios = manager
+        .get_termios(pty.master.description.id())
+        .expect("read default termios");
+    assert_eq!(termios.cc.vkill, 0x15);
+    assert_eq!(termios.cc.vwerase, 0x17);
+
+    manager
+        .set_termios(
+            pty.master.description.id(),
+            PartialTermios {
+                cc: Some(PartialTermiosControlChars {
+                    vkill: Some(0x18),
+                    vwerase: Some(0x1a),
+                    ..PartialTermiosControlChars::default()
+                }),
+                ..PartialTermios::default()
+            },
+        )
+        .expect("merge termios update");
+
+    let termios = manager
+        .get_termios(pty.master.description.id())
+        .expect("read merged termios");
+    assert_eq!(termios.cc.vkill, 0x18);
+    assert_eq!(termios.cc.vwerase, 0x1a);
+    // Unspecified control chars keep their defaults.
+    assert_eq!(termios.cc.verase, 0x7f);
+}

crates/sidecar-core/src/guest_pty.rs

@@ -190,6 +190,8 @@ where
             "vsusp": termios.cc.vsusp,
             "veof": termios.cc.veof,
             "verase": termios.cc.verase,
+            "vkill": termios.cc.vkill,
+            "vwerase": termios.cc.vwerase,
         },
     }))
 }
@@ -221,6 +223,8 @@ fn parse_partial_termios(request: &Value) -> PartialTermios {
         vsusp: optional_u8(cc, "vsusp"),
         veof: optional_u8(cc, "veof"),
         verase: optional_u8(cc, "verase"),
+        vkill: optional_u8(cc, "vkill"),
+        vwerase: optional_u8(cc, "vwerase"),
     });
     PartialTermios {
         icrnl: optional_bool(request, "icrnl"),

crates/sidecar/src/execution.rs

@@ -17621,6 +17621,35 @@ fn service_javascript_kernel_tty_size_sync_rpc(
     }))
 }
 
+// TODO(double-print): The recovered pre-convergence work (94e935d0) added a
+// guard here because in THAT model the sidecar owned a per-process
+// `tty_master_fd`, drained the PTY master into `Stdout`/`ProcessOutput` events,
+// and therefore had to suppress a TTY child's own `process_output` event to
+// avoid surfacing the same bytes twice (once via the child's Stdout event, once
+// via the drained master).
+//
+// The current "real terminal client" model on main is different and does NOT
+// exhibit that bug as far as I can determine:
+//   * `tty_master_fd` here is only a LOCAL used to derive
+//     `kernel_stdin_writer_fd`; there is no `ActiveProcess::tty_master_fd`
+//     field and no `drain_tty_master_output` — the sidecar never surfaces the
+//     PTY master as host `ProcessOutput` at all. The in-guest terminal client
+//     reads the master itself.
+//   * A wasm/python CHILD of a cooked-TTY JS shell is spawned WITHOUT the
+//     parent's PTY slave dup2'd onto its fds (the child spawn path does no
+//     `open_pty`/slave dup2), so the child's stdout does not flow through the
+//     parent's master. It is surfaced exactly once, via the `Stdout` event
+//     queued below.
+// So there is no second surfacing path to suppress, and porting 94e935d0's
+// master-drain guard would guard against a drain that no longer exists.
+//
+// UNRESOLVED: I could not exercise the live JS-shell-spawns-wasm-child scenario
+// end-to-end to positively confirm the host sees exactly one copy; the analysis
+// above is from the code paths only. If a real double-print is later observed
+// for a TTY child, the fix belongs here (or at the `ProcessOutput` emission in
+// `event_frame_for_execution_event`): suppress this child `Stdout` event when
+// the same bytes already reach the host through a PTY master owned by an
+// ancestor. Deliberately NOT fabricating that guard now, per instructions.
 fn service_javascript_kernel_stdio_write_sync_rpc(
     kernel: &mut SidecarKernel,
     process: &mut ActiveProcess,