diff --git a/src/delivery.rs b/src/delivery.rs index 383d0dc3..90bf5af5 100644 --- a/src/delivery.rs +++ b/src/delivery.rs @@ -860,9 +860,11 @@ pub struct ScreenState { /// Latched Cursor/Codex approval signal. Their TUI redraws can briefly erase /// both the dialog and title, which would flicker `approval` false while the /// prompt is still up. Latch true on any positive detection and only clear - /// once output has settled. Antigravity keeps its immediate scrape. - /// See `APPROVAL_SCRAPE_CLEAR_MS`. + /// once the scrape has stayed negative. Antigravity keeps its immediate + /// scrape. See `APPROVAL_SCRAPE_CLEAR_MS`. pub approval_scrape_latched: bool, + /// When the approval scrape last turned negative (`None` while positive). + pub approval_scrape_negative_since: Option, /// A Claude TUI overlay is focused whose input box is NOT the current /// session's root prompt — the subagent navigator (a human may be typing /// into a subagent's box) or the `←` session switcher (input box is a @@ -890,6 +892,7 @@ impl Default for ScreenState { cols: 80, last_prompt_submit: None, approval_scrape_latched: false, + approval_scrape_negative_since: None, nav_overlay: false, startup_loading: false, } @@ -911,22 +914,33 @@ pub(crate) fn pre_enter_settle(tool: &str) -> Option { (tool == "qoder").then(|| Duration::from_millis(250)) } -/// How long screen output must be quiet before a negative approval scrape is -/// trusted to clear the latched signal. Redraw bursts (cursor's approval prompt -/// animating its selection / spinner) emit partial frames that scrape as "no -/// approval"; requiring a settled screen before clearing keeps the latch up -/// through the burst so the gate reports `approval`, not `prompt_has_text`. +/// How long a negative approval scrape must hold before it clears the latched +/// signal. Redraw bursts (cursor's approval prompt animating its selection / +/// spinner, Codex on focus changes) emit partial frames that scrape as "no +/// approval"; the next full frame shows the prompt again and restarts the wait, +/// so the gate keeps reporting `approval`, not `prompt_has_text`. +/// +/// The wait is measured on the scrape, not on quiet output: a TUI that never +/// stops redrawing (Codex's /voice meters, an animated status line) would +/// otherwise hold an answered approval forever. pub(crate) const APPROVAL_SCRAPE_CLEAR_MS: u64 = 400; -/// Latch decision for screen-scraped approval (cursor). A partial-render frame -/// mid-redraw scrapes as "no approval"; holding the previous latch through such -/// transient false reads keeps the gate reporting `approval` instead of falling -/// through to `prompt_has_text`. The latch only clears once `output_settled` -/// (no redraw churn) confirms the prompt has genuinely left the screen. -pub(crate) fn latch_scraped_approval(prev: bool, scraped: bool, output_settled: bool) -> bool { +/// Whether a negative approval scrape has held for `APPROVAL_SCRAPE_CLEAR_MS`. +pub(crate) fn approval_scrape_cleared(negative_since: Option, now: Instant) -> bool { + negative_since.is_some_and(|t| { + now.saturating_duration_since(t) >= Duration::from_millis(APPROVAL_SCRAPE_CLEAR_MS) + }) +} + +/// Latch decision for screen-scraped approval (Cursor, Codex). A partial-render +/// frame mid-redraw scrapes as "no approval"; holding the previous latch through +/// such transient false reads keeps the gate reporting `approval` instead of +/// falling through to `prompt_has_text`. The latch only clears once `settled` +/// confirms the prompt has genuinely left the screen. +pub(crate) fn latch_scraped_approval(prev: bool, scraped: bool, settled: bool) -> bool { if scraped { true - } else if output_settled { + } else if settled { false } else { prev @@ -2786,6 +2800,7 @@ mod tests { cols: 80, last_prompt_submit: None, approval_scrape_latched: false, + approval_scrape_negative_since: None, nav_overlay: false, startup_loading: false, } diff --git a/src/pty/mod.rs b/src/pty/mod.rs index fb223c4c..e4385285 100644 --- a/src/pty/mod.rs +++ b/src/pty/mod.rs @@ -1007,6 +1007,16 @@ impl Proxy { if child_exited { poll_timeout = poll_timeout.min(100); } + // A latched approval clears on a timer (`APPROVAL_SCRAPE_CLEAR_MS`), + // and once the dialog is gone the tool may go quiet. Wake to + // re-evaluate instead of holding delivery until the next 10s timeout. + if self + .delivery_state + .read() + .is_ok_and(|s| s.approval_scrape_latched) + { + poll_timeout = poll_timeout.min(crate::delivery::APPROVAL_SCRAPE_CLEAR_MS as u16); + } match poll(&mut poll_fds, PollTimeout::from(poll_timeout)) { Ok(0) => { if !ready_signaled { diff --git a/src/pty/shared.rs b/src/pty/shared.rs index 8fcb503c..1a367837 100644 --- a/src/pty/shared.rs +++ b/src/pty/shared.rs @@ -19,8 +19,8 @@ use anyhow::Result; use crate::config::Config; use crate::db::HcomDb; use crate::delivery::{ - APPROVAL_SCRAPE_CLEAR_MS, DeliveryState, ScreenState, ToolConfig, latch_scraped_approval, - run_delivery_loop, + APPROVAL_SCRAPE_CLEAR_MS, DeliveryState, ScreenState, ToolConfig, approval_scrape_cleared, + latch_scraped_approval, run_delivery_loop, }; use crate::log::{log_error, log_info, log_warn}; use crate::notify::NotifyServer; @@ -182,17 +182,25 @@ pub(super) fn update_delivery_state( state.ready = screen.is_ready(); // Cursor and Codex can briefly erase their approval surfaces during // redraws (Codex does this on focus changes). Latch positive detection - // until output settles so a partial frame cannot clear blocked status. + // until the scrape stays negative (or output goes quiet on a negative + // frame) so a partial frame cannot clear blocked status. let scrape_latched_tool = matches!(target.known_tool(), Some(Tool::Codex | Tool::Cursor)); let scraped_approval = match target.known_tool() { Some(Tool::Codex) => screen.is_waiting_approval() || screen.is_codex_approval_visible(), Some(Tool::Cursor) => screen.is_cursor_approval_visible(), _ => false, }; + let now = Instant::now(); + state.approval_scrape_negative_since = if scraped_approval { + None + } else { + Some(state.approval_scrape_negative_since.unwrap_or(now)) + }; state.approval_scrape_latched = latch_scraped_approval( state.approval_scrape_latched, scraped_approval, - screen.is_output_stable(APPROVAL_SCRAPE_CLEAR_MS), + screen.is_output_stable(APPROVAL_SCRAPE_CLEAR_MS) + || approval_scrape_cleared(state.approval_scrape_negative_since, now), ); let approval = (scrape_latched_tool && state.approval_scrape_latched) || (target.name() == "antigravity" && screen.is_antigravity_approval_visible()); @@ -1934,6 +1942,64 @@ No session found matching 'never-saved'" assert_eq!(calls.get(), 0, "cursor keystroke must not publish"); } + #[test] + fn approval_clears_while_output_keeps_churning() { + // Codex's /voice meters redraw continuously, so output is never quiet. + // An answered approval must still clear once the scrape stays negative. + let target = PtyTarget::Known(Tool::Codex); + let mut screen = ScreenTracker::new_with_instance(24, 100, &[], None); + let state = Arc::new(RwLock::new(ScreenState::default())); + let launch = Arc::new(AtomicBool::new(false)); + let edges = std::cell::RefCell::new(Vec::new()); + let publish = |a: bool| edges.borrow_mut().push(a); + let dialog = "\x1b[2J\x1b[H\u{203a} 1. Yes, proceed (y)\r\n 2. No (esc)\r\n\r\n Press enter to confirm or esc to cancel\r\n"; + let idle = |screen: &mut ScreenTracker, level: char| { + screen.process( + format!( + "\x1b[2J\x1b[H voice \u{25cc} muted\r\n mic {level}{level}{level}\r\n\r\n\u{203a} Ask Codex to do anything\r\n\r\n ? for shortcuts\r\n" + ) + .as_bytes(), + ) + }; + let backdate = |state: &Arc>| { + let mut s = state.write().unwrap(); + s.approval_scrape_negative_since = + Some(Instant::now() - Duration::from_millis(APPROVAL_SCRAPE_CLEAR_MS)); + }; + + screen.process(dialog.as_bytes()); + update_delivery_state(&state, &screen, &target, &launch, &publish); + assert!(state.read().unwrap().approval); + + // A negative frame alone may be a partial redraw. + idle(&mut screen, '\u{2581}'); + update_delivery_state(&state, &screen, &target, &launch, &publish); + assert!(state.read().unwrap().approval); + + // The dialog coming back restarts the wait. + backdate(&state); + screen.process(dialog.as_bytes()); + update_delivery_state(&state, &screen, &target, &launch, &publish); + assert!( + state + .read() + .unwrap() + .approval_scrape_negative_since + .is_none() + ); + idle(&mut screen, '\u{2582}'); + update_delivery_state(&state, &screen, &target, &launch, &publish); + assert!(state.read().unwrap().approval); + + // Negative for the full wait while the meters still churn: clear. + backdate(&state); + idle(&mut screen, '\u{2583}'); + assert!(!screen.is_output_stable(APPROVAL_SCRAPE_CLEAR_MS)); + update_delivery_state(&state, &screen, &target, &launch, &publish); + assert!(!state.read().unwrap().approval); + assert_eq!(*edges.borrow(), vec![true, false]); + } + #[test] fn codex_input_preserves_approval_until_screen_clears() { let target = PtyTarget::Known(Tool::Codex);