From b30214e02edf89ee820d7a0caae4c427eeb5e937 Mon Sep 17 00:00:00 2001 From: Val Alexander <68980965+BunsDev@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:08:50 -0500 Subject: [PATCH 1/2] fix(query): recover announce-then-stop stalls instead of ending the turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model sometimes announces work and then ends the turn with stop_reason end_turn and zero tool calls ("Writing the CLI uninstall module now." — then nothing). Both provider paths treated end_turn as authoritative, so the loop fired Stop hooks and returned EndTurn; the issue #149 placeholder made the empty variant visible but nothing made either variant recoverable. Users had to keep typing "continue". Mirror the max_tokens recovery pattern: when a round ends end_turn with no tool calls and its text is empty or closes on an imminent-action announcement, inject a continuation nudge as a user message and keep looping. Bounded at STALL_RECOVERY_LIMIT (2) per user turn; the counter resets whenever a tool round actually executes. The intermediate TurnComplete is suppressed while a nudge is pending, exactly like an in-budget max_tokens recovery. Deliberate handoffs (questions, "let me know…", "I'll wait for your review.") never trigger recovery, and COVEN_CODE_DISABLE_STALL_RECOVERY=1 turns the whole mechanism off. The announcement heuristic works on the final sentence-like chunk, skipping punctuation debris from dotted identifiers (`main.rs`.), and is deliberately cheap to false-positive: the nudge tells the model to state the final outcome if the work is already done, so a wrong guess costs one bounded round-trip. Observed transcripts from the stalled session are pinned as unit tests for both the stall and non-stall (completion/handoff) shapes. fixes #165 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src-rust/crates/query/src/lib.rs | 289 +++++++++++++++++++++++++++++-- 1 file changed, 278 insertions(+), 11 deletions(-) diff --git a/src-rust/crates/query/src/lib.rs b/src-rust/crates/query/src/lib.rs index e25e1ad..f48b3c4 100644 --- a/src-rust/crates/query/src/lib.rs +++ b/src-rust/crates/query/src/lib.rs @@ -789,7 +789,137 @@ const MAX_TOKENS_RECOVERY_MSG: &str = you were doing. Pick up mid-thought if that is where the cut happened. \ Break remaining work into smaller pieces."; -fn should_emit_turn_complete(stop: &str, max_tokens_recovery_count: u32) -> bool { +/// Maximum automatic continuation nudges per user turn when the model ends +/// the turn right after announcing an action it never executed (issue #165: +/// "agent announces work and just stops"). Bounded like the max_tokens +/// recovery so a model that keeps narrating can't loop forever. +const STALL_RECOVERY_LIMIT: u32 = 2; + +/// Message injected when the model ends a turn with announced-but-unexecuted +/// work (or with no output at all) and no tool calls. +const STALL_RECOVERY_MSG: &str = + "You ended the turn right after announcing an action, without executing \ + it. Do not narrate what you are about to do. If work remains, call the \ + tools and do it now; if everything is actually complete, state the \ + final outcome without announcing further actions."; + +/// Stall recovery is on by default; set COVEN_CODE_DISABLE_STALL_RECOVERY=1 +/// to restore the old end-the-turn-as-announced behavior. +fn stall_recovery_enabled() -> bool { + std::env::var_os("COVEN_CODE_DISABLE_STALL_RECOVERY").is_none() +} + +/// Heuristic for an "announce-then-stop" stall: an assistant round that +/// ended with `end_turn`, executed no tools, and whose visible text is +/// either empty or closes on an imminent-action announcement ("Writing the +/// CLI uninstall module now.") rather than a completed-work statement or a +/// deliberate handoff back to the user ("Should I…?", "Let me know…"). +/// +/// False positives are cheap by design: the recovery nudge explicitly tells +/// the model to state the final outcome if the work is already done, and +/// the whole mechanism is bounded by [`STALL_RECOVERY_LIMIT`]. +fn is_stalled_announcement(text: &str) -> bool { + let trimmed = text.trim(); + // No visible output at all is always a stall (the #149 placeholder case). + if trimmed.is_empty() { + return true; + } + + // Work on the final sentence-like chunk. Chunks with fewer than two + // words are punctuation debris from dotted identifiers (`main.rs`.), + // not sentences — skip past them. + let last_sentence = trimmed + .rsplit(['.', '!', '?', '…', '\n']) + .find(|chunk| { + chunk + .split_whitespace() + .filter(|word| word.chars().any(|c| c.is_alphabetic())) + .count() + >= 2 + }) + .unwrap_or(trimmed) + .trim() + .to_ascii_lowercase(); + + // A question mark anywhere in the tail means the model handed control + // back to the user on purpose. + let tail: String = trimmed + .chars() + .rev() + .take(240) + .collect::() + .chars() + .rev() + .collect::() + .to_ascii_lowercase(); + if tail.contains('?') { + return false; + } + + // Deliberate wait-for-user phrasings are handoffs, not stalls. + const HANDOFF_MARKERS: [&str; 10] = [ + "let me know", + "should i", + "do you", + "would you", + "if you", + "wait", + "once you", + "when you", + "your call", + "your review", + ]; + if HANDOFF_MARKERS + .iter() + .any(|marker| last_sentence.contains(marker)) + { + return false; + } + + // Imminent-action openers ("Writing the module now.", "Now running the + // tests.") and first-person intent markers ("then I'll wire it in"). + const ANNOUNCEMENT_OPENERS: [&str; 15] = [ + "writing ", + "creating ", + "implementing ", + "building ", + "adding ", + "updating ", + "editing ", + "wiring ", + "running ", + "starting ", + "proceeding ", + "next,", + "next up", + "on to ", + "time to ", + ]; + const INTENT_MARKERS: [&str; 6] = [ + "let me ", + "i'll ", + "i will ", + "going to ", + "about to ", + "now to ", + ]; + let opener = last_sentence.strip_prefix("now ").unwrap_or(&last_sentence); + ANNOUNCEMENT_OPENERS + .iter() + .any(|pattern| opener.starts_with(pattern)) + || INTENT_MARKERS + .iter() + .any(|marker| last_sentence.contains(marker)) +} + +fn should_emit_turn_complete( + stop: &str, + max_tokens_recovery_count: u32, + stall_recovery_pending: bool, +) -> bool { + if stall_recovery_pending { + return false; + } match stop { "tool_use" => false, "max_tokens" => max_tokens_recovery_count >= MAX_TOKENS_RECOVERY_LIMIT, @@ -994,6 +1124,9 @@ pub async fn run_query_loop( // Tracks how many consecutive max_tokens recoveries we've attempted so // we don't loop forever on a model that can't finish within any budget. let mut max_tokens_recovery_count: u32 = 0; + // Automatic continuation nudges used for announce-then-stop stalls this + // user turn (issue #165). Reset whenever a tool round actually executes. + let mut stall_recovery_count: u32 = 0; // Active model — may switch to fallback on overloaded errors. // Agent model override takes priority over the session model when set. let mut effective_model = selected_model_for_query(config); @@ -1683,9 +1816,39 @@ pub async fn run_query_loop( cost: None, snapshot_patch: None, }); + // Real tool work happened; give the next + // announce-then-stop stall a fresh recovery budget. + stall_recovery_count = 0; continue; // loop for next turn } + // Announce-then-stop stall detection (issue #165), before + // the end-of-turn placeholder/TurnComplete below: a round + // that executed no tools and whose text is empty or ends + // on an imminent-action announcement gets a bounded + // continuation nudge instead of ending the turn. + if stall_recovery_enabled() + && stall_recovery_count < STALL_RECOVERY_LIMIT + && is_stalled_announcement(&combined_text) + { + stall_recovery_count += 1; + warn!( + attempt = stall_recovery_count, + limit = STALL_RECOVERY_LIMIT, + provider = %provider_id_str, + "end_turn with announced-but-unexecuted work — injecting continuation nudge" + ); + if let Some(ref tx) = event_tx { + let _ = tx.send(QueryEvent::Status(format!( + "Model announced work but ended the turn — nudging it to \ + continue (attempt {}/{})", + stall_recovery_count, STALL_RECOVERY_LIMIT + ))); + } + messages.push(Message::user(STALL_RECOVERY_MSG)); + continue; + } + // End turn — notify TUI and return. // Issue #149 follow-up: providers occasionally end the // turn after a tool round without emitting any text or @@ -2035,7 +2198,18 @@ pub async fn run_query_loop( } } - if should_emit_turn_complete(stop, max_tokens_recovery_count) { + // Announce-then-stop stall detection (issue #165): an `end_turn` + // round with no tool calls whose text is empty or closes on an + // imminent-action announcement gets a bounded continuation nudge + // instead of ending the turn. Decided before TurnComplete emission + // so recovered rounds stay invisible, like max_tokens recovery. + let stall_recovery_pending = stop == "end_turn" + && stall_recovery_enabled() + && stall_recovery_count < STALL_RECOVERY_LIMIT + && assistant_msg.get_tool_use_blocks().is_empty() + && is_stalled_announcement(&assistant_msg.get_all_text()); + + if should_emit_turn_complete(stop, max_tokens_recovery_count, stall_recovery_pending) { if let Some(ref tx) = event_tx { let _ = tx.send(QueryEvent::TurnComplete { turn, @@ -2068,6 +2242,27 @@ pub async fn run_query_loop( match stop { "end_turn" => { + if stall_recovery_pending { + stall_recovery_count += 1; + warn!( + attempt = stall_recovery_count, + limit = STALL_RECOVERY_LIMIT, + "end_turn with announced-but-unexecuted work — injecting continuation nudge" + ); + if let Some(ref tx) = event_tx { + let _ = tx.send(QueryEvent::Status(format!( + "Model announced work but ended the turn — nudging it to continue \ + (attempt {}/{})", + stall_recovery_count, STALL_RECOVERY_LIMIT + ))); + } + // The stalled assistant message is already in the history, + // so the nudge reads in context. Stop hooks are not fired: + // the turn is not over. + messages.push(Message::user(STALL_RECOVERY_MSG)); + continue; + } + fire_stop_hook!(assistant_msg); // T1-3: Fire Stop hooks in background (fire-and-forget). @@ -2242,6 +2437,9 @@ pub async fn run_query_loop( // A completed tool-use turn counts as a successful recovery // boundary; reset the max_tokens retry counter. max_tokens_recovery_count = 0; + // Real tool work happened, so any earlier announce-then-stop + // stall resolved itself; give the next stall a fresh budget. + stall_recovery_count = 0; // Extract tool calls and execute them let tool_blocks = assistant_msg.get_tool_use_blocks(); if tool_blocks.is_empty() { @@ -3018,25 +3216,94 @@ mod tests { #[test] fn turn_complete_emission_skips_intermediate_tool_turns() { - assert!(!should_emit_turn_complete("tool_use", 0)); + assert!(!should_emit_turn_complete("tool_use", 0, false)); } #[test] fn turn_complete_emission_skips_recoverable_max_tokens_turns() { - assert!(!should_emit_turn_complete("max_tokens", 0)); - assert!(!should_emit_turn_complete("max_tokens", 1)); - assert!(!should_emit_turn_complete("max_tokens", 2)); + assert!(!should_emit_turn_complete("max_tokens", 0, false)); + assert!(!should_emit_turn_complete("max_tokens", 1, false)); + assert!(!should_emit_turn_complete("max_tokens", 2, false)); assert!(should_emit_turn_complete( "max_tokens", - MAX_TOKENS_RECOVERY_LIMIT + MAX_TOKENS_RECOVERY_LIMIT, + false )); } #[test] fn turn_complete_emission_keeps_terminal_stop_reasons() { - assert!(should_emit_turn_complete("end_turn", 0)); - assert!(should_emit_turn_complete("stop_sequence", 0)); - assert!(should_emit_turn_complete("content_filtered", 0)); - assert!(should_emit_turn_complete("unknown_stop", 0)); + assert!(should_emit_turn_complete("end_turn", 0, false)); + assert!(should_emit_turn_complete("stop_sequence", 0, false)); + assert!(should_emit_turn_complete("content_filtered", 0, false)); + assert!(should_emit_turn_complete("unknown_stop", 0, false)); + } + + #[test] + fn turn_complete_emission_skips_stall_recovery_rounds() { + // While a stall nudge is pending the turn is not over, exactly like + // an in-budget max_tokens recovery round. + assert!(!should_emit_turn_complete("end_turn", 0, true)); + assert!(should_emit_turn_complete("end_turn", 0, false)); + } + + #[test] + fn stalled_announcement_matches_observed_stall_transcripts() { + // Real assistant texts from the issue #165 session that ended with + // end_turn, zero tool calls, and no follow-through. + assert!(is_stalled_announcement("")); + assert!(is_stalled_announcement(" \n")); + assert!(is_stalled_announcement( + "Writing the CLI uninstall module now." + )); + assert!(is_stalled_announcement( + "Now writing the CLI uninstall module." + )); + assert!(is_stalled_announcement( + "Enough exploration — I have the wiring points. Writing the CLI \ + command now. First the self-contained module with unit-tested \ + pure helpers, then I'll wire it into `main.rs`." + )); + assert!(is_stalled_announcement( + "Implementing now. Let me confirm available deps, then write the \ + CLI uninstall module." + )); + assert!(is_stalled_announcement( + "I'll start by reading the manifest, then patch the loader." + )); + } + + #[test] + fn stalled_announcement_ignores_completed_work_statements() { + assert!(!is_stalled_announcement( + "Done — the module is wired in and all tests pass." + )); + assert!(!is_stalled_announcement( + "The uninstall command is implemented and tested; docs updated." + )); + // "now" inside a completion statement is not an announcement. + assert!(!is_stalled_announcement("All 45 tests pass now.")); + } + + #[test] + fn stalled_announcement_respects_deliberate_handoffs() { + assert!(!is_stalled_announcement("Should I also update the docs?")); + assert!(!is_stalled_announcement( + "Let me know if you want the Windows path covered too." + )); + assert!(!is_stalled_announcement("I'll wait for your review.")); + assert!(!is_stalled_announcement( + "Two options: (1) uninstall module in the CLI, (2) Cave-side \ + cleanup. Which do you want?" + )); + assert!(!is_stalled_announcement( + "Waiting on your decision before touching main.rs." + )); + } + + #[test] + fn stall_recovery_disable_env_is_read() { + // Not set in the test environment → enabled by default. + assert!(stall_recovery_enabled()); } } From 6792a98a668d6f953f3e31ed49c0c747f0f98556 Mon Sep 17 00:00:00 2001 From: Val Alexander <68980965+BunsDev@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:21:56 -0500 Subject: [PATCH 2/2] fix(query): honor documented =1 contract for the stall-recovery kill switch Review follow-up: stall_recovery_enabled() checked env presence, so COVEN_CODE_DISABLE_STALL_RECOVERY=0 also disabled recovery. Parse the value with feature_gates::is_env_truthy so only 1/true/yes/on disable, matching the documented contract and the repo's gate convention. Refs #165 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src-rust/crates/query/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src-rust/crates/query/src/lib.rs b/src-rust/crates/query/src/lib.rs index f48b3c4..e982231 100644 --- a/src-rust/crates/query/src/lib.rs +++ b/src-rust/crates/query/src/lib.rs @@ -804,9 +804,13 @@ const STALL_RECOVERY_MSG: &str = final outcome without announcing further actions."; /// Stall recovery is on by default; set COVEN_CODE_DISABLE_STALL_RECOVERY=1 -/// to restore the old end-the-turn-as-announced behavior. +/// (or true/yes/on) to restore the old end-the-turn-as-announced behavior. fn stall_recovery_enabled() -> bool { - std::env::var_os("COVEN_CODE_DISABLE_STALL_RECOVERY").is_none() + !claurst_core::feature_gates::is_env_truthy( + std::env::var("COVEN_CODE_DISABLE_STALL_RECOVERY") + .ok() + .as_deref(), + ) } /// Heuristic for an "announce-then-stop" stall: an assistant round that