fix(tui): suppress verbose CLI logging on Windows alt-screen

[aboimpinto]

This PR is the cleaned replacement for the old #1910 branch.

It carries the same Windows alt-screen verbose-leak fix, but rebased onto current main so the review starts from a clean history instead of the stale DeepSeek-era workspace snapshot.

What changed:

• suppress verbose CLI logging on Windows while the TUI alt-screen is active
• restore the previous verbose state when leaving alt-screen or during cleanup

Validation:

• cargo fmt --all
• cargo check -p codewhale-tui --all-features --locked

Reference:

• Closed PR #1910, especially the maintainer note explaining that the broader Windows verbose-leak work was being split into defense-in-depth layers (#2259 and #2262). This PR keeps only the alt-screen suppression fix and replays it cleanly on current main.

The old PR branch was superseded by this clean cherry-pick on top of main.

Paulo Aboim Pinto

Greptile Summary

This PR suppresses verbose CLI logging on Windows while the TUI alt-screen is active, replacing the approach from #1910 with a snapshot/restore pattern that correctly preserves the pre-suppression verbose state (including --verbose CLI flags) across alt-screen entry and exit.

• logging.rs gains snapshot_verbose_state / restore_verbose_state backed by a VERBOSE_SNAPSHOT atomic; ui.rs calls these at every alt-screen enter/leave site (run_tui, pause_terminal, resume_terminal), all correctly guarded inside if use_alt_screen.
• Adding #[cfg(windows)] to the whole mod tests block silently drops the cross-platform log_value_parser_accepts_common_rust_log_directives test from Linux/macOS CI runs; only the new snapshot/restore tests need the Windows gate.

Confidence Score: 5/5

Safe to merge; the core suppress/restore logic is correct and properly scoped to alt-screen sessions.

The suppress/restore pairs are consistently placed inside if use_alt_screen guards across all changed call sites, and the snapshot correctly captures the full verbose state including CLI flags rather than re-reading the environment. The only issue is a test-module attribute that drops cross-platform coverage for an existing test, which does not affect runtime behavior.

crates/tui/src/logging.rs — the #[cfg(windows)] gate on the test module needs to be narrowed so the cross-platform parser test keeps running on Linux/macOS.

Important Files Changed

Filename	Overview
crates/tui/src/logging.rs	Adds Windows-only VERBOSE_SNAPSHOT atomic, snapshot_verbose_state, and restore_verbose_state helpers. The new snapshot/restore tests are correct, but gating the entire mod tests block with #[cfg(windows)] accidentally removes cross-platform coverage for the pre-existing log_value_parser_accepts_common_rust_log_directives test.
crates/tui/src/tui/ui.rs	Integrates snapshot/restore calls at all alt-screen transitions: run_tui entry, normal exit, pause_terminal, and resume_terminal. The suppress/restore pairs are correctly placed inside if use_alt_screen guards.

Sequence Diagram

sequenceDiagram
    participant main
    participant run_tui
    participant logging
    participant pause_terminal
    participant resume_terminal

    main->>run_tui: "run_tui(use_alt_screen=true)"
    run_tui->>logging: snapshot_verbose_state() [Windows]
    run_tui->>logging: set_verbose(false) [Windows]
    Note over run_tui: TUI event loop running...
    run_tui->>pause_terminal: "pause_terminal(use_alt_screen=true)"
    pause_terminal->>logging: restore_verbose_state() [Windows]
    Note over pause_terminal: LeaveAlternateScreen
    pause_terminal-->>run_tui: Ok
    Note over run_tui: child process running...
    run_tui->>resume_terminal: "resume_terminal(use_alt_screen=true)"
    Note over resume_terminal: EnterAlternateScreen
    resume_terminal->>logging: set_verbose(false) [Windows]
    resume_terminal-->>run_tui: Ok
    Note over run_tui: TUI event loop running...
    run_tui->>logging: restore_verbose_state() [Windows, normal exit]
    run_tui-->>main: Ok

Loading

Comments Outside Diff (2)

1. crates/tui/src/tui/ui.rs, line 626-628 (link)

   TerminalCleanupGuard::drop skips restore_verbose_state on Windows

   The drop handler is the safety net for every early-return path between guard creation (line ~338) and defused = true (line ~558). On Windows with use_alt_screen = true, it leaves the alt-screen correctly but never calls crate::logging::restore_verbose_state(). Any ?-propagated error that escapes run_tui before the normal cleanup will leave VERBOSE permanently cleared, silencing all subsequent crate::logging::info/warn calls in the same process — including any post-TUI diagnostics or retry logic in the caller.

   

2. crates/tui/src/tui/ui.rs, line 616-637 (link)

   TerminalCleanupGuard::drop is the safety net for all early-return paths between guard creation and cleanup_guard.defused = true. On Windows with use_alt_screen = true, the drop correctly issues LeaveAlternateScreen, but never calls crate::logging::restore_verbose_state(). Any ?-propagated error that escapes run_tui before line 558 will leave VERBOSE permanently cleared — silencing all subsequent crate::logging::info/warn calls in the same process, including any post-TUI diagnostics in the caller.

   The fix is to add the same #[cfg(windows)] restore call that the normal-exit path already has, inside the if self.use_alt_screen { … } branch of Drop::drop.

   

_{Reviews (5): Last reviewed commit: "test(tui): cover windows verbose state r..." | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request introduces changes to suppress verbose CLI logging on Windows when the alternate screen is active, preventing stderr leaks into the TUI buffer, and restoring the logging state when leaving the alternate screen. The reviewer identified two critical issues where querying the environment via env_requests_verbose_logging() in emergency_restore_terminal() and TerminalCleanupGuard::drop introduces deadlock risks during panics or signal handling due to memory allocation and lock acquisition. It is recommended to remove these calls from the emergency and cleanup paths.

[aboimpinto]

Addressed the review feedback in the associated threads and pushed the fixes.

[aboimpinto]

Removed the cleanup-path restore from the review comments and kept the final exit behavior aligned with the panic-safe path.