Skip to content

refactor(cli): replace hand-rolled terminal code with crossterm - #127

Draft
thebrandonlucas wants to merge 1 commit into
masterfrom
brandon/dev-2904-voltage-cli-replace-hand-rolled-terminal-code-with-crossterm
Draft

thebrandonlucas wants to merge 1 commit into
masterfrom
brandon/dev-2904-voltage-cli-replace-hand-rolled-terminal-code-with-crossterm

Conversation

@thebrandonlucas

@thebrandonlucas thebrandonlucas commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Fixes DEV-2904

Problem

terminal.rs kept an approved unsafe exception: TerminalModes called libc::tcgetattr and libc::tcsetattr to put echo back when Ctrl-C cancelled the hidden API-key prompt, because rpassword restores echo only when its read returns. The restore was Unix-only. The picker, live-watch, and status-line guards also wrote escape sequences as literal bytes, although crossterm, which the CLI already uses for the width and the pickers, has a command for each.

Change

Before After
rpassword reads the secret from /dev/tty with echo and signals off crossterm raw mode reads key events on the detached thread; Enter (or a pasted newline) ends the read, Backspace and Ctrl-U edit it
TerminalModes saves and restores termios with unsafe libc calls (Unix only) A RawMode guard calls crossterm::terminal::disable_raw_mode, which restores the modes crossterm saved when raw mode went on. It covers the hidden prompt and inquire's pickers on every platform
A typed Ctrl-C: rpassword raises SIGINT, the prompt waits for the handler A typed Ctrl-C returns Interrupted before any resource change was submitted (exit 130), the same as the pickers
\x1b[?2004l\x1b[?25h, \x1b[?1049h…, \x1b[H\x1b[2J, \r\x1b[2K as literals DisableBracketedPaste, cursor::Show/Hide, Enter/LeaveAlternateScreen, MoveTo, Clear, rendered through a terminal::ansi helper
  • Raw mode is turned on in the async function before the read thread starts, so the guard is in place before the terminal changes. Dropping the guard when raw mode is already off does nothing.
  • ansi() uses Command::write_ansi, not execute!. On Windows, execute! can call the console API directly, which would reorder the sequence with the buffered bytes around it. The bytes written are unchanged, except that home is ESC[1;1H instead of ESC[H.
  • The approved exception is removed from docs/quality.md. rpassword is removed (−22 lines in Cargo.lock). The crossterm entry now lists events and bracketed-paste; inquire already turned both on, so the dependency graph does not change.
  • unsafe and libc in src/ are now limited to check_owner and O_NOFOLLOW in config.rs. The integration tests still use libc to open and inspect pseudo-terminals.

User-visible, so there is a CHANGELOG line: the hidden prompt is written to stderr instead of /dev/tty, Ctrl-U clears it, and the restore works on every platform.

Try it on staging

Setup: Running against staging. This saves an API key under a throwaway account name and then logs it out.

alias voltage="$PWD/scripts/voltage-staging"
voltage --build
voltage login --account me --credential-store file
voltage context --account me

Use the organization and environment from context:

voltage auth import-key --account try-key --org ORG_ID --env ENV_ID --credential-store file

At Environment API key:, type a few characters and press Ctrl-C. Expect: nothing you typed is shown, Interrupted before any resource change was submitted appears, echo $? prints 130, and typing in the shell echoes again (no stty sane needed).

Run the command again and type wrong, press Ctrl-U, type a real or dummy key, use Backspace once, and press Enter. Expect: the key is not shown, and the command reports imported. Then remove it:

voltage logout --account try-key

To check the picker guard, run voltage wallets list --account me without --org. If your login sees more than one organization, a picker opens. Press Ctrl-C there, and check that the shell still echoes and shows its cursor.

Validation

  • cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, cargo machete --with-metadata (in nix develop), cargo test --locked (200 unit, 150 integration), and python3 scripts/check-coverage.py all pass.
  • New unit tests: type_hidden key handling (typing, Shift, Backspace, Ctrl-U, ignored Alt/Ctrl chords, arrows, and key releases; Enter, Ctrl-J, and Ctrl-C), the read-result mapping (secret, interruption, and the transport error for a terminal that cannot enter raw mode or be read), and the ansi byte output.
  • New integration test the_hidden_key_prompt_saves_the_edited_key_without_showing_it: a pseudo-terminal types a key with Ctrl-U and Backspace edits. It checks the saved key, that the terminal output never contains it, and that the terminal's local modes match their values before the run.
  • The existing pseudo-terminal tests still pass unchanged: SIGINT and typed Ctrl-C at the hidden prompt restore echo and exit 130, and the picker restores modes and bracketed paste. With the guard's disable_raw_mode call removed, all three hidden-prompt tests fail, so the guard is what restores the terminal.
  • CRAP (scripts/crap-report.sh, cargo-llvm-cov 0.8.5, knots 1.16.0): 849 of 1060 functions measured, 0 violations. The highest score is 18.10, wait_for_payment in src/api.rs, which this PR does not change. The new functions score read_hidden_keys 5.0, type_hidden 4.0, read_hidden 3.01, and the rest 1–2.

Limits

  • style.rs still writes the OSC 8 hyperlink sequence by hand, because neither crossterm 0.29 nor anstyle has a hyperlink command. The wordmark.rs escapes are test assertions only.
  • Checks not run: the Windows and Linux restore paths were not tested by hand. CI builds and tests them, but there is no pseudo-terminal test on Windows.

The hidden API-key prompt reads keys in crossterm's raw mode instead of
rpassword, and a guard turns raw mode off through crossterm, which restores
the saved terminal modes when Ctrl-C cancels a read that cannot return. This
removes the unsafe tcgetattr/tcsetattr exception, the rpassword dependency,
and the Unix-only restore: pickers and the hidden prompt now restore the
terminal on every platform. A typed Ctrl-C at the hidden prompt exits 130
with the same interruption message as before.

The picker, live-watch, and status-line escape sequences come from
crossterm commands instead of literal bytes; the bytes written are the
same, except that home is now ESC[1;1H.
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.

1 participant