From 7504e306f8a0c103ed1072217425afd7b56accf8 Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 2 Oct 2026 07:41:45 +0900 Subject: [PATCH 1/4] fix(app): resume auto-approve when re-enabled right after the breaker The runaway breaker latched `disabled_by_runaway` in the io thread, and only a scan that saw the mode off cleared it. Turning the mode back on before the next pty output therefore left the pane suppressed for good. The tab flag, which the main thread clears in the same step that sends the limit-reaching approval, is already the breaker's off switch, so drop the latch and start the io state over on that feedback. --- crates/noa-app/src/auto_approve.rs | 79 ++++++++++++++++++++++++------ docs/specs/auto-approve-mode.md | 7 +++ 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/crates/noa-app/src/auto_approve.rs b/crates/noa-app/src/auto_approve.rs index b6d6679..a433841 100644 --- a/crates/noa-app/src/auto_approve.rs +++ b/crates/noa-app/src/auto_approve.rs @@ -205,7 +205,6 @@ pub(crate) struct DetectContext { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum SuppressReason { - Disabled, #[cfg(test)] UnknownAgent, ViewportNotLive, @@ -234,7 +233,6 @@ pub(crate) struct AutoApproveState { pending_fire: Option, awaiting_change: Option, approvals: VecDeque, - disabled_by_runaway: bool, } impl AutoApproveState { @@ -243,8 +241,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 +271,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 +345,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 +375,7 @@ pub(crate) fn rescan_signature( cursor: Point, ctx: DetectContext, ) -> Option { - if suppression(ctx, false).is_some() { + if suppression(ctx).is_some() { return None; } find_signature(rows, cursor, signature(signature_id)) @@ -419,7 +428,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 +573,7 @@ fn apply_decision_state( } } -fn suppression(ctx: DetectContext, disabled_by_runaway: bool) -> Option { - if disabled_by_runaway { - return Some(SuppressReason::Disabled); - } +fn suppression(ctx: DetectContext) -> Option { if !ctx.alt_screen && ctx.scrollback_offset != 0 { return Some(SuppressReason::ViewportNotLive); } @@ -2584,9 +2590,50 @@ 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, + .. + } )); } } diff --git a/docs/specs/auto-approve-mode.md b/docs/specs/auto-approve-mode.md index 9b0be7a..071c4f7 100644 --- a/docs/specs/auto-approve-mode.md +++ b/docs/specs/auto-approve-mode.md @@ -5,6 +5,13 @@ - owner: simota - build-path decision: **apex** (`/nexus apex` — live AC: T-1 signature capture and AC-11/12/13 GUI visual checks remain manual) +## 2026-10-01 revision — breaker re-enable + +The runaway breaker no longer keeps a latch in the io thread. Its off switch +is only the tab flag, which the main thread clears when it sends the sixth +approval. Before this change, turning the mode back on before the next pty +output left the pane suppressed. + ## 2026-09-14 investigation — agy approval remains pending The supplied log-search dialog passes both text and VT/grid detection tests. From 6b7ead02a6dba20d970678424b846653d1c8d13a Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 2 Oct 2026 07:42:32 +0900 Subject: [PATCH 2/4] fix(app): withhold destructive commands from auto-approve Codex and agy command dialogs were approved whatever they ran, which contradicts FR-11 ("never approve dangerous operations"). Check the displayed command against a fixed denylist and leave a matching dialog for the user, as if the mode were off. The command is read off the grid, so the check has to survive what the grid loses: a row boundary may be a wrap between words or inside one, so words meeting at a boundary are tried both apart and glued; words are split both shell-quoted and plainly; and rules look for flags and git subcommands anywhere after the program rather than at fixed positions. Every ambiguity resolves toward denying, since a false hit costs only a manual approval. --- crates/noa-app/src/auto_approve.rs | 116 ++++++ crates/noa-app/src/auto_approve/denylist.rs | 390 ++++++++++++++++++++ docs/specs/auto-approve-mode.md | 36 ++ 3 files changed, 542 insertions(+) create mode 100644 crates/noa-app/src/auto_approve/denylist.rs diff --git a/crates/noa-app/src/auto_approve.rs b/crates/noa-app/src/auto_approve.rs index a433841..431849f 100644 --- a/crates/noa-app/src/auto_approve.rs +++ b/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; @@ -637,6 +639,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()), @@ -2331,6 +2349,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, Decision) { + let prompt: Vec = 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 -` | `-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(); @@ -2636,4 +2730,26 @@ mod tests { } )); } + + #[test] + fn destructive_codex_command_is_withheld() { + let now = fixed_now(); + let prompt: Vec = 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() + ); + } } diff --git a/crates/noa-app/src/auto_approve/denylist.rs b/crates/noa-app/src/auto_approve/denylist.rs new file mode 100644 index 0000000..d793272 --- /dev/null +++ b/crates/noa-app/src/auto_approve/denylist.rs @@ -0,0 +1,390 @@ +//! Destructive-command denylist for command-execution approvals (FR-11). +//! +//! Best-effort by construction: it reads only what the dialog paints, so a +//! command an agent elides (agy's `⋯ (n lines hidden)`) or hides behind a +//! script can still pass. A hit only withholds the automatic answer — the +//! dialog stays up for the user, exactly as with auto-approve off — so every +//! ambiguity below resolves toward denying. + +/// The rule the command display trips, or `None`. `rows` are the lowercased +/// rows above the dialog's choices; a blank row ends a paragraph. +/// +/// Inside a paragraph each row boundary is a wrap either between words or +/// inside one (`rm -` / `rf`), and the grid cannot tell which. The words on +/// both sides of a boundary are therefore kept apart *and* also offered glued, +/// and every rule is positional only in "appears after", never "appears at", +/// so the extra candidates can only add denials. This stays linear in the +/// paragraph however its wraps mix. +pub(super) fn denied_rule(rows: &[String]) -> Option<&'static str> { + rows.split(|row| row.trim().is_empty()) + .filter(|paragraph| !paragraph.is_empty()) + .find_map(paragraph_rule) +} + +fn paragraph_rule(rows: &[String]) -> Option<&'static str> { + let mut line = String::new(); + let mut boundaries = Vec::new(); + for (idx, row) in rows.iter().enumerate() { + if idx > 0 { + boundaries.push(line.len()); + line.push(' '); + } + line.push_str(row.trim()); + } + let compact: String = line.split_whitespace().collect(); + for phrase in ["droptable", "dropdatabase", "truncatetable"] { + if compact.contains(phrase) { + return Some("drop/truncate table"); + } + } + // Shell-quoted words keep `"/tmp/my project"` whole; plain splitting + // also exposes commands quoted into `sh -c '…'` and similar. + [shell_tokens(&line), plain_tokens(&line)] + .into_iter() + .find_map(|tokens| tokens_rule(&with_glued_wraps(tokens, &boundaries))) +} + +#[derive(Clone, Debug, PartialEq, Eq)] +enum Token { + Word(String), + Separator(char), +} + +/// A token and the byte span of `line` it was read from. +type Spanned = (Token, usize, usize); + +const SEPARATORS: [char; 6] = [';', '&', '|', '(', ')', '`']; + +/// Also offer the two words meeting at a wrap glued as one word, between them. +fn with_glued_wraps(tokens: Vec, boundaries: &[usize]) -> Vec { + let mut out = Vec::with_capacity(tokens.len()); + for (idx, (token, _, end)) in tokens.iter().enumerate() { + out.push(token.clone()); + if let Token::Word(left) = token + && let Some((Token::Word(right), start, _)) = tokens.get(idx + 1) + && *start == end + 1 + && boundaries.contains(end) + { + out.push(Token::Word(format!("{left}{right}"))); + } + } + out +} + +fn shell_tokens(line: &str) -> Vec { + let mut tokens = Vec::new(); + let mut word: Option<(String, usize)> = None; + let mut chars = line.char_indices().peekable(); + while let Some((at, ch)) = chars.next() { + match ch { + '\'' => { + let (word, _) = word.get_or_insert_with(|| (String::new(), at)); + for (_, ch) in chars.by_ref() { + if ch == '\'' { + break; + } + word.push(ch); + } + } + '"' => { + let (word, _) = word.get_or_insert_with(|| (String::new(), at)); + while let Some((_, ch)) = chars.next() { + match ch { + '"' => break, + '\\' => word.extend(chars.next().map(|(_, ch)| ch)), + _ => word.push(ch), + } + } + } + '\\' => { + let (word, _) = word.get_or_insert_with(|| (String::new(), at)); + word.extend(chars.next().map(|(_, ch)| ch)); + } + _ if ch.is_whitespace() || SEPARATORS.contains(&ch) => { + if let Some((text, start)) = word.take() { + tokens.push((Token::Word(text), start, at)); + } + if !ch.is_whitespace() { + tokens.push((Token::Separator(ch), at, at + 1)); + } + } + _ => word.get_or_insert_with(|| (String::new(), at)).0.push(ch), + } + } + if let Some((text, start)) = word { + tokens.push((Token::Word(text), start, line.len())); + } + tokens +} + +fn plain_tokens(line: &str) -> Vec { + let mut tokens = Vec::new(); + let mut start = None; + for (at, ch) in line.char_indices().chain([(line.len(), ' ')]) { + let breaks = + ch.is_whitespace() || SEPARATORS.contains(&ch) || matches!(ch, '\'' | '"' | '\\'); + if !breaks { + start.get_or_insert(at); + continue; + } + if let Some(start) = start.take() { + tokens.push((Token::Word(line[start..at].to_string()), start, at)); + } + if SEPARATORS.contains(&ch) { + tokens.push((Token::Separator(ch), at, at + 1)); + } + } + tokens +} + +fn tokens_rule(tokens: &[Token]) -> Option<&'static str> { + let mut words: Vec<&str> = Vec::new(); + let mut after_pipe = false; + for token in tokens.iter().map(Some).chain([None]) { + if let Some(Token::Word(word)) = token { + words.push(word); + continue; + } + // Any word, not just the first: a wrap can split the program name + // (`ba` / `sh`). `| grep sh` is denied too, which only costs a manual + // approval. + if after_pipe && words.iter().any(|word| is_shell(program_name(word))) { + return Some("pipe to shell"); + } + // Any word may be the program (`xargs rm -rf`, `time git …`), and any + // later word its argument. + let rule = (0..words.len()).find_map(|idx| program_rule(words[idx], &words[idx + 1..])); + if rule.is_some() { + return rule; + } + words.clear(); + after_pipe = token == Some(&Token::Separator('|')); + } + None +} + +fn is_shell(program: &str) -> bool { + matches!( + program, + "sh" | "bash" | "zsh" | "dash" | "ksh" | "fish" | "sudo" + ) +} + +fn program_rule(word: &str, args: &[&str]) -> Option<&'static str> { + let recursive = |arg: &&str| short_flag_has(arg, 'r') || *arg == "--recursive"; + let rule = match program_name(word) { + "sudo" | "doas" => "sudo", + "shutdown" | "reboot" | "halt" => "shutdown", + "--no-verify" => "--no-verify", + "rm" if args.iter().any(recursive) => "rm -r", + "chmod" | "chown" if args.iter().any(recursive) => "recursive chmod/chown", + "dd" if args.iter().any(|arg| arg.starts_with("of=")) => "dd of=", + "diskutil" if args.iter().any(|arg| arg.starts_with("erase")) => "diskutil erase", + "git" => return git_rule(args), + program if program.starts_with("mkfs") => "mkfs", + _ => return None, + }; + Some(rule) +} + +/// The subcommand is found by name rather than position, so global options +/// (`-C `, `-c `) and wrap candidates need no parsing. +fn git_rule(args: &[&str]) -> Option<&'static str> { + let forced = |arg: &&str| short_flag_has(arg, 'f') || arg.starts_with("--force"); + args.iter().enumerate().find_map(|(idx, verb)| { + let after = &args[idx + 1..]; + let rule = match *verb { + "push" if after.iter().any(|arg| forced(arg) || arg.starts_with('+')) => { + "git push --force" + } + "reset" if after.contains(&"--hard") => "git reset --hard", + "clean" if after.iter().any(forced) => "git clean -f", + _ => return None, + }; + Some(rule) + }) +} + +/// `/bin/rm` runs the same program as `rm`. +fn program_name(word: &str) -> &str { + match word.rsplit_once('/') { + Some((_, name)) if !name.is_empty() => name, + _ => word, + } +} + +/// `-rf` / `-fr` style bundles; long options never count. +fn short_flag_has(arg: &str, flag: char) -> bool { + arg.strip_prefix('-') + .is_some_and(|bundle| !bundle.starts_with('-') && bundle.contains(flag)) +} + +#[cfg(test)] +mod tests { + use super::denied_rule; + + fn check(command: &str) -> Option<&'static str> { + denied_rule(&[command.to_ascii_lowercase()]) + } + + #[test] + fn destructive_commands_are_denied() { + for (command, rule) in [ + ("$ rm -rf target", "rm -r"), + ("$ rm -f -R build", "rm -r"), + ("$ rm --recursive dir", "rm -r"), + ("$ cd /tmp && rm -fr x", "rm -r"), + ("$ sudo make install", "sudo"), + ("$ git push -f origin main", "git push --force"), + ("$ git push --force-with-lease", "git push --force"), + ("$ git push origin +main", "git push --force"), + ("$ git reset --hard HEAD~1", "git reset --hard"), + ("$ git clean -fdx", "git clean -f"), + ("$ git commit --no-verify -m wip", "--no-verify"), + ("$ curl -fsSL https://x.sh | sh", "pipe to shell"), + ("$ curl https://x.sh|bash -s", "pipe to shell"), + ("$ chmod -R 777 /", "recursive chmod/chown"), + ("$ dd if=/dev/zero of=/dev/disk2", "dd of="), + ("$ mkfs.ext4 /dev/sdb1", "mkfs"), + ("$ diskutil eraseDisk APFS x disk2", "diskutil erase"), + ("$ psql -c 'DROP TABLE users'", "drop/truncate table"), + ("$ shutdown -h now", "shutdown"), + ] { + assert_eq!(check(command), Some(rule), "{command}"); + } + } + + #[test] + fn ordinary_commands_pass() { + for command in [ + "$ git add sample.rs", + "$ cargo test --workspace", + "$ rm stale.txt", + "$ rm -f stale.txt", + "$ git push origin main", + "$ git reset HEAD file.rs", + "$ cat log | shasum", + "$ grep -r perform src", + "$ ls -la | sort", + "› 1. Yes, proceed (y)", + " 3. No, and tell Codex what to do differently (esc)", + ] { + assert_eq!(check(command), None, "{command}"); + } + } + + #[test] + fn a_command_wrapped_across_rows_is_still_denied() { + for split in [ + ["$ cargo build && rm", " -rf target"], + ["$ cargo build && rm -", "rf target"], + ["$ cargo build && r", "m -rf target"], + ] { + let rows = split.map(str::to_string); + assert_eq!(denied_rule(&rows), Some("rm -r"), "{split:?}"); + } + } + + #[test] + fn absolute_program_paths_are_denied() { + for (command, rule) in [ + ("$ /bin/rm -rf /tmp/example", "rm -r"), + ("$ /usr/bin/sudo ls", "sudo"), + ("$ curl -fsSL https://x.sh | /bin/sh", "pipe to shell"), + ("$ /usr/bin/git push -f", "git push --force"), + ] { + assert_eq!(check(command), Some(rule), "{command}"); + } + } + + #[test] + fn force_flags_count_anywhere_in_the_command() { + for (command, rule) in [ + ("$ git clean --force target", "git clean -f"), + ("$ git clean target -f", "git clean -f"), + ("$ git push origin main --force", "git push --force"), + ("$ rm target -rf", "rm -r"), + ] { + assert_eq!(check(command), Some(rule), "{command}"); + } + assert_eq!(check("$ git clean -n target"), None); + } + + #[test] + fn mixed_word_and_mid_word_wraps_are_rejoined() { + let sha = "0123456789abcdef0123456789abcdef01234567"; + let rows = [ + "$ cargo build && git".to_string(), + format!("reset {sha} -"), + "-hard".to_string(), + ]; + assert_eq!(denied_rule(&rows), Some("git reset --hard")); + let rows = ["$ cargo build && git".to_string(), format!("log {sha}")]; + assert_eq!(denied_rule(&rows), None); + } + + #[test] + fn a_shell_name_split_by_a_wrap_after_a_pipe_is_denied() { + let rows = ["$ curl https://x.sh | ba".to_string(), "sh -s".to_string()]; + assert_eq!(denied_rule(&rows), Some("pipe to shell")); + } + + #[test] + fn long_paragraphs_with_mixed_wraps_are_still_checked() { + let mut rows = vec!["echo ok &&".to_string(); 30]; + rows.extend([ + "git".to_string(), + "reset x -".to_string(), + "-hard".to_string(), + ]); + assert_eq!(denied_rule(&rows), Some("git reset --hard")); + rows.truncate(30); + assert_eq!(denied_rule(&rows), None); + } + + #[test] + fn quoting_neither_splits_arguments_nor_hides_programs() { + for (command, rule) in [ + ( + r#"$ git -C "/tmp/my project" reset --hard HEAD"#, + "git reset --hard", + ), + (r"$ git -C /tmp/my\ project clean -f", "git clean -f"), + ("$ curl https://example.test/x | 'bash' -s", "pipe to shell"), + ( + r#"$ curl https://example.test/x | "/bin/sh""#, + "pipe to shell", + ), + ("$ bash -c 'rm -rf build'", "rm -r"), + ] { + assert_eq!(check(command), Some(rule), "{command}"); + } + assert_eq!(check(r#"$ git commit -m "reset the cache""#), None); + assert_eq!(check("$ cat notes | grep shell | sort"), None); + } + + #[test] + fn git_global_options_precede_the_subcommand() { + for (command, rule) in [ + ( + "$ git -C /tmp/example reset --hard HEAD", + "git reset --hard", + ), + ("$ git -C /tmp/example push --force", "git push --force"), + ("$ git -C /tmp/example clean -fd", "git clean -f"), + ( + "$ git -c core.pager=cat --no-pager reset --hard", + "git reset --hard", + ), + ( + "$ git --git-dir .git --work-tree . clean -f", + "git clean -f", + ), + ("$ git --git-dir=.git clean -f", "git clean -f"), + ] { + assert_eq!(check(command), Some(rule), "{command}"); + } + assert_eq!(check("$ git -C /tmp/example log --oneline"), None); + assert_eq!(check("$ git -C /tmp/example status"), None); + } +} diff --git a/docs/specs/auto-approve-mode.md b/docs/specs/auto-approve-mode.md index 071c4f7..e7a1914 100644 --- a/docs/specs/auto-approve-mode.md +++ b/docs/specs/auto-approve-mode.md @@ -5,6 +5,42 @@ - owner: simota - build-path decision: **apex** (`/nexus apex` — live AC: T-1 signature capture and AC-11/12/13 GUI visual checks remain manual) +## 2026-10-01 revision — command denylist + +This revision supersedes the "not filtered by an allowlist or denylist" +statements below for Codex and agy command dialogs, to restore FR-11 ("never +approve dangerous operations"). + +- Every recognized command dialog is checked against a fixed denylist + (`crates/noa-app/src/auto_approve/denylist.rs`). The check reads the rows + above the choices (the choices only echo the command), and a blank row ends + a paragraph. + - **Wraps:** a row boundary may fall between words or inside one, and the + grid cannot tell which. The words on both sides are kept as separate words + and are also offered glued together. The check stays linear in the length + of the paragraph, however its wraps mix. + - **Quoting:** words are split twice, once shell-quoted and once plainly. A + quoted path such as `"/tmp/my project"` stays one argument, and a quoted + program (`| 'bash'`, `sh -c '…'`) is still seen. + - **Position:** rules ask what appears after what, never where. A flag can + appear anywhere in the command (`git clean target -f`). A git subcommand is + found by name, so global options such as `-C ` need no parsing. + Programs are matched by basename (`/bin/rm`). + - **Rules:** `rm -r`/`--recursive`, `sudo`/`doas`, forced `git push`, + `git reset --hard`, `git clean -f`/`--force`, `--no-verify`, a pipe into a + shell (`sh`, `bash`, `zsh`, `dash`, `ksh`, `fish`) or `sudo`, recursive + `chmod`/`chown`, `dd of=`, `mkfs*`, `diskutil erase*`, + `shutdown`/`reboot`/`halt`, and `DROP TABLE`/`DROP DATABASE`/`TRUNCATE + TABLE`. + - **Ambiguity resolves toward denying.** A false hit costs only a manual + approval. For example, `| grep sh` is denied, as is a destructive command + named only inside a commit message. +- A hit withholds the automatic answer, and the dialog waits for the user as + it would with the mode off. `NOA_AUTO_APPROVE_TRACE=1` logs the rule that + matched. The check is best-effort: it sees only what the dialog paints, so a + command an agent elides (agy's `⋯ (n lines hidden)`) or wraps in a script can + still pass. + ## 2026-10-01 revision — breaker re-enable The runaway breaker no longer keeps a latch in the io thread. Its off switch From 9f0c747c51949bc9a9e1843d74157d130576f2f3 Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 2 Oct 2026 07:43:10 +0900 Subject: [PATCH 3/4] fix(app): clear a departed agent's status from its card An agent that exits without its end-of-session report (crash, kill) left its last explicit status on the card, so the sidebar and Overview kept showing it as running or waiting. Clear the status when the foreground leaves recognized agents. A switch from one agent straight to another keeps it: process changes post only on change, so the arriving agent's first report can land before the poll that sees it. Route the clearing through the Overview label invalidation an AgentStatus delta already uses. --- crates/noa-app/src/app/sidebar/state.rs | 18 +++++++- crates/noa-app/src/session_store.rs | 60 +++++++++++++++++++++++++ docs/specs/agent-workflow.md | 7 +++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/crates/noa-app/src/app/sidebar/state.rs b/crates/noa-app/src/app/sidebar/state.rs index 8ff7ba1..d209eb9 100644 --- a/crates/noa-app/src/app/sidebar/state.rs +++ b/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 }); } diff --git a/crates/noa-app/src/session_store.rs b/crates/noa-app/src/session_store.rs index 1b7ff80..4c60f07 100644 --- a/crates/noa-app/src/session_store.rs +++ b/crates/noa-app/src/session_store.rs @@ -18,6 +18,7 @@ use std::collections::{HashMap, HashSet, VecDeque}; use noa_core::Color; +use crate::sidebar::{AgentKind, classify_agent}; use crate::split_tree::PaneId; /// One color run within a card's last-output preview (FR-2): a maximal span of @@ -171,6 +172,13 @@ pub struct SessionCard { activity: u64, } +fn agent_left(previous: Option<&str>, current: Option<&str>) -> bool { + let is_agent = |process: Option<&str>| { + process.is_some_and(|process| classify_agent(process) != AgentKind::Generic) + }; + is_agent(previous) && !is_agent(current) +} + impl SessionCard { pub fn is_running(&self) -> bool { self.agent_status.as_ref().map_or(self.busy, |report| { @@ -698,6 +706,17 @@ impl SessionStore { SessionDelta::ProgressError { .. } => {} SessionDelta::Process { id, process } => { if let Some(card) = self.cards.get_mut(&id) { + // An agent that dies without its end-of-session report + // (crash, kill) would otherwise leave its last status on + // the card. Only leaving recognized agents altogether + // clears it: process deltas post on change only, so a + // report that beat the poll may already belong to the + // arriving agent (shell → agent, or one agent replacing + // another between polls) and must survive that arrival. + if agent_left(card.process.as_deref(), process.as_deref()) { + card.agent_status = None; + card.agent_status_at = None; + } card.process = process; } } @@ -1823,6 +1842,47 @@ mod tests { assert!(store.get(&id).unwrap().agent_status.is_none()); } + #[test] + fn agent_status_clears_when_the_agent_leaves_the_foreground() { + let mut store = SessionStore::new(); + let id = card_id(1, 1); + store.apply(upsert(id, 1, "agent")); + let process = |name: &str| SessionDelta::Process { + id, + process: Some(name.into()), + }; + let report = |state| SessionDelta::AgentStatus { + id, + status: Some(noa_grid::AgentStatus { + state, + detail: String::new(), + }), + at: wall(10, 0), + }; + store.apply(process("zsh")); + // The hook's first report can beat the 1s poll noticing the agent. + store.apply(report(noa_grid::AgentState::Running)); + store.apply(process("claude")); + assert!(store.get(&id).unwrap().agent_status.is_some()); + + // codex exits and claude starts between polls; claude's report lands + // before the poll that sees it. + store.apply(process("codex")); + store.apply(report(noa_grid::AgentState::Permission)); + store.apply(process("claude")); + let card = store.get(&id).unwrap(); + assert_eq!( + card.agent_status.as_ref().map(|status| status.state), + Some(noa_grid::AgentState::Permission) + ); + assert!(!card.is_running()); + + store.apply(process("zsh")); + let card = store.get(&id).unwrap(); + assert!(card.agent_status.is_none()); + assert!(card.agent_status_at.is_none()); + } + #[test] fn days_from_civil_matches_known_epoch_anchors() { assert_eq!(days_from_civil(1970, 1, 1), 0); diff --git a/docs/specs/agent-workflow.md b/docs/specs/agent-workflow.md index 0edc60a..abd2dfb 100644 --- a/docs/specs/agent-workflow.md +++ b/docs/specs/agent-workflow.md @@ -2,6 +2,13 @@ Status: implemented (2026-09-05). +Amended 2026-10-01: a pane's explicit agent status is cleared when its +foreground process changes from a recognized agent to a non-agent, so an agent +that exits without an end-of-session report leaves no stale status. A direct +switch from one agent to another keeps the status, because process changes are +posted only when they happen, and the arriving agent's first report can land +before that post. + Scope: pane-scoped unread notifications and next-unread navigation; local file links with editor line/column navigation; explicit agent status reports; multiline prompt drafts and a readable output view. Project grouping and a From 967ba92d04f5f2544e19b7c5b341254decd08938 Mon Sep 17 00:00:00 2001 From: "shingo.imota" Date: Fri, 2 Oct 2026 07:43:14 +0900 Subject: [PATCH 4/4] chore(app): mark the Claude auto-approve anchors as unverified The Claude Code prompt anchors were written against synthetic fixtures, and no real Claude permission dialog has been captured to confirm them. Leave the follow-up at the table so it is not lost. --- crates/noa-app/src/auto_approve.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/noa-app/src/auto_approve.rs b/crates/noa-app/src/auto_approve.rs index 431849f..3397806 100644 --- a/crates/noa-app/src/auto_approve.rs +++ b/crates/noa-app/src/auto_approve.rs @@ -89,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,