Skip to content

fix(tui): suppress verbose CLI logging on Windows alt-screen to prevent TUI leak #1905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aboimpinto wants to merge 1 commit into from
Closed

fix(tui): suppress verbose CLI logging on Windows alt-screen to prevent TUI leak #1905

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion crates/tui/src/tui/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> {
if use_alt_screen {
execute!(stdout, EnterAlternateScreen)?;
}
// On Windows, stderr cannot be redirected to the log file (no dup2).
// Suppress verbose CLI logging once the alt-screen is active so
// eprintln! calls from crate::logging don't leak into the TUI buffer.
#[cfg(windows)]
crate::logging::set_verbose(false);
Comment on lines +282 to +283
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 21, 2026

Choose a reason for hiding this comment

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

medium

This logic is duplicated in resume_terminal (line 6428). To improve maintainability and follow the project's established patterns, consider moving this Windows-specific verbosity suppression into recover_terminal_modes (line 6562). That function is already documented as the canonical location for terminal-mode setup and is called by both run_tui and resume_terminal.

// Initialize the file-backed TUI log and (on Unix) redirect raw stderr
// away from the alt-screen for the lifetime of this guard. Any
// `eprintln!`, panic message, or third-party stderr write that would
Expand Down Expand Up @@ -6417,6 +6422,10 @@ fn resume_terminal(
if use_alt_screen {
execute!(terminal.backend_mut(), EnterAlternateScreen)?;
}
// Re-entering alt-screen after mode recovery — suppress verbose
// CLI logging again so eprintln! doesn't leak into the TUI.
#[cfg(windows)]
crate::logging::set_verbose(false);
Comment on lines +6425 to +6428
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 21, 2026

Choose a reason for hiding this comment

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

medium

This call to set_verbose(false) is redundant because verbosity is already disabled at the start of run_tui (line 283) and is never re-enabled within the TUI lifecycle (including in pause_terminal). If the intention is to allow verbose logging while the TUI is paused (e.g., during a shell command), then pause_terminal should restore the state. If not, this call can be safely removed or refactored into recover_terminal_modes as suggested in the previous comment.

recover_terminal_modes(
terminal.backend_mut(),
use_mouse_capture,
Expand Down Expand Up @@ -7580,4 +7589,4 @@ fn extract_reasoning_header(text: &str) -> Option<String> {
}

#[cfg(test)]
mod tests;
mod tests;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 21, 2026

Choose a reason for hiding this comment

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

medium

The trailing newline was removed from the end of the file. It is a standard convention in Rust (and enforced by rustfmt) to end files with a newline to ensure compatibility with various Unix tools and to avoid unnecessary diff noise in future changes.

mod tests;

Loading