diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index 52bf5e0e..2aa6c7c9 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -94,4 +94,4 @@ objc2 = "0.6.3" objc2-foundation = { version = "0.3.2", default-features = false, features = ["std", "NSArray", "NSDictionary", "NSError", "NSObject", "NSString", "NSURL"] } [target.'cfg(target_os = "windows")'.dependencies] -windows = { version = "0.60", features = ["Win32_Foundation", "Win32_System_Console", "Win32_UI_WindowsAndMessaging", "Win32_System_Diagnostics_Debug"] } +windows = { version = "0.60", features = ["Win32_Foundation", "Win32_System_Console", "Win32_UI_WindowsAndMessaging", "Win32_System_Diagnostics_Debug", "Win32_System_Threading"] } diff --git a/crates/tui/src/runtime_log.rs b/crates/tui/src/runtime_log.rs index fd631f66..32ce6508 100644 --- a/crates/tui/src/runtime_log.rs +++ b/crates/tui/src/runtime_log.rs @@ -1,7 +1,7 @@ //! TUI runtime logging. Initializes a `tracing-subscriber` that writes to a //! per-process file under `~/.codewhale/logs/tui-YYYY-MM-DD-PID.log`, and (on -//! Unix) redirects the process's `stderr` fd to that same file for the lifetime -//! of the alt-screen TUI. +//! Unix and Windows) redirects the process's `stderr` handle/fd to that same +//! file for the lifetime of the alt-screen TUI. //! //! Why this exists: //! @@ -26,12 +26,12 @@ //! `tracing::error!` calls go somewhere observable instead of //! disappearing into the void (the TUI previously had no global //! subscriber, so contributors reached for `eprintln!`). -//! 2. On Unix the process's stderr fd is redirected (via `dup2`) to the -//! same log file for the lifetime of `TuiLogGuard`. Any raw stderr +//! 2. On Unix and Windows the process's stderr handle/fd is redirected to +//! the same log file for the lifetime of `TuiLogGuard`. Any raw stderr //! write — ours, a dependency's, a panic message — lands in the log //! file instead of the alt-screen. The guard restores the original -//! stderr fd on drop so post-TUI shutdown messages still reach the -//! user's terminal. +//! stderr handle/fd on drop so post-TUI shutdown messages still reach +//! the user's terminal. //! 3. Crate-level `#![deny(clippy::print_stderr, clippy::print_stdout)]` //! on the TUI runtime modules forbids new `eprintln!` / `println!` //! calls at compile time. CLI-output paths (`main.rs` eval, init, @@ -50,12 +50,16 @@ const DEFAULT_LOG_RETENTION_DAYS: u64 = 7; const LOG_RETENTION_ENV: &str = "DEEPSEEK_LOG_RETENTION_DAYS"; const SECONDS_PER_DAY: u64 = 24 * 60 * 60; -/// Owns the active tracing subscriber and (on Unix) a saved copy of the -/// original `stderr` fd so it can be restored on drop. Dropped when the TUI -/// exits the alt-screen. +/// Owns the active tracing subscriber and (on Unix/Windows) a saved copy of +/// the original `stderr` handle/fd so it can be restored on drop. Dropped when +/// the TUI exits the alt-screen. pub struct TuiLogGuard { #[cfg(unix)] saved_stderr_fd: Option, + #[cfg(windows)] + saved_stderr_handle: Option, + #[cfg(windows)] + redirected_stderr_handle: Option, _file: File, // Exposed via `log_path()` for diagnostics (e.g. `/doctor`, // `--print-log-path`). Currently no caller — keep the accessor @@ -90,7 +94,29 @@ impl Drop for TuiLogGuard { } } -#[cfg(not(unix))] +#[cfg(windows)] +impl Drop for TuiLogGuard { + fn drop(&mut self) { + if let Some(handle) = self.saved_stderr_handle.take() { + unsafe { + let _ = windows::Win32::System::Console::SetStdHandle( + windows::Win32::System::Console::STD_ERROR_HANDLE, + handle, + ); + } + } + // Close the duplicated handle that was serving as the redirected + // stderr target. This is safe because `SetStdHandle` above already + // restored the original handle, so nothing references this one. + if let Some(dup) = self.redirected_stderr_handle.take() { + unsafe { + let _ = windows::Win32::Foundation::CloseHandle(dup); + } + } + } +} + +#[cfg(not(any(unix, windows)))] impl Drop for TuiLogGuard { fn drop(&mut self) {} } @@ -147,10 +173,19 @@ pub fn init() -> Result { #[cfg(unix)] let saved_stderr_fd = redirect_stderr_to(&file).ok(); + #[cfg(windows)] + let (saved_stderr_handle, redirected_stderr_handle) = match redirect_stderr_to(&file) { + Ok((saved, dup)) => (Some(saved), Some(dup)), + Err(_) => (None, None), + }; Ok(TuiLogGuard { #[cfg(unix)] saved_stderr_fd, + #[cfg(windows)] + saved_stderr_handle, + #[cfg(windows)] + redirected_stderr_handle, _file: file, log_path, }) @@ -250,6 +285,59 @@ fn redirect_stderr_to(file: &File) -> Result { } } +#[cfg(windows)] +fn redirect_stderr_to( + file: &File, +) -> Result<( + windows::Win32::Foundation::HANDLE, + windows::Win32::Foundation::HANDLE, +)> { + use std::os::windows::io::AsRawHandle; + use windows::Win32::Foundation::{CloseHandle, DUPLICATE_SAME_ACCESS, DuplicateHandle, HANDLE}; + use windows::Win32::System::Console::{GetStdHandle, STD_ERROR_HANDLE, SetStdHandle}; + use windows::Win32::System::Threading::GetCurrentProcess; + + // SAFETY: GetStdHandle is always available; returns INVALID_HANDLE_VALUE + // on failure or null-like handles for console-less processes. + let saved = + unsafe { GetStdHandle(STD_ERROR_HANDLE) }.context("GetStdHandle(STD_ERROR_HANDLE)")?; + if saved.is_invalid() { + return Err(anyhow::anyhow!("GetStdHandle(STD_ERROR_HANDLE) failed")); + } + + // Duplicate the file handle so the redirected stderr owns an + // independent HANDLE — mirroring the Unix path's `libc::dup`. + // Without this, `_file` and stderr would alias the same HANDLE; + // a rogue `CloseHandle` on stderr would silently invalidate `_file`. + let raw = HANDLE(file.as_raw_handle()); + let process = unsafe { GetCurrentProcess() }; + let mut dup = HANDLE::default(); + unsafe { + DuplicateHandle( + process, + raw, + process, + &mut dup, + 0, + false, + DUPLICATE_SAME_ACCESS, + ) + .context("DuplicateHandle for stderr redirect")?; + } + + // SAFETY: SetStdHandle redirects stderr to the duplicated handle. + // We save the original handle so the guard can restore it on drop. + unsafe { + if let Err(e) = SetStdHandle(STD_ERROR_HANDLE, dup) { + let _ = CloseHandle(dup); + return Err(anyhow::anyhow!( + "SetStdHandle(STD_ERROR_HANDLE) failed: {e}" + )); + } + } + Ok((saved, dup)) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 6021e32c..7b4f77e5 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -313,27 +313,16 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> { enable_windows_ime_console_mode(); let mut stdout = io::stdout(); - 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::snapshot_verbose_state(); - #[cfg(windows)] - crate::logging::set_verbose(false); - } - // 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 - // otherwise leak into the alt-screen buffer and shift ratatui's - // diff-renderer view (the "scroll demon" reported in #1085) now lands - // in `~/.deepseek/logs/tui-YYYY-MM-DD.log` instead. The guard is held - // until the function returns; dropping it (after `LeaveAlternateScreen` - // below) restores the original stderr fd so shutdown messages reach - // the user's terminal. We accept the init failing (e.g., read-only - // `$HOME`) and continue without the redirect rather than refusing to - // start the TUI. + // Initialize the file-backed TUI log and redirect raw stderr away from + // the alt-screen for the lifetime of this guard. MUST run BEFORE + // EnterAlternateScreen; otherwise logging between alt-screen entry and + // redirect init leaks raw bytes into the TUI buffer, causing the "scroll + // demon" on Windows (#1909) and garbled output on all platforms (#1085). + // The guard is held until the function returns; dropping it after + // LeaveAlternateScreen restores the original stderr handle/fd so shutdown + // messages reach the user's terminal. We accept the init failing (e.g., + // read-only $HOME) and continue without the redirect rather than refusing + // to start the TUI. let _tui_log_guard = match crate::runtime_log::init() { Ok(guard) => Some(guard), Err(err) => { @@ -341,6 +330,16 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> { None } }; + if use_alt_screen { + execute!(stdout, EnterAlternateScreen)?; + // Windows also suppresses CodeWhale's own verbose CLI logger while + // the alt-screen is active. The stderr redirect above catches raw + // writes; this prevents the known verbose source at the origin. + #[cfg(windows)] + crate::logging::snapshot_verbose_state(); + #[cfg(windows)] + crate::logging::set_verbose(false); + } // Mouse capture, bracketed paste, focus events, and the Kitty // keyboard-protocol escape-disambiguation flag (#442). Single source // of truth shared with the FocusGained recovery path and