From c75d169b9c4ff8f2ca549b3b63f40317d5ab8cbb Mon Sep 17 00:00:00 2001 From: simota Date: Fri, 25 Sep 2026 08:17:04 +0900 Subject: [PATCH] fix(app): let codex approval prompts settle before auto-approving Codex 0.156.1 drops an Enter that lands right after its approval modal's first paint. Noa fired on the second consecutive match, which can come milliseconds later, and then treated the ignored prompt as consumed, so it sat unanswered. Hold codex prompts until they have been on screen for 300ms; the stability rescan fires once they settle. --- crates/noa-app/src/auto_approve.rs | 110 +++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 15 deletions(-) diff --git a/crates/noa-app/src/auto_approve.rs b/crates/noa-app/src/auto_approve.rs index 2ec2ed7..b6d6679 100644 --- a/crates/noa-app/src/auto_approve.rs +++ b/crates/noa-app/src/auto_approve.rs @@ -78,8 +78,15 @@ struct Signature { yes_label: Option<&'static str>, requires_marker: bool, bytes: &'static [u8], + /// How long a prompt must stay on screen before the answer is sent. Codex + /// drops an Enter that lands right after its approval modal's first paint + /// (measured against codex 0.156.1: ignored at 0ms, accepted at 100ms), + /// and an ignored send is never retried because the prompt is consumed. + settle: Duration, } +const CODEX_PROMPT_SETTLE: Duration = Duration::from_millis(300); + const SIGNATURES: &[Signature] = &[ Signature { id: AutoApproveSignature::ClaudeEdit, @@ -89,6 +96,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. Yes"), requires_marker: true, bytes: b"1\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::ClaudeWrite, @@ -98,6 +106,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. Yes"), requires_marker: true, bytes: b"1\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::ClaudeRead, @@ -107,6 +116,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. Yes"), requires_marker: true, bytes: b"1\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::ClaudeAskUserQuestion, @@ -116,6 +126,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1."), requires_marker: true, bytes: b"1\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::ClaudeEnterConfirm, @@ -125,6 +136,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: None, requires_marker: false, bytes: b"\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::CodexCommand, @@ -134,6 +146,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. Yes, proceed (y)"), requires_marker: true, bytes: b"\r", + settle: CODEX_PROMPT_SETTLE, }, Signature { id: AutoApproveSignature::AgyAskUserQuestion, @@ -143,6 +156,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. (Recommended) "), requires_marker: true, bytes: b"\r", + settle: Duration::ZERO, }, Signature { id: AutoApproveSignature::AgyCommand, @@ -152,6 +166,7 @@ const SIGNATURES: &[Signature] = &[ yes_label: Some("1. Yes"), requires_marker: true, bytes: b"\r", + settle: Duration::ZERO, }, ]; @@ -213,6 +228,8 @@ pub(crate) enum Decision { #[derive(Clone, Debug, Default, PartialEq, Eq)] pub(crate) struct AutoApproveState { last_match: Option, + /// When `last_match` was first observed, for [`Signature::settle`]. + last_match_since: Option, match_count: u8, pending_fire: Option, awaiting_change: Option, @@ -263,6 +280,7 @@ impl AutoApproveState { region_hash, }); self.last_match = None; + self.last_match_since = None; self.match_count = 0; self.disabled_by_runaway = pending.disable_after; } @@ -436,6 +454,13 @@ fn decide( if state.last_match != Some(key) || state.match_count < 1 { return Decision::Hold; } + let settle = signature(matched.signature).settle; + if state + .last_match_since + .is_none_or(|since| ctx.now.saturating_duration_since(since) < settle) + { + return Decision::Hold; + } let approvals_in_window = count_recent_approvals(&state.approvals, ctx.now); Decision::Fire { @@ -493,10 +518,12 @@ fn apply_decision_state( state.match_count = state.match_count.saturating_add(1); } else { state.last_match = Some(key); + state.last_match_since = Some(ctx.now); state.match_count = 1; } } else { state.last_match = None; + state.last_match_since = None; state.match_count = 0; state.pending_fire = None; // A partial status redraw can invalidate the live tail while @@ -513,7 +540,7 @@ fn apply_decision_state( // back to the live rows need not produce any pty output. Keep the // known prompt tracked, while requiring two unsuppressed matches // before sending. - state.last_match = if matches!( + let tracked = if matches!( reason, SuppressReason::ImePreedit | SuppressReason::RecentUserInput @@ -527,6 +554,10 @@ fn apply_decision_state( } else { None }; + if tracked != state.last_match { + state.last_match_since = tracked.map(|_| ctx.now); + } + state.last_match = tracked; state.match_count = 0; state.pending_fire = None; } @@ -981,6 +1012,15 @@ mod tests { } } + /// `ctx` once a prompt first seen at `ctx.now` has outlived every + /// signature's settle delay. + fn settled(ctx: DetectContext) -> DetectContext { + DetectContext { + now: ctx.now + CODEX_PROMPT_SETTLE, + ..ctx + } + } + fn cursor(row: u16) -> Point { Point { x: 0, y: row } } @@ -1295,6 +1335,7 @@ mod tests { Decision::Hold ); assert!(state.needs_static_rescan()); + ctx.now += CODEX_PROMPT_SETTLE; let Decision::Fire { signature, region_hash, @@ -1338,7 +1379,7 @@ mod tests { Decision::Hold ); assert!(matches!( - detect_and_update_any_agent(&prompt, cursor, ctx, &mut state), + detect_and_update_any_agent(&prompt, cursor, settled(ctx), &mut state), Decision::Fire { .. } )); } @@ -1561,7 +1602,7 @@ mod tests { Decision::Hold ); assert!(matches!( - detect_and_update_any_agent(&prompt, cursor(0), ctx, &mut state), + detect_and_update_any_agent(&prompt, cursor(0), settled(ctx), &mut state), Decision::Fire { .. } )); } @@ -1578,8 +1619,8 @@ mod tests { (agy_review_command_prompt(), AgentKind::Agy), ] { let mut state = AutoApproveState::default(); - let ctx = base_ctx(now); - let _ = detect_and_update_any_agent(&prompt, cursor(0), ctx, &mut state); + let _ = detect_and_update_any_agent(&prompt, cursor(0), base_ctx(now), &mut state); + let ctx = settled(base_ctx(now)); assert!(matches!( detect(&prompt, cursor(0), agent, ctx, &state), Decision::Fire { .. } @@ -1616,7 +1657,7 @@ mod tests { #[test] fn menu_hash_covers_command_question_and_all_choices_but_not_cost() { let now = fixed_now(); - let ctx = base_ctx(now); + let ctx = settled(base_ctx(now)); for (mut prompt, change_row) in [ (codex_command_prompt(), 6), (agy_question_prompt(), 8), @@ -1624,7 +1665,7 @@ mod tests { (agy_run_command_prompt(), 3), ] { let mut state = AutoApproveState::default(); - let _ = detect_and_update_any_agent(&prompt, cursor(0), ctx, &mut state); + let _ = detect_and_update_any_agent(&prompt, cursor(0), base_ctx(now), &mut state); let Decision::Fire { signature, region_hash, @@ -1659,7 +1700,7 @@ mod tests { Decision::Hold ); assert!(matches!( - detect_and_update_any_agent(&prompt, cursor(0), ctx, &mut state), + detect_and_update_any_agent(&prompt, cursor(0), settled(ctx), &mut state), Decision::Fire { .. } )); let selected = prompt @@ -1713,6 +1754,44 @@ mod tests { assert!(!state.needs_static_rescan()); } + #[test] + fn codex_prompt_waits_for_settle_before_firing() { + // Codex 0.156.1 drops an Enter sent right after the modal's first + // paint; a second frame arriving within the settle window must not + // fire, and must keep the prompt armed for the stability rescan. + let prompt = codex_command_prompt(); + let first_seen = base_ctx(fixed_now()); + let mut state = AutoApproveState::default(); + assert_eq!( + detect_and_update_any_agent(&prompt, cursor(0), first_seen, &mut state), + Decision::Hold + ); + let mut early = first_seen; + early.now += CODEX_PROMPT_SETTLE - Duration::from_millis(1); + assert_eq!( + detect_and_update_any_agent(&prompt, cursor(0), early, &mut state), + Decision::Hold + ); + assert!(state.needs_static_rescan()); + assert!(matches!( + detect_and_update_any_agent(&prompt, cursor(0), settled(first_seen), &mut state), + Decision::Fire { + signature: AutoApproveSignature::CodexCommand, + .. + } + )); + + // Other agents keep firing on the second consecutive match. + let prompt = claude_edit_prompt(); + let ctx = base_ctx(fixed_now()); + let mut state = AutoApproveState::default(); + let _ = detect_and_update_any_agent(&prompt, cursor(1), ctx, &mut state); + assert!(matches!( + detect_and_update_any_agent(&prompt, cursor(1), ctx, &mut state), + Decision::Fire { .. } + )); + } + #[test] fn scrolled_back_prompt_rearms_when_viewport_returns_live() { let now = fixed_now(); @@ -1737,7 +1816,7 @@ mod tests { Decision::Hold ); assert!(matches!( - detect_and_update_any_agent(&prompt, cursor, ctx, &mut state), + detect_and_update_any_agent(&prompt, cursor, settled(ctx), &mut state), Decision::Fire { .. } )); } @@ -1832,7 +1911,7 @@ mod tests { signature, region_hash, .. - } = detect_and_update_any_agent(&prompt, cursor(0), ctx, &mut state) + } = detect_and_update_any_agent(&prompt, cursor(0), settled(ctx), &mut state) else { panic!("stable menu should fire"); }; @@ -1936,7 +2015,7 @@ mod tests { Decision::Hold ); assert!( - matches!(detect_and_update_any_agent(&screen, cursor, ctx, &mut state), Decision::Fire { signature, .. } if signature == expected) + matches!(detect_and_update_any_agent(&screen, cursor, settled(ctx), &mut state), Decision::Fire { signature, .. } if signature == expected) ); } } @@ -2120,7 +2199,7 @@ mod tests { Decision::Hold ); let Decision::Fire { signature, .. } = - detect_and_update_any_agent(&prompt, cursor, ctx, &mut state) + detect_and_update_any_agent(&prompt, cursor, settled(ctx), &mut state) else { panic!("codex prompt without Environment row should fire: {prompt:?}"); }; @@ -2174,7 +2253,7 @@ mod tests { detect_and_update_any_agent(&screen, cursor, ctx, &mut state), Decision::Hold ); - let d = detect_and_update_any_agent(&screen, cursor, ctx, &mut state); + let d = detect_and_update_any_agent(&screen, cursor, settled(ctx), &mut state); assert!( matches!( d, @@ -2204,14 +2283,15 @@ mod tests { .position(|row| row.trim() == "esc)") .expect("the rejection shortcut should wrap onto another physical row"); let mut state = AutoApproveState::default(); - let ctx = base_ctx(fixed_now()); + let first_seen = base_ctx(fixed_now()); + let ctx = settled(first_seen); let position = terminal.active().cursor; let cursor = Point { x: position.x, y: position.y, }; assert_eq!( - detect_and_update_any_agent(&screen, cursor, ctx, &mut state), + detect_and_update_any_agent(&screen, cursor, first_seen, &mut state), Decision::Hold ); let Decision::Fire {