From 87a9ec176308373099a633defb1f3bcc74387fcf Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Wed, 22 Jul 2026 16:03:25 -0700 Subject: [PATCH] fix waiting state not firing --- src/orchestrator/mod.rs | 82 ++++++++++++++++++++++++++++++++--------- src/patterns.rs | 36 ++++++++++++++++-- 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/src/orchestrator/mod.rs b/src/orchestrator/mod.rs index deae625..f9b5306 100644 --- a/src/orchestrator/mod.rs +++ b/src/orchestrator/mod.rs @@ -131,23 +131,36 @@ impl OrchestratorMsg { fn coalesce_batch( msgs: &[OrchestratorMsg], history: &mut Vec, -) -> (Option, bool) { +) -> (Option, bool, bool) { let mut parts = Vec::new(); let mut any_user = false; + let mut any_actionable = false; for m in msgs { if matches!(m, OrchestratorMsg::Reset) { history.clear(); parts.clear(); any_user = false; + any_actionable = false; } else { any_user |= m.is_user_chat(); + // WAITING / ERROR / DEAD events (and system notes) must always + // produce a visible report; only pure informational batches + // (e.g. READY transitions) may be answered with a silent `ok`. + any_actionable |= match m { + OrchestratorMsg::SessionEvent { state, .. } => { + let s = state.trim_end_matches('!'); + !matches!(s, "READY" | "STARTING" | "THINKING" | "RUNNING") + } + OrchestratorMsg::SystemNote(_) => true, + _ => false, + }; parts.push(m.render()); } } if parts.is_empty() { - (None, any_user) + (None, any_user, any_actionable) } else { - (Some(parts.join("\n\n")), any_user) + (Some(parts.join("\n\n")), any_user, any_actionable) } } @@ -169,7 +182,7 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche while let Ok(next) = rx.try_recv() { msgs.push(next); } - let (user_text, any_user) = coalesce_batch(&msgs, &mut history); + let (user_text, any_user, any_actionable) = coalesce_batch(&msgs, &mut history); let Some(user_text) = user_text else { // Pure reset, nothing to say to the model. continue; @@ -213,12 +226,21 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche Ok(text) => text, Err(e) => format!("[{}: error: {}]", cfg.name, e), }; - // Always answer a human; stay quiet only if an event turn produced - // nothing, or the bare `ok` the system prompt designates as the - // "nothing to report" acknowledgment for informational events. + // Always answer a human. The bare `ok` acknowledgment is only a + // valid no-op for purely informational batches; small models + // over-apply it, so for actionable events (WAITING/ERROR/DEAD) + // an `ok` or empty reply is replaced with the raw event text — + // the user must hear about a blocked session even when the + // model under-delivers. + let trimmed = text.trim(); let noop_ack = - text.trim().eq_ignore_ascii_case("ok") || text.trim().eq_ignore_ascii_case("ok."); - if any_user || (!text.trim().is_empty() && !noop_ack) { + trimmed.eq_ignore_ascii_case("ok") || trimmed.eq_ignore_ascii_case("ok."); + let text = if any_actionable && (noop_ack || trimmed.is_empty()) { + user_text.clone() + } else { + text + }; + if any_user || any_actionable || (!text.trim().is_empty() && !noop_ack) { let _ = event_tx .send(AppEvent::ChatReply { from: cfg.name.clone(), @@ -639,11 +661,12 @@ session's process (SIGSTOP) without losing its context — its state shows PAUSE which is the right lever when concurrent sessions contend for limited CPU or RAM.\n\ \n\ Messages starting with [linkshell event] are automatic notifications that a session \ -changed state. For WAITING, ERROR, and DEAD events: investigate briefly (read_output) \ -and tell the user in one or two sentences what happened, what it needs, and what you \ -suggest. For purely informational events that require no action from you or the user \ -(a session simply became READY/idle and nothing depends on it), do NOT call any tools \ -and reply with exactly `ok` — that reply is suppressed and never shown to the user. \ +changed state. For WAITING, ERROR, and DEAD events you MUST report: investigate \ +briefly (read_output) and tell the user in one or two sentences what happened, what \ +it needs, and what you suggest — never answer these with just `ok`. Only when ALL \ +events in the message are informational (READY/STARTING/THINKING/RUNNING) and nothing \ +depends on them: make no tool calls and reply with exactly `ok` — that reply is \ +suppressed and never shown to the user. \ Messages starting \ with [linkshell] are system notes.\n\ \n\ @@ -1085,14 +1108,14 @@ mod tests { OrchestratorMsg::Reset, OrchestratorMsg::UserChat("two".into()), ]; - let (text, any_user) = coalesce_batch(&msgs, &mut history); + let (text, any_user, _) = coalesce_batch(&msgs, &mut history); assert_eq!(text.as_deref(), Some("two")); assert!(any_user); assert!(history.is_empty()); // Pure reset: history wiped, nothing to send. let mut history = vec![serde_json::json!({"role": "user", "content": "old"})]; - let (text, _) = coalesce_batch(&[OrchestratorMsg::Reset], &mut history); + let (text, _, _) = coalesce_batch(&[OrchestratorMsg::Reset], &mut history); assert!(text.is_none()); assert!(history.is_empty()); @@ -1102,12 +1125,37 @@ mod tests { OrchestratorMsg::UserChat("a".into()), OrchestratorMsg::SystemNote("b".into()), ]; - let (text, any_user) = coalesce_batch(&msgs, &mut history); + let (text, any_user, _) = coalesce_batch(&msgs, &mut history); assert_eq!(text.as_deref(), Some("a\n\n[linkshell] b")); assert!(any_user); assert_eq!(history.len(), 1); } + #[test] + fn waiting_events_are_actionable_ready_events_are_not() { + let ev = |state: &str| OrchestratorMsg::SessionEvent { + session_id: 1, + name: "claude".into(), + kind: "claude".into(), + state: state.into(), + waiting_prompt: None, + tail: String::new(), + }; + let mut h = Vec::new(); + let (_, _, actionable) = coalesce_batch(&[ev("READY")], &mut h); + assert!(!actionable, "READY alone must be suppressible"); + let (_, _, actionable) = coalesce_batch(&[ev("READY"), ev("WAITING!")], &mut h); + assert!( + actionable, + "WAITING (with bar decoration) must always surface" + ); + let (_, _, actionable) = coalesce_batch(&[ev("ERROR")], &mut h); + assert!(actionable); + let (_, _, actionable) = + coalesce_batch(&[OrchestratorMsg::SystemNote("note".into())], &mut h); + assert!(actionable, "system notes must always surface"); + } + #[test] fn aging_stubs_only_old_tool_results() { let big = "x".repeat(500); diff --git a/src/patterns.rs b/src/patterns.rs index 7bd2c5d..d6958ba 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -47,8 +47,14 @@ impl PatternMatcher { .unwrap(), claude_thinking: Regex::new(r"(Thinking|Processing|Analyzing)\.\.\.|⠋|⠙|⠹|⠸").unwrap(), claude_ready: Regex::new(r"^>\s*$|Human:\s*$").unwrap(), + // Modern Claude Code renders permission prompts and questions as + // numbered option menus ("Do you want to proceed?" with a + // ❯-selected "1. Yes" / "2. Yes, and don't ask again" / "3. No" + // list) — none of which the legacy y/n markers match. The ❯ + // selector before a numbered item is required so ordinary + // numbered lists in Claude's prose don't read as dialogs. claude_waiting: Regex::new( - r"\[y/n\]|\[Y/n\]|\(yes/no\)|Press Enter|continue\?|proceed\?", + r"\[y/n\]|\[Y/n\]|\(yes/no\)|Press Enter|continue\?|proceed\?|^\s*❯\s*\d+\.\s|(?i)^\s*do you want\b|(?i)don't ask again", ) .unwrap(), codex_ready: Regex::new(r"codex>\s*$|>\s*$").unwrap(), @@ -105,12 +111,16 @@ impl PatternMatcher { } match base { BaseKind::Claude => { - if self.claude_thinking.is_match(line) { - return Some(SessionState::Thinking); - } + // Waiting before thinking, like every other kind: a dialog + // line that also carries a spinner glyph must read as + // WAITING, and dialogs are the transition the orchestrator + // and the user most need to hear about. if self.claude_waiting.is_match(line) { return Some(SessionState::Waiting); } + if self.claude_thinking.is_match(line) { + return Some(SessionState::Thinking); + } if self.claude_ready.is_match(line) { return Some(SessionState::Ready); } @@ -357,6 +367,24 @@ mod tests { matcher.infer_state("shall I proceed?", BaseKind::Claude), Some(SessionState::Waiting) ); + // Modern Claude Code dialogs: header + ❯-selected numbered options. + for line in [ + "Do you want to make this edit to main.rs?", + " ❯ 1. Yes", + " 2. Yes, and don't ask again this session", + "⠙ Do you want to proceed?", // spinner glyph must not win + ] { + assert_eq!( + matcher.infer_state(line, BaseKind::Claude), + Some(SessionState::Waiting), + "{line}" + ); + } + // Ordinary numbered lists in prose stay non-waiting. + assert_eq!( + matcher.infer_state("1. First, refactor the parser", BaseKind::Claude), + Some(SessionState::Running) + ); } #[test]