From e09fc7db1790958bb57b39d7c1c38637eadc0bc8 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sat, 5 Sep 2026 23:15:25 -0700 Subject: [PATCH] test(tui): name the command a dispatch smoke stalls on, and skip /pin on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two dispatch-everything smoke tests burned nextest's full 600 s timeout on windows-latest with no attribution (#5919). The stderr breadcrumbs from run 34013201624 / job 101432543520 name the offender: the registered-command test stopped at `/pin` and the alias test at `/mini` — the same handler, `commands::groups::core::pin`, neither of which printed a "returned in" line. `/pin` is not a state toggle. It calls `tui::window_control::toggle_pin()`, which resolves the *host terminal window* and drives it with Win32 `SetWindowPos`/`ShowWindow`. That HWND belongs to another process, so those calls are delivered to that window's thread and block until it pumps them. On a headless CI window station there is nothing to pump, and the call never returns. Skip `/pin` in the smoke tests on Windows only, with the reason in the doc comment; macOS and Linux compile `toggle_pin()` to a no-op, so dispatch coverage for the command is kept there. Fixing only the known offender leaves the next one just as opaque, so each dispatch now runs under a per-command watchdog: the app is built and the command executed on its own thread and the test waits 30 s for the result. A blocking handler fails the test in milliseconds naming the invocation instead of costing a ten-minute CI slot. Handler panics still surface as that panic (the payload is resumed), which is the property the old inline loop had and the reason the smoke tests exist. The unconditional per-command eprintln pair is gone — 15k log lines whose only job the watchdog now does — replaced by a single line for any dispatch slower than a second. Verified locally on macOS: cargo fmt --all -- --check clean cargo test -p codewhale-tui --lib -- commands::tests::every_ test result: ok. 3 passed; 0 failed; 0 ignored; 11704 filtered out cargo test -p codewhale-tui --lib -- commands:: test result: ok. 977 passed; 0 failed; 0 ignored; 10730 filtered out The watchdog itself was proved by temporarily setting DISPATCH_WATCHDOG to Duration::ZERO (failed in 0.05 s with "/anchor did not return within 0ns") and by temporarily panicking inside the worker (the panic propagated to the test with the handler's name on the thread). Both temporaries were reverted before the runs quoted above. The Windows hang itself is only provable on windows-latest CI. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0188XYyJaw9Mh9uSrqQBoqhm Signed-off-by: CodeWhale Bot --- crates/tui/src/commands/mod.rs | 96 ++++++++++++++++++++++++++-------- 1 file changed, 73 insertions(+), 23 deletions(-) diff --git a/crates/tui/src/commands/mod.rs b/crates/tui/src/commands/mod.rs index 10698d7f25..ef512b604f 100644 --- a/crates/tui/src/commands/mod.rs +++ b/crates/tui/src/commands/mod.rs @@ -1587,8 +1587,78 @@ mod tests { /// `scoped_home` (snapshot repo init shells out to git, which races /// against parallel-running tests). Skip it here so this smoke test /// stays parallel-safe. + /// + /// `/pin` is skipped on Windows only. Its handler is not a state toggle: + /// it drives the *host terminal window* through Win32 (see + /// `tui::window_control`), and the resolved `HWND` belongs to another + /// process. `SetWindowPos`/`ShowWindow` against a foreign window are + /// delivered to that window's thread and block until it pumps them, so on + /// a headless CI window station the call never returns — this is the + /// 600 s nextest timeout in #5919 (the CI breadcrumb stalled on `/pin` + /// and on its `/mini` alias, the same handler). On macOS and Linux + /// `toggle_pin()` is a compiled-out no-op, so dispatch coverage for + /// `/pin` is kept there. fn skip_in_dispatch_smoke(name: &str) -> bool { - name == "restore" + name == "restore" || (cfg!(windows) && name == "pin") + } + + /// Upper bound on a single command dispatch in the smoke tests. + /// + /// Generous next to the millisecond each handler actually takes, and far + /// below nextest's 600 s test timeout, so a handler that blocks fails the + /// test *by name* instead of burning a ten-minute CI slot with no + /// attribution (#5919). + const DISPATCH_WATCHDOG: std::time::Duration = std::time::Duration::from_secs(30); + + /// Dispatch one command under a per-command watchdog and return the + /// handler's message. + /// + /// The app is built and the command executed on a dedicated thread; the + /// test thread waits on the result with a timeout. A handler that never + /// returns leaves its thread parked, but the test itself fails + /// immediately, naming the invocation. A handler that panics still + /// surfaces as that panic — the smoke tests are the repo's only + /// panic-in-a-handler net, so the payload is resumed rather than + /// swallowed. + fn dispatch_under_watchdog(command_name: &str, alias_or_name: &str) -> Option { + let label = format!("/{alias_or_name}"); + let (tx, rx) = std::sync::mpsc::channel(); + let name = command_name.to_string(); + let alias = alias_or_name.to_string(); + let handle = std::thread::Builder::new() + .name(format!("dispatch-smoke-{alias_or_name}")) + // Command handlers are deeply recursive in debug builds; match the + // 16 MiB the CI runner sets via RUST_MIN_STACK for the main thread. + .stack_size(16 * 1024 * 1024) + .spawn(move || { + let (mut app, tmpdir, _guard) = create_isolated_test_app(); + let invocation = invocation_for(&name, &alias, tmpdir.path()); + let result = execute(&invocation, &mut app); + let _ = tx.send(result.message); + }) + .expect("spawn dispatch smoke thread"); + + let started = std::time::Instant::now(); + match rx.recv_timeout(DISPATCH_WATCHDOG) { + Ok(message) => { + let _ = handle.join(); + // Quiet on the common path; a handler heading for the + // watchdog still leaves a named breadcrumb in the log. + let elapsed = started.elapsed(); + if elapsed > std::time::Duration::from_secs(1) { + eprintln!("dispatch smoke: {label} took {elapsed:?}"); + } + message + } + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => panic!( + "{label} did not return within {DISPATCH_WATCHDOG:?}: its handler blocks. \ + Fix the handler or add it to skip_in_dispatch_smoke with a reason." + ), + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => match handle.join() { + Ok(()) => panic!("{label} dispatch thread ended without producing a result"), + Err(payload) => std::panic::resume_unwind(payload), + }, + } } #[test] @@ -1690,18 +1760,7 @@ mod tests { if skip_in_dispatch_smoke(command.name) { continue; } - let (mut app, tmpdir, _guard) = create_isolated_test_app(); - let invocation = invocation_for(command.name, command.name, tmpdir.path()); - // Breadcrumb for a terminated run: the last line names the handler - // that never returned. - eprintln!("dispatch smoke: {invocation}"); - let started = std::time::Instant::now(); - let result = execute(&invocation, &mut app); - eprintln!( - "dispatch smoke: {invocation} returned in {:?}", - started.elapsed() - ); - if let Some(msg) = &result.message { + if let Some(msg) = dispatch_under_watchdog(command.name, command.name) { assert!( !msg.contains("Unknown command"), "/{} fell through to the unknown-command branch: {msg}", @@ -1720,16 +1779,7 @@ mod tests { continue; } for alias in command.aliases { - let (mut app, tmpdir, _guard) = create_isolated_test_app(); - let invocation = invocation_for(command.name, alias, tmpdir.path()); - eprintln!("dispatch smoke: {invocation}"); - let started = std::time::Instant::now(); - let result = execute(&invocation, &mut app); - eprintln!( - "dispatch smoke: {invocation} returned in {:?}", - started.elapsed() - ); - if let Some(msg) = &result.message { + if let Some(msg) = dispatch_under_watchdog(command.name, alias) { assert!( !msg.contains("Unknown command"), "/{alias} (alias of /{}) fell through to unknown: {msg}",