Skip to content

fix(wasix): preserve CR and LF in noncanonical tty mode - #6863

Open
theduke wants to merge 2 commits into
mainfrom
fix/wasix-tty-enter
Open

fix(wasix): preserve CR and LF in noncanonical tty mode#6863
theduke wants to merge 2 commits into
mainfrom
fix/wasix-tty-enter

Conversation

@theduke

@theduke theduke commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What changed

When a WASIX guest disables canonical input, the host terminal now disables all three CR/LF input translations: ICRNL, INLCR, and IGNCR. Re-enabling canonical input restores the conventional cooked mapping (ICRNL on, INLCR and IGNCR off). Terminal reset uses the same cooked configuration.

Why

Interactive programs need to distinguish Enter (CR, byte 0x0d) from line feed (LF, byte 0x0a). WASIX previously disabled ICANON but left ICRNL enabled on the host terminal. The terminal driver therefore converted Enter to LF before the guest could read it.

This showed up in the Pi coding agent running through EdgeJS: pressing Enter inserted a newline instead of submitting the prompt. Pi was acting on the byte it received; the conversion happened in the WASIX host TTY bridge. Running with stty -icrnl confirmed the source of the problem.

This change is intentionally limited to newline translation. Noncanonical mode is not necessarily full raw mode, and the current WASIX TTY ABI does not carry enough termios state to infer or reproduce cfmakeraw(). Flags such as ISIG, IXON, and OPOST are therefore left unchanged.

Tests

The flag-level tests cover both mode transitions and verify that unrelated signal, flow-control, echo, and output settings are preserved. A real pseudoterminal regression test sends CR and LF through the kernel terminal driver, verifies that they remain distinct in noncanonical mode, and then verifies that cooked mode maps CR to LF again. PTY reads use a timeout so a regression fails instead of hanging.

  • nix develop -c cargo test -p wasmer-wasix --features sys --lib — 226 passed, 2 ignored
  • nix develop -c cargo fmt --all -- --check
  • git diff --check
  • verified interactively that Enter submits a Pi prompt under Wasmer

Copilot AI lite review requested due to automatic review settings August 6, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes WASIX TTY raw-mode behavior on Unix by preventing host-side carriage-return translation when switching to non-canonical (raw-ish) input, and restoring canonical CR→LF handling when returning to cooked mode. This addresses interactive apps that need to distinguish Enter (\r) from other newline-like inputs (e.g. LF).

Changes:

  • Centralizes canonical/raw toggling via set_line_buffering, including input-flag handling (ICRNL/IGNCR).
  • Restores canonical CR-to-LF translation when re-enabling line buffering or resetting the terminal.
  • Adds flag-level tests and a PTY regression test to validate CR/LF behavior.
Suppressed comments (2)

lib/wasix/src/os/tty/tty_sys.rs:255

  • The unit test for raw input only asserts that ICRNL/IGNCR are cleared. If INLCR is set, LF can still be translated to CR, so it would be good for the test to cover that flag as well (especially since the comment mentions Shift+Enter behavior).
            state.c_iflag = ICRNL | IGNCR | IXON;

lib/wasix/src/os/tty/tty_sys.rs:268

  • Similarly, the cooked-mode flag test could include INLCR to ensure it’s cleared when returning to canonical mode (so LF isn’t unexpectedly mapped to CR).
            state.c_iflag = IGNCR | IXON;

Comment thread lib/wasix/src/os/tty/tty_sys.rs
Comment thread lib/wasix/src/os/tty/tty_sys.rs
@theduke
theduke marked this pull request as draft August 6, 2026 20:30
@theduke theduke changed the title fix(wasix): preserve carriage returns in raw tty mode fix(wasix): preserve CR and LF in noncanonical tty mode Aug 6, 2026
@marxin
marxin requested a review from Arshia001 August 7, 2026 06:44
@theduke
theduke marked this pull request as ready for review August 7, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants