diff --git a/crates/codegen/kigi-shell/src/session/acp_session.rs b/crates/codegen/kigi-shell/src/session/acp_session.rs index a3dc2b4..2f74f85 100644 --- a/crates/codegen/kigi-shell/src/session/acp_session.rs +++ b/crates/codegen/kigi-shell/src/session/acp_session.rs @@ -121,6 +121,8 @@ mod model_switch; mod prompt_queue; #[path = "acp_session_impl/slash_exec.rs"] mod slash_exec; +#[path = "acp_session_impl/stationarity_seam.rs"] +mod stationarity_seam; #[path = "acp_session_impl/swarm.rs"] mod swarm; use super::PromptOrigin; diff --git a/crates/codegen/kigi-shell/src/session/acp_session_impl/hook_dispatch.rs b/crates/codegen/kigi-shell/src/session/acp_session_impl/hook_dispatch.rs index 8641e27..98c1641 100644 --- a/crates/codegen/kigi-shell/src/session/acp_session_impl/hook_dispatch.rs +++ b/crates/codegen/kigi-shell/src/session/acp_session_impl/hook_dispatch.rs @@ -8,7 +8,12 @@ pub(super) fn turn_result_to_hook_outcome( ) -> kigi_tool_protocol::turn_hook::TurnHookOutcome { use kigi_tool_protocol::turn_hook::TurnHookOutcome; match result { - Ok(TurnOutcome::Completed { .. }) => TurnHookOutcome::Completed, + // A stationarity halt is a completed turn for hook purposes: nobody + // cancelled it, and a `Cancelled` outcome would tell every Stop hook + // the user interrupted the model. + Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityHalted { .. }) => { + TurnHookOutcome::Completed + } Ok(TurnOutcome::Cancelled { .. }) | Ok(TurnOutcome::MaxTurnsReached { .. }) => { TurnHookOutcome::Cancelled } diff --git a/crates/codegen/kigi-shell/src/session/acp_session_impl/stationarity_seam.rs b/crates/codegen/kigi-shell/src/session/acp_session_impl/stationarity_seam.rs new file mode 100644 index 0000000..5425c13 --- /dev/null +++ b/crates/codegen/kigi-shell/src/session/acp_session_impl/stationarity_seam.rs @@ -0,0 +1,120 @@ +//! Turn-loop seam for the stationarity detector. + +use super::*; +use crate::session::stationarity::{ + IdenticalToolCallRun, NUDGE_AFTER_IDENTICAL_TOOL_CALLS, command_is_true, hash_batch, +}; + +/// Sent once, naming the blocking wait a poller should use. +const STATIONARITY_NUDGE: &str = "\ +You have called the same tool with the same arguments repeatedly and are in a \ +polling loop. Stop repeating that call. If you are waiting on a background \ +task, block on it with the wait-tasks tool instead of re-checking its output; \ +if you are waiting on anything else, sleep once and check once. If you cannot \ +make progress, stop and tell the user what you are waiting for. This turn will \ +be halted automatically if the identical call keeps repeating."; + +impl SessionActor { + /// Enforces the ceilings; `Some` halts the turn. Silent but logged. + pub(crate) async fn observe_tool_call_stationarity( + self: &Arc, + run: &mut IdenticalToolCallRun, + tool_calls: &[kigi_sampling_types::conversation::ToolCall], + loop_index: usize, + ) -> Option { + let batch_hash = hash_batch( + tool_calls + .iter() + .map(|tc| (tc.name.as_str(), tc.arguments.as_ref())), + ); + let tool_name = tool_calls + .first() + .map(|tc| tc.name.clone()) + .unwrap_or_default(); + let is_true_noop = self.is_run_true_step(tool_calls).await; + let run_len = run.observe(batch_hash, &tool_name, is_true_noop); + + if run_len == NUDGE_AFTER_IDENTICAL_TOOL_CALLS { + tracing::warn!( + tool_name = %run.tool_name(), + run_len, + loop_index, + "action stationarity: nudging a repeating tool call" + ); + kigi_log::unified_log::warn( + "shell.turn.action_stationarity_nudge", + Some(self.session_info.id.0.as_ref()), + Some(serde_json::json!({ + "tool_name": run.tool_name(), + "run_len": run_len, + "loop_index": loop_index, + })), + ); + self.push_system_reminder(STATIONARITY_NUDGE); + } + + if run_len < run.hard_stop_threshold() { + return None; + } + + tracing::warn!( + tool_name = %run.tool_name(), + run_len, + loop_index, + true_noop = run.is_true_noop_run(), + "action stationarity: halting the turn" + ); + kigi_log::unified_log::warn( + "shell.turn.action_stationarity_stop", + Some(self.session_info.id.0.as_ref()), + Some(serde_json::json!({ + "tool_name": run.tool_name(), + "run_len": run_len, + "loop_index": loop_index, + "true_noop": run.is_true_noop_run(), + })), + ); + Some(StationarityHalt { + tool_name: run.tool_name().to_string(), + run_len, + true_noop: run.is_true_noop_run(), + }) + } + + /// Whether this batch is a single shell call that does nothing. + /// + /// Size-gated, not name-gated: the shell tool is renamed + /// `run_terminal_command`, so a name gate would silently disable this. + /// Parse failure fails open; multi-call batches use the ordinary ceiling. + async fn is_run_true_step( + &self, + tool_calls: &[kigi_sampling_types::conversation::ToolCall], + ) -> bool { + /// Bound on `{"command":"true"}` plus sibling fields. + const MAX_NOOP_ARGS_BYTES: usize = 512; + + let [tc] = tool_calls else { + return false; + }; + if tc.arguments.as_ref().len() > MAX_NOOP_ARGS_BYTES { + return false; + } + let Ok(args) = serde_json::from_str::(tc.arguments.as_ref()) else { + return false; + }; + let Ok(input) = self.tool_bridge_handle().try_parse(&tc.name, args).await else { + return false; + }; + matches!( + input, + kigi_tools::types::tool_io::ToolInput::Bash(ref b) if command_is_true(&b.command) + ) + } +} + +/// Why a turn was halted, carried to its outcome. +pub(crate) struct StationarityHalt { + pub(crate) tool_name: String, + pub(crate) run_len: u32, + pub(crate) true_noop: bool, +} diff --git a/crates/codegen/kigi-shell/src/session/acp_session_impl/turn.rs b/crates/codegen/kigi-shell/src/session/acp_session_impl/turn.rs index 102c03f..746837f 100644 --- a/crates/codegen/kigi-shell/src/session/acp_session_impl/turn.rs +++ b/crates/codegen/kigi-shell/src/session/acp_session_impl/turn.rs @@ -790,7 +790,7 @@ impl SessionActor { let turn_tool_count = self.events.tool_count_this_turn(); let bridge_outcome = turn_result_to_hook_outcome(&result); match &result { - Ok(TurnOutcome::Completed { .. }) => { + Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityHalted { .. }) => { self.emit_turn_ended( crate::session::events::TurnOutcomeLabel::Completed, None, @@ -880,7 +880,9 @@ impl SessionActor { let doom_tally = std::mem::take(&mut *self.doom_loop_turn_tally.lock()); doom_tally.fired(); let stop_reason_str = match &result { - Ok(TurnOutcome::Completed { .. }) => "end_turn", + Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityHalted { .. }) => { + "end_turn" + } Ok(TurnOutcome::Cancelled { .. }) | Ok(TurnOutcome::MaxTurnsReached { .. }) => { "cancelled" } @@ -896,7 +898,7 @@ impl SessionActor { ) .await; match &result { - Ok(TurnOutcome::Completed { .. }) => { + Ok(TurnOutcome::Completed { .. }) | Ok(TurnOutcome::StationarityHalted { .. }) => { for contributor in self.extension_registry.turn_lifecycle_contributors() { contributor .on_turn_done(&kigi_agent_lifecycle::TurnDoneInput) @@ -981,6 +983,26 @@ impl SessionActor { PromptCompletionKind::MaxTurnsReached { limit }, None, ), + // `EndTurn`, not `Cancelled`: the client must not render + // an interrupted turn for something nobody interrupted. + // The halt's detail rides `completion_kind`, which is + // where a bug report can still read it. + TurnOutcome::StationarityHalted { + snapshot, + tool_name, + run_len, + true_noop, + .. + } => ( + acp::StopReason::EndTurn, + *snapshot, + PromptCompletionKind::StationarityHalted { + tool_name, + run_len, + true_noop, + }, + None, + ), }; if let Some(snapshot) = snapshot.as_mut() { self.apply_prompt_modes_to_snapshot(snapshot); @@ -1282,7 +1304,11 @@ impl SessionActor { let mut result = self .process_conversation_turn(req_id, json_schema.clone()) .await; - if matches!(result, Ok(TurnOutcome::MaxTurnsReached { .. })) { + // Harness stopped the turn; retrying re-enters the same wall. + if matches!( + result, + Ok(TurnOutcome::MaxTurnsReached { .. }) | Ok(TurnOutcome::StationarityHalted { .. }) + ) { return result; } if let Ok(TurnOutcome::Completed { @@ -1599,6 +1625,10 @@ impl SessionActor { self.record_turn_model().await; let mut metrics_drop_guard = TurnMetrics::new(); let mut turn_tools_called: Vec = Vec::new(); + let mut identical_tool_calls = + crate::session::stationarity::IdenticalToolCallRun::default(); + // Retained across execute: observed only after results land. + let mut last_batch: Vec = Vec::new(); let mut tool_turn_count: usize = 1; let mut loop_index: u32 = 0; let mut todo_gate_fires: u32 = 0; @@ -2015,6 +2045,8 @@ impl SessionActor { } turn_tools_called.push(tc.name.clone()); } + last_batch.clear(); + last_batch.extend(tool_calls.iter().cloned()); let tool_call_responses: Vec = tool_calls .into_iter() .map(|tc| ToolCallResponse { @@ -2057,6 +2089,34 @@ impl SessionActor { } _ => {} } + // After execute: every call has a result, so nothing dangles + // and the nudge cannot trigger the "cancelled" repair. + // Ok-gated: one `?` inside can leave a call resultless. + if execute_tool_calls_result.is_ok() + && let Some(halt) = self + .observe_tool_call_stationarity( + &mut identical_tool_calls, + &last_batch, + tool_turn_count, + ) + .await + { + let snapshot = self + .finalize_turn_bookkeeping( + req_id, + conv_turn_start, + &turn_span_totals, + model_fingerprint.clone(), + ) + .await; + return Ok(TurnOutcome::StationarityHalted { + snapshot: Box::new(snapshot), + tools_called: std::mem::take(&mut turn_tools_called), + tool_name: halt.tool_name, + run_len: halt.run_len, + true_noop: halt.true_noop, + }); + } let next_turn = tool_turn_count + 1; if let Some(limit) = self.max_turns && next_turn > limit diff --git a/crates/codegen/kigi-shell/src/session/acp_session_impl/types.rs b/crates/codegen/kigi-shell/src/session/acp_session_impl/types.rs index 6590044..e8068bc 100644 --- a/crates/codegen/kigi-shell/src/session/acp_session_impl/types.rs +++ b/crates/codegen/kigi-shell/src/session/acp_session_impl/types.rs @@ -62,6 +62,19 @@ pub(crate) enum TurnOutcome { }, /// The `--max-turns` limit was reached after a tool-execution cycle. MaxTurnsReached { limit: usize }, + /// One tool call repeated past its ceiling; the turn was halted. + /// + /// Groups with [`Self::Completed`], NOT [`Self::Cancelled`]: nobody + /// cancelled anything. As a cancellation it would report + /// `StopReason::Cancelled`, fire the abort lifecycle, kill the turn's + /// subagents, grow the goal back-off streak, and let recovery re-run it. + StationarityHalted { + snapshot: Box>, + tools_called: Vec, + tool_name: String, + run_len: u32, + true_noop: bool, + }, } #[derive(Debug)] diff --git a/crates/codegen/kigi-shell/src/session/commands.rs b/crates/codegen/kigi-shell/src/session/commands.rs index 908ff7a..e809a10 100644 --- a/crates/codegen/kigi-shell/src/session/commands.rs +++ b/crates/codegen/kigi-shell/src/session/commands.rs @@ -29,6 +29,13 @@ pub enum PromptCompletionKind { MaxTurnsReached { limit: usize, }, + /// One tool call repeated past its ceiling. Reported as `EndTurn`: + /// the turn ended, nobody interrupted it. + StationarityHalted { + tool_name: String, + run_len: u32, + true_noop: bool, + }, Rewound, /// A queued prompt was removed (or cleared) from the server-authoritative /// queue before it ever ran. Used to resolve the still-pending diff --git a/crates/codegen/kigi-shell/src/session/mod.rs b/crates/codegen/kigi-shell/src/session/mod.rs index 8c9c6a8..087dc27 100644 --- a/crates/codegen/kigi-shell/src/session/mod.rs +++ b/crates/codegen/kigi-shell/src/session/mod.rs @@ -326,6 +326,7 @@ pub mod restore; pub mod result; pub mod signals; pub(crate) mod slash_commands; +pub(crate) mod stationarity; pub mod storage; pub(crate) mod streaming_capture; pub(crate) mod summary; diff --git a/crates/codegen/kigi-shell/src/session/stationarity.rs b/crates/codegen/kigi-shell/src/session/stationarity.rs new file mode 100644 index 0000000..3219468 --- /dev/null +++ b/crates/codegen/kigi-shell/src/session/stationarity.rs @@ -0,0 +1,216 @@ +//! Detects a model stuck repeating one identical tool call. + +/// Consecutive identical batches after which the turn is halted. +pub(crate) const MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS: u32 = 16; + +/// Nudge threshold; half the budget remains after it. +pub(crate) const NUDGE_AFTER_IDENTICAL_TOOL_CALLS: u32 = 8; + +/// Below the nudge on purpose: no-op runs get none. +pub(crate) const MAX_CONSECUTIVE_TRUE_NOOPS: u32 = 4; + +const _: () = assert!(NUDGE_AFTER_IDENTICAL_TOOL_CALLS < MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS); +const _: () = assert!(MAX_CONSECUTIVE_TRUE_NOOPS < NUDGE_AFTER_IDENTICAL_TOOL_CALLS); + +/// A shell command that does nothing whatsoever. +pub(crate) fn command_is_true(cmd: &str) -> bool { + cmd.trim().eq_ignore_ascii_case("true") +} + +/// Hashes name+args per call; separators prevent concatenation collisions. +pub(crate) fn hash_batch<'a>(calls: impl IntoIterator) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + for (name, args) in calls { + name.hash(&mut hasher); + '\u{1f}'.hash(&mut hasher); + args.hash(&mut hasher); + '\u{1e}'.hash(&mut hasher); + } + hasher.finish() +} + +/// One identity, so re-spelling a no-op cannot reset it. +const TRUE_NOOP_HASH: u64 = u64::MAX; + +/// How many times the current batch has repeated unchanged. +#[derive(Default)] +pub(crate) struct IdenticalToolCallRun { + /// Hash only: signatures hold raw tool arguments. + last_hash: Option, + tool_name: String, + run_len: u32, + is_true_noop_run: bool, +} + +impl IdenticalToolCallRun { + /// Records one batch and returns the length of the run it belongs to. + pub(crate) fn observe(&mut self, batch_hash: u64, tool_name: &str, is_true_noop: bool) -> u32 { + let hash = if is_true_noop { + TRUE_NOOP_HASH + } else { + batch_hash + }; + if self.last_hash == Some(hash) { + self.run_len += 1; + } else { + self.run_len = 1; + self.last_hash = Some(hash); + self.is_true_noop_run = is_true_noop; + } + self.tool_name = tool_name.to_string(); + self.run_len + } + + /// The run length at which this turn must be halted. + pub(crate) fn hard_stop_threshold(&self) -> u32 { + if self.is_true_noop_run { + MAX_CONSECUTIVE_TRUE_NOOPS + } else { + MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS + } + } + + pub(crate) fn tool_name(&self) -> &str { + &self.tool_name + } + + pub(crate) fn is_true_noop_run(&self) -> bool { + self.is_true_noop_run + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn call(name: &str, args: &str) -> u64 { + hash_batch([(name, args)]) + } + + #[test] + fn an_unchanged_batch_accumulates_a_run() { + let mut run = IdenticalToolCallRun::default(); + let h = call("read_file", "a.rs"); + assert_eq!(run.observe(h, "read_file", false), 1); + assert_eq!(run.observe(h, "read_file", false), 2); + assert_eq!(run.observe(h, "read_file", false), 3); + } + + #[test] + fn any_change_in_the_arguments_restarts_the_run() { + let mut run = IdenticalToolCallRun::default(); + run.observe(call("read_file", "a.rs"), "read_file", false); + run.observe(call("read_file", "a.rs"), "read_file", false); + assert_eq!( + run.observe(call("read_file", "b.rs"), "read_file", false), + 1, + "a different argument is a different action" + ); + } + + #[test] + fn two_batches_cannot_collide_by_concatenation() { + // Unseparated these would hash identically. + assert_ne!( + hash_batch([("ab", "cd")]), + hash_batch([("a", "b"), ("c", "d")]) + ); + } + + #[test] + fn no_ops_share_one_run_however_they_are_spelled() { + let mut run = IdenticalToolCallRun::default(); + assert_eq!(run.observe(call("bash", "true"), "bash", true), 1); + assert_eq!( + run.observe(call("bash", " TRUE "), "bash", true), + 2, + "re-spelling a no-op must not reset the tighter ceiling" + ); + assert_eq!(run.hard_stop_threshold(), MAX_CONSECUTIVE_TRUE_NOOPS); + } + + #[test] + fn a_real_call_after_a_noop_run_restores_the_ordinary_ceiling() { + let mut run = IdenticalToolCallRun::default(); + run.observe(call("bash", "true"), "bash", true); + assert_eq!(run.hard_stop_threshold(), MAX_CONSECUTIVE_TRUE_NOOPS); + run.observe(call("read_file", "a.rs"), "read_file", false); + assert_eq!( + run.hard_stop_threshold(), + MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS + ); + } + + #[test] + fn an_ordinary_repeated_call_is_halted_at_its_ceiling_and_not_before() { + let mut run = IdenticalToolCallRun::default(); + let h = call("read_file", "a.rs"); + for expected in 1..MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS { + let len = run.observe(h, "read_file", false); + assert_eq!(len, expected); + assert!(len < run.hard_stop_threshold(), "must not halt early"); + } + assert_eq!( + run.observe(h, "read_file", false), + run.hard_stop_threshold(), + "the turn halts on the 16th identical call" + ); + } + + #[test] + fn a_repeated_no_op_is_halted_far_sooner_and_without_a_nudge() { + let mut run = IdenticalToolCallRun::default(); + let h = call("bash", "true"); + let mut len = 0; + while len < run.hard_stop_threshold() { + len = run.observe(h, "bash", true); + } + assert_eq!(len, MAX_CONSECUTIVE_TRUE_NOOPS); + assert!( + len < NUDGE_AFTER_IDENTICAL_TOOL_CALLS, + "documented: a no-op run is halted before any nudge could fire" + ); + } + + #[test] + fn the_nudge_lands_with_budget_left_to_act_on_it() { + let mut run = IdenticalToolCallRun::default(); + let h = call("read_file", "a.rs"); + let mut len = 0; + while len < NUDGE_AFTER_IDENTICAL_TOOL_CALLS { + len = run.observe(h, "read_file", false); + } + assert!( + len < run.hard_stop_threshold(), + "a warning the model cannot act on is not a warning" + ); + } + + #[test] + fn work_interleaved_with_repeats_is_never_halted() { + let mut run = IdenticalToolCallRun::default(); + for i in 0..MAX_CONSECUTIVE_IDENTICAL_TOOL_CALLS * 2 { + let h = if i % 2 == 0 { + call("read_file", "a.rs") + } else { + call("read_file", "b.rs") + }; + let len = run.observe(h, "read_file", false); + assert!( + len < run.hard_stop_threshold(), + "alternating calls are progress, not a loop" + ); + } + } + + #[test] + fn only_a_bare_true_counts_as_a_no_op() { + assert!(command_is_true("true")); + assert!(command_is_true(" true ")); + assert!(command_is_true("TRUE")); + assert!(!command_is_true("true && make")); + assert!(!command_is_true("truely")); + assert!(!command_is_true("")); + } +}