* fix(tui): init runtime log before EnterAlternateScreen, add Windows stderr redirect Two root causes of verbose logging leaking into the TUI alt-screen: 1. EnterAlternateScreen was called BEFORE runtime_log::init(), so any logging between alt-screen entry and redirect init leaked raw bytes into the TUI buffer — causing the 'scroll demon' on all platforms. 2. redirect_stderr_to was #[cfg(unix)] only — Windows had no stderr redirect at all, so every eprintln!/tracing call during the TUI session wrote directly into the alt-screen buffer. Fixes: - Swap order: init runtime_log (with stderr redirect) BEFORE entering the alt-screen, so the redirect is active from the start. - Add #[cfg(windows)] redirect_stderr_to using SetStdHandle from the already-available windows crate, with corresponding Drop impl to restore the original handle. Fixes #1909 * fix(tui): handle Windows stderr redirect API types * docs(tui): update runtime log stderr redirect notes * fix: propagate SetStdHandle error instead of using .context() Use .map_err() with a descriptive message and cast target to isize for the HANDLE constructor, so the error is properly propagated if SetStdHandle fails. Co-Authored-By: bot_apk <apk@cognition.ai> * fix(tui): pass raw Windows stderr handle * fix: use DuplicateHandle for Windows stderr redirect to avoid handle aliasing The previous implementation used file.as_raw_handle() directly with SetStdHandle, causing both _file and the process stderr table entry to share the same HANDLE. If a third-party library called CloseHandle on the stderr handle during the TUI session, it would silently invalidate _file and cause a double-CloseHandle on guard drop. Now we call DuplicateHandle before SetStdHandle to produce a truly independent handle for the redirected stderr, mirroring the Unix path's use of libc::dup. The duplicated handle is tracked in the guard struct and properly closed on drop after the original stderr is restored. Co-Authored-By: bot_apk <apk@cognition.ai> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: bot_apk <apk@cognition.ai>
This commit is contained in:
@@ -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"] }
|
||||
|
||||
@@ -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<libc::c_int>,
|
||||
#[cfg(windows)]
|
||||
saved_stderr_handle: Option<windows::Win32::Foundation::HANDLE>,
|
||||
#[cfg(windows)]
|
||||
redirected_stderr_handle: Option<windows::Win32::Foundation::HANDLE>,
|
||||
_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<TuiLogGuard> {
|
||||
|
||||
#[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<libc::c_int> {
|
||||
}
|
||||
}
|
||||
|
||||
#[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::*;
|
||||
|
||||
+20
-21
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user