From cb5e16fddf83f581d96258df3caef1ddd92fcca4 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 20 Oct 2026 18:52:04 -0700 Subject: [PATCH 1/4] test(chief): typed approval answers read loosely, allow all, allow for this turn, allow for 1h (red) Leo, 2026-10-20: he answered "allow", "allow..." and "ALLOW" from his phone, the turn kept waiting, and one turn sent six prompts. A typed answer is read loosely (case, punctuation, emoji, yes/ok/no). It answers the oldest prompt or all of them, can cover the rest of the turn or a time window, and gets a one-line confirmation. The Chief's own send and status read are described in words, never as tool JSON. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c01-a495-7013-b37d-abe71de99881&host=cosmic-gray Co-Authored-By: Claude Opus 5.5 --- .../optchat-chief/tests/approval_gates.rs | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/Native/OptChat/optchat-chief/tests/approval_gates.rs b/Native/OptChat/optchat-chief/tests/approval_gates.rs index 1017463154e3..e164c8e733f1 100644 --- a/Native/OptChat/optchat-chief/tests/approval_gates.rs +++ b/Native/OptChat/optchat-chief/tests/approval_gates.rs @@ -675,3 +675,227 @@ fn a_card_in_a_side_conversation_is_answered_there() { assert_eq!(answered(&h, "p1"), Some(Some("allow".into()))); finish(&mut h); } + +// (f) Answers a person actually types (Leo, 2026-10-20: "allow", "allow...", +// "ALLOW" from his phone left the turn waiting), and answers that reach +// further than one request. + +#[test] +fn a_typed_answer_is_read_loosely_and_never_swallows_a_real_message() { + use optchat_chief::approval::{Answer, Reply, Scope}; + let allow = |scope| { + Some(Reply { + answer: Answer::Allow, + scope, + }) + }; + for text in [ + "allow", + "Allow", + "ALLOW", + "allow...", + "Allow!", + " allowed ", + "yes", + "Y", + "ok", + "okay", + "👍", + "✅", + "approve", + "yes please", + "allow it", + ] { + assert_eq!(Reply::parse(text), allow(Scope::Oldest), "{text:?}"); + } + for text in ["deny", "Deny.", "no", "nope", "👎", "❌", "reject"] { + assert_eq!( + Reply::parse(text).map(|r| (r.answer, r.scope)), + Some((Answer::Deny, Scope::Oldest)), + "{text:?}" + ); + } + for text in [ + "allow all", + "Allow them all", + "yes to all", + "approve everything", + ] { + assert_eq!(Reply::parse(text), allow(Scope::Pending), "{text:?}"); + } + assert_eq!( + Reply::parse("deny all").map(|r| (r.answer, r.scope)), + Some((Answer::Deny, Scope::Pending)) + ); + for text in [ + "allow for this turn", + "Allow for the rest of this turn", + "allow all for this turn", + "yes, this turn", + ] { + assert_eq!(Reply::parse(text), allow(Scope::Turn), "{text:?}"); + } + let hour = 60 * 60 * 1000; + for (text, ms) in [ + ("allow for 1h", hour), + ("allow for an hour", hour), + ("Allow for 2 hours", 2 * hour), + ("allow 30m", 30 * 60 * 1000), + ("allow for 15 min", 15 * 60 * 1000), + ] { + assert_eq!(Reply::parse(text), allow(Scope::For(ms)), "{text:?}"); + } + // A window is at most a day. + assert_eq!(Reply::parse("allow for 100h"), allow(Scope::For(24 * hour))); + // Real messages stay messages. + for text in [ + "hello?", + "allowance", + "yes, and also check the PR", + "no idea, look at CI", + "ok so what about the build", + "allow for", + "", + ] { + assert_eq!(Reply::parse(text), None, "{text:?}"); + } +} + +#[test] +fn a_loose_allow_from_the_phone_answers_the_oldest_approval_and_says_so() { + let mut h = gated(started()); + running(&mut h, true, "clean up"); + h.brain.step(permission( + "p1", + "Bash", + json!({"command": "rm -rf target"}), + )); + h.brain + .step(permission("p2", "Bash", json!({"command": "cargo clean"}))); + assert_eq!(approvals(&h).len(), 2); + deliver(&mut h, true, "ALLOW..."); + assert_eq!(answered(&h, "p1"), Some(Some("allow".into()))); + assert_eq!(answered(&h, "p2"), None, "one answer, the oldest request"); + deliver(&mut h, true, "👍"); + assert_eq!(answered(&h, "p2"), Some(Some("allow".into()))); + let ops = format!("{:?}", h.owner.lock().unwrap().ops); + assert!(ops.contains("Allowed: run `rm -rf target`"), "{ops}"); + assert!(ops.contains("Allowed: run `cargo clean`"), "{ops}"); + finish(&mut h); +} + +#[test] +fn allow_all_answers_every_pending_approval() { + let mut h = gated(started()); + running(&mut h, true, "clean up"); + for (id, command) in [("p1", "ls"), ("p2", "pwd"), ("p3", "whoami")] { + h.brain + .step(permission(id, "Bash", json!({"command": command}))); + } + deliver(&mut h, true, "allow all"); + for id in ["p1", "p2", "p3"] { + assert_eq!(answered(&h, id), Some(Some("allow".into())), "{id}"); + } + // The next request still asks: "all" is what was pending. + h.brain.step(permission( + "p4", + "Bash", + json!({"command": "rm -rf /tmp/x"}), + )); + assert_eq!(answered(&h, "p4"), None); + finish(&mut h); +} + +#[test] +fn allow_for_this_turn_lets_the_rest_of_the_turn_run_and_ends_with_it() { + let mut h = gated(default_script()); + running(&mut h, true, "clean up"); + h.brain + .step(permission("p1", "Bash", json!({"command": "ls"}))); + deliver(&mut h, true, "allow for this turn"); + assert_eq!(answered(&h, "p1"), Some(Some("allow".into()))); + h.brain + .step(permission("p2", "Bash", json!({"command": "cargo clean"}))); + assert_eq!(answered(&h, "p2"), Some(Some("allow".into()))); + assert_eq!(approvals(&h).len(), 1, "one card for the whole turn"); + finish(&mut h); + + // The next turn asks again. + running(&mut h, true, "and again"); + let session = h + .brain + .state() + .turn + .as_ref() + .and_then(|t| t.session_id.clone()) + .unwrap(); + h.brain.step(permission_in( + &session, + "p3", + "Bash", + json!({"command": "ls"}), + )); + assert_eq!(answered(&h, "p3"), None); + finish(&mut h); +} + +#[test] +fn allow_for_an_hour_covers_the_devices_later_turns_until_it_ends() { + let mut h = gated(default_script()); + let now = Arc::new(std::sync::Mutex::new(1_000_000_000_000u64)); + let clock = now.clone(); + h.brain.set_clock(Arc::new(move || *clock.lock().unwrap())); + running(&mut h, true, "clean up"); + h.brain + .step(permission("p1", "Bash", json!({"command": "ls"}))); + deliver(&mut h, true, "allow for 1h"); + assert_eq!(answered(&h, "p1"), Some(Some("allow".into()))); + let ops = format!("{:?}", h.owner.lock().unwrap().ops); + assert!(ops.contains("Allowed for 1h: run `ls`"), "{ops}"); + finish(&mut h); + + let session = |h: &Harness| { + h.brain + .state() + .turn + .as_ref() + .and_then(|t| t.session_id.clone()) + .unwrap() + }; + // A later turn from the phone, within the hour: runs. + running(&mut h, true, "next"); + let s = session(&h); + h.brain.step(permission_in( + &s, + "p2", + "Bash", + json!({"command": "cargo clean"}), + )); + assert_eq!(answered(&h, "p2"), Some(Some("allow".into()))); + // After the hour: asks again. + *now.lock().unwrap() += 61 * 60 * 1000; + h.brain.step(permission_in( + &s, + "p3", + "Bash", + json!({"command": "cargo clean"}), + )); + assert_eq!(answered(&h, "p3"), None); + finish(&mut h); +} + +#[test] +fn the_chiefs_own_messages_read_as_what_they_say_never_as_tool_json() { + use optchat_chief::approval::describe; + assert_eq!( + describe( + "mcp__optchat__send", + Some(&json!({"text": "Here.", "attachments": ["/tmp/a.png"]})) + ), + "send you \u{201c}Here.\u{201d} with a file from this Mac" + ); + assert_eq!( + describe("mcp__optchat__control", Some(&json!({"action": "status"}))), + "check on its subagents" + ); +} From 64bc2cba14b6a05d4a2524575a243182351401cc Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 20 Oct 2026 18:52:14 -0700 Subject: [PATCH 2/4] fix(chief): typed approval answers read loosely; allow all, for this turn or for 1h; confirm in one line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Leo answered "allow", "allow..." and "ALLOW" from his phone and the Chief kept waiting. Only a bare allow/deny counted, one answer settled one card, and one turn sent six cards. - A typed answer ignores case, punctuation and emoji, and accepts yes/ok/👍/✅ and no/👎/❌. Anything else in the message keeps it a message. - "allow all" / "deny all" answer every pending card. "allow for this turn" lets the rest of the turn run. "allow for 1h" (at most a day) lets the owner's own gated turns (paired device, Home) run until then, never another participant's. A non-owner joining the turn cancels the turn allow. - One line confirms what was answered ("Allowed: run `ls`"). - The Chief's own send and status read are described by what they do ("send you “...”", "check on its subagents"), not by tool name. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c01-a495-7013-b37d-abe71de99881&host=cosmic-gray Co-Authored-By: Claude Opus 5.5 --- Native/OptChat/optchat-chief/README.md | 9 +- Native/OptChat/optchat-chief/src/approval.rs | 124 +++++++++++++++++- .../optchat-chief/src/brain/approvals.rs | 104 +++++++++++++-- .../OptChat/optchat-chief/src/brain/inbox.rs | 6 +- Native/OptChat/optchat-chief/src/brain/mod.rs | 9 ++ .../OptChat/optchat-chief/src/brain/side.rs | 6 +- .../OptChat/optchat-chief/src/brain/steer.rs | 4 + .../OptChat/optchat-chief/src/brain/turns.rs | 3 + 8 files changed, 239 insertions(+), 26 deletions(-) diff --git a/Native/OptChat/optchat-chief/README.md b/Native/OptChat/optchat-chief/README.md index 34769d9f1087..0046484c613a 100644 --- a/Native/OptChat/optchat-chief/README.md +++ b/Native/OptChat/optchat-chief/README.md @@ -1171,7 +1171,14 @@ In a turn that asks, the turn session runs with acpmux policy `ask` instead of card's buttons (`question.answer` on the card in the conversation it was posted in, a side conversation included; only the owner's answer counts, and another participant's answer closes that card, so the owner gets a fresh - one) or with a message `allow` or `deny`. The line shows a line break as + one) or with a typed answer, read loosely (case, punctuation and emoji + ignored): `allow`, `ALLOW...`, `yes`, `ok`, 👍 or ✅ answer the oldest card; + `deny`, `no`, 👎 or ❌ deny it; `allow all` / `deny all` answer every pending + card; `allow for this turn` also lets the rest of the turn run; `allow for + 1h` (or `30m`, `2 hours`, at most a day) also lets the owner's own gated + turns (a paired device, Home) run until then, never another participant's. + A message with anything else in it ("yes, and check CI") is a message. One + line confirms what was answered ("Allowed: run `ls`"). The line shows a line break as `⏎` and a cut as "… (cut)". A client that draws no card shows its line and options, and an owner that refuses the part gets the same two lines and the arguments as text. diff --git a/Native/OptChat/optchat-chief/src/approval.rs b/Native/OptChat/optchat-chief/src/approval.rs index b9ca9ea7b80d..26a4258ebab6 100644 --- a/Native/OptChat/optchat-chief/src/approval.rs +++ b/Native/OptChat/optchat-chief/src/approval.rs @@ -89,22 +89,114 @@ pub fn is_memory_tool(request: &Value) -> bool { MEMORY_TOOLS.contains(&tool_name(request).as_str()) } -/// A person's answer in a chat message: `allow` or `deny` (any case, alone). +/// A person's answer: allow or deny. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Answer { Allow, Deny, } -impl Answer { - pub fn parse(text: &str) -> Option { - match text.trim().to_ascii_lowercase().as_str() { - "allow" => Some(Answer::Allow), - "deny" => Some(Answer::Deny), - _ => None, +/// How far a typed answer reaches. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Scope { + /// The conversation's oldest pending request. + Oldest, + /// Every pending request of the conversation. + Pending, + /// Every pending request, and the running turn's later ones. + Turn, + /// Every pending request, and for this many ms the later requests of + /// the owner's own gated turns (a paired device, Home). + For(u64), +} + +/// A chat message that answers approvals: "allow", "ALLOW...", "yes", +/// "👍", "allow all", "allow for this turn", "allow for 1h". Anything +/// else in the message makes it a message, not an answer. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Reply { + pub answer: Answer, + pub scope: Scope, +} + +/// The longest window `allow for ...` opens. +const MAX_WINDOW_MS: u64 = 24 * 60 * 60 * 1000; + +impl Reply { + pub fn parse(text: &str) -> Option { + let mut normal = String::new(); + for c in text.chars() { + match c { + '\u{1f44d}' | '\u{2705}' | '\u{2714}' => normal.push_str(" allow "), + '\u{1f44e}' | '\u{274c}' | '\u{1f6ab}' => normal.push_str(" deny "), + c if c.is_alphanumeric() => normal.extend(c.to_lowercase()), + _ => normal.push(' '), + } } + let words: Vec<&str> = normal.split_whitespace().collect(); + let (first, rest) = words.split_first()?; + let answer = match *first { + "allow" | "allowed" | "approve" | "approved" | "yes" | "y" | "yep" | "yeah" | "ok" + | "okay" | "sure" => Answer::Allow, + "deny" | "denied" | "reject" | "no" | "n" | "nope" => Answer::Deny, + _ => return None, + }; + let mut scope = Scope::Oldest; + let mut dangling_for = false; + let mut k = 0; + while k < rest.len() { + let word = rest[k]; + k += 1; + match word { + "please" | "pls" | "thanks" | "it" | "to" | "of" | "the" | "this" | "rest" => {} + "all" | "them" | "everything" => { + if scope == Scope::Oldest { + scope = Scope::Pending; + } + } + "for" => dangling_for = true, + "turn" => { + scope = Scope::Turn; + dangling_for = false; + } + _ => { + // A duration: "1h", "30m", "2 hours", "an hour". + let digits = word + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(word.len()); + let (count, unit) = if matches!(word, "a" | "an" | "one") { + k += 1; + (1, rest.get(k - 1).copied()) + } else if digits == 0 { + return None; + } else if digits == word.len() { + k += 1; + (word.parse::().ok()?, rest.get(k - 1).copied()) + } else { + (word[..digits].parse::().ok()?, Some(&word[digits..])) + }; + let unit_ms: u64 = match unit? { + "h" | "hr" | "hrs" | "hour" | "hours" => 60 * 60 * 1000, + "m" | "min" | "mins" | "minute" | "minutes" => 60 * 1000, + _ => return None, + }; + scope = Scope::For(count.saturating_mul(unit_ms).min(MAX_WINDOW_MS)); + dangling_for = false; + } + } + } + if dangling_for { + return None; + } + // Deny reaches only what is pending. + if answer == Answer::Deny && scope != Scope::Oldest { + scope = Scope::Pending; + } + Some(Reply { answer, scope }) } +} +impl Answer { pub fn as_str(self) -> &'static str { match self { Answer::Allow => "allow", @@ -164,6 +256,24 @@ pub fn describe(tool: &str, input: Option<&Value>) -> String { (Some(id), None) => format!("tell subagent {id}"), _ => "tell a subagent".to_owned(), }, + "mcp__optchat__send" => { + let files = input + .and_then(|i| i.get("attachments")) + .and_then(Value::as_array) + .map_or(0, Vec::len); + let text = + field("text").map(|t| format!("send you \u{201c}{}\u{201d}", one_line(&t, 80))); + match (text, files) { + (Some(t), 0) => t, + (Some(t), 1) => format!("{t} with a file from this Mac"), + (Some(t), n) => format!("{t} with {n} files from this Mac"), + (None, 0) => "send you a message".to_owned(), + (None, _) => "send you files from this Mac".to_owned(), + } + } + "mcp__optchat__control" if field("action").is_none_or(|a| a == "status") => { + "check on its subagents".to_owned() + } "mcp__optchat__control" => match (field("action"), field("id")) { (Some(a), Some(id)) => format!("{a} subagent {id}"), (Some(a), None) => format!("change subagents ({a})"), diff --git a/Native/OptChat/optchat-chief/src/brain/approvals.rs b/Native/OptChat/optchat-chief/src/brain/approvals.rs index 44fa63dceefd..ab321af7f99f 100644 --- a/Native/OptChat/optchat-chief/src/brain/approvals.rs +++ b/Native/OptChat/optchat-chief/src/brain/approvals.rs @@ -7,7 +7,9 @@ use cmux_conversation::{Message, Origin}; use serde_json::{Value, json}; use super::{Brain, reply_entry}; -use crate::approval::{Answer, Pending, is_memory_tool, option_for, question, tool_name}; +use crate::approval::{ + Answer, Pending, Reply, Scope, describe, is_memory_tool, option_for, question, tool_name, +}; /// A turn that asks again for a request it was denied this many times is /// stopped: the model is not left looping on a refused tool. @@ -88,6 +90,11 @@ impl Brain { } return; } + // The owner allowed the rest of this turn, or for a while. + if let Some(standing) = self.standing_allow() { + self.respond(&pending, Answer::Allow, standing, None); + return; + } let key = format!( "approval:{}:{}", self.state.turn.as_ref().map_or("", |t| t.key.as_str()), @@ -96,6 +103,22 @@ impl Brain { self.ask_person(pending, &key); } + /// The trace's approver when an earlier answer of the owner covers the + /// running turn's requests: "allow for this turn", or an "allow for 1h" + /// window, which covers only the owner's own gated turns (a paired + /// device, Home), never another participant's. + fn standing_allow(&self) -> Option<&'static str> { + if self.turn_allowed { + return Some("the owner (allowed for this turn)"); + } + let owners = matches!( + self.turn_why, + Some(crate::approval::Why::Device(_) | crate::approval::Why::Home) + ); + let open = self.allow_until.is_some_and(|until| (self.clock)() < until); + (owners && open).then_some("the owner (allowed for a while)") + } + /// The name of the paired device that sent one of `items` (its /// participant's display name), else "another device". pub(super) fn device_name(&self, items: &[super::Queued]) -> String { @@ -332,30 +355,77 @@ impl Brain { .any(|p| p.conversation.as_deref() == conversation) } - /// A person in `conversation` (None: main) answered that conversation's - /// oldest pending approval with `message`; another conversation's - /// approvals are never answered from here. + /// The owner in `conversation` (None: main) answered with `reply`: the + /// conversation's oldest pending approval, or all of them (and, for + /// "allow for this turn" or "allow for 1h", the later requests too); + /// another conversation's approvals are never answered from here. One + /// short line says what was answered. pub(super) fn answer_approval( &mut self, - answer: Answer, + reply: Reply, message: &Message, conversation: Option<&str>, ) { - let Some(k) = self + let mut answered = Vec::new(); + while let Some(k) = self .approvals .iter() .position(|p| p.conversation.as_deref() == conversation) - else { - return; - }; - let Some(pending) = self.approvals.remove(k) else { + { + let Some(pending) = self.approvals.remove(k) else { + break; + }; + answered.push(pending); + if reply.scope == Scope::Oldest { + break; + } + } + if answered.is_empty() { return; - }; + } + if reply.answer == Answer::Allow { + match reply.scope { + Scope::Turn => self.turn_allowed = true, + Scope::For(ms) => self.allow_until = Some((self.clock)().saturating_add(ms)), + Scope::Oldest | Scope::Pending => {} + } + } let install = message .origin .as_ref() .map(|Origin::Remote { install }| install.clone()); - self.respond(&pending, answer, &message.author, install.as_deref()); + for pending in &answered { + self.respond(pending, reply.answer, &message.author, install.as_deref()); + } + let label = match (reply.answer, reply.scope) { + (Answer::Deny, _) => "Denied".to_owned(), + (Answer::Allow, Scope::Turn) => "Allowed for this turn".to_owned(), + (Answer::Allow, Scope::For(ms)) => format!("Allowed for {}", window(ms)), + (Answer::Allow, _) => "Allowed".to_owned(), + }; + let what = answered + .iter() + .map(|p| { + let action = describe(&p.tool, p.request.pointer("/toolCall/rawInput")); + match &p.child { + Some(child) => format!("{child}: {action}"), + None => action, + } + }) + .collect::>() + .join("; "); + let Some(target) = conversation + .map(str::to_owned) + .or_else(|| self.state.conversation.clone()) + else { + return; + }; + let key = format!("approval-answered:{}", message.id); + self.state + .outbox + .push(reply_entry(target, &key, &format!("{label}: {what}"))); + self.save(); + self.flush_outbox(); } /// Denies the running turn's pending approvals (`why` is the trace's @@ -417,3 +487,13 @@ impl Brain { } } } + +/// A window's length the way a person writes it: "1h", "30m", "1h 30m". +fn window(ms: u64) -> String { + let minutes = ms / 60_000; + match (minutes / 60, minutes % 60) { + (0, m) => format!("{m}m"), + (h, 0) => format!("{h}h"), + (h, m) => format!("{h}h {m}m"), + } +} diff --git a/Native/OptChat/optchat-chief/src/brain/inbox.rs b/Native/OptChat/optchat-chief/src/brain/inbox.rs index 6e94978c7d91..534323f13738 100644 --- a/Native/OptChat/optchat-chief/src/brain/inbox.rs +++ b/Native/OptChat/optchat-chief/src/brain/inbox.rs @@ -266,12 +266,12 @@ impl Brain { // An answer to a pending approval of the running turn: it // answers, is logged, and is not a new message (it neither // queues nor interrupts the turn). - match crate::approval::Answer::parse(&text) { - Some(answer) + match crate::approval::Reply::parse(&text) { + Some(reply) if self.settings.membership.is_owner_message(summary, &message) && self.has_approval(None) => { - self.answer_approval(answer, &message, None); + self.answer_approval(reply, &message, None); if let Err(e) = self.chat.append(optchat_core::Kind::User, &text) { (self.log)(&format!("logging an approval failed: {e}")); } diff --git a/Native/OptChat/optchat-chief/src/brain/mod.rs b/Native/OptChat/optchat-chief/src/brain/mod.rs index 4eb4ba1e8a4a..2a22361c14c3 100644 --- a/Native/OptChat/optchat-chief/src/brain/mod.rs +++ b/Native/OptChat/optchat-chief/src/brain/mod.rs @@ -677,6 +677,13 @@ pub struct Brain { /// Requests of the running turn denied again because they repeat one /// already denied (`approvals::REPEATS_BEFORE_STOP` stops the turn). repeat_denials: u32, + /// The owner answered "allow for this turn": the running turn's later + /// requests run without asking (until it ends or another participant + /// joins it). + turn_allowed: bool, + /// The owner answered "allow for 1h": until this wall clock (ms), the + /// owner's own gated turns (a paired device, Home) run without asking. + allow_until: Option, /// A remote-origin turn was stopped for a newer message: the turn that /// answers both keeps its origin. remote_taint: bool, @@ -804,6 +811,8 @@ impl Brain { turn_why: None, turn_denied: Vec::new(), repeat_denials: 0, + turn_allowed: false, + allow_until: None, remote_taint: false, approvals: VecDeque::new(), after_turn: None, diff --git a/Native/OptChat/optchat-chief/src/brain/side.rs b/Native/OptChat/optchat-chief/src/brain/side.rs index 06bbd8696f77..6708998ad78f 100644 --- a/Native/OptChat/optchat-chief/src/brain/side.rs +++ b/Native/OptChat/optchat-chief/src/brain/side.rs @@ -228,11 +228,11 @@ impl Brain { labelled(summary, &text) }; let by_owner = self.settings.membership.is_owner_message(summary, &message); - match crate::approval::Answer::parse(&text) { + match crate::approval::Reply::parse(&text) { // An answer to this conversation's pending approval (only a // person answers one, never another Chief). - Some(answer) if by_owner && self.has_approval(Some(conversation)) => { - self.answer_approval(answer, &message, Some(conversation)); + Some(reply) if by_owner && self.has_approval(Some(conversation)) => { + self.answer_approval(reply, &message, Some(conversation)); if let Err(e) = self.chat.append(optchat_core::Kind::User, &labelled) { (self.log)(&format!("logging an approval failed: {e}")); } diff --git a/Native/OptChat/optchat-chief/src/brain/steer.rs b/Native/OptChat/optchat-chief/src/brain/steer.rs index ab968da4c5ad..0e323c26ef92 100644 --- a/Native/OptChat/optchat-chief/src/brain/steer.rs +++ b/Native/OptChat/optchat-chief/src/brain/steer.rs @@ -178,6 +178,10 @@ impl Brain { { self.turn_why = Some(why); } + // "Allow for this turn" was the owner's word on the owner's turn. + if non_owner { + self.turn_allowed = false; + } self.turn_ask = self.turn_why.is_some(); self.interrupt.set_gate(self.turn_ask); } diff --git a/Native/OptChat/optchat-chief/src/brain/turns.rs b/Native/OptChat/optchat-chief/src/brain/turns.rs index 00e76175d503..515aefe2521d 100644 --- a/Native/OptChat/optchat-chief/src/brain/turns.rs +++ b/Native/OptChat/optchat-chief/src/brain/turns.rs @@ -470,6 +470,7 @@ impl Brain { self.turn_ask = self.turn_why.is_some(); self.turn_denied.clear(); self.repeat_denials = 0; + self.turn_allowed = false; self.clear_turn_approvals(); self.interrupt.set_gate(self.turn_ask); // The shared rule (cmux_chief::policy::turn_policy, the corpus's @@ -1266,6 +1267,7 @@ impl Brain { self.turn_why = None; self.turn_denied.clear(); self.repeat_denials = 0; + self.turn_allowed = false; self.clear_turn_approvals(); self.interrupt.set_gate(false); if let Some(orphan) = outcome.orphan { @@ -1582,6 +1584,7 @@ impl Brain { self.turn_why = None; self.turn_denied.clear(); self.repeat_denials = 0; + self.turn_allowed = false; self.denial_stopped = None; self.clear_turn_approvals(); self.interrupt.set_gate(false); From 72096814ce567ff5b656167cbb453a3cbc590f8d Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 20 Oct 2026 18:57:52 -0700 Subject: [PATCH 3/4] test(chief): allow for this turn on a subagent's card leaves the turn asking; 1h 30m adds up; 0h is no answer (red) Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c01-a495-7013-b37d-abe71de99881&host=cosmic-gray Co-Authored-By: Claude Opus 5.5 --- .../optchat-chief/tests/approval_gates.rs | 2 + .../optchat-chief/tests/remote_policy.rs | 48 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/Native/OptChat/optchat-chief/tests/approval_gates.rs b/Native/OptChat/optchat-chief/tests/approval_gates.rs index e164c8e733f1..49ae2479ce96 100644 --- a/Native/OptChat/optchat-chief/tests/approval_gates.rs +++ b/Native/OptChat/optchat-chief/tests/approval_gates.rs @@ -742,6 +742,7 @@ fn a_typed_answer_is_read_loosely_and_never_swallows_a_real_message() { ("Allow for 2 hours", 2 * hour), ("allow 30m", 30 * 60 * 1000), ("allow for 15 min", 15 * 60 * 1000), + ("allow for 1h 30m", hour + 30 * 60 * 1000), ] { assert_eq!(Reply::parse(text), allow(Scope::For(ms)), "{text:?}"); } @@ -755,6 +756,7 @@ fn a_typed_answer_is_read_loosely_and_never_swallows_a_real_message() { "no idea, look at CI", "ok so what about the build", "allow for", + "allow for 0h", "", ] { assert_eq!(Reply::parse(text), None, "{text:?}"); diff --git a/Native/OptChat/optchat-chief/tests/remote_policy.rs b/Native/OptChat/optchat-chief/tests/remote_policy.rs index 32b0437f0a68..473a1f95d496 100644 --- a/Native/OptChat/optchat-chief/tests/remote_policy.rs +++ b/Native/OptChat/optchat-chief/tests/remote_policy.rs @@ -554,6 +554,54 @@ fn a_child_spawned_from_an_ask_turn_asks_too() { assert_eq!(h.brain.spawn_policy(), None); } +/// "Allow for this turn" that only answers a subagent's card does not cover +/// the running turn: the owner never saw a card from it. +#[test] +fn allow_for_this_turn_on_a_subagents_card_leaves_the_turn_asking() { + let mut h = harness(started()); + h.agents.hold(true); + deliver(&mut h, true, "start an agent that cleans the cache"); + h.step(); + h.agents.wait_prompts(1); + wait_session(&mut h); + h.brain + .step(Input::from(AgentEvent::SessionChanged(ask_child( + "running", + )))); + h.brain.step(permission_of( + "c1", + "p9", + "Bash", + json!({"command": "rm -rf ~/.cache"}), + )); + deliver(&mut h, true, "allow for this turn"); + assert!(h.agents.inner.lock().unwrap().responses.contains(&( + "c1".into(), + "p9".into(), + Some("allow".into()) + ))); + // The turn's own shell call still waits for a person. + h.brain.step(permission( + "p1", + "Bash", + json!({"command": "rm -rf target"}), + )); + assert!( + !h.agents + .inner + .lock() + .unwrap() + .responses + .iter() + .any(|r| r.1 == "p1"), + "the turn's shell call waits" + ); + assert!(sends(&h).last().unwrap().contains("rm -rf target")); + h.agents.hold(false); + h.agents.release(); + h.settle(); +} + /// Security floor for section 9's host-served `spawn`: a subagent spawned /// during a remote-origin (ask) turn runs with policy ask and carries /// `optchat.policy=ask`; its shell call waits for a person, asked in the From b8dcde5c263905d7abff43126060808aab086266 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 20 Oct 2026 18:57:56 -0700 Subject: [PATCH 4/4] fix(chief): a turn allow needs an answer to the turn's own card; durations add up; a zero window is no answer Review of #22755: "allow for this turn" typed to approve a leftover subagent card set the turn allow for a running turn the owner never saw a card from. "allow for 1h 30m" kept only 30m, and "allow for 0h" posted "Allowed for 0m". Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c01-a495-7013-b37d-abe71de99881&host=cosmic-gray Co-Authored-By: Claude Opus 5.5 --- Native/OptChat/optchat-chief/src/approval.rs | 10 ++++++++-- Native/OptChat/optchat-chief/src/brain/approvals.rs | 8 +++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Native/OptChat/optchat-chief/src/approval.rs b/Native/OptChat/optchat-chief/src/approval.rs index 26a4258ebab6..a050e6e58615 100644 --- a/Native/OptChat/optchat-chief/src/approval.rs +++ b/Native/OptChat/optchat-chief/src/approval.rs @@ -180,12 +180,18 @@ impl Reply { "m" | "min" | "mins" | "minute" | "minutes" => 60 * 1000, _ => return None, }; - scope = Scope::For(count.saturating_mul(unit_ms).min(MAX_WINDOW_MS)); + // "1h 30m" adds up. + let before = match scope { + Scope::For(ms) => ms, + _ => 0, + }; + let ms = before.saturating_add(count.saturating_mul(unit_ms)); + scope = Scope::For(ms.min(MAX_WINDOW_MS)); dangling_for = false; } } } - if dangling_for { + if dangling_for || scope == Scope::For(0) { return None; } // Deny reaches only what is pending. diff --git a/Native/OptChat/optchat-chief/src/brain/approvals.rs b/Native/OptChat/optchat-chief/src/brain/approvals.rs index ab321af7f99f..c3b248fac42b 100644 --- a/Native/OptChat/optchat-chief/src/brain/approvals.rs +++ b/Native/OptChat/optchat-chief/src/brain/approvals.rs @@ -385,7 +385,13 @@ impl Brain { } if reply.answer == Answer::Allow { match reply.scope { - Scope::Turn => self.turn_allowed = true, + // Only an answer to the running turn's own request covers the turn, + // never one to a subagent's card left over from earlier. + Scope::Turn => { + if answered.iter().any(|pending| pending.child.is_none()) { + self.turn_allowed = true; + } + } Scope::For(ms) => self.allow_until = Some((self.clock)().saturating_add(ms)), Scope::Oldest | Scope::Pending => {} }