feat(turn): halt a turn stuck repeating one identical tool call
Ports grok-build's action stationarity breaker. A polling loop looks like progress from inside the turn — every step succeeds — so nothing stops it and it burns the whole budget re-reading one file. Nudge at 8 identical batches, halt at 16, halt a pure no-op run at 4. Three defects were found and fixed before this landed, none of which the gates caught: - The check runs AFTER execute_tool_calls, not before. Before it, the assistant's tool_calls are recorded with no results, so pushing the nudge triggers repair_dangling_tool_calls: synthetic "cancelled by the user" results get spliced in, the real results land after a user message, and dedup misses them — the orphan tool_result shape that draws a provider 400. It is Ok-gated too, since one `?` inside can leave a call resultless. - `TurnOutcome::StationarityHalted` is its own variant grouped with Completed, not a reuse of Cancelled. As a cancellation it would report StopReason::Cancelled, fire the abort lifecycle, kill the turn's subagents, grow the goal harness back-off streak until three halts paused a running goal, and let completion-requirement recovery re-run the very turn halted for looping. - The no-op gate is size-based, not name-based. kigi registers the shell tool as `run_terminal_cmd` and renames it `run_terminal_command`, so matching on "bash" silently disabled the tighter ceiling while every test still passed.
This commit is contained in:
1 parent
a07d889ad9
commit
ac23ebc9a1
8 files changed
+429
-5
No files matched your search
@@ -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
|
||||
}
|
||||
|
||||
@@ -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<Self>,
|
||||
run: &mut IdenticalToolCallRun,
|
||||
tool_calls: &[kigi_sampling_types::conversation::ToolCall],
|
||||
loop_index: usize,
|
||||
) -> Option<StationarityHalt> {
|
||||
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::<serde_json::Value>(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,
|
||||
}
|
||||
@@ -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<String> = 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<kigi_sampling_types::conversation::ToolCall> = 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<ToolCallResponse> = 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
|
||||
|
||||
@@ -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<Option<TurnDeltaSnapshot>>,
|
||||
tools_called: Vec<String>,
|
||||
tool_name: String,
|
||||
run_len: u32,
|
||||
true_noop: bool,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
||||
Reference in new issue
Block a user