fix(app): withhold destructive commands from auto-approve

crates/noa-app/src/app/sidebar/state.rs

@@ -124,7 +124,7 @@ impl App {
         // label. Remember that before `apply` moves the delta; the shared
         // label-dirty path below invalidates the filter after the new state is
         // visible and redraws every tile when the live result set can reflow.
-        let flags_overview_tile = delta_changes_overview_label(&delta);
+        let mut flags_overview_tile = delta_changes_overview_label(&delta);
         let upsert_window = match &delta {
             SessionDelta::Upsert { id, .. } => Some(id.window_id),
             _ => None,
@@ -153,7 +153,23 @@ impl App {
         // below (a process change is otherwise invisible to the render path,
         // which reads the store, not the delta stream).
         let is_process_delta = matches!(delta, SessionDelta::Process { .. });
+        // A process change can clear a departed agent's status, which the
+        // Overview label shows like an `AgentStatus` delta would.
+        let card_id = delta.id();
+        let had_status = is_process_delta
+            && self
+                .session_store
+                .get(&card_id)
+                .is_some_and(|card| card.agent_status.is_some());
         self.session_store.apply(delta);
+        if had_status
+            && self
+                .session_store
+                .get(&card_id)
+                .is_some_and(|card| card.agent_status.is_none())
+        {
+            flags_overview_tile = true;
+        }
         if let Some(id) = agent_notification {
             self.apply_session_delta(SessionDelta::Attention { id });
         }

crates/noa-app/src/auto_approve.rs

@@ -10,6 +10,8 @@ use noa_grid::{Cell, Terminal};
 
 use crate::sidebar::AgentKind;
 
+mod denylist;
+
 pub(crate) const USER_INPUT_SUPPRESSION: Duration = Duration::from_secs(3);
 pub(crate) const APPROVAL_WINDOW: Duration = Duration::from_secs(60);
 pub(crate) const APPROVAL_LIMIT: usize = 6;
@@ -87,6 +89,9 @@ struct Signature {
 
 const CODEX_PROMPT_SETTLE: Duration = Duration::from_millis(300);
 
+// #TODO(agent): UNVERIFIED — the Claude anchors below come from synthetic
+// fixtures only; capture a real Claude Code permission dialog and either fix
+// them or move Claude approval to its `PermissionRequest` hook.
 const SIGNATURES: &[Signature] = &[
     Signature {
         id: AutoApproveSignature::ClaudeEdit,
@@ -205,7 +210,6 @@ pub(crate) struct DetectContext {
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub(crate) enum SuppressReason {
-    Disabled,
     #[cfg(test)]
     UnknownAgent,
     ViewportNotLive,
@@ -234,7 +238,6 @@ pub(crate) struct AutoApproveState {
     pending_fire: Option<PendingPrompt>,
     awaiting_change: Option<ConsumedPrompt>,
     approvals: VecDeque<Instant>,
-    disabled_by_runaway: bool,
 }
 
 impl AutoApproveState {
@@ -243,8 +246,7 @@ impl AutoApproveState {
     }
 
     pub(crate) fn needs_static_rescan(&self) -> bool {
-        !self.disabled_by_runaway
-            && self.pending_fire.is_none()
+        self.pending_fire.is_none()
             && self.last_match.is_some_and(|key| {
                 self.awaiting_change.is_none_or(|consumed| {
                     consumed.signature != key.signature || consumed.region_hash != key.region_hash
@@ -274,15 +276,27 @@ impl AutoApproveState {
             return;
         }
 
-        self.approvals.push_back(now);
-        self.awaiting_change = Some(ConsumedPrompt {
+        let consumed = Some(ConsumedPrompt {
             signature,
             region_hash,
         });
+        if pending.disable_after {
+            // The breaker's off switch is the tab flag, cleared in the same
+            // main-thread step that sent this feedback, so every candidate
+            // the main thread handles afterwards is rejected as mode-off.
+            // Latching here too would outlive a re-enable that arrives
+            // before the next pty output, so start over as a fresh mode-on.
+            *self = Self {
+                awaiting_change: consumed,
+                ..Self::default()
+            };
+            return;
+        }
+        self.approvals.push_back(now);
+        self.awaiting_change = consumed;
         self.last_match = None;
         self.last_match_since = None;
         self.match_count = 0;
-        self.disabled_by_runaway = pending.disable_after;
     }
 }
 
@@ -336,7 +350,7 @@ pub(crate) fn detect_and_update_any_agent(
     ctx: DetectContext,
     state: &mut AutoApproveState,
 ) -> Decision {
-    let (decision, matched) = match suppression(ctx, state.disabled_by_runaway) {
+    let (decision, matched) = match suppression(ctx) {
         Some(reason) => {
             // Temporary input/viewport guards keep tracking the prompt
             // (see `apply_decision_state`), so only they pay for a scan.
@@ -366,7 +380,7 @@ pub(crate) fn rescan_signature(
     cursor: Point,
     ctx: DetectContext,
 ) -> Option<MatchedPrompt> {
-    if suppression(ctx, false).is_some() {
+    if suppression(ctx).is_some() {
         return None;
     }
     find_signature(rows, cursor, signature(signature_id))
@@ -419,7 +433,7 @@ fn detect_inner(
     ctx: DetectContext,
     state: &AutoApproveState,
 ) -> Decision {
-    if let Some(reason) = suppression(ctx, state.disabled_by_runaway) {
+    if let Some(reason) = suppression(ctx) {
         return Decision::Suppressed(reason);
     }
     decide(find_prompt(rows, cursor, agent).as_ref(), ctx, state)
@@ -564,10 +578,7 @@ fn apply_decision_state(
     }
 }
 
-fn suppression(ctx: DetectContext, disabled_by_runaway: bool) -> Option<SuppressReason> {
-    if disabled_by_runaway {
-        return Some(SuppressReason::Disabled);
-    }
+fn suppression(ctx: DetectContext) -> Option<SuppressReason> {
     if !ctx.alt_screen && ctx.scrollback_offset != 0 {
         return Some(SuppressReason::ViewportNotLive);
     }
@@ -631,6 +642,22 @@ fn find_signature_with_lowercase(
         if !menu_has_live_tail(rows, *region.end(), sig) {
             return None;
         }
+        // The choices below only echo the command ("… start with git add").
+        let choices = region
+            .clone()
+            .find(|&idx| selected_option(&rows[idx]).is_some())
+            .expect("a matched menu has a selected choice");
+        if sig.kind == PromptKind::Command
+            && let Some(rule) = denylist::denied_rule(&lowercase_rows[*region.start()..choices])
+        {
+            if trace_enabled() {
+                eprintln!(
+                    "[auto-approve] withheld {:?}: denylist rule {rule:?}",
+                    sig.id
+                );
+            }
+            return None;
+        }
         return Some(MatchedPrompt {
             signature: sig.id,
             region_hash: region_hash(rows, region.clone()),
@@ -2325,6 +2352,82 @@ mod tests {
         }
     }
 
+    const DENYLIST_GRID_COLS: usize = 48;
+
+    /// Paint a Codex command dialog showing `command` on a 48-column grid,
+    /// returning its rows and the settled decision.
+    fn decide_codex_command_on_grid(command: &str) -> (Vec<RowText>, Decision) {
+        let prompt: Vec<RowText> = codex_command_prompt()
+            .into_iter()
+            .map(|row| {
+                if row.starts_with("$ ") {
+                    command.to_string()
+                } else {
+                    row
+                }
+            })
+            .collect();
+        let mut terminal = Terminal::new(noa_core::GridSize::new(DENYLIST_GRID_COLS as u16, 30));
+        noa_vt::Stream::new().feed(prompt.join("\r\n").as_bytes(), &mut terminal);
+        let screen = viewport_rows_from_terminal(&terminal);
+        let position = terminal.active().cursor;
+        let cursor = Point {
+            x: position.x,
+            y: position.y,
+        };
+        let mut state = AutoApproveState::default();
+        let first_seen = base_ctx(fixed_now());
+        let _ = detect_and_update_any_agent(&screen, cursor, first_seen, &mut state);
+        let decision =
+            detect_and_update_any_agent(&screen, cursor, settled(first_seen), &mut state);
+        (screen, decision)
+    }
+
+    #[test]
+    fn denylist_sees_a_flag_split_by_the_grid_wrap() {
+        for (program, approved) in [("ls", true), ("rm", false)] {
+            // The first physical row ends exactly at `… && rm -`.
+            let tail = format!(" && {program} -");
+            let filler = "x".repeat(DENYLIST_GRID_COLS - "$ ".len() - tail.len());
+            let (screen, decision) =
+                decide_codex_command_on_grid(&format!("$ {filler}{tail}rf target"));
+            assert!(
+                screen.iter().any(|row| row.trim_end().ends_with(&tail))
+                    && screen.iter().any(|row| row.starts_with("rf target")),
+                "the flag should wrap mid-token: {screen:?}"
+            );
+            assert_eq!(
+                matches!(decision, Decision::Fire { .. }),
+                approved,
+                "{program}"
+            );
+        }
+    }
+
+    #[test]
+    fn denylist_sees_mixed_word_and_mid_word_grid_wraps() {
+        // `… && git` | `reset <sha> -` | `-hard`: the first wrap falls between
+        // words, the second inside `--hard`.
+        let tail = " && git";
+        let filler = "x".repeat(DENYLIST_GRID_COLS - "$ ".len() - tail.len());
+        let reset = format!("reset {} -", &"0123456789abcdef".repeat(3)[..40]);
+        assert_eq!(reset.len(), DENYLIST_GRID_COLS);
+        for (mode, approved) in [("-soft", true), ("-hard", false)] {
+            let (screen, decision) =
+                decide_codex_command_on_grid(&format!("$ {filler}{tail}{reset}{mode}"));
+            assert!(
+                screen.iter().any(|row| row.trim_end() == reset)
+                    && screen.iter().any(|row| row.trim_end() == mode),
+                "expected three physical rows: {screen:?}"
+            );
+            assert_eq!(
+                matches!(decision, Decision::Fire { .. }),
+                approved,
+                "{mode}"
+            );
+        }
+    }
+
     #[test]
     fn detect_holds_for_generic_agent_even_with_known_signature() {
         let now = fixed_now();
@@ -2584,9 +2687,72 @@ mod tests {
         };
         assert!(disable_after);
         state.apply_feedback(signature, region_hash, true, now);
-        assert!(matches!(
+        // The accepted prompt is never answered twice ...
+        assert_eq!(
             detect_and_update_any_agent(&prompt, cursor(1), base_ctx(now), &mut state),
-            Decision::Suppressed(SuppressReason::Disabled)
+            Decision::Hold
+        );
+        assert!(state.approvals.is_empty());
+    }
+
+    #[test]
+    fn re_enabling_after_the_breaker_needs_no_intervening_output() {
+        let now = fixed_now();
+        let mut state = AutoApproveState {
+            approvals: VecDeque::from(vec![now - Duration::from_secs(1); APPROVAL_LIMIT - 1]),
+            ..Default::default()
+        };
+        let edit = claude_edit_prompt();
+        let _ = detect_and_update_any_agent(&edit, cursor(1), base_ctx(now), &mut state);
+        let Decision::Fire {
+            signature,
+            region_hash,
+            disable_after: true,
+        } = detect_and_update_any_agent(&edit, cursor(1), base_ctx(now), &mut state)
+        else {
+            panic!("the limit-reaching approval should fire with disable_after");
+        };
+        state.apply_feedback(signature, region_hash, true, now);
+
+        // The user turns the mode back on before any pty output reaches the
+        // io thread's mode-off reset: the next prompt is still answered.
+        let codex = codex_command_prompt();
+        let cursor = cursor((codex.len() - 1) as u16);
+        let mut ctx = base_ctx(now);
+        assert_eq!(
+            detect_and_update_any_agent(&codex, cursor, ctx, &mut state),
+            Decision::Hold
+        );
+        ctx.now += CODEX_PROMPT_SETTLE;
+        assert!(matches!(
+            detect_and_update_any_agent(&codex, cursor, ctx, &mut state),
+            Decision::Fire {
+                signature: AutoApproveSignature::CodexCommand,
+                disable_after: false,
+                ..
+            }
         ));
     }
+
+    #[test]
+    fn destructive_codex_command_is_withheld() {
+        let now = fixed_now();
+        let prompt: Vec<RowText> = codex_command_prompt()
+            .into_iter()
+            .map(|row| row.replace("git add sample.rs", "rm -rf ~/src"))
+            .collect();
+        let cursor = cursor((prompt.len() - 1) as u16);
+        let mut state = AutoApproveState::default();
+        let mut ctx = base_ctx(now);
+        for _ in 0..3 {
+            assert_eq!(
+                detect_and_update_any_agent(&prompt, cursor, ctx, &mut state),
+                Decision::Hold
+            );
+            ctx.now += CODEX_PROMPT_SETTLE;
+        }
+        assert!(
+            rescan_signature(&prompt, AutoApproveSignature::CodexCommand, cursor, ctx).is_none()
+        );
+    }
 }