Skip to content

fix(pty): drain late terminal query reply on teardown#1258

Open
sweb wants to merge 2 commits into
nolabs-ai:mainfrom
sweb:fix/cpr-leak-on-tui-exit
Open

fix(pty): drain late terminal query reply on teardown#1258
sweb wants to merge 2 commits into
nolabs-ai:mainfrom
sweb:fix/cpr-leak-on-tui-exit

Conversation

@sweb

@sweb sweb commented Jun 25, 2026

Copy link
Copy Markdown

Linked Issue

Closes #1257

Summary

Quitting an interactive TUI (e.g. Claude Code) under nono run left stray characters such as 3;1R in the next shell prompt.

3;1R is the tail of a terminal Cursor Position Report (ESC [ 3 ; 1 R), the reply to a ESC[6n query the TUI emits during shutdown. The proxy forwards the query to the terminal, but the reply round-trips back after the child has exited, so nothing on the PTY consumes it. release_terminal_for_prompt restores cooked mode with tcsetattr(..., TCSANOW, ...), which does not flush the input queue, so the stray reply is handed to the shell.

A bare tcflush does not help: the reply has usually not arrived at that instant. The fix waits for it, then flushes.

Agent Disclosure (if applicable)

This change was prepared with the assistance of an AI coding agent (Claude Code), reviewed, altered and applied by the human contributor.

Test Plan

  • make fmt-check
  • make clippy
  • make test-cli
  • Manual: nono run -- claude, /q, confirmed no 3;1R in the shell prompt.

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates
  • Release note has been added to CHANGELOG.md if needed

@github-actions github-actions Bot added bug Something isn't working nono-cli size/medium labels Jun 25, 2026
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +175
Lines removed -2
Total changed 177
Classification Medium (50–300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a mechanism to discard late terminal query replies (such as cursor-position reports) during final teardown to prevent them from leaking into the shell prompt. This is achieved by waiting briefly via poll and flushing the input queue using tcflush. Feedback is provided regarding a potential race condition where tcflush might be called before the entire multi-byte escape sequence has fully arrived, suggesting a brief sleep and a check on pfd.revents before flushing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/nono-cli/src/pty_proxy.rs Outdated
Comment on lines +1841 to +1845
if unsafe { libc::poll(&mut pfd, 1, timeout_ms) } > 0 {
// SAFETY: `fd` is the live terminal fd for the duration of the call.
let tty = unsafe { std::os::fd::BorrowedFd::borrow_raw(fd) };
let _ = nix::sys::termios::tcflush(tty, nix::sys::termios::FlushArg::TCIFLUSH);
}

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.

medium

When poll returns > 0, it indicates that at least one byte of the terminal query reply (such as the initial ESC character) has arrived in the input queue. However, because the terminal query reply is a multi-byte sequence (e.g., ESC[3;1R), calling tcflush immediately might only discard the first few bytes if the remaining bytes are still in transit or have not yet been processed by the OS tty layer. This race condition can result in the tail of the sequence (like 3;1R) still being left in the input queue and handed to the shell.

To prevent this, we should:

  1. Verify that POLLIN is actually set in pfd.revents to ensure we only proceed when there is readable data.
  2. Introduce a very brief sleep (e.g., 10ms) after poll succeeds to allow the entire multi-byte sequence to be fully received before flushing.
    if unsafe { libc::poll(&mut pfd, 1, timeout_ms) } > 0 && (pfd.revents & libc::POLLIN) != 0 {
        // Sleep briefly to ensure the entire multi-byte escape sequence has arrived in the input queue.
        std::thread::sleep(Duration::from_millis(10));
        // SAFETY: fd is the live terminal fd for the duration of the call.
        let tty = unsafe { std::os::fd::BorrowedFd::borrow_raw(fd) };
        let _ = nix::sys::termios::tcflush(tty, nix::sys::termios::FlushArg::TCIFLUSH);
    }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

addressed in 6bff299

sweb added 2 commits June 25, 2026 20:23
Signed-off-by: Florian Müller <florian@tomueller.de>
…view

Signed-off-by: Florian Müller <florian@tomueller.de>
@sweb sweb force-pushed the fix/cpr-leak-on-tui-exit branch from 41fe77c to 92d7874 Compare June 25, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nono-cli size/medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stray cursor-position report (3;1R) pasted into shell after exiting a TUI under nono run

1 participant