diff --git a/Native/OptChat/optchat-chief/src/agents.rs b/Native/OptChat/optchat-chief/src/agents.rs index b0e1f6beccda..e985614bd316 100644 --- a/Native/OptChat/optchat-chief/src/agents.rs +++ b/Native/OptChat/optchat-chief/src/agents.rs @@ -436,7 +436,17 @@ pub fn run(flags: &Flags) -> Result { } client .request("_acpmux/permission_respond", params) - .map_err(|e| e.to_string())?; + .map_err(|e| { + if verb == "allow" { + // acpmux takes an allow only from the person's app, + // and a child has no tab there: say what still works. + format!( + "{e}\nOnly the person's cmux app can allow, and {name} has no tab there: `chief agents deny {name}` denies it, or tell it what to do instead with `chief agents prompt {name} \"...\"`." + ) + } else { + e.to_string() + } + })?; client.close(); Ok(format!( "{}: {}", diff --git a/Native/OptChat/optchat-chief/src/approval.rs b/Native/OptChat/optchat-chief/src/approval.rs index 5455bb9184a8..4773189f5ab6 100644 --- a/Native/OptChat/optchat-chief/src/approval.rs +++ b/Native/OptChat/optchat-chief/src/approval.rs @@ -203,14 +203,23 @@ fn code_span(text: &str) -> String { /// A request's identity within one turn: the tool and its input. A /// repeat of a refused request counts toward stopping the turn. pub fn signature(pending: &Pending) -> String { - let input = pending - .request - .pointer("/toolCall/rawInput") - .map(Value::to_string) - .unwrap_or_default(); - format!("{}\u{0}{input}", pending.tool) + signature_of(&pending.tool, pending.request.pointer("/toolCall/rawInput")) } +/// [`signature`] of a call of `tool` with `input` (a permission request's +/// `rawInput`, or the arguments a Chief tool runs with). +pub fn signature_of(tool: &str, input: Option<&Value>) -> String { + let input = input.map(Value::to_string).unwrap_or_default(); + format!("{tool}\u{0}{input}") +} + +/// The longest a permission request of the Chief's turn session or of one +/// of its children may wait with no answer before the host denies it +/// (`Brain::deny_overdue_permissions_at`): a request nobody sees never holds +/// a turn forever. A subagent's prompt that the person's app shows with an +/// approval button waits for the person instead. +pub const PERMISSION_WAIT: std::time::Duration = std::time::Duration::from_secs(120); + /// Appends one `approval` event to the trace in `dir` /// (`traces/YYYY-MM-DD.jsonl`, the monitoring trace's layout: /// `{"ts", "ev", ...}`, 0600). A failed write is reported, never fatal. diff --git a/Native/OptChat/optchat-chief/src/brain/approvals.rs b/Native/OptChat/optchat-chief/src/brain/approvals.rs index 537149545ac1..7bfa7401ab2f 100644 --- a/Native/OptChat/optchat-chief/src/brain/approvals.rs +++ b/Native/OptChat/optchat-chief/src/brain/approvals.rs @@ -5,6 +5,22 @@ //! from sessions that run with policy `ask`: a turn that carries another //! person's text, and the children spawned under it (the spawn floor). Each //! refusal goes to the trace. +//! +//! No request waits on an answer nobody can give: +//! - acpmux takes an allow only from the person's app, and the app shows no +//! button for a turn session (a turn session never gets a tab): a turn +//! session's request the host does not allow is denied at once, in any +//! turn. +//! - A request the host leaves open (a subagent's or child's prompt with no +//! working approval button in the app, a deny acpmux did not take) is +//! denied once it waits [`crate::approval::PERMISSION_WAIT`] +//! (`deny_overdue_permissions_at`), with the reason in host.log and the +//! trace. +//! - Every host-side deny of the turn's own request counts toward +//! [`REPEATS_BEFORE_STOP`], so a model that calls a denied tool again and +//! again is stopped. + +use std::time::{Duration, Instant}; use serde_json::{Value, json}; @@ -18,6 +34,43 @@ const REPEATS_BEFORE_STOP: u32 = 3; /// The trace's approver of a refusal. const REFUSED_BY: &str = "another person's turn"; +/// Why the host watches a request whose deny acpmux did not take. +pub(super) const NOT_DELIVERED: &str = "its deny was not delivered"; + +/// The trace's approver of a request nobody could answer. +const NO_ANSWER: &str = "no approver"; + +/// Why a turn was denied a request (the stop line's end). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum DenyWhy { + /// A turn another person started may not run it. + OtherPerson, + /// It needs an approval nobody can give (only the person's app allows, + /// and the app shows no button for it). + NoApprover, +} + +impl DenyWhy { + /// The end of the turn's stop line: "I stopped: I kept trying to X, and ...". + pub(super) fn reason(self) -> &'static str { + match self { + DenyWhy::OtherPerson => "a turn another person started may not do that", + DenyWhy::NoApprover => "nobody can approve that here", + } + } +} + +/// A permission request the host left open: denied once it waits longer +/// than [`crate::approval::PERMISSION_WAIT`]. +#[derive(Clone, Debug)] +pub(super) struct Unanswered { + pending: Pending, + /// When the host first saw it (the brain's clock, ms since the epoch). + since_ms: u64, + /// Why nobody answers it, for host.log. + why: &'static str, +} + impl Brain { /// Whether `session_id` is the running turn's session. pub(super) fn is_turn_session(&self, session_id: &str) -> bool { @@ -40,30 +93,25 @@ impl Brain { // Reading the memory has no local effect; the Chief's own tools // are Chief talking to the person and its subagents. let option = option_for(&request, Answer::Allow); - if let Err(e) = + let Err(e) = self.agents .respond_permission(&session_id, &permission_id, option.as_deref()) - { - // acpmux takes an allow only from the person's app. A - // request left open holds the turn forever: refuse it, so - // the model hears why and the turn goes on. The turn's - // settings allow these tools, so this is a session started - // before them. - (self.log)(&format!("allowing {tool}: {e}; refusing it instead")); - let deny = option_for(&request, Answer::Deny); - if let Err(e) = - self.agents - .respond_permission(&session_id, &permission_id, deny.as_deref()) - { - (self.log)(&format!("refusing {tool}: {e}")); - } - } - return; - } - if !self.turn_ask { - (self.log)(&format!( - "turn session asked for {tool} outside a turn another person started; left to its harness" - )); + else { + return; + }; + // acpmux takes an allow only from the person's app. A request + // left open holds the turn forever: refuse it, so the model + // hears why and the turn goes on. The turn's settings allow + // these tools, so this is a session started before them. + (self.log)(&format!("allowing {tool}: {e}; refusing it instead")); + let pending = Pending { + session_id, + permission_id, + tool, + request, + child: None, + }; + self.refuse_as(&pending, DenyWhy::NoApprover, NO_ANSWER); return; } let pending = Pending { @@ -73,24 +121,60 @@ impl Brain { request, child: None, }; - let repeat = self - .turn_denied - .contains(&crate::approval::signature(&pending)); - self.refuse(&pending); - if repeat && self.denial_stopped.is_none() { - self.repeat_denials += 1; - if self.repeat_denials >= REPEATS_BEFORE_STOP { - // The model keeps asking for what was refused: end the turn. - (self.log)(&format!( - "{} was refused and asked for again {} times: stopping the turn", - pending.tool, self.repeat_denials - )); - self.denial_stopped = Some(crate::approval::describe( - &pending.tool, - pending.request.pointer("/toolCall/rawInput"), - )); - self.interrupt.request_hard(); - } + if self.turn_ask { + self.refuse_as(&pending, DenyWhy::OtherPerson, REFUSED_BY); + } else { + // The owner's turn under an asking policy, or a question the + // policy never answers: the app shows no button for a turn + // session, so nobody could answer it and the turn would wait + // forever. The Chief never asks: refused at once. + (self.log)(&format!( + "the turn session asked for {}; nobody can answer a turn session's request: refusing it", + pending.tool + )); + self.refuse_as(&pending, DenyWhy::NoApprover, NO_ANSWER); + } + } + + /// Refuses `pending`, a request of the running turn's session, as + /// `approver` says, and counts the refusal: a turn that keeps asking for + /// what it was refused stops ([`REPEATS_BEFORE_STOP`]). + fn refuse_as(&mut self, pending: &Pending, why: DenyWhy, approver: &'static str) { + self.refuse_by(pending, approver); + self.count_denial( + &pending.tool, + pending.request.pointer("/toolCall/rawInput"), + why, + ); + } + + /// Counts one host-side deny of the running turn's call of `tool` with + /// `input` (a refused request, a deny after a refused allow, a Chief + /// tool the turn gate refused, an overdue request): its third repeat + /// in the turn stops the turn, so the model is not left calling a + /// denied tool again and again. + pub(super) fn count_denial(&mut self, tool: &str, input: Option<&Value>, why: DenyWhy) { + if self.state.turn.is_none() { + return; + } + let signature = crate::approval::signature_of(tool, input); + let repeat = self.turn_denied.contains(&signature); + if !repeat { + self.turn_denied.push(signature); + return; + } + if self.denial_stopped.is_some() { + return; + } + self.repeat_denials += 1; + if self.repeat_denials >= REPEATS_BEFORE_STOP { + // The model keeps asking for what was refused: end the turn. + (self.log)(&format!( + "{tool} was refused and asked for again {} times: stopping the turn", + self.repeat_denials + )); + self.denial_stopped = Some((crate::approval::describe(tool, input), why)); + self.interrupt.request_hard(); } } @@ -166,12 +250,17 @@ impl Brain { /// with `input` now. The turn's settings allow the Chief's tools with /// no permission request, so this is where a turn another person /// started is held to [`Self::internal_allowed`]. - pub fn turn_gate(&self, tool: &str, input: &Value) -> Result<(), String> { + pub fn turn_gate(&mut self, tool: &str, input: &Value) -> Result<(), String> { let ask = self.turn_ask || (self.last_turn_ask && self.state.turn.is_none()); if self.internal_allowed_input(tool, Some(input), ask) { return Ok(()); } (self.log)(&format!("refused {tool} ({REFUSED_BY})")); + // A model that calls it again and again is stopped, as for a + // refused permission request. + if self.turn_ask { + self.count_denial(tool, Some(input), DenyWhy::OtherPerson); + } Err(format!( "refused: this turn carries another person's text, so {tool} may not run with these arguments" )) @@ -227,12 +316,13 @@ impl Brain { /// Refuses `pending` (its reject option) and records the refusal in /// the trace. fn refuse(&mut self, pending: &Pending) { - if pending.child.is_none() { - let signature = crate::approval::signature(pending); - if !self.turn_denied.contains(&signature) { - self.turn_denied.push(signature); - } - } + self.refuse_by(pending, REFUSED_BY); + } + + /// [`Self::refuse`] with `approver` in host.log and the trace. A refusal + /// acpmux did not take is watched: the host denies it again once it is + /// overdue (`deny_overdue_permissions_at`). + pub(super) fn refuse_by(&mut self, pending: &Pending, approver: &'static str) { let option = option_for(&pending.request, Answer::Deny); let result = self.agents.respond_permission( &pending.session_id, @@ -240,7 +330,7 @@ impl Brain { option.as_deref(), ); (self.log)(&format!( - "approval {}: refused {} ({REFUSED_BY}){}", + "approval {}: refused {} ({approver}){}", pending.permission_id, pending.tool, match &result { @@ -248,6 +338,11 @@ impl Brain { Err(e) => format!(" (not delivered: {e})"), } )); + if result.is_err() { + self.watch_unanswered(pending.clone(), NOT_DELIVERED); + } else { + self.forget_unanswered(&pending.session_id, &pending.permission_id); + } if let Some(dir) = &self.settings.trace_dir { let fields = json!({ "turn": self.state.turn.as_ref().map(|t| t.key.clone()), @@ -257,7 +352,7 @@ impl Brain { "child": pending.child, "decision": Answer::Deny.as_str(), "option": option, - "approver": REFUSED_BY, + "approver": approver, "delivered": result.is_ok(), }); if let Err(e) = crate::approval::record(dir, fields) { @@ -265,4 +360,138 @@ impl Brain { } } } + /// Watches `pending`, a request the host leaves open: once it waits + /// [`crate::approval::PERMISSION_WAIT`] with no answer, it is denied. + pub(super) fn watch_unanswered(&mut self, pending: Pending, why: &'static str) { + if self.unanswered.iter().any(|u| { + u.pending.session_id == pending.session_id + && u.pending.permission_id == pending.permission_id + }) { + return; + } + let since_ms = (self.clock)(); + self.unanswered.push(Unanswered { + pending, + since_ms, + why, + }); + } + + /// `permission_id` of `session_id` was answered (or is gone). + pub(super) fn forget_unanswered(&mut self, session_id: &str, permission_id: &str) { + self.unanswered.retain(|u| { + !(u.pending.session_id == session_id && u.pending.permission_id == permission_id) + }); + } + + /// `session_id` has no pending request any more (its summary says 0). + pub(super) fn forget_unanswered_of(&mut self, session_id: &str) { + self.unanswered + .retain(|u| u.pending.session_id != session_id); + } + + /// The requests the host watches now: (session, permission), oldest + /// first. + pub fn unanswered_permissions(&self) -> Vec<(String, String)> { + self.unanswered + .iter() + .map(|u| { + ( + u.pending.session_id.clone(), + u.pending.permission_id.clone(), + ) + }) + .collect() + } + + /// When the oldest watched request is due, on the run loop's clock. + pub(super) fn next_unanswered_due(&self) -> Option { + let due = self.unanswered.iter().map(|u| u.since_ms).min()? + + crate::approval::PERMISSION_WAIT.as_millis() as u64; + let now = (self.clock)(); + Some(Instant::now() + Duration::from_millis(due.saturating_sub(now))) + } + + /// Denies every watched request that waited + /// [`crate::approval::PERMISSION_WAIT`] or longer by `now_ms` (the + /// brain's clock): nobody answered it, and nobody may be able to. The + /// reason goes to host.log and the trace; a deny of the running turn's + /// own request counts toward stopping the turn. + pub fn deny_overdue_permissions_at(&mut self, now_ms: u64) { + let limit = crate::approval::PERMISSION_WAIT.as_millis() as u64; + let (due, keep): (Vec, Vec) = std::mem::take(&mut self.unanswered) + .into_iter() + .partition(|u| now_ms.saturating_sub(u.since_ms) >= limit); + self.unanswered = keep; + for u in due { + let p = &u.pending; + // A subagent whose workspace opened after it asked: the person + // sees the prompt with a working Allow button now. + if u.why != NOT_DELIVERED + && let Some(id) = self.state.sub_by_session(&p.session_id) + && self + .state + .sub(&id) + .is_some_and(|(_, s)| Self::approval_shown(s)) + { + continue; + } + let option = option_for(&p.request, Answer::Deny); + let result = + self.agents + .respond_permission(&p.session_id, &p.permission_id, option.as_deref()); + (self.log)(&format!( + "permission {} of {} ({}{}) waited {} s with no answer ({}): denied{}", + p.permission_id, + p.session_id, + p.tool, + p.child + .as_deref() + .map(|c| format!(", {c}")) + .unwrap_or_default(), + now_ms.saturating_sub(u.since_ms) / 1000, + u.why, + match &result { + Ok(()) => String::new(), + // Answered or gone meanwhile: nothing more to do. + Err(e) => format!(" (not delivered: {e})"), + } + )); + if let Some(dir) = &self.settings.trace_dir { + let fields = json!({ + "turn": self.state.turn.as_ref().map(|t| t.key.clone()), + "session": p.session_id, + "permission": p.permission_id, + "tool": p.tool, + "child": p.child, + "decision": Answer::Deny.as_str(), + "option": option, + "approver": NO_ANSWER, + "reason": format!("no answer for {} s ({})", limit / 1000, u.why), + "delivered": result.is_ok(), + }); + if let Err(e) = crate::approval::record(dir, fields) { + (self.log)(&format!("recording a refusal in the trace: {e}")); + } + } + if let Some(id) = self.state.sub_by_session(&p.session_id) + && let Some(sub) = self.state.sub_mut(&id) + && sub + .permission + .as_ref() + .is_some_and(|q| q.id == p.permission_id) + { + sub.permission = None; + self.save(); + } + // A deny that was not delivered was counted when it was made. + if p.child.is_none() && u.why != NOT_DELIVERED && self.is_turn_session(&p.session_id) { + self.count_denial( + &p.tool, + p.request.pointer("/toolCall/rawInput"), + DenyWhy::NoApprover, + ); + } + } + } } diff --git a/Native/OptChat/optchat-chief/src/brain/children.rs b/Native/OptChat/optchat-chief/src/brain/children.rs index b15dac7e0ea0..6e5f7bd54cfb 100644 --- a/Native/OptChat/optchat-chief/src/brain/children.rs +++ b/Native/OptChat/optchat-chief/src/brain/children.rs @@ -98,6 +98,35 @@ impl Brain { } else if self.is_child(&session) { let text = permission_text(&session.name, &request); self.queue(text, Source::Note); + // A child has no tab in the person's app, so nobody + // can allow it: denied once overdue, unless the Chief + // answers or steers it first. + self.watch_unanswered( + crate::approval::Pending { + session_id: session.session_id.clone(), + permission_id, + tool: crate::approval::tool_name(&request), + request, + child: Some(session.name.clone()), + }, + "a child agent has no approval button in the app", + ); + } else if self.is_own_turn_session(&session) { + // A turn session of this Chief that is not the running + // turn (a kept, adopted or ended one): nobody sees its + // request, so it is refused at once. + let pending = crate::approval::Pending { + session_id: session.session_id.clone(), + permission_id, + tool: crate::approval::tool_name(&request), + request, + child: None, + }; + (self.log)(&format!( + "turn session {} that no turn runs on asked for {}: refusing it", + pending.session_id, pending.tool + )); + self.refuse_by(&pending, "no approver"); } } } @@ -141,6 +170,16 @@ impl Brain { )); } + /// A turn session of this Chief (`cmux.chief=`, role `turn`). + fn is_own_turn_session(&self, session: &SessionSummary) -> bool { + session.tags.get(crate::acpmux::CHIEF_TAG) == Some(&self.settings.chief_id) + && session + .tags + .get(crate::acpmux::CHIEF_ROLE_TAG) + .map(String::as_str) + == Some("turn") + } + /// One of the Chief's `chief agents` children; a subagent (section 9, /// `optchat.subagent`) is not: its spawn reports for it. fn is_child(&self, session: &SessionSummary) -> bool { @@ -149,6 +188,10 @@ impl Brain { } fn on_session(&mut self, session: SessionSummary) { + if session.pending_permissions == 0 { + // Answered (by the person's app, or a deny): nothing to watch. + self.forget_unanswered_of(&session.session_id); + } // An adopted turn the Chief waits for has ended: fold it, then the // turns that waited start (the resume first). if self.orphan_wait.is_some() @@ -308,8 +351,8 @@ impl Brain { .collect(); for s in asking { let text = format!( - "[{}] has {} pending permission request(s). See `chief agents list`; answer with `chief agents allow {} [OPTION_ID]` or `chief agents deny {}`.", - s.name, s.pending_permissions, s.name, s.name + "[{}] has {} pending permission request(s). Nobody can allow them (only the person's cmux app allows, and it has no tab there): deny with `chief agents deny {}`, or tell it what to do instead.", + s.name, s.pending_permissions, s.name ); self.queue(text, Source::Note); } @@ -494,6 +537,7 @@ pub(super) fn permission_text(name: &str, request: &Value) -> String { options.join(", ") }; format!( - "[{name}] asks permission: {title}{input}\nOptions: {options}\nAnswer with `chief agents allow {name} OPTION_ID` or `chief agents deny {name}`." + "[{name}] asks permission: {title}{input}\nOptions: {options}\nNobody can allow it: only the person's cmux app allows, and {name} has no tab there. Deny it with `chief agents deny {name}`, or tell it what to do instead; the host denies it after {} s.", + crate::approval::PERMISSION_WAIT.as_secs() ) } diff --git a/Native/OptChat/optchat-chief/src/brain/control.rs b/Native/OptChat/optchat-chief/src/brain/control.rs index 4b37f4c7ad25..94808978e49f 100644 --- a/Native/OptChat/optchat-chief/src/brain/control.rs +++ b/Native/OptChat/optchat-chief/src/brain/control.rs @@ -161,8 +161,10 @@ pub(super) fn prompt_parts(request: &Value) -> (String, Vec<(String, String)>) { } /// A subagent's permission prompt as the Chief's note: the prompt, its -/// options, and how to answer, steer or ask the user. -pub(super) fn permission_note(id: &str, request: &Value) -> String { +/// options, and how to deny or steer it. Only the person's app can allow +/// (acpmux), so the note never asks the Chief to allow; `shown`: the app +/// shows the prompt with a working Allow button (`approval_shown`). +pub(super) fn permission_note(id: &str, request: &Value, shown: bool) -> String { let (title, options) = prompt_parts(request); let input = request .get("toolCall") @@ -182,8 +184,18 @@ pub(super) fn permission_note(id: &str, request: &Value) -> String { .collect::>() .join(", ") }; + let who = if shown { + format!( + "Only the user can allow it, with the Allow button in {id}'s workspace; tell them in one line if it matters." + ) + } else { + format!( + "Nobody can allow it: the app shows no working Allow button for {id}. The host denies it after {} s.", + crate::approval::PERMISSION_WAIT.as_secs() + ) + }; format!( - "[{id}] waits on a prompt: {title}{input}\nOptions: {options}\nAnswer: control(action: \"answer\", id: \"{id}\", option: OPTION_ID or \"deny\"). Or steer it: control(action: \"interrupt\", id: \"{id}\", message: \"do this instead\"). Approve only safe steps (reads, edits and tests in its worktree); ask the user for money, legal, destructive or credential steps." + "[{id}] waits on a prompt: {title}{input}\nOptions: {options}\n{who} Deny it: control(action: \"answer\", id: \"{id}\", option: \"deny\"). Or steer it: control(action: \"interrupt\", id: \"{id}\", message: \"do this instead\")." ) } @@ -384,7 +396,11 @@ impl Brain { } if let Some(p) = &sub.permission { out.push('\n'); - out.push_str(&permission_note(&sub.id, &p.request)); + out.push_str(&permission_note( + &sub.id, + &p.request, + Self::approval_shown(sub), + )); } out } @@ -475,26 +491,133 @@ impl Brain { ), ); } - let answer = if deny { None } else { Some(option) }; + // A deny answers with the prompt's reject option (a cancel when it + // has none). + // Never `reject_always`: a lasting rule would refuse a later allow + // of the person's. + let reject = p + .request + .get("options") + .and_then(Value::as_array) + .and_then(|o| { + o.iter() + .find(|o| o.get("kind").and_then(Value::as_str) == Some("reject_once")) + }) + .and_then(|o| o.get("optionId").and_then(Value::as_str)) + .map(str::to_owned); + let answer = if deny { + reject.as_deref() + } else { + Some(option) + }; match self.agents.respond_permission(&session, &p.id, answer) { Ok(()) => { if let Some(sub) = self.state.sub_mut(id) { sub.permission = None; } + self.forget_unanswered(&session, &p.id); self.save(); self.trace .emit("control.answer", json!({"id": id, "option": option})); outcome("done", format!("{id}'s prompt is answered: {option}")) } - Err(e) => outcome( + Err(e) if deny => outcome( "refused", - format!( - "{id}'s prompt was not answered ({e}). acpmux takes an allow only from the person's app; deny works, or steer it: control(action: \"interrupt\", id: \"{id}\", message: \"...\"), or ask the user to answer in its workspace" - ), + format!("{id}'s prompt was not denied ({e}); it may be answered already"), + ), + Err(e) if !e.contains("approve this on the Mac app") => outcome( + "refused", + format!("{id}'s prompt was not answered ({e}); it may be answered already"), ), + Err(e) => { + // acpmux takes an allow only from the person's app (the + // user's "allow" relayed by the Chief, too). Never leave + // the prompt open silently: the person hears where to + // allow it, or, with no working button for it, it is denied + // so the subagent goes on. + (self.log)(&format!("allowing {id}'s prompt {}: {e}", p.id)); + let shown = Self::approval_shown(sub); + let what = crate::approval::describe( + &crate::approval::tool_name(&p.request), + p.request.pointer("/toolCall/rawInput"), + ); + if shown { + self.tell_person_once( + &p.id, + &format!( + "{id} waits for your approval to {what}: use the Allow button in its workspace." + ), + ); + return outcome( + "refused", + format!( + "{id}'s prompt was not allowed: only the user can allow, with the Allow button in {id}'s workspace, and they were just told so in the chat. Do not try to allow it again; you may deny it or steer it: control(action: \"interrupt\", id: \"{id}\", message: \"...\")" + ), + ); + } + let denied = self + .agents + .respond_permission(&session, &p.id, reject.as_deref()); + if denied.is_ok() { + if let Some(sub) = self.state.sub_mut(id) { + sub.permission = None; + } + self.forget_unanswered(&session, &p.id); + self.save(); + } else { + self.watch_unanswered( + crate::approval::Pending { + session_id: session.clone(), + permission_id: p.id.clone(), + tool: crate::approval::tool_name(&p.request), + request: p.request.clone(), + child: Some(id.to_owned()), + }, + super::approvals::NOT_DELIVERED, + ); + } + if denied.is_ok() { + self.tell_person_once( + &p.id, + &format!( + "{id} asked to {what}; I denied it, because only the cmux app can approve and it has no Allow button for {id}." + ), + ); + } + outcome( + "refused", + format!( + "{id}'s prompt cannot be allowed: only the user's app allows, and it shows no working Allow button for {id}. {} The user was told. Steer it: control(action: \"interrupt\", id: \"{id}\", message: \"...\")", + match denied { + Ok(()) => "It was denied.".to_owned(), + Err(e) => format!( + "Denying it failed too ({e}); the host denies it again later." + ), + } + ), + ) + } } } + /// Posts `text` to the person in the main conversation, in the front + /// desk's voice, once per `permission` (a model that relays the same + /// "allow" again posts nothing more). + fn tell_person_once(&mut self, permission: &str, text: &str) { + let Some(conversation) = self.state.conversation.clone() else { + return; + }; + if !self.approval_told.insert(permission.to_owned()) { + return; + } + let key = crate::voice::key(crate::voice::DESK, &format!("approval:{permission}")); + self.state + .outbox + .push(super::reply_entry(conversation, &key, text)); + self.save(); + self.flush_outbox(); + } + fn control_pause(&mut self, sub: &SubRecord) -> Result { let id = sub.id.as_str(); if sub.paused { @@ -1047,10 +1170,11 @@ mod tests { "toolCall": {"title": "Bash: rm -rf build", "rawInput": {"command": "rm -rf build"}}, "options": [{"optionId": "allow_once", "name": "Allow"}, {"optionId": "reject", "name": "Reject"}], }); - let note = permission_note("a3", &request); + let note = permission_note("a3", &request, true); assert!(note.starts_with("[a3] waits on a prompt: Bash: rm -rf build")); assert!(note.contains("allow_once (Allow), reject (Reject)")); - assert!(note.contains("ask the user for money, legal, destructive or credential steps")); + assert!(note.contains("Only the user can allow it")); + assert!(permission_note("a3", &request, false).contains("Nobody can allow it")); } #[test] diff --git a/Native/OptChat/optchat-chief/src/brain/mod.rs b/Native/OptChat/optchat-chief/src/brain/mod.rs index 67c228f9bcd4..0284560cc8e6 100644 --- a/Native/OptChat/optchat-chief/src/brain/mod.rs +++ b/Native/OptChat/optchat-chief/src/brain/mod.rs @@ -721,7 +721,13 @@ pub struct Brain { /// The running turn was stopped for asking again and again for a tool /// call it was denied (approvals.rs): later requests are denied, and the /// turn posts one line saying so (what it asked for) instead of its text. - denial_stopped: Option, + denial_stopped: Option<(String, approvals::DenyWhy)>, + /// Permission requests the host left open, denied once overdue + /// (`approvals::deny_overdue_permissions_at`). + unanswered: Vec, + /// Subagent prompts whose refused allow the person was told about + /// (`control answer`): the line is posted once per prompt. + approval_told: HashSet, /// Fetches link previews for replies' URL lines (None: replies stay text). previewer: Option>, /// Turns cut in a row by a lost acpmux link (`Brain::turn_cut`): past @@ -854,6 +860,8 @@ impl Brain { sent_keys: std::collections::HashSet::new(), tool_stopped: false, denial_stopped: None, + unanswered: Vec::new(), + approval_told: HashSet::new(), previewer: None, cuts: 0, resume_at: None, @@ -1038,16 +1046,24 @@ impl Brain { } // The `sleep` wakes run on the same loop (wakes.rs); they stay // out of `next_timer`, which waits only for the outbox and resume. - let timer = match (self.next_timer(), self.next_wake()) { - (Some(a), Some(b)) => Some(a.min(b)), - (a, b) => a.or(b), - }; + // So does the watch on permission requests nobody answers + // (approvals.rs). + let timer = [ + self.next_timer(), + self.next_wake(), + self.next_unanswered_due(), + ] + .into_iter() + .flatten() + .min(); let input = match timer { Some(at) => match rx.recv_timeout(at.saturating_duration_since(Instant::now())) { Ok(input) => input, Err(RecvTimeoutError::Timeout) => { self.on_timer(); self.fire_wakes_at(now_ms()); + let now = (self.clock)(); + self.deny_overdue_permissions_at(now); continue; } Err(RecvTimeoutError::Disconnected) => return "input channel closed".into(), diff --git a/Native/OptChat/optchat-chief/src/brain/spawns.rs b/Native/OptChat/optchat-chief/src/brain/spawns.rs index 1483fe4191d0..779eed7bf834 100644 --- a/Native/OptChat/optchat-chief/src/brain/spawns.rs +++ b/Native/OptChat/optchat-chief/src/brain/spawns.rs @@ -317,14 +317,21 @@ impl Brain { } pub(super) fn sub_workspace(&mut self, id: &str, key: String, name: String) { + let mut shown_session = None; let done = if let Some(sub) = self.state.sub_mut(id) { sub.workspace = Some(key); sub.title = name; + shown_session = sub.session_id.clone().filter(|_| sub.peer.is_none()); matches!(sub.status, SubStatus::Done | SubStatus::Reported) } else { false }; self.save(); + // Its prompt shows in its tab now, with a working Allow button: it + // waits for the person, not for the watchdog. + if let Some(session) = shown_session { + self.forget_unanswered_of(&session); + } // It finished before its workspace was made. if done { self.mark_workspace(id, true); @@ -1682,12 +1689,38 @@ impl Brain { self.save(); if ask { self.child_permission(&summary, permission_id.to_owned(), request.clone()); - } else { - let text = super::control::permission_note(&id, request); - self.queue(text, Source::Note); + return true; + } + let shown = self + .state + .sub(&id) + .is_some_and(|(_, s)| Self::approval_shown(s)); + let text = super::control::permission_note(&id, request, shown); + self.queue(text, Source::Note); + if !shown { + // Nobody can allow it: denied once overdue, unless the Chief + // denies or steers it first. + self.watch_unanswered( + crate::approval::Pending { + session_id: session_id.to_owned(), + permission_id: permission_id.to_owned(), + tool: crate::approval::tool_name(request), + request: request.clone(), + child: Some(id.clone()), + }, + "the app shows no working approval button for this subagent", + ); } true } + + /// Whether the person's app shows `sub`'s prompt with an Allow button + /// that works: a local subagent with a workspace (its agent tab draws + /// the prompt). A cloud subagent's peer refuses an allow from this + /// Mac's app, and one with no workspace has no tab. + pub(super) fn approval_shown(sub: &crate::state::SubRecord) -> bool { + sub.workspace.is_some() && sub.peer.is_none() + } } /// A tell as the subagent's next prompt; its answer goes to the brain. diff --git a/Native/OptChat/optchat-chief/src/brain/turns.rs b/Native/OptChat/optchat-chief/src/brain/turns.rs index 0802603157ce..15c720091b20 100644 --- a/Native/OptChat/optchat-chief/src/brain/turns.rs +++ b/Native/OptChat/optchat-chief/src/brain/turns.rs @@ -1225,11 +1225,9 @@ impl Brain { text }; let text = match &denial_stopped { - Some(what) if !quiet => { + Some((what, why)) if !quiet => { (self.log)(&format!("turn {key} stopped: it asked again for {what}")); - format!( - "I stopped: I kept trying to {what}, and a turn another person started may not do that." - ) + format!("I stopped: I kept trying to {what}, and {}.", why.reason()) } _ => text, }; diff --git a/Native/OptChat/optchat-chief/src/codex_home.rs b/Native/OptChat/optchat-chief/src/codex_home.rs index d616de6742e9..18abb91a0cdb 100644 --- a/Native/OptChat/optchat-chief/src/codex_home.rs +++ b/Native/OptChat/optchat-chief/src/codex_home.rs @@ -150,6 +150,20 @@ enabled = false /// hooks, plugins, skills or apps) and no native subagents: the Chief's /// subagents are `chief spawn` sessions. The session directory's AGENTS.md /// (the Chief's instructions) stays. +/// +/// No approval config for the Chief's own tools: codex has one per MCP tool +/// (`[mcp_servers..tools.] approval_mode = "approve"`, or the +/// server's `default_tools_approval_mode`), but acpmux gives a codex turn no +/// `optchat` MCP server, so its Chief tools are `chief ` shell +/// commands (`prompt::Tools::Cli`), which no per-tool setting covers. In a +/// turn another person started (policy `ask`) each of them is a shell +/// permission request, which the host refuses at once (acpmux takes an +/// allow only from the person's app; `brain/approvals.rs`): the turn +/// answers with its final text, and a model that runs a refused command +/// again and again is stopped. An owner's turn under the default policy +/// (approve-all) runs codex with full access and asks nothing. Serving +/// `optchat` to codex as an MCP server with +/// `default_tools_approval_mode = "approve"` would lift this. pub fn codex_turn_config(user: Option<&str>) -> Result { let mut out = toml::Table::new(); if let Some(text) = user { diff --git a/Native/OptChat/optchat-chief/src/fold.rs b/Native/OptChat/optchat-chief/src/fold.rs index 836a3017a90e..69ff02ab0e6a 100644 --- a/Native/OptChat/optchat-chief/src/fold.rs +++ b/Native/OptChat/optchat-chief/src/fold.rs @@ -145,6 +145,9 @@ pub struct TurnFold { /// Reply text since the last tool call. talk: String, tools: HashMap, + /// Permission requests of the turn's session that wait for an answer + /// (acpmux `permission_request` with no `permission_decision` yet). + asking: std::collections::BTreeSet, /// The last finished reply: what the turn posts. last_talk: Option, /// Answers Claude Code finished (`result`) before a steered message @@ -217,6 +220,12 @@ impl TurnFold { self.ended.is_none() && self.tools.values().any(|t| !t.done) } + /// The turn waits on a permission request nobody answered yet: its + /// running tool is not at work, so the idle watchdog counts this time. + pub fn awaits_permission(&self) -> bool { + self.ended.is_none() && !self.asking.is_empty() + } + /// The turn's final assistant text: its last finished reply. pub fn final_text(&self) -> Option { let parts: Vec<&str> = self @@ -348,6 +357,16 @@ impl TurnFold { self.tool_update(update, event.at, &mut out); } } + ("mux", "permission_request") => { + if let Some(id) = event.msg.get("permissionId").and_then(Value::as_str) { + self.asking.insert(id.to_owned()); + } + } + ("mux", "permission_decision" | "permission_auto") => { + if let Some(id) = event.msg.get("permissionId").and_then(Value::as_str) { + self.asking.remove(id); + } + } ("mux", kind @ ("turn_end" | "turn_error")) => { self.finish_talk(&mut out); // The shared rule (cmux_chief::acp::turn_error_text): the same diff --git a/Native/OptChat/optchat-chief/src/mcp.rs b/Native/OptChat/optchat-chief/src/mcp.rs index 54b90c6f9775..60e5d0057170 100644 --- a/Native/OptChat/optchat-chief/src/mcp.rs +++ b/Native/OptChat/optchat-chief/src/mcp.rs @@ -69,7 +69,7 @@ pub fn tools_granted(subagent: bool, grant: &crate::chief_profile::Grant) -> Val "action": {"type": "string", "enum": crate::tools::CONTROL_ACTIONS}, "id": {"type": "string", "minLength": 1, "description": "The subagent (a); status without it shows all."}, "message": {"type": "string", "minLength": 1, "description": "interrupt/restart/resume: the prompt sent once it has stopped."}, - "option": {"type": "string", "minLength": 1, "description": "answer: the option id of its pending prompt, or deny."}, + "option": {"type": "string", "minLength": 1, "description": "answer: deny (only the user can allow, in the app)."}, "n": {"type": "integer", "minimum": 0, "description": "cap: subagents at work at once; 0 is no cap."}, "peer": {"type": "string", "minLength": 1, "description": "cloud: the acpmux peers (acpmux host ls, comma-separated) new subagents run on by default, each on the least busy, or off; without it, shows the setting."}}, "required": ["action"], "additionalProperties": false}})); diff --git a/Native/OptChat/optchat-chief/src/prompt.rs b/Native/OptChat/optchat-chief/src/prompt.rs index 46984fb229c5..cf5df5afda97 100644 --- a/Native/OptChat/optchat-chief/src/prompt.rs +++ b/Native/OptChat/optchat-chief/src/prompt.rs @@ -555,7 +555,7 @@ pub const STOP_DESCRIPTION: &str = "End your turn now, when everything is said ( pub const SLEEP_DESCRIPTION: &str = "End your turn and wake later: after `seconds` (30 to 86400) a message \"[wake] \" starts a new turn with the whole view. Use it to look at a run, deploy or subagent again instead of waiting or polling. until: event lets a user message or subagent report that comes first replace the wake. seconds 0 clears every pending wake. Never call it to wait for a subagent's report alone: reports always wake you."; /// The `control` tool's description. -pub const CONTROL_DESCRIPTION: &str = "Control your subagents; every answer starts with its outcome (done, queued, noop, refused). status [id]: state, elapsed, last output, pending prompt with its options. interrupt id [message]: stop its turn now, keep its context, then send message. answer id option: answer its pending prompt with an option id, or deny; approve only safe steps (reads, edits and tests in its worktree), ask the user for money, legal, destructive or credential steps. pause id / resume id [message]. cancel id: stop for good. restart id [message]: stop and continue in the same session. priority id: a queued one starts next. cap n: subagents at work at once (0: no cap). There is no cap by default; set one only when the user asks for one, and say how to lift it. cloud [peer[,peer...]|off]: the acpmux peers new subagents run on by default (each goes to the least busy; an idle VM pauses and resumes when needed), or off (here); without peer, shows it. Cloud subagents take no local slot. Never poll: reports, stuck notes and prompts come to you as messages."; +pub const CONTROL_DESCRIPTION: &str = "Control your subagents; every answer starts with its outcome (done, queued, noop, refused). status [id]: state, elapsed, last output, pending prompt with its options. interrupt id [message]: stop its turn now, keep its context, then send message. answer id deny: deny its pending prompt; only the user can allow one, with the Allow button in its workspace (an allow from you is refused: the user is told where to allow, or, when the app shows no Allow button for it, the prompt is denied). pause id / resume id [message]. cancel id: stop for good. restart id [message]: stop and continue in the same session. priority id: a queued one starts next. cap n: subagents at work at once (0: no cap). There is no cap by default; set one only when the user asks for one, and say how to lift it. cloud [peer[,peer...]|off]: the acpmux peers new subagents run on by default (each goes to the least busy; an idle VM pauses and resumes when needed), or off (here); without peer, shows it. Cloud subagents take no local slot. Never poll: reports, stuck notes and prompts come to you as messages."; /// How long a cache entry lives after its last read: Anthropic's two TTLs. /// Every mark of one request has the same TTL (the API refuses a 1h mark diff --git a/Native/OptChat/optchat-chief/src/turn.rs b/Native/OptChat/optchat-chief/src/turn.rs index 0e073bce3fd7..c679d2af3966 100644 --- a/Native/OptChat/optchat-chief/src/turn.rs +++ b/Native/OptChat/optchat-chief/src/turn.rs @@ -635,9 +635,11 @@ pub fn run_with_drafts( last_cancel = Some(Instant::now()); } let resend = last_cancel.filter(|_| stopping).map(|t| t + CANCEL_RESEND); + // A turn that waits on a permission request is idle too: its + // running tool is not at work, and nobody may ever answer. let idle_at = start .idle_limit - .filter(|_| !fold.tool_running() && last_cancel.is_none()) + .filter(|_| (!fold.tool_running() || fold.awaits_permission()) && last_cancel.is_none()) .map(|limit| last_event + limit); if idle_at.is_some_and(|at| Instant::now() >= at) { answered = true; diff --git a/Native/OptChat/optchat-chief/tests/approval_gates.rs b/Native/OptChat/optchat-chief/tests/approval_gates.rs index dc42c93da0e4..a9b5fe7200fa 100644 --- a/Native/OptChat/optchat-chief/tests/approval_gates.rs +++ b/Native/OptChat/optchat-chief/tests/approval_gates.rs @@ -512,3 +512,364 @@ fn another_persons_turn_is_held_to_the_input_rules_when_a_chief_tool_runs() { let upload = json!({"text": "here", "attachments": ["/tmp/report.txt"]}); assert_eq!(h.brain.turn_gate("mcp__optchat__send", &upload), Ok(())); } + +// No request of a Chief turn or of its subagents waits on an answer nobody +// can give (2026-10-20: "Using optchat: control ยท 34m"). + +/// A spawner of the owner's subagents. +fn owner_spawner(h: &Harness) -> Arc { + use optchat_chief::subagents::{Spawner, SubagentSettings}; + Arc::new(Spawner::new( + h.chat.clone(), + h.agents.clone(), + SubagentSettings { + harness: "claude-sr".into(), + policy: "approve-all".into(), + model: None, + effort: None, + preset: Some("optchat-sub-h0me".into()), + cwd: h.dir.path().join("subagent"), + prefix: "optchat-sub-h0me".into(), + parent: optchat_chief::brain::PARENT.into(), + claude_md: None, + }, + h.tx.clone(), + Arc::new(|_: &str| {}), + )) +} + +/// Runs `f` on its own thread while the brain takes its inputs, then lets +/// the brain take what is left. +fn run_with(h: &mut Harness, f: impl FnOnce() -> T + Send + 'static) -> T { + let worker = std::thread::spawn(f); + while !worker.is_finished() { + if let Ok(input) = h.rx.recv_timeout(Duration::from_millis(10)) { + h.brain.step(input); + } + } + drain(h); + worker.join().unwrap() +} + +fn drain(h: &mut Harness) { + while let Ok(input) = h.rx.recv_timeout(Duration::from_millis(80)) { + h.brain.step(input); + } +} + +/// The owner's subagent `a1`, finished, with a prompt `p1` to run +/// `make deploy` pending; with a workspace (its agent tab shows the +/// prompt with an Allow button) when `workspace`. Returns its session. +fn subagent_asking(h: &mut Harness, workspace: bool) -> String { + use optchat_chief::tools::Orchestrator; + let spawner = owner_spawner(h); + let spawned = run_with(h, move || { + spawner.spawn(vec!["deploy the docs".into()], None) + }); + assert!(spawned.is_ok(), "{spawned:?}"); + let session = { + let agents = h.agents.inner.lock().unwrap(); + let k = agents + .specs + .iter() + .position(|s| s.tags.contains_key("optchat.subagent")) + .expect("the subagent session"); + format!("s{}", k + 1) + }; + if workspace { + h.brain.step(Input::SubagentWorkspace { + id: "a1".into(), + key: "workspace:7".into(), + name: "deploy the docs".into(), + }); + } + h.brain.step(permission_in( + &session, + "p1", + "Bash", + json!({"command": "make deploy"}), + )); + drain(h); + session +} + +/// The Chief relays the user's "allow" for `a1` (`control answer`). +fn relay_allow(h: &mut Harness) -> String { + use optchat_chief::tools::{ControlCall, Orchestrator}; + let spawner = owner_spawner(h); + run_with(h, move || { + spawner.control(ControlCall { + action: "answer".into(), + id: Some("a1".into()), + option: Some("allow".into()), + ..ControlCall::default() + }) + }) + .unwrap_or_else(|e| e) +} + +/// The front desk's lines about prompt `permission`. +fn approval_lines(h: &Harness, permission: &str) -> Vec { + let key = format!("voice:desk:approval:{permission}"); + h.owner + .lock() + .unwrap() + .sends() + .into_iter() + .filter(|(k, _)| *k == key) + .map(|(_, text)| text) + .collect() +} + +fn pending_prompt(h: &Harness) -> bool { + h.brain + .state() + .sub("a1") + .is_some_and(|(_, s)| s.permission.is_some()) +} + +#[test] +fn a_relayed_allow_acpmux_refuses_tells_the_person_where_to_allow_once_and_keeps_the_prompt() { + // The user typed "allow" in the Chief chat; the Chief relayed it, and + // acpmux took it only from the person's app: "not delivered", and the + // prompt stayed open with nobody told. + let mut h = harness(None, false, default_script()); + subagent_asking(&mut h, true); + h.agents.inner.lock().unwrap().refuse_allows = true; + let answer = relay_allow(&mut h); + assert!(answer.starts_with("outcome: refused."), "{answer}"); + assert!(answer.contains("they were just told so"), "{answer}"); + h.settle_posts(); + let lines = approval_lines(&h, "p1"); + assert_eq!(lines.len(), 1, "{lines:?}"); + assert!( + lines[0].contains("a1 waits for your approval to run `make deploy`: use the Allow button in its workspace."), + "{lines:?}" + ); + // The app shows the prompt with a working button: it stays for the person. + assert_eq!(answered(&h, "p1"), None); + assert!(pending_prompt(&h)); + assert!(h.brain.unanswered_permissions().is_empty()); + // A second relay posts no second line. + relay_allow(&mut h); + h.settle_posts(); + assert_eq!(approval_lines(&h, "p1").len(), 1); + assert_eq!(questions(&h), 0, "no approval card"); +} + +#[test] +fn a_relayed_allow_for_a_prompt_the_app_does_not_show_denies_it_and_says_so() { + let mut h = harness(None, false, default_script()); + subagent_asking(&mut h, false); + h.agents.inner.lock().unwrap().refuse_allows = true; + let answer = relay_allow(&mut h); + assert!(answer.contains("It was denied."), "{answer}"); + assert_eq!(answered(&h, "p1"), Some(Some("reject".into()))); + assert!(!pending_prompt(&h)); + assert!(h.brain.unanswered_permissions().is_empty()); + h.settle_posts(); + let lines = approval_lines(&h, "p1"); + assert_eq!(lines.len(), 1, "{lines:?}"); + assert!(lines[0].contains("I denied it"), "{lines:?}"); +} + +#[test] +fn a_prompt_nobody_can_answer_is_denied_once_it_waits_two_minutes() { + let mut h = harness(None, false, default_script()); + const T0: u64 = 1_800_000_000_000; + h.brain.set_clock(Arc::new(|| T0)); + let session = subagent_asking(&mut h, false); + assert_eq!( + h.brain.unanswered_permissions(), + vec![(session.clone(), "p1".to_owned())] + ); + let wait = optchat_chief::approval::PERMISSION_WAIT.as_millis() as u64; + h.brain.deny_overdue_permissions_at(T0 + wait - 1); + assert_eq!(answered(&h, "p1"), None, "not overdue yet"); + h.brain.deny_overdue_permissions_at(T0 + wait); + assert_eq!(answered(&h, "p1"), Some(Some("reject".into()))); + assert!(h.brain.unanswered_permissions().is_empty()); + assert!(!pending_prompt(&h)); + let denial = traces(&h) + .into_iter() + .find(|t| t["ev"] == "approval" && t["permission"] == "p1") + .expect("the deny is in the trace"); + assert_eq!(denial["approver"], "no approver"); + assert!( + denial["reason"] + .as_str() + .is_some_and(|r| r.starts_with("no answer for 120 s")), + "{denial}" + ); +} + +#[test] +fn a_prompt_the_app_shows_waits_for_the_person() { + let mut h = harness(None, false, default_script()); + const T0: u64 = 1_800_000_000_000; + h.brain.set_clock(Arc::new(|| T0)); + subagent_asking(&mut h, true); + h.brain.deny_overdue_permissions_at(T0 + 3_600_000); + assert_eq!(answered(&h, "p1"), None); + assert!(pending_prompt(&h)); +} + +#[test] +fn a_prompt_whose_workspace_opens_after_it_asked_waits_for_the_person() { + // The tab opens after the subagent's first prompt was sent, so its + // first request can come before its workspace. + let mut h = harness(None, false, default_script()); + const T0: u64 = 1_800_000_000_000; + h.brain.set_clock(Arc::new(|| T0)); + subagent_asking(&mut h, false); + h.brain.step(Input::SubagentWorkspace { + id: "a1".into(), + key: "workspace:7".into(), + name: "deploy the docs".into(), + }); + assert!(h.brain.unanswered_permissions().is_empty()); + h.brain.deny_overdue_permissions_at(T0 + 3_600_000); + assert_eq!(answered(&h, "p1"), None); + assert!(pending_prompt(&h)); +} + +#[test] +fn the_owners_turn_session_is_refused_at_once_what_nobody_can_answer() { + // The app shows no button for a turn session (it lives on the Chief's + // own acpmux daemon): a request the host left "to its harness" held the + // turn for its whole limit. + let mut h = harness(None, false, started()); + h.agents.hold(true); + deliver(&mut h, false, "build it"); + h.step(); + h.agents.wait_prompts(1); + wait_session(&mut h); + let session = h + .brain + .state() + .turn + .as_ref() + .unwrap() + .session_id + .clone() + .unwrap(); + h.brain.step(permission_in( + &session, + "p1", + "Bash", + json!({"command": "make"}), + )); + assert_eq!(answered(&h, "p1"), Some(Some("reject".into()))); + assert_eq!(questions(&h), 0); + finish(&mut h); + // A turn session no turn runs on any more (kept, adopted, ended): + // refused at once too. + let turn_session = { + let agents = h.agents.inner.lock().unwrap(); + let k = agents + .specs + .iter() + .position(|s| s.tags.get("cmux.chief.role").map(String::as_str) == Some("turn")) + .expect("a turn session"); + format!("s{}", k + 1) + }; + h.brain.step(permission_in( + &turn_session, + "p2", + "Bash", + json!({"command": "make"}), + )); + assert_eq!(answered(&h, "p2"), Some(Some("reject".into()))); +} + +#[test] +fn a_chief_tool_denied_after_a_refused_allow_again_and_again_stops_the_turn() { + // 2026-10-20: the model called `control` again after each deny. + let mut h = shared(started()); + h.agents.inner.lock().unwrap().refuse_allows = true; + teammate_turn(&mut h, "@Chief hi"); + for k in 1..=4 { + let id = format!("p{k}"); + h.brain + .step(permission(&id, "mcp__optchat__send", json!({"text": "hi"}))); + assert_eq!(answered(&h, &id), Some(Some("reject".into())), "{id}"); + } + h.agents.wait_cancels(1); + h.agents.hold(false); + h.settle(); + let ops = format!("{:?}", h.owner.lock().unwrap().ops); + assert!( + ops.contains( + "I stopped: I kept trying to use send (optchat), and nobody can approve that here." + ), + "{ops}" + ); +} + +#[test] +fn a_chief_tool_the_turn_gate_refuses_again_and_again_stops_the_turn() { + let mut h = shared(started()); + teammate_turn(&mut h, "@Chief allow a1"); + let allow = json!({"action": "answer", "id": "a1", "option": "allow"}); + for _ in 0..4 { + assert!(h.brain.turn_gate("mcp__optchat__control", &allow).is_err()); + } + h.agents.wait_cancels(1); + h.agents.hold(false); + h.settle(); + let ops = format!("{:?}", h.owner.lock().unwrap().ops); + assert!( + ops.contains("I stopped: I kept trying to answer subagent a1, and a turn another person started may not do that."), + "{ops}" + ); +} + +#[test] +fn a_turn_that_waits_on_a_permission_nobody_answers_counts_as_idle() { + // The idle watchdog skipped a turn with a running tool, and a tool that + // waits on a permission runs until someone answers: the turn waited + // for its 3-hour limit. A request the host never heard of (a + // notification lost across a restart) is the case left to it. + let dir = tempfile::tempdir().unwrap(); + let owner = Arc::new(std::sync::Mutex::new(Owner { + summary: Some(members(false)), + ..Owner::default() + })); + let configured = optchat_chief::brain::Settings { + turn_idle_limit: Some(Duration::from_secs(1)), + ..common::settings(dir.path()) + }; + let mut h = Harness::configured(dir, started(), owner, configured, Arc::new(|_: &str| {})); + h.connect(); + h.agents.inner.lock().unwrap().answer_never = true; + deliver(&mut h, false, "deploy it"); + h.step(); + h.agents.wait_prompts(1); + wait_session(&mut h); + let session = h + .brain + .state() + .turn + .as_ref() + .unwrap() + .session_id + .clone() + .unwrap(); + h.agents.push_events( + &session, + vec![ + update( + "tool_call", + json!({"toolCallId": "t1", "title": "Bash", "status": "pending", + "rawInput": {"command": "make deploy"}, "_meta": {"claude": {"tool": "Bash"}}}), + ), + json!({"dir": "mux", "kind": "permission_request", + "msg": {"permissionId": "p9", "request": {"toolCall": {"toolCallId": "t1"}}}}), + ], + ); + h.agents.wait_cancels(1); + // The watchdog's turn runs once again, and answers. + h.settle(); + let sends = h.owner.lock().unwrap().sends(); + assert!(sends.iter().any(|(_, text)| text == "done"), "{sends:?}"); +} diff --git a/Native/OptChat/optchat-chief/tests/common/mod.rs b/Native/OptChat/optchat-chief/tests/common/mod.rs index fd9ff95e5378..957fc95aec8f 100644 --- a/Native/OptChat/optchat-chief/tests/common/mod.rs +++ b/Native/OptChat/optchat-chief/tests/common/mod.rs @@ -883,6 +883,7 @@ impl AgentPort for FakeAgents { }); Ok(Some(serde_json::from_value(json!({ "sessionId": id, "name": spec.name, "harness": harness, "cwd": spec.cwd, "status": status, + "tags": spec.tags, })).unwrap())) }