From c2332e85987e8acf30390a126272b662d07317c4 Mon Sep 17 00:00:00 2001 From: David Date: Sat, 25 Jul 2026 16:34:51 -0700 Subject: [PATCH 1/2] fix(cli): never block startup on optional live model metadata Startup awaited a provider model-listing probe that can hang indefinitely. The agent now always starts immediately with conservative limits; the optional live metadata refresh is skipped entirely, since applying it later would require safely reconfiguring the already-built agent. Co-Authored-By: Claude Fable 5 --- crates/hi-cli/src/main.rs | 37 ++++++++++++++++------------------- crates/hi-cli/src/provider.rs | 6 ++++++ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/crates/hi-cli/src/main.rs b/crates/hi-cli/src/main.rs index 292f7fe..7c4b9d3 100644 --- a/crates/hi-cli/src/main.rs +++ b/crates/hi-cli/src/main.rs @@ -66,8 +66,8 @@ use landing::{effective_prompt, print_landing, profile_infos, resolve_session}; use orchestration::{build_sync_config, run_best_of, run_hf_cli, run_mcp_command}; use project_context::auto_memory_enabled; use provider::{ - LiveModelMetadata, build_chain, default_skeptic_model, effective_max_tokens_for_model, - provider_label, resolve_live_model_metadata, + build_chain, default_skeptic_model, effective_max_tokens_for_model, provider_label, + startup_live_model_metadata, }; use repl::repl; use report::{ @@ -324,14 +324,10 @@ async fn run() -> Result<()> { let provider = rsi_bundle.provider; let rsi_control = rsi_bundle.rsi_control; let rsi_remote_switch = rsi_bundle.rsi_remote_switch; - let live_metadata = if settings.provider == ProviderName::Pipenetwork { - resolve_live_model_metadata(provider.as_ref(), &settings.model).await - } else { - LiveModelMetadata { - context_window: None, - max_output_tokens: None, - } - }; + // Optional provider metadata must not delay startup. The agent begins with + // conservative limits; applying a later refresh would require safely + // reconfiguring the already-built agent. + let live_metadata = startup_live_model_metadata(); let max_tokens = effective_max_tokens_for_model(&settings, live_metadata.max_output_tokens); rsi_bootstrap::bind_managed_effective( rsi.managed_runtime.as_ref(), @@ -1334,8 +1330,7 @@ mod tests { use crate::landing::write_landing; use crate::project_context::{auto_memory_enabled, memory_context}; use crate::provider::{ - default_skeptic_model, effective_max_tokens_for_model, - resolve_live_model_metadata_with_timeout, + default_skeptic_model, effective_max_tokens_for_model, startup_live_model_metadata, }; use crate::report::{ one_shot_exit_code, report_tool_records, report_verification_stages, @@ -1393,18 +1388,20 @@ mod tests { } #[tokio::test] - async fn hanging_optional_model_metadata_cannot_stall_startup() { + async fn hanging_optional_model_metadata_cannot_delay_startup_preparation() { + let provider = HangingModelListProvider; + let mut discovery = std::pin::pin!(provider.list_models()); + assert!(matches!( + futures_util::poll!(&mut discovery), + std::task::Poll::Pending + )); + let started = std::time::Instant::now(); - let metadata = resolve_live_model_metadata_with_timeout( - &HangingModelListProvider, - "test-model", - std::time::Duration::from_millis(20), - ) - .await; + let metadata = startup_live_model_metadata(); assert_eq!(metadata.context_window, None); assert_eq!(metadata.max_output_tokens, None); - assert!(started.elapsed() < std::time::Duration::from_secs(1)); + assert!(started.elapsed() < std::time::Duration::from_millis(50)); } #[test] diff --git a/crates/hi-cli/src/provider.rs b/crates/hi-cli/src/provider.rs index 5036ee0..8d0569f 100644 --- a/crates/hi-cli/src/provider.rs +++ b/crates/hi-cli/src/provider.rs @@ -148,6 +148,12 @@ pub(crate) struct LiveModelMetadata { pub(crate) max_output_tokens: Option, } +/// Metadata used while preparing startup. Live discovery is deliberately not +/// polled here because it is optional tuning and may hang indefinitely. +pub(crate) fn startup_live_model_metadata() -> LiveModelMetadata { + LiveModelMetadata::default() +} + pub(crate) fn effective_max_tokens_for_model( settings: &Settings, advertised_max_output_tokens: Option, From 1e11df79d5c58d5364ca8f2aa96b99b7a23d620b Mon Sep 17 00:00:00 2001 From: David Date: Sat, 25 Jul 2026 16:35:06 -0700 Subject: [PATCH 2/2] =?UTF-8?q?feat:=20stop=20punishing=20long-running=20w?= =?UTF-8?q?ork=20=E2=80=94=20no=20implicit=20step=20limit,=20cheap=20backg?= =?UTF-8?q?round=20waits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driven by real transcripts (sol-view sessions): one turn babysitting two large downloads burned 200 model rounds / 14M input tokens (383M in an earlier session) — nearly all of it instant polls of still-running processes, plus 85 plan-continue nudges overriding the model's honest "work remains in progress" status answers. Step limit: - Remove the implicit intent-based per-turn caps (80/120/200). Turns are uncapped unless --max-steps / `/config steps` sets one; runaway loops are ended by the repeat/no-progress/stall budgets instead. A misclassified intent can no longer kill a productive turn early. - Remove the dead max_steps_explicit flag; internal subagent budgets (explore 10, background tasks 10/20) become real caps as written. `/config steps auto` is now an alias for `off`. - A capped turn gets ONE tool-free wrap-up round to report where the work stands before stopping (still Incomplete · StepLimit). Background waiting: - bash_output gains wait_secs (max 600): blocks server-side until new output or process exit — one waiting call replaces a poll loop, and a kill wakes it immediately. - Waiting detection keys on the process lifecycle, not output novelty: a download progress bar no longer reads as progress. After three consecutive waiting rounds the model is steered to wait once with wait_secs or wrap up; continued polling forces a tool-free status answer. Subsumes the byte-identical idle-poll special case. - A status answer ends a waiting turn even with an incomplete plan — the plan-continue nudge no longer ping-pongs against polls. - Superseded bash_output results fold to a one-line digest, so a long watch stops re-sending stale progress dumps with every request. - Tool descriptions and nudges steer toward wait_secs and chaining follow-ups in-shell (fetch && convert) instead of babysitting. Co-Authored-By: Claude Fable 5 --- README.md | 9 +- crates/hi-agent/src/agent/explore_turn.rs | 1 - crates/hi-agent/src/agent/lifecycle.rs | 14 +- crates/hi-agent/src/agent/turn/helpers.rs | 80 ++---- crates/hi-agent/src/agent/turn/loop_.rs | 7 +- crates/hi-agent/src/agent/turn/model_round.rs | 54 +++- crates/hi-agent/src/agent/turn/progress.rs | 22 ++ crates/hi-agent/src/agent/turn/state.rs | 1 + .../src/agent/turn/steer/implementation.rs | 75 +++-- .../hi-agent/src/agent/turn/steer/review.rs | 41 ++- crates/hi-agent/src/agent/turn/tools/batch.rs | 115 ++++++-- crates/hi-agent/src/command.rs | 6 +- crates/hi-agent/src/config.rs | 16 +- crates/hi-agent/src/domain.rs | 3 + crates/hi-agent/src/lib.rs | 10 +- crates/hi-agent/src/steering/constants.rs | 33 ++- .../hi-agent/src/steering/tool_guardrail.rs | 47 ++- crates/hi-agent/src/tests/goal.rs | 11 +- crates/hi-agent/src/tests/goal_contract.rs | 6 +- crates/hi-agent/src/tests/turn.rs | 272 +++++++++++++++++- crates/hi-agent/src/transcript.rs | 116 ++++++++ crates/hi-cli/src/agent_build.rs | 1 - crates/hi-cli/src/commands.rs | 2 +- crates/hi-cli/src/config/cli.rs | 3 +- crates/hi-tools/src/background.rs | 109 +++++++ crates/hi-tools/src/catalog.rs | 7 +- crates/hi-tools/src/lib.rs | 51 ++++ crates/hi-tools/src/tools/mod.rs | 15 +- crates/hi-tui/src/app/commands.rs | 2 +- crates/hi-tui/src/tests.rs | 8 +- docs/0.2-migration.md | 8 +- 31 files changed, 964 insertions(+), 181 deletions(-) diff --git a/README.md b/README.md index 9bacbcf..5a081ab 100644 --- a/README.md +++ b/README.md @@ -172,10 +172,11 @@ hi "..." # auto-detects cargo check+test, go build+test, Automatic verification builds a **multi-stage pipeline** per project: `cargo check` then `cargo test`, `go build` then `go test`, `tsc` then `npm test` (when a tsconfig is present), `ruff check` then `pytest` (when ruff is configured), or `make test`. Repeat `--verify CMD` to replace detection with exact ordered stages. `--no-verify` produces an explicitly unverified outcome; a mutating one-shot still exits nonzero unless `--allow-unverified` is also given. -A `--max-steps` cap stops runaway tool loops. When it is not set explicitly, -the task contract selects 80 model calls for clearly read-only work, 120 for -recognized implementation work, and 200 for general or ambiguous turns. Each turn prints -`[N in · N out · N total · k/k ctx]`. +There is no per-turn step limit by default: runaway tool loops are ended by +the repeat/no-progress stall budgets, so long productive turns are never cut +off mid-flight. `--max-steps N` (or `/config steps `) sets an optional hard +cap; a capped turn gets one final tool-free round to report where it left the +work before stopping. Each turn prints `[N in · N out · N total · k/k ctx]`. `--max-tool-calls N` is a separate hard execution budget. Parallel batches reserve the remaining budget before dispatch and return typed denials for the diff --git a/crates/hi-agent/src/agent/explore_turn.rs b/crates/hi-agent/src/agent/explore_turn.rs index 6857896..c651b90 100644 --- a/crates/hi-agent/src/agent/explore_turn.rs +++ b/crates/hi-agent/src/agent/explore_turn.rs @@ -126,7 +126,6 @@ impl crate::Agent { }, loop_limits: crate::AgentLoopLimits { max_steps: EXPLORE_MAX_STEPS, - max_steps_explicit: true, max_parallel_tools: EXPLORE_MAX_PARALLEL_TOOLS, // A read-only explorer's text output IS its answer — don't nudge it to // keep going after it stops with text. diff --git a/crates/hi-agent/src/agent/lifecycle.rs b/crates/hi-agent/src/agent/lifecycle.rs index 57f36a8..925e434 100644 --- a/crates/hi-agent/src/agent/lifecycle.rs +++ b/crates/hi-agent/src/agent/lifecycle.rs @@ -1699,12 +1699,10 @@ impl crate::Agent { self.config.routing.temperature = temperature; } - /// Human-readable live step-limit setting. `auto` uses the intent-aware - /// defaults; `off` uses no practical per-turn cap. + /// Human-readable live step-limit setting. `off` (the default) means no + /// per-turn cap. pub fn max_steps_setting(&self) -> String { - if !self.config.loop_limits.max_steps_explicit { - "auto".to_string() - } else if self.config.loop_limits.max_steps == u32::MAX { + if self.config.loop_limits.max_steps == u32::MAX { "off".to_string() } else { self.config.loop_limits.max_steps.to_string() @@ -1718,12 +1716,12 @@ impl crate::Agent { /// Set a fixed per-turn step cap, or disable the cap with `None`. pub fn set_max_steps_limit(&mut self, limit: Option) { self.config.loop_limits.max_steps = limit.unwrap_or(u32::MAX).max(1); - self.config.loop_limits.max_steps_explicit = true; } - /// Restore intent-aware automatic step limits for subsequent turns. + /// Restore the default: no per-turn step cap (`auto` is an alias for `off` + /// now that the intent-aware implicit limits are gone). pub fn set_max_steps_auto(&mut self) { - self.config.loop_limits.max_steps_explicit = false; + self.config.loop_limits.max_steps = u32::MAX; } pub fn rsi_status(&self) -> (&'static str, &'static str, Option) { diff --git a/crates/hi-agent/src/agent/turn/helpers.rs b/crates/hi-agent/src/agent/turn/helpers.rs index f8688f4..432ef09 100644 --- a/crates/hi-agent/src/agent/turn/helpers.rs +++ b/crates/hi-agent/src/agent/turn/helpers.rs @@ -3,10 +3,9 @@ use crate::heuristics::humanize_count; use hi_ai::{RateLimitBucket, RateLimitState}; -use crate::steering::{EvidenceTracker, ReviewIntent}; +use crate::steering::EvidenceTracker; use crate::{ - EffectiveModelRoute, ReviewStatus, TaskContract, TaskIntent, ToolCallEntry, TurnAttribution, - TurnTelemetry, + EffectiveModelRoute, ReviewStatus, TaskContract, ToolCallEntry, TurnAttribution, TurnTelemetry, }; use super::progress::{ProgressTracker, ToolProgressLabel}; @@ -75,25 +74,15 @@ pub(super) fn build_turn_telemetry( } } -pub(super) fn effective_max_steps_for_turn( - config: &crate::AgentConfig, - contract_intent: TaskIntent, - read_only_intent: Option, - implementation_intent: Option, -) -> u32 { - if config.loop_limits.max_steps_explicit { - return config.loop_limits.max_steps.max(1); - } - // Intent-aware per-turn cap, regardless of `long_horizon`. A long-horizon goal - // spans many turns (each advancing one sub-goal), so each turn gets the normal - // per-intent budget — not a flat 200 that would also apply when no goal is set. - if contract_intent == TaskIntent::ReadOnly || read_only_intent.is_some() { - 80 - } else if implementation_intent.is_some() { - 120 - } else { - 200 - } +/// The per-turn model-call cap. There is no implicit cap: unless a cap was +/// deliberately configured (`--max-steps`, `/config steps `, or an internal +/// subagent budget), turns run until the model stops or the stall machinery +/// ends them. Runaway-loop protection is the repeat/no-progress/silent-continue +/// budgets, not a step ceiling — a ceiling mostly punished long *productive* +/// turns, and its size depended on intent classification, so a misclassified +/// turn died early. +pub(super) fn effective_max_steps_for_turn(config: &crate::AgentConfig) -> u32 { + config.loop_limits.max_steps.max(1) } pub(super) fn task_needs_repository_context(task: &str, contract: &TaskContract) -> bool { @@ -358,7 +347,6 @@ pub(super) fn format_rate_limit_reset(seconds: u64) -> String { #[cfg(test)] mod step_cap_tests { use super::*; - use crate::steering::ImplementationIntent; use hi_tools::{FileChange, FileChangeKind}; fn cfg(long_horizon: bool) -> crate::AgentConfig { @@ -367,10 +355,6 @@ mod step_cap_tests { long_horizon, ..crate::AgentSubagents::default() }, - loop_limits: crate::AgentLoopLimits { - max_steps_explicit: false, - ..crate::AgentLoopLimits::default() - }, ..Default::default() } } @@ -403,47 +387,25 @@ mod step_cap_tests { } #[test] - fn max_steps_is_intent_aware_even_with_long_horizon() { - // Decoupled from `long_horizon`: each turn gets its per-intent cap whether - // or not a long-horizon goal is active (the goal spans many turns). + fn no_implicit_step_cap_by_default() { + // Removing the intent-aware defaults means a turn with no configured + // cap is unbounded regardless of intent classification or horizon. for lh in [false, true] { assert_eq!( - effective_max_steps_for_turn( - &cfg(lh), - TaskIntent::Mutation, - None, - Some(ImplementationIntent::default()) - ), - 120, - "implementation intent (long_horizon={lh})" - ); - assert_eq!( - effective_max_steps_for_turn( - &cfg(lh), - TaskIntent::ReadOnly, - Some(ReviewIntent::Security), - None - ), - 80, - "read-only intent (long_horizon={lh})" - ); - assert_eq!( - effective_max_steps_for_turn(&cfg(lh), TaskIntent::Mutation, None, None), - 200, - "no intent (long_horizon={lh})" + effective_max_steps_for_turn(&cfg(lh)), + u32::MAX, + "default is uncapped (long_horizon={lh})" ); } } #[test] - fn explicit_max_steps_always_wins() { + fn configured_max_steps_is_honored_and_zero_is_clamped() { let mut c = cfg(true); - c.loop_limits.max_steps_explicit = true; c.loop_limits.max_steps = 42; - assert_eq!( - effective_max_steps_for_turn(&c, TaskIntent::ReadOnly, None, None), - 42 - ); + assert_eq!(effective_max_steps_for_turn(&c), 42); + c.loop_limits.max_steps = 0; + assert_eq!(effective_max_steps_for_turn(&c), 1); } #[test] diff --git a/crates/hi-agent/src/agent/turn/loop_.rs b/crates/hi-agent/src/agent/turn/loop_.rs index 86d2595..cc6d5d7 100644 --- a/crates/hi-agent/src/agent/turn/loop_.rs +++ b/crates/hi-agent/src/agent/turn/loop_.rs @@ -477,12 +477,7 @@ impl crate::Agent { }; // Mid-turn LSP + affected cargo check state (dedupes packages across batches). let fast_feedback = super::fast_feedback::FastFeedbackState::default(); - let max_steps = effective_max_steps_for_turn( - &self.config, - task_contract.intent, - read_only_intent, - implementation_intent, - ); + let max_steps = effective_max_steps_for_turn(&self.config); let max_parallel_tools = self.config.loop_limits.max_parallel_tools.max(1); let steps = 0u32; let empty_retries = 0u32; diff --git a/crates/hi-agent/src/agent/turn/model_round.rs b/crates/hi-agent/src/agent/turn/model_round.rs index 7a0504e..ea68e09 100644 --- a/crates/hi-agent/src/agent/turn/model_round.rs +++ b/crates/hi-agent/src/agent/turn/model_round.rs @@ -26,8 +26,8 @@ use crate::{MAX_TOOL_PROTOCOL_RETRIES, TRUNCATED_TOOL_CALL_NUDGE, TRUNCATION_NUD use super::helpers::{build_turn_telemetry, effective_model_route}; use super::phase::TurnPhase; use super::progress::{ - NO_PROGRESS_FINAL_ANSWER_NUDGE, ProgressKind, ProgressTracker, forced_final_answer_is_unusable, - no_progress_signature_for_calls, + AWAITING_BACKGROUND_REASON, NO_PROGRESS_FINAL_ANSWER_NUDGE, ProgressKind, ProgressTracker, + STEP_LIMIT_WRAP_UP_NUDGE, forced_final_answer_is_unusable, no_progress_signature_for_calls, }; use super::retry::{ INCOMPLETE_STATUS, ReviewRepairState, TurnRetryState, @@ -69,6 +69,7 @@ pub(super) struct ModelRoundState<'a> { pub sched_serial_runs: &'a mut u32, pub tool_schema_tokens: &'a mut u64, pub ended_at_cap: &'a mut bool, + pub cap_wrap_up_requested: &'a mut bool, pub prev_call_sig: &'a mut Option>, pub retry_state: &'a mut TurnRetryState, pub request_max_tokens_override: &'a mut Option, @@ -195,6 +196,7 @@ impl crate::Agent { let sched_serial_runs = *state.sched_serial_runs; let mut tool_schema_tokens = *state.tool_schema_tokens; let ended_at_cap = *state.ended_at_cap; + let mut cap_wrap_up_requested = *state.cap_wrap_up_requested; let mut prev_call_sig = std::mem::take(state.prev_call_sig); let mut retry_state = std::mem::take(state.retry_state); let mut request_max_tokens_override = std::mem::take(state.request_max_tokens_override); @@ -225,8 +227,21 @@ impl crate::Agent { let result = async { self.set_turn_phase(TurnPhase::Model); + // Reaching the cap grants ONE tool-free wrap-up round so the model can + // report where it left the work, instead of the turn dying mid-flight + // with no final answer. The sticky flag makes the second hit terminal. + let mut request_cap_wrap_up = false; if steps >= max_steps { - return Ok(ModelRoundControl::BreakInner(true)); + if cap_wrap_up_requested { + return Ok(ModelRoundControl::BreakInner(true)); + } + cap_wrap_up_requested = true; + request_cap_wrap_up = true; + ui.nudge(&format!( + "reached step limit ({max_steps}); asking for a final wrap-up before stopping" + )); + self.messages + .push_nudge(NudgeKind::Continue, STEP_LIMIT_WRAP_UP_NUDGE); } steps += 1; @@ -372,6 +387,7 @@ impl crate::Agent { let tool_mode = if request_text_tool_fallback || request_text_answer || request_no_progress_final_answer + || request_cap_wrap_up { ToolMode::ChatOnly } else if force_tools_next && self.config.routing.tool_mode == ToolMode::Auto { @@ -382,6 +398,7 @@ impl crate::Agent { let tool_availability_mode = if request_text_tool_fallback || request_text_answer || request_no_progress_final_answer + || request_cap_wrap_up { ToolMode::ChatOnly } else if read_only_intent.is_some() @@ -669,7 +686,7 @@ impl crate::Agent { } let calls: Vec<(String, String, String)> = - if request_text_answer || request_no_progress_final_answer { + if request_text_answer || request_no_progress_final_answer || request_cap_wrap_up { Vec::new() } else { completion @@ -695,6 +712,7 @@ impl crate::Agent { let calls = if calls.is_empty() && !request_text_answer && !request_no_progress_final_answer + && !request_cap_wrap_up { let full_text: String = completion .content @@ -1229,11 +1247,33 @@ If the task is already complete, stop and give your final recap." .join("\n"); let has_text = !assistant_text.trim().is_empty(); + if request_cap_wrap_up { + // Whatever the model reports here is accepted as the wrap-up — the + // turn ends at the cap either way, so no usability gating applies. + if has_text { + if buffer_read_only_review_text || !streamed_assistant_text { + let text_to_emit = if buffered_assistant_text.is_empty() { + assistant_text.as_str() + } else { + buffered_assistant_text.as_str() + }; + self.emit_assistant_text(ui, text_to_emit); + ui.assistant_end(); + } + progress_tracker.record(ProgressKind::Weak, "step-limit wrap-up report", None); + } + self.messages + .push_assistant_text_only(std::mem::take(&mut completion.content)); + return Ok(ModelRoundControl::BreakInner(true)); + } + if request_no_progress_final_answer { + let background_status_answer = + progress_tracker.last_progress_reason == AWAITING_BACKGROUND_REASON; let unusable = forced_final_answer_is_unusable( &assistant_text, - self.goals.plan_incomplete(), - ); + self.goals.plan_incomplete() && !background_status_answer, + ) && !(background_status_answer && has_text); if has_text && (buffer_read_only_review_text || !streamed_assistant_text) { let text_to_emit = if buffered_assistant_text.is_empty() { assistant_text.as_str() @@ -1309,6 +1349,7 @@ If the task is already complete, stop and give your final recap." &mut force_tools_next, &mut force_text_answer_next, &mut text_tool_fallback_next, + &mut stalled_repeating, &mut stalled_unfinished, &mut buffered_assistant_text, buffer_read_only_review_text, @@ -1366,6 +1407,7 @@ If the task is already complete, stop and give your final recap." *state.sched_serial_runs = sched_serial_runs; *state.tool_schema_tokens = tool_schema_tokens; *state.ended_at_cap = ended_at_cap; + *state.cap_wrap_up_requested = cap_wrap_up_requested; *state.prev_call_sig = prev_call_sig; *state.retry_state = retry_state; *state.request_max_tokens_override = request_max_tokens_override; diff --git a/crates/hi-agent/src/agent/turn/progress.rs b/crates/hi-agent/src/agent/turn/progress.rs index 34b678c..fe370e4 100644 --- a/crates/hi-agent/src/agent/turn/progress.rs +++ b/crates/hi-agent/src/agent/turn/progress.rs @@ -14,6 +14,21 @@ use crate::steering::{ pub(super) const PROGRESS_EVENT_LIMIT: usize = 20; pub(super) const NO_PROGRESS_FINAL_ANSWER_NUDGE_THRESHOLD: u32 = 2; pub(super) const NO_PROGRESS_FINAL_ANSWER_NUDGE: &str = "You have not made new progress after repeated tool-use nudges. Stop using tools now and give the best final answer from the evidence already in the conversation. If the task cannot be completed from that evidence, say exactly what is missing."; +/// Sent when a turn reaches its configured step cap: one final tool-free round +/// so the model reports where it left the work instead of the turn dying +/// mid-flight with no answer. Only a deliberately set cap (`--max-steps`, +/// `/config steps `, or an internal subagent budget) can trigger this — +/// there is no implicit per-turn step limit. +pub(super) const STEP_LIMIT_WRAP_UP_NUDGE: &str = "You have reached this turn's step limit. Stop using tools now. In a short final answer, report what you completed, what remains unfinished, and the exact state you are leaving the work in (files changed, checks not yet run). Do not claim the task is complete unless it actually is; the user can raise or remove the limit with /config steps."; +/// Progress reason shared between the waiting-round recovery (Steer) and the +/// final-answer acceptance paths: it marks the turn as blocked only on live +/// background work, so a status answer is a valid terminal outcome. +pub(super) const AWAITING_BACKGROUND_REASON: &str = "background process is still running"; +/// Consecutive waiting rounds (only polls/status probes of a still-running +/// background process) tolerated before the turn is steered to end with a +/// status report. Three rounds ≈ one launch check plus two follow-ups — enough +/// to catch a fast finish without funding an open-ended babysitting loop. +pub(super) const WAITING_ROUND_BUDGET: u32 = 3; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(super) enum ProgressKind { @@ -60,6 +75,13 @@ pub(super) struct ProgressTracker { pub(super) forced_final_answer_attempts: u32, pub(super) last_progress_reason: String, pub(super) last_stall_reason: String, + /// Consecutive tool rounds that only watched still-running background + /// work (see [`WAITING_ROUND_BUDGET`]). Reset by any non-waiting round. + pub(super) waiting_rounds: u32, + /// Sticky once the waiting budget is spent: the turn is blocked on live + /// background work, so plan-continue nudges are suppressed and a status + /// answer ends the turn. Cleared by a round that does real work. + pub(super) awaiting_background: bool, pub(super) events: Vec, } diff --git a/crates/hi-agent/src/agent/turn/state.rs b/crates/hi-agent/src/agent/turn/state.rs index 9256de4..d40698f 100644 --- a/crates/hi-agent/src/agent/turn/state.rs +++ b/crates/hi-agent/src/agent/turn/state.rs @@ -119,6 +119,7 @@ impl TurnState { stalled_repeating: &mut self.flags.stalled_repeating, stalled_unfinished: &mut self.flags.stalled_unfinished, ended_at_cap: &mut self.flags.ended_at_cap, + cap_wrap_up_requested: &mut self.flags.cap_wrap_up_requested, prev_added_no_evidence: &mut self.prev_added_no_evidence, turn_start: &mut self.turn_start, context_generation_seen: &mut self.context_generation_seen, diff --git a/crates/hi-agent/src/agent/turn/steer/implementation.rs b/crates/hi-agent/src/agent/turn/steer/implementation.rs index d6cf70a..1db3aaf 100644 --- a/crates/hi-agent/src/agent/turn/steer/implementation.rs +++ b/crates/hi-agent/src/agent/turn/steer/implementation.rs @@ -2,16 +2,18 @@ use crate::agent::mutation_recovery_turn::MutationRecoveryControl; use crate::steering::{ - BG_POLL_IDLE_NUDGE, EvidenceTracker, IMPLEMENTATION_NO_CHANGES_NUDGE, ImplementationIntent, - ImplementationTracker, MutationRecovery, REREAD_NUDGE, ReviewIntent, WAIT_POLL_STATIC_NUDGE, - bash_call_waits, implementation_text_tool_nudge, + BACKGROUND_WAIT_FINAL_NUDGE, BACKGROUND_WAIT_STATUS_NUDGE, EvidenceTracker, + IMPLEMENTATION_NO_CHANGES_NUDGE, ImplementationIntent, ImplementationTracker, + MutationRecovery, REREAD_NUDGE, ReviewIntent, WAIT_POLL_STATIC_NUDGE, bash_call_waits, + implementation_text_tool_nudge, }; use crate::transcript::NudgeKind; use crate::ui::Ui; use super::super::phase::TurnPhase; use super::super::progress::{ - NO_PROGRESS_FINAL_ANSWER_NUDGE, ProgressKind, ProgressTracker, no_progress_signature_for_calls, + AWAITING_BACKGROUND_REASON, NO_PROGRESS_FINAL_ANSWER_NUDGE, ProgressKind, ProgressTracker, + WAITING_ROUND_BUDGET, no_progress_signature_for_calls, }; use super::super::retry::INCOMPLETE_STATUS; use super::super::tools::ToolBatchOutcome; @@ -45,7 +47,8 @@ impl crate::Agent { hash_guard_applies, hashable_idempotent_results, repeated_idempotent_results, - idle_background_poll_results, + running_background_poll_results, + wait_flavored_results, ref tool_progress_labels, plan_changed_this_batch, interrupted_calls, @@ -54,7 +57,6 @@ impl crate::Agent { let plan_changed_this_batch = plan_changed_this_batch; let hashable_idempotent_results = hashable_idempotent_results; let repeated_idempotent_results = repeated_idempotent_results; - let idle_background_poll_results = idle_background_poll_results; let hash_guard_applies = hash_guard_applies; // Post-tool policy (mutation recovery, inspection sprawl, …) is Steer. self.set_turn_phase(TurnPhase::Steer); @@ -90,6 +92,51 @@ impl crate::Agent { MutationRecoveryControl::None => {} MutationRecoveryControl::Continue => return RoundControl::Continue, } + // Waiting-round detection keys on the process *lifecycle*, not output + // novelty: a live progress bar makes every poll deliver fresh bytes, + // which defeated the byte-identical idle guard for hours while the + // turn burned a model round per poll. A round that only watched + // still-running background work is waiting, full stop. After the + // budget, steer to a terminal status answer; a quiet-but-running + // process is not a stalled turn, so no repeat budgets are consumed + // and no sticky stall flags are left behind. + let waiting_round = + running_background_poll_results > 0 && wait_flavored_results == calls.len(); + if waiting_round { + progress_tracker.waiting_rounds = progress_tracker.waiting_rounds.saturating_add(1); + } else { + progress_tracker.waiting_rounds = 0; + progress_tracker.awaiting_background = false; + } + if waiting_round && progress_tracker.waiting_rounds >= WAITING_ROUND_BUDGET { + let first_request = !progress_tracker.awaiting_background; + progress_tracker.awaiting_background = true; + *stalled_repeating = false; + *stalled_unfinished = false; + *repeat_nudges = 0; + *force_tools_next = false; + *force_no_progress_final_answer_next = false; + progress_tracker.record( + ProgressKind::Weak, + AWAITING_BACKGROUND_REASON, + no_progress_signature_for_calls(calls), + ); + if first_request { + ui.nudge( + "the background process is still running; asking the model to wait once with wait_secs or wrap up with a status report", + ); + self.messages + .push_nudge(NudgeKind::Continue, BACKGROUND_WAIT_STATUS_NUDGE); + } else { + // Still polling after the wrap-up request — force the next + // round tool-free so the status answer actually lands. + *force_no_progress_final_answer_next = true; + ui.nudge("still polling after the wrap-up request — forcing a final status answer"); + self.messages + .push_nudge(NudgeKind::Continue, BACKGROUND_WAIT_FINAL_NUDGE); + } + return RoundControl::Continue; + } let repeated_result_no_progress = hash_guard_applies && hashable_idempotent_results == calls.len() && repeated_idempotent_results == calls.len(); @@ -104,23 +151,15 @@ impl crate::Agent { let waiting_round = calls .iter() .any(|(_, name, args)| name == "bash" && bash_call_waits(args)); - let idle_bg_round = idle_background_poll_results == calls.len(); let force_final_after_nudge = progress_tracker.record_no_progress_nudge( - if idle_bg_round { - "idle background poll tight loop" - } else if waiting_round { + if waiting_round { "wait poll returned static output" } else { "repeated idempotent tool output" }, no_progress_signature_for_calls(&calls), ) && implementation_intent.is_none(); - if idle_bg_round { - ui.nudge(&format!( - "the model tight-polled a quiet background process — nudging it to wait or run in the foreground ({repeat_nudges}/{})", - self.config.loop_limits.max_repeat_nudges - )); - } else if waiting_round { + if waiting_round { ui.nudge(&format!( "the wait-and-check poll returned the same output — nudging the model to diagnose the stalled process ({repeat_nudges}/{})", self.config.loop_limits.max_repeat_nudges @@ -131,9 +170,7 @@ impl crate::Agent { self.config.loop_limits.max_repeat_nudges )); } - let base_nudge = if idle_bg_round { - BG_POLL_IDLE_NUDGE - } else if waiting_round { + let base_nudge = if waiting_round { WAIT_POLL_STATIC_NUDGE } else { REREAD_NUDGE diff --git a/crates/hi-agent/src/agent/turn/steer/review.rs b/crates/hi-agent/src/agent/turn/steer/review.rs index ce7671c..3c24207 100644 --- a/crates/hi-agent/src/agent/turn/steer/review.rs +++ b/crates/hi-agent/src/agent/turn/steer/review.rs @@ -12,7 +12,7 @@ use crate::transcript::NudgeKind; use crate::{PLAN_CONTINUE_NUDGE, SILENT_CONTINUE_NUDGE, Ui}; use super::super::phase::TurnPhase; -use super::super::progress::{ProgressKind, ProgressTracker}; +use super::super::progress::{AWAITING_BACKGROUND_REASON, ProgressKind, ProgressTracker}; use super::super::retry::{INCOMPLETE_STATUS, ReviewRepairState}; use super::RoundControl; @@ -36,6 +36,7 @@ impl crate::Agent { force_tools_next: &mut bool, force_text_answer_next: &mut bool, text_tool_fallback_next: &mut bool, + stalled_repeating: &mut bool, stalled_unfinished: &mut bool, buffered_assistant_text: &mut String, buffer_read_only_review_text: bool, @@ -63,6 +64,34 @@ impl crate::Agent { // Q&A answer. Bounded so it can't loop forever. let looks_unfinished = looks_like_unfinished_step(assistant_text); let plan_incomplete = self.goals.plan_incomplete(); + // A plan can be structurally incomplete while every remaining step is + // blocked on a live background process. A status answer is then the + // correct terminal outcome — re-nudging "continue with the next + // pending step" only makes the model poll again (observed in real + // transcripts: 85 plan nudges in one turn babysitting two downloads, + // each cycle a full model round). The waiting classifier in + // steer_after_tools sets `awaiting_background` after consecutive + // waiting rounds; any non-waiting tool round clears it. + if progress_tracker.awaiting_background && !assistant_text.trim().is_empty() { + if buffer_read_only_review_text { + let text_to_emit = if buffered_assistant_text.is_empty() { + assistant_text + } else { + buffered_assistant_text + }; + self.emit_assistant_text(ui, text_to_emit); + ui.assistant_end(); + } + self.messages + .push_assistant(std::mem::take(completion_content)); + *stalled_repeating = false; + *stalled_unfinished = false; + progress_tracker.no_progress_streak = 0; + progress_tracker.last_stall_reason.clear(); + progress_tracker.record(ProgressKind::Weak, AWAITING_BACKGROUND_REASON, None); + ui.status("background work continues; ending the turn with the status report"); + return RoundControl::BreakInner(false); + } if let Some(intent) = read_only_intent && (looks_unfinished || plan_incomplete) { @@ -219,6 +248,12 @@ impl crate::Agent { if (looks_unfinished || plan_incomplete) && *silent_continues < self.config.loop_limits.max_silent_continues { + // A real final answer after a forced no-progress recovery resolves + // pending unfinished state from the preceding tool round. + *stalled_repeating = false; + *stalled_unfinished = false; + progress_tracker.no_progress_streak = 0; + progress_tracker.last_stall_reason.clear(); *silent_continues += 1; *continue_total_nudges += 1; // Force the next round to actually call a tool, so the @@ -249,6 +284,10 @@ impl crate::Agent { if looks_unfinished || plan_incomplete { progress_tracker.record(ProgressKind::Weak, "text answer looked unfinished", None); } else { + *stalled_repeating = false; + *stalled_unfinished = false; + progress_tracker.no_progress_streak = 0; + progress_tracker.last_stall_reason.clear(); progress_tracker.record_final_answer(); } RoundControl::BreakInner(false) diff --git a/crates/hi-agent/src/agent/turn/tools/batch.rs b/crates/hi-agent/src/agent/turn/tools/batch.rs index 91138a4..7d5c16e 100644 --- a/crates/hi-agent/src/agent/turn/tools/batch.rs +++ b/crates/hi-agent/src/agent/turn/tools/batch.rs @@ -47,9 +47,16 @@ pub(in crate::agent::turn) struct ToolBatchOutcome { pub(in crate::agent::turn) hash_guard_applies: bool, pub(in crate::agent::turn) hashable_idempotent_results: usize, pub(in crate::agent::turn) repeated_idempotent_results: usize, - /// How many results in this batch were idle `bash_output` polls - /// (running, no new output). Used to pick the tight-poll nudge. - pub(in crate::agent::turn) idle_background_poll_results: usize, + /// How many results were `bash_output` polls of a still-running process — + /// with or without new output. A live progress bar produces fresh bytes on + /// every poll, so waiting-detection keys on the process lifecycle instead + /// of output novelty. + pub(in crate::agent::turn) running_background_poll_results: usize, + /// How many calls in this batch were wait-flavored: background polls, plan + /// bookkeeping, or non-mutating non-validating shell probes (log tails, + /// size checks). A batch of only these while a process runs is a turn + /// waiting on external work, not making progress toward the plan. + pub(in crate::agent::turn) wait_flavored_results: usize, pub(in crate::agent::turn) tool_progress_labels: Vec, pub(in crate::agent::turn) plan_changed_this_batch: bool, pub(in crate::agent::turn) interrupted_calls: usize, @@ -97,7 +104,8 @@ impl crate::Agent { }); let mut hashable_idempotent_results = 0usize; let mut repeated_idempotent_results = 0usize; - let mut idle_background_poll_results = 0usize; + let mut running_background_poll_results = 0usize; + let mut wait_flavored_results = 0usize; self.set_turn_phase(TurnPhase::Tools); let mut tool_progress_labels: Vec = Vec::new(); let mut plan_changed_this_batch = false; @@ -527,8 +535,11 @@ impl crate::Agent { validation_succeeded, ); let progress = tool_guardrail.record_tool_result(name, arguments, &semantic_output); - if progress.idle_background_poll { - idle_background_poll_results += 1; + if progress.running_background_poll { + running_background_poll_results += 1; + } + if wait_flavored_call(name, arguments, &output) { + wait_flavored_results += 1; } if progress.hashable_idempotent { hashable_idempotent_results += 1; @@ -682,9 +693,6 @@ impl crate::Agent { ); let progress = tool_guardrail.record_tool_result("explore", arguments, &semantic_output); - if progress.idle_background_poll { - idle_background_poll_results += 1; - } if progress.hashable_idempotent { hashable_idempotent_results += 1; if progress.repeated_idempotent_result { @@ -894,9 +902,6 @@ impl crate::Agent { arguments, &semantic_output, ); - if progress.idle_background_poll { - idle_background_poll_results += 1; - } if progress.hashable_idempotent { hashable_idempotent_results += 1; if progress.repeated_idempotent_result { @@ -1116,9 +1121,6 @@ impl crate::Agent { validation_succeeded, ); let progress = tool_guardrail.record_tool_result(name, arguments, &semantic_output); - if progress.idle_background_poll { - idle_background_poll_results += 1; - } if progress.hashable_idempotent { hashable_idempotent_results += 1; if progress.repeated_idempotent_result { @@ -1409,8 +1411,11 @@ impl crate::Agent { ); let progress = tool_guardrail.record_tool_result(name, &calls[i].2, &semantic_output); - if progress.idle_background_poll { - idle_background_poll_results += 1; + if progress.running_background_poll { + running_background_poll_results += 1; + } + if wait_flavored_call(name, &calls[i].2, &output) { + wait_flavored_results += 1; } if progress.hashable_idempotent { hashable_idempotent_results += 1; @@ -1640,6 +1645,19 @@ impl crate::Agent { } self.messages .push_assistant_with_results(std::mem::take(completion_content), results); + // Collapse older polls of the handles polled this batch to one-line + // digests: only the newest poll carries information the model still + // needs, and a long watch otherwise re-sends every stale progress + // dump on every subsequent request. + let mut folded_handles: BTreeSet = BTreeSet::new(); + for (_, name, arguments) in calls { + if name == "bash_output" + && let Some(handle) = crate::transcript::background_poll_handle(arguments) + && folded_handles.insert(handle.clone()) + { + self.messages.fold_superseded_background_polls(&handle); + } + } // A fully cancelled batch did not execute discovery or implementation // work, so it must not burn the mutation-recovery round budget. if interrupted_calls < calls.len() { @@ -1650,7 +1668,8 @@ impl crate::Agent { hash_guard_applies, hashable_idempotent_results, repeated_idempotent_results, - idle_background_poll_results, + running_background_poll_results, + wait_flavored_results, tool_progress_labels, plan_changed_this_batch, interrupted_calls, @@ -1658,3 +1677,63 @@ impl crate::Agent { }) } } + +/// Whether a call is compatible with a turn that is merely waiting on live +/// background work: the poll itself, plan bookkeeping, and non-mutating, +/// non-validating shell probes (log tails, size checks, process listings). +/// Real work — edits, builds, tests, file reads — is deliberately not +/// wait-flavored, so interleaved genuine progress resets the waiting streak. +fn wait_flavored_call(name: &str, arguments: &str, output: &hi_tools::ToolOutcome) -> bool { + match name { + "bash_output" | "update_plan" => true, + "bash" => { + !output.effects.mutation_applied + && !crate::steering::implementation_tool_call_validates(name, arguments) + } + _ => false, + } +} + +#[cfg(test)] +mod wait_flavored_tests { + use super::*; + + fn outcome(mutation_applied: bool) -> hi_tools::ToolOutcome { + let mut output = synthetic_tool_outcome("ok".into(), hi_tools::ToolStatus::Succeeded); + output.effects.mutation_applied = mutation_applied; + output + } + + #[test] + fn polls_bookkeeping_and_status_probes_are_wait_flavored_but_real_work_is_not() { + assert!(wait_flavored_call( + "bash_output", + r#"{"id":"bg_1"}"#, + &outcome(false) + )); + assert!(wait_flavored_call("update_plan", "{}", &outcome(false))); + assert!(wait_flavored_call( + "bash", + r#"{"command":"tail -c 200 /tmp/download.log"}"#, + &outcome(false) + )); + // Real work resets the waiting streak: mutations, validation runs, + // and file reads are progress, not babysitting. + assert!(!wait_flavored_call( + "bash", + r#"{"command":"echo hi > f.txt"}"#, + &outcome(true) + )); + assert!(!wait_flavored_call( + "bash", + r#"{"command":"cargo test"}"#, + &outcome(false) + )); + assert!(!wait_flavored_call( + "read", + r#"{"path":"src/main.rs"}"#, + &outcome(false) + )); + assert!(!wait_flavored_call("edit", "{}", &outcome(true))); + } +} diff --git a/crates/hi-agent/src/command.rs b/crates/hi-agent/src/command.rs index 5cd9e9e..27e4a9a 100644 --- a/crates/hi-agent/src/command.rs +++ b/crates/hi-agent/src/command.rs @@ -925,8 +925,10 @@ pub enum ConfigArg { /// it, leaving the provider default). Temperature(Option), /// `/config steps ` — set a fixed cap, or disable it (`None`). + /// There is no cap by default. MaxSteps(Option), - /// `/config steps auto` — restore intent-aware per-turn defaults. + /// `/config steps auto` — alias for `off` (kept for muscle memory; the + /// intent-aware implicit limits it used to restore are gone). MaxStepsAuto, /// `/config moe-streaming ` — control MLX MoE expert streaming. /// `On` forces streaming, `Off` forces resident, `Auto` (the default) lets @@ -1416,7 +1418,7 @@ pub const COMMANDS: &[CommandSpec] = &[ ("temp", "set sampling temperature: 0.0-2.0, or off"), ( "steps", - "set the turn step limit: positive integer, auto, or off", + "set a turn step limit: positive integer, or off (the default; auto = off)", ), ("verify", "show/set/clear the verify command"), ("lsp", "toggle LSP or show status"), diff --git a/crates/hi-agent/src/config.rs b/crates/hi-agent/src/config.rs index c8b75c5..6e2489b 100644 --- a/crates/hi-agent/src/config.rs +++ b/crates/hi-agent/src/config.rs @@ -410,11 +410,12 @@ impl Default for AgentGates { pub struct AgentLoopLimits { /// Optional wall-clock budget for one turn. `None` keeps legacy behavior. pub turn_timeout: Option, - /// Safety cap on model calls per turn, to stop runaway tool loops. + /// Cap on model calls per turn. `u32::MAX` (the default) means **no cap**: + /// runaway loops are ended by the repeat/no-progress/stall budgets, not a + /// step ceiling. Set deliberately via `--max-steps`, `/config steps `, + /// or an internal subagent budget; when a capped turn hits the limit it is + /// granted one tool-free wrap-up round to report where the work stands. pub max_steps: u32, - /// Whether `max_steps` was explicitly requested by the caller. When false, - /// the turn loop chooses a conservative dynamic cap from the turn intent. - pub max_steps_explicit: bool, /// Hard cap on executed tool calls per turn. This is independent of the /// model-call (`max_steps`) cap. pub max_tool_calls: u32, @@ -444,7 +445,6 @@ impl Default for AgentLoopLimits { Self { turn_timeout: None, max_steps: u32::MAX, - max_steps_explicit: false, max_tool_calls: u32::MAX, max_repeat_nudges: MAX_REPEAT_NUDGES, max_silent_continues: MAX_SILENT_CONTINUES, @@ -643,7 +643,11 @@ mod tests { assert_eq!(config.gates.review, ReviewPolicy::Risk); assert_eq!(config.gates.lsp_mode, LspMode::Auto); assert_eq!(config.memory.tool_set, ToolSet::Dynamic); - assert!(!config.loop_limits.max_steps_explicit); + assert_eq!( + config.loop_limits.max_steps, + u32::MAX, + "no implicit per-turn step cap" + ); assert!(!config.gates.allow_unverified); assert!(config.gates.allow_no_checkpoint); assert!(config.subagents.explore_subagents, "explore on by default"); diff --git a/crates/hi-agent/src/domain.rs b/crates/hi-agent/src/domain.rs index d13743f..e3e2b1b 100644 --- a/crates/hi-agent/src/domain.rs +++ b/crates/hi-agent/src/domain.rs @@ -370,6 +370,9 @@ pub(crate) struct TurnControlFlags { pub stalled_repeating: bool, pub stalled_unfinished: bool, pub ended_at_cap: bool, + /// Whether this turn already granted the one tool-free wrap-up round after + /// reaching the step cap. Sticky: the next cap hit ends the turn for real. + pub cap_wrap_up_requested: bool, pub obligation_nudge_fired: bool, } diff --git a/crates/hi-agent/src/lib.rs b/crates/hi-agent/src/lib.rs index d066e89..48449f6 100644 --- a/crates/hi-agent/src/lib.rs +++ b/crates/hi-agent/src/lib.rs @@ -264,8 +264,9 @@ pub struct TurnPhaseLatencies { pub struct TurnTelemetry { /// Cumulative wall-clock time spent in major turn phases. pub phase_latencies: TurnPhaseLatencies, - /// Effective model-call cap used for this turn after dynamic defaults and - /// explicit overrides are resolved. + /// Per-turn model-call cap in effect (`u32::MAX` when uncapped — the + /// default; only `--max-steps`, `/config steps`, or an internal subagent + /// budget set a real cap). pub effective_max_steps: u32, /// How many verify rounds ran this turn (0 = verify off or skipped). pub verify_rounds: u32, @@ -527,8 +528,9 @@ pub const MAX_PARALLEL_TOOLS: usize = 8; /// Max times one turn will nudge a model that re-issues the *exact same* tool /// call as the previous round — a repetition loop where the model re-runs an /// identical command, gets the same output, and re-emits it again. Bounds the -/// recovery before the turn ends with an honest "stuck repeating" notice; -/// `max_steps` is the hard backstop. +/// recovery before the turn ends with an honest "stuck repeating" notice. +/// There is no implicit step cap: this budget and the no-progress forced +/// final answer are what end a looping turn. pub const MAX_REPEAT_NUDGES: u32 = 2; /// Max times a turn will silently re-prompt the model to continue after it /// stops with text but no tool calls (when it was actively working). Keeps the diff --git a/crates/hi-agent/src/steering/constants.rs b/crates/hi-agent/src/steering/constants.rs index 4aee557..83ba107 100644 --- a/crates/hi-agent/src/steering/constants.rs +++ b/crates/hi-agent/src/steering/constants.rs @@ -197,17 +197,28 @@ stop and give your final recap. Do not re-read files you have already inspected. /// diagnosing the stalled process instead of quitting or looping. pub(crate) const WAIT_POLL_STATIC_NUDGE: &str = "Your wait-and-check command returned exactly the same \ output as before — whatever you are waiting on has not progressed since the last check. Do not simply \ -re-run the same poll. Check the underlying process directly (bash_output on its handle, its log file, or \ -the process list), fix what is stuck if you can, or if the wait is genuinely still in progress use a much \ -longer interval. If you cannot make progress now, stop and report the current state and what remains."; -/// Sent when the model tight-polls `bash_output` while a background process is -/// still running with no new output. Re-polling immediately burns turns and -/// makes the UI look hung; push the model toward a real wait or foreground run. -pub(crate) const BG_POLL_IDLE_NUDGE: &str = "That background process is still running and has produced no \ -new output since your last checks. Do not tight-poll bash_output in a loop — that looks hung and wastes \ -turns. For a finite build or test suite, raise the bash timeout and run it in the foreground instead. \ -Otherwise sleep for a meaningful interval before the next bash_output, or do other useful work and check \ -back later. If the process appears stuck, inspect it (logs, process list) or bash_kill it and recover."; +re-run the same poll. Check the underlying process directly (bash_output on its handle — pass wait_secs \ +to block for new output instead of re-polling — its log file, or the process list), fix what is stuck if \ +you can, or if the wait is genuinely still in progress use a much longer interval. If you cannot make \ +progress now, stop and report the current state and what remains."; +/// Sent when the turn has spent its waiting budget: several consecutive tool +/// rounds did nothing but watch still-running background work (with or without +/// fresh output — a live progress bar makes every poll look new). Babysitting a +/// long process one model round at a time is the most expensive failure mode +/// observed in real transcripts (hundreds of rounds re-polling two downloads). +/// Steer the model to either block once server-side or end the turn honestly. +pub(crate) const BACKGROUND_WAIT_STATUS_NUDGE: &str = "The background process is still running. Stop \ +polling it round after round. If it should produce output or finish within a few minutes, make ONE \ +bash_output call with wait_secs (up to 600) to block until then. Otherwise stop now and give a concise \ +final status: the work remains in progress, what has been completed so far, and what remains once it \ +finishes. Do not claim completion or failure, and do not keep watching a process that will run for a \ +long time."; +/// Sent when the model keeps polling after [`BACKGROUND_WAIT_STATUS_NUDGE`] — +/// the next round is forced tool-free so the status answer actually lands. +pub(crate) const BACKGROUND_WAIT_FINAL_NUDGE: &str = "The background process is still running and you \ +were already asked to stop polling it. Give your final status answer now: state that the work remains \ +in progress, what has been completed so far, and what remains. Do not call any tools and do not claim \ +completion or failure."; pub(crate) const SECURITY_BROAD_SEARCH_NUDGE: &str = "This security review searched and read some evidence, \ but it has not covered all required pattern families yet. Do not use mutating tools. Search for \ unsafe/unwrap/expect/panic, command execution/filesystem/env access, and secret/token/auth \ diff --git a/crates/hi-agent/src/steering/tool_guardrail.rs b/crates/hi-agent/src/steering/tool_guardrail.rs index 8e6cef6..c8057ec 100644 --- a/crates/hi-agent/src/steering/tool_guardrail.rs +++ b/crates/hi-agent/src/steering/tool_guardrail.rs @@ -22,6 +22,11 @@ pub(crate) struct ToolResultProgress { /// output). Used to pick a dedicated nudge instead of the wait-poll or /// re-read copy. pub(crate) idle_background_poll: bool, + /// True when this result was a `bash_output` poll of a process that is + /// still running — with or without new output. A live progress bar makes + /// every poll look like fresh output, so waiting-detection must key on the + /// process lifecycle, not output novelty. + pub(crate) running_background_poll: bool, } impl ToolLoopGuardrail { @@ -36,6 +41,7 @@ impl ToolLoopGuardrail { // same poll returning byte-identical output means the awaited state // stopped changing. let wait_poll = name == "bash" && super::implementation::bash_call_waits(arguments); + let running_bg = name == "bash_output" && bash_output_is_running(output); let idle_bg = name == "bash_output" && bash_output_is_idle(output); if idle_bg { return self.record_idle_bg_poll(arguments); @@ -48,7 +54,10 @@ impl ToolLoopGuardrail { } } if !(is_hashable_idempotent_tool(name) || wait_poll) || output.starts_with("Error:") { - return ToolResultProgress::default(); + return ToolResultProgress { + running_background_poll: running_bg && !output.starts_with("Error:"), + ..ToolResultProgress::default() + }; } // Inspections dedup on output alone: the same content reached through // different arguments (another path to the same file, a wider grep) is @@ -70,6 +79,7 @@ impl ToolLoopGuardrail { hashable_idempotent: true, repeated_idempotent_result: repeated, idle_background_poll: false, + running_background_poll: running_bg, } } @@ -79,6 +89,7 @@ impl ToolLoopGuardrail { hashable_idempotent: true, repeated_idempotent_result: false, idle_background_poll: true, + running_background_poll: true, }; }; let strikes = self.idle_bg_poll_strikes.entry(id).or_insert(0); @@ -89,6 +100,7 @@ impl ToolLoopGuardrail { // ones are the tight-loop case the UI used to render as hung. repeated_idempotent_result: *strikes > IDLE_BG_POLL_FREE_STRIKES, idle_background_poll: true, + running_background_poll: true, } } } @@ -100,6 +112,16 @@ fn bash_output_is_idle(output: &str) -> bool { .is_some_and(|status| status.contains("running — no new output")) } +/// The poll's status line says the process is still running, whether or not +/// it delivered fresh output (`[bg_1: running]` or `[bg_1: running — no new +/// output]`). +fn bash_output_is_running(output: &str) -> bool { + output + .lines() + .next() + .is_some_and(|status| status.starts_with('[') && status.contains(": running")) +} + fn background_handle_id(arguments: &str) -> Option { let value: serde_json::Value = serde_json::from_str(arguments).ok()?; let id = value.get("id")?.as_str()?; @@ -266,6 +288,29 @@ mod tests { ); } + #[test] + fn running_polls_are_flagged_regardless_of_output_novelty() { + let mut guard = ToolLoopGuardrail::default(); + let args = r#"{"id":"bg_1"}"#; + + // A progress bar delivers fresh bytes on every poll: not idle, but + // still a poll of a running process — the waiting classifier keys on + // this, not on output novelty. + let progressing = + guard.record_tool_result("bash_output", args, "[bg_1: running]\n42.1 GiB / 767.7 GiB"); + assert!(progressing.running_background_poll); + assert!(!progressing.idle_background_poll); + + let idle = guard.record_tool_result("bash_output", args, "[bg_1: running — no new output]"); + assert!(idle.running_background_poll && idle.idle_background_poll); + + let exited = guard.record_tool_result("bash_output", args, "[bg_1: exited code 0]\ndone"); + assert!(!exited.running_background_poll); + + let errored = guard.record_tool_result("bash_output", args, "Error: no background process"); + assert!(!errored.running_background_poll); + } + #[test] fn mutating_tools_are_not_hash_guarded() { let mut guard = ToolLoopGuardrail::default(); diff --git a/crates/hi-agent/src/tests/goal.rs b/crates/hi-agent/src/tests/goal.rs index 7ed9705..21b15e5 100644 --- a/crates/hi-agent/src/tests/goal.rs +++ b/crates/hi-agent/src/tests/goal.rs @@ -1354,7 +1354,6 @@ async fn step_capped_turn_with_progress_is_a_continuation_not_a_failure() { cfg.subagents.long_horizon = true; cfg.gates.review = ReviewPolicy::Off; cfg.loop_limits.max_steps = 1; // the write below consumes the whole turn budget - cfg.loop_limits.max_steps_explicit = true; let responses = vec![ write_content_completion( &changed.to_string_lossy(), @@ -1395,7 +1394,6 @@ async fn step_capped_turn_past_continuation_budget_records_failure() { cfg.subagents.long_horizon = true; cfg.gates.review = ReviewPolicy::Off; cfg.loop_limits.max_steps = 1; - cfg.loop_limits.max_steps_explicit = true; let responses = vec![ write_content_completion( &changed.to_string_lossy(), @@ -1440,7 +1438,6 @@ async fn step_capped_barren_turn_continues_under_the_barren_limit() { cfg.subagents.long_horizon = true; cfg.gates.review = ReviewPolicy::Off; cfg.loop_limits.max_steps = 1; // the non-editing bash call below consumes the whole budget - cfg.loop_limits.max_steps_explicit = true; let responses = vec![ bash_completion("echo exploring"), completion(vec![Content::Text("ran out of turn budget".into())], 1, 1), @@ -1476,7 +1473,6 @@ async fn step_capped_barren_run_fails_at_the_limit() { cfg.subagents.long_horizon = true; cfg.gates.review = ReviewPolicy::Off; cfg.loop_limits.max_steps = 1; - cfg.loop_limits.max_steps_explicit = true; let responses = vec![ bash_completion("echo exploring"), completion(vec![Content::Text("ran out of turn budget".into())], 1, 1), @@ -1518,7 +1514,6 @@ async fn oversized_milestone_decomposes_into_substeps() { cfg.gates.review = ReviewPolicy::Off; cfg.subagents.planner_model = Some("planner".into()); cfg.loop_limits.max_steps = 1; // the write consumes the whole turn budget (one model call) - cfg.loop_limits.max_steps_explicit = true; let substeps = "Scaffold the crate with core types\n\ Implement the encoder with tests\n\ Implement the decoder with tests\n"; @@ -1529,6 +1524,12 @@ Implement the decoder with tests\n"; &changed.to_string_lossy(), "a substantial implementation body, comfortably past the trivial-diff exemption", )), + // The capped turn's tool-free wrap-up round. + ProviderStep::Completion(completion( + vec![Content::Text("ran out of turn budget".into())], + 1, + 1, + )), // Then the milestone-split planner call returns the sub-steps. ProviderStep::Completion(completion(vec![Content::Text(substeps.into())], 1, 1)), ], diff --git a/crates/hi-agent/src/tests/goal_contract.rs b/crates/hi-agent/src/tests/goal_contract.rs index 7b8c425..285ad02 100644 --- a/crates/hi-agent/src/tests/goal_contract.rs +++ b/crates/hi-agent/src/tests/goal_contract.rs @@ -553,7 +553,11 @@ async fn exact_plan_goal_continuation_uses_real_context_and_implementation_guard ui.statuses, agent.last_turn_telemetry() ); - assert_eq!(agent.last_turn_telemetry().effective_max_steps, 120); + assert_eq!( + agent.last_turn_telemetry().effective_max_steps, + u32::MAX, + "implementation intent no longer imposes an implicit step cap" + ); assert_eq!( agent .task diff --git a/crates/hi-agent/src/tests/turn.rs b/crates/hi-agent/src/tests/turn.rs index 004b66f..8f0869b 100644 --- a/crates/hi-agent/src/tests/turn.rs +++ b/crates/hi-agent/src/tests/turn.rs @@ -1649,7 +1649,7 @@ async fn repeated_successful_background_output_poll_is_not_repeat_nudged() { } #[tokio::test] -async fn idle_background_output_tight_poll_is_nudged() { +async fn idle_background_output_tight_poll_reports_active_work() { let provider = std::sync::Arc::new(Canned(Mutex::new(Vec::new()))); let mut agent = Agent::new(provider.clone(), config()).unwrap(); let id = agent @@ -1697,8 +1697,8 @@ async fn idle_background_output_tight_poll_is_nudged() { assert!( ui.statuses .iter() - .any(|s| s.contains("tight-polled a quiet background process")), - "third consecutive idle poll should be nudged: {:?}", + .any(|s| s.contains("background process is still running")), + "third consecutive idle poll should trigger an active-work report: {:?}", ui.statuses ); assert!( @@ -1710,6 +1710,173 @@ async fn idle_background_output_tight_poll_is_nudged() { ); } +#[tokio::test] +async fn idle_background_poll_budget_exhaustion_reports_progress_without_stalling() { + let provider = std::sync::Arc::new(Canned(Mutex::new(Vec::new()))); + let mut cfg = config(); + cfg.loop_limits.max_repeat_nudges = 1; + let mut agent = Agent::new(provider.clone(), cfg).unwrap(); + let id = agent + .runtime + .background() + .spawn(agent.runtime.process_runner(), "sleep 600") + .unwrap(); + let bash_output = |id: &str| { + completion( + vec![Content::ToolCall { + id: "bo".into(), + name: "bash_output".into(), + arguments: serde_json::json!({ "id": id }).to_string(), + }], + 1, + 1, + ) + }; + provider.0.lock().unwrap().extend(vec![ + bash_output(&id), + bash_output(&id), + bash_output(&id), + bash_output(&id), + completion( + vec![Content::Text("Download is still running.".into())], + 1, + 1, + ), + ]); + let mut ui = RecUi::default(); + + let outcome = agent + .run_turn("watch the long download", &mut ui) + .await + .unwrap(); + + let _ = agent.runtime.background().kill(&id); + assert_ne!( + outcome.stop_reason, + crate::TurnStopReason::Stalled, + "statuses={:?}; telemetry={:?}", + ui.statuses, + agent.last_turn_telemetry() + ); + assert!(!agent.last_turn_telemetry().stalled_repeating); + assert!( + !agent.last_turn_telemetry().stalled_unfinished, + "a live background process must not mark the turn stalled" + ); + assert!( + ui.statuses + .iter() + .any(|status| status.contains("background process is still running")), + "expected immediate progress-report recovery: {:?}", + ui.statuses + ); +} + +#[tokio::test] +async fn waiting_on_live_background_with_fresh_output_ends_with_status_report() { + // The sol-view failure mode: a download with a progress bar delivers new + // bytes on every poll, so byte-identical idle detection never fires, and + // an incomplete plan re-arms the plan-continue nudge after every status + // answer — 85 nudge/poll cycles in one observed turn. The waiting + // classifier must key on the process lifecycle and the status answer must + // end the turn even though the plan still has pending steps. + let provider = std::sync::Arc::new(Canned(Mutex::new(Vec::new()))); + let mut agent = Agent::new(provider.clone(), config()).unwrap(); + let id = agent + .runtime + .background() + .spawn( + agent.runtime.process_runner(), + "i=0; while true; do i=$((i+1)); echo progress-$i; sleep 0.05; done", + ) + .unwrap(); + let bash_output = |id: &str| { + completion( + vec![Content::ToolCall { + id: "bo".into(), + name: "bash_output".into(), + arguments: serde_json::json!({ "id": id }).to_string(), + }], + 1, + 1, + ) + }; + provider.0.lock().unwrap().extend(vec![ + completion( + vec![ + Content::ToolCall { + id: "plan".into(), + name: "update_plan".into(), + arguments: serde_json::json!({ + "steps": [ + { "title": "Watch the download", "status": "active" }, + { "title": "Convert the file", "status": "pending" }, + ] + }) + .to_string(), + }, + Content::ToolCall { + id: "bo0".into(), + name: "bash_output".into(), + arguments: serde_json::json!({ "id": id }).to_string(), + }, + ], + 1, + 1, + ), + bash_output(&id), + bash_output(&id), + completion( + vec![Content::Text( + "Work remains in progress: the download is still running; conversion has not started.".into(), + )], + 1, + 1, + ), + ]); + let mut ui = RecUi::default(); + + let outcome = agent + .run_turn("watch the download and report status", &mut ui) + .await + .unwrap(); + + let _ = agent.runtime.background().kill(&id); + assert_ne!( + outcome.stop_reason, + crate::TurnStopReason::Stalled, + "statuses={:?}; telemetry={:?}", + ui.statuses, + agent.last_turn_telemetry() + ); + assert!(!agent.last_turn_telemetry().stalled_repeating); + assert!(!agent.last_turn_telemetry().stalled_unfinished); + assert!( + ui.statuses + .iter() + .any(|s| s.contains("wait once with wait_secs or wrap up")), + "the waiting budget should trigger the wrap-up request despite fresh output: {:?}", + ui.statuses + ); + assert!( + ui.statuses + .iter() + .any(|s| s.contains("ending the turn with the status report")), + "an incomplete plan must not re-arm the continue nudge while waiting: {:?}", + ui.statuses + ); + assert!( + agent + .messages() + .last() + .unwrap() + .text() + .contains("remains in progress"), + "the status answer is the turn's final message" + ); + agent.messages.validate_for_provider().unwrap(); +} + #[tokio::test] async fn repeated_completed_background_output_poll_is_bounded() { let id = "bg_1".to_string(); @@ -3222,26 +3389,27 @@ async fn zero_max_steps_is_clamped_to_one_model_round() { } #[tokio::test] -async fn dynamic_max_steps_apply_only_without_explicit_override() { - let mut cfg = config(); - cfg.loop_limits.max_steps = 999; - cfg.loop_limits.max_steps_explicit = false; +async fn no_implicit_step_cap_and_configured_cap_is_honored() { + // The intent-aware implicit caps (80/120/200) are gone: an unconfigured + // turn is uncapped regardless of how its intent is classified. Only a + // deliberately configured cap applies. let mut first_agent = agent( vec![completion(vec![Content::Text("done".into())], 4, 2)], - cfg, + config(), ); let mut ui = RecUi::default(); first_agent.run_turn("answer once", &mut ui).await.unwrap(); - assert_eq!(first_agent.last_turn_telemetry().effective_max_steps, 80); + assert_eq!( + first_agent.last_turn_telemetry().effective_max_steps, + u32::MAX, + "no implicit cap for a plain turn" + ); let inspected_path = temp_file("dynamic-read-only-steps"); std::fs::write(&inspected_path, "pub fn reviewed() {}\n").unwrap(); let inspected = inspected_path.to_string_lossy().to_string(); - let mut cfg = config(); - cfg.loop_limits.max_steps = 999; - cfg.loop_limits.max_steps_explicit = false; let mut read_only_agent = agent( vec![ completion( @@ -3261,7 +3429,7 @@ async fn dynamic_max_steps_apply_only_without_explicit_override() { 2, ), ], - cfg, + config(), ); let mut ui = RecUi::default(); @@ -3272,13 +3440,13 @@ async fn dynamic_max_steps_apply_only_without_explicit_override() { assert_eq!( read_only_agent.last_turn_telemetry().effective_max_steps, - 80 + u32::MAX, + "read-only intent classification must not impose a cap" ); let _ = std::fs::remove_file(inspected_path); let mut cfg = config(); cfg.loop_limits.max_steps = 7; - cfg.loop_limits.max_steps_explicit = true; let mut second_agent = agent( vec![completion(vec![Content::Text("done".into())], 4, 2)], cfg, @@ -3290,6 +3458,80 @@ async fn dynamic_max_steps_apply_only_without_explicit_override() { assert_eq!(second_agent.last_turn_telemetry().effective_max_steps, 7); } +#[tokio::test] +async fn capped_turn_gets_one_tool_free_wrap_up_round() { + // Hitting a configured step cap no longer kills the turn mid-flight: the + // model gets exactly one chat-only round to report where it left the work, + // and the turn still ends Incomplete · StepLimit. + let mut cfg = config(); + cfg.loop_limits.max_steps = 1; + let modes = std::sync::Arc::new(Mutex::new(Vec::new())); + let provider = RecordToolModes { + responses: Mutex::new(vec![ + bash_completion("echo working"), + completion( + vec![Content::Text( + "Ran the first check; the remaining verification has not run yet.".into(), + )], + 1, + 1, + ), + ]), + modes: modes.clone(), + }; + let mut agent = Agent::new(std::sync::Arc::new(provider), cfg).unwrap(); + let mut ui = RecUi::default(); + + let outcome = agent.run_turn("run the checks", &mut ui).await.unwrap(); + + assert_eq!(outcome.stop_reason, crate::TurnStopReason::StepLimit); + assert!(agent.last_turn_telemetry().hit_step_cap); + assert!( + ui.assistant.contains("remaining verification"), + "the wrap-up answer must reach the user: {}", + ui.assistant + ); + assert!( + ui.statuses + .iter() + .any(|s| s.contains("asking for a final wrap-up")), + "expected the wrap-up request status: {:?}", + ui.statuses + ); + assert_eq!( + modes.lock().unwrap().last(), + Some(&ToolMode::ChatOnly), + "the wrap-up round must be tool-free" + ); + agent.messages.validate_for_provider().unwrap(); +} + +#[tokio::test] +async fn capped_turn_wrap_up_round_is_granted_only_once() { + // If the wrap-up round comes back with tool-call noise instead of text + // (chat-only requests suppress calls, so they are dropped), the turn ends + // at the cap rather than granting further rounds. + let mut cfg = config(); + cfg.loop_limits.max_steps = 1; + let responses = vec![ + bash_completion("echo working"), + bash_completion("echo trying to keep going"), + ]; + let mut agent = agent(responses, cfg); + let mut ui = RecUi::default(); + + let outcome = agent.run_turn("run the checks", &mut ui).await.unwrap(); + + assert_eq!(outcome.stop_reason, crate::TurnStopReason::StepLimit); + assert_eq!( + ui.tool_results.len(), + 1, + "the wrap-up round must not execute tools: {:?}", + ui.tool_results + ); + agent.messages.validate_for_provider().unwrap(); +} + #[tokio::test] async fn read_only_review_sprawl_is_bounded() { // The "inspection sprawl" failure mode: a read-only review turn reads many diff --git a/crates/hi-agent/src/transcript.rs b/crates/hi-agent/src/transcript.rs index 715fe78..c15e6f7 100644 --- a/crates/hi-agent/src/transcript.rs +++ b/crates/hi-agent/src/transcript.rs @@ -216,6 +216,13 @@ fn is_nudge_text(text: &str) -> bool { text.starts_with("[hi:nudge:") } +/// The background handle a `bash_output` call polls (`{"id":"bg_1"}` → `bg_1`). +pub(crate) fn background_poll_handle(arguments: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(arguments).ok()?; + let id = value.get("id")?.as_str()?; + (!id.is_empty()).then(|| id.to_string()) +} + /// The conversation transcript, enforcing provider-safety invariants. #[derive(Clone, Debug)] pub(crate) struct Transcript { @@ -429,6 +436,63 @@ impl Transcript { /// call it just made instead of a bare placeholder. /// /// [`push_assistant_with_results`]: Self::push_assistant_with_results + /// Shrink every superseded `bash_output` result for `handle` to a one-line + /// digest, keeping only the newest poll verbatim. A turn (or session) + /// watching a long-running background process otherwise accumulates + /// hundreds of stale progress dumps that are re-sent — and re-billed — + /// with every subsequent model request, while only the latest poll carries + /// information the model still needs. + pub(crate) fn fold_superseded_background_polls(&mut self, handle: &str) { + let mut poll_call_ids: Vec = Vec::new(); + for message in self.messages.iter() { + for block in &message.content { + if let Content::ToolCall { + id, + name, + arguments, + } = block + && name == "bash_output" + && background_poll_handle(arguments).as_deref() == Some(handle) + { + poll_call_ids.push(id.clone()); + } + } + } + if poll_call_ids.len() < 2 { + return; + } + let superseded: std::collections::HashSet<&str> = poll_call_ids + [..poll_call_ids.len() - 1] + .iter() + .map(String::as_str) + .collect(); + let digest = format!("[{handle}: superseded poll — see the latest bash_output result]"); + // Skip the rewrite (and the copy-on-write clone) when every superseded + // poll is already folded — the common case after the first fold. + let already_folded = self + .messages + .iter() + .flat_map(|message| &message.content) + .all(|block| match block { + Content::ToolResult { call_id, output } if superseded.contains(call_id.as_str()) => { + *output == digest + } + _ => true, + }); + if already_folded { + return; + } + for message in self.make_mut().iter_mut() { + for block in &mut message.content { + if let Content::ToolResult { call_id, output } = block + && superseded.contains(call_id.as_str()) + { + *output = digest.clone(); + } + } + } + } + pub(crate) fn push_assistant_text_only(&mut self, content: Vec) { let mut text_only: Vec = content .into_iter() @@ -973,6 +1037,58 @@ mod tests { transcript.validate_for_provider().unwrap(); } + #[test] + fn superseded_background_polls_fold_to_a_one_line_digest() { + let mut t = Transcript::new(vec![user("watch the download")]); + let poll = |call_id: &str, handle: &str| Content::ToolCall { + id: call_id.into(), + name: "bash_output".into(), + arguments: format!(r#"{{"id":"{handle}"}}"#), + }; + t.push_assistant_with_results( + vec![poll("p1", "bg_1"), poll("q1", "bg_2")], + vec![ + ("p1".into(), "[bg_1: running]\n10 GiB / 100 GiB".into()), + ("q1".into(), "[bg_2: running]\n1 GiB / 50 GiB".into()), + ], + ); + t.push_assistant_with_results( + vec![poll("p2", "bg_1")], + vec![("p2".into(), "[bg_1: running]\n11 GiB / 100 GiB".into())], + ); + t.push_assistant_with_results( + vec![poll("p3", "bg_1")], + vec![("p3".into(), "[bg_1: running]\n12 GiB / 100 GiB".into())], + ); + + t.fold_superseded_background_polls("bg_1"); + + let output_of = |t: &Transcript, id: &str| -> String { + t.as_slice() + .iter() + .flat_map(|m| &m.content) + .find_map(|c| match c { + Content::ToolResult { call_id, output } if call_id == id => { + Some(output.clone()) + } + _ => None, + }) + .unwrap() + }; + let digest = "[bg_1: superseded poll — see the latest bash_output result]"; + assert_eq!(output_of(&t, "p1"), digest); + assert_eq!(output_of(&t, "p2"), digest); + assert!( + output_of(&t, "p3").contains("12 GiB"), + "the newest poll keeps its full output" + ); + assert!( + output_of(&t, "q1").contains("1 GiB"), + "other handles are untouched" + ); + t.validate_for_provider().unwrap(); + } + #[test] fn push_assistant_with_results_pairs_every_call() { let mut t = Transcript::new(vec![user("do it")]); diff --git a/crates/hi-cli/src/agent_build.rs b/crates/hi-cli/src/agent_build.rs index 5dfb270..89e424d 100644 --- a/crates/hi-cli/src/agent_build.rs +++ b/crates/hi-cli/src/agent_build.rs @@ -69,7 +69,6 @@ pub(crate) fn build_agent( }, loop_limits: hi_agent::AgentLoopLimits { max_steps: cli.max_steps.unwrap_or(u32::MAX), - max_steps_explicit: cli.max_steps.is_some(), max_tool_calls: cli.max_tool_calls.unwrap_or(u32::MAX), ..hi_agent::AgentLoopLimits::default() }, diff --git a/crates/hi-cli/src/commands.rs b/crates/hi-cli/src/commands.rs index 8ef6b8e..45d35c2 100644 --- a/crates/hi-cli/src/commands.rs +++ b/crates/hi-cli/src/commands.rs @@ -279,7 +279,7 @@ pub(crate) fn handle_command(agent: &mut Agent, command: hi_agent::Command) -> b } ConfigArg::MaxStepsAuto => { agent.set_max_steps_auto(); - println!("\x1b[2mstep limit → auto (intent-aware; applies next turn)\x1b[0m"); + println!("\x1b[2mstep limit → off (auto = off; applies next turn)\x1b[0m"); } ConfigArg::MoeStreaming(mode) => { // Set the env var that the MLX backend reads at model load diff --git a/crates/hi-cli/src/config/cli.rs b/crates/hi-cli/src/config/cli.rs index 35e2f26..8fee4a8 100644 --- a/crates/hi-cli/src/config/cli.rs +++ b/crates/hi-cli/src/config/cli.rs @@ -212,7 +212,8 @@ pub struct Cli { #[arg(long, value_enum)] pub tool_set: Option, - /// Safety cap on model calls per turn (stops runaway tool loops). + /// Optional hard cap on model calls per turn (no cap by default; a capped + /// turn gets one tool-free wrap-up round when the limit is hit). #[arg(long)] pub max_steps: Option, diff --git a/crates/hi-tools/src/background.rs b/crates/hi-tools/src/background.rs index 370b9f3..ec5e943 100644 --- a/crates/hi-tools/src/background.rs +++ b/crates/hi-tools/src/background.rs @@ -42,6 +42,9 @@ struct BgProc { effect_baseline: Option>, inner: Mutex, reaped: Notify, + /// Woken on every output append and lifecycle transition, so a blocking + /// [`BackgroundRegistry::poll_wait`] sleeps instead of spinning. + changed: Notify, } struct EffectBaseline { @@ -168,6 +171,7 @@ impl BackgroundRegistry { terminal_effects: None, }), reaped: Notify::new(), + changed: Notify::new(), }); { let mut reg = self.processes.lock().unwrap(); @@ -216,6 +220,7 @@ impl BackgroundRegistry { terminal_effects: None, }), reaped: Notify::new(), + changed: Notify::new(), }); { @@ -238,6 +243,35 @@ impl BackgroundRegistry { poll_from(self, id) } + /// Like [`poll`](Self::poll), but blocks up to `wait` until the process + /// produces new output or reaches a terminal state — so one tool call can + /// cover minutes of waiting instead of a tight model-round poll loop. On + /// timeout it returns the normal idle status. The wait sleeps on a + /// notification (no spinning) and holds no locks while parked. + pub async fn poll_wait(&self, id: &str, wait: std::time::Duration) -> Result { + let proc = lookup(self, id)?; + let deadline = tokio::time::Instant::now() + wait; + loop { + // Register interest before checking the condition so an append or + // exit that lands between the check and the await still wakes us. + let notified = proc.changed.notified(); + tokio::pin!(notified); + notified.as_mut().enable(); + { + let inner = proc.inner.lock().unwrap(); + let has_new_output = inner.output.len() > inner.read_offset; + if has_new_output || !matches!(inner.state, BgState::Running) { + break; + } + } + tokio::select! { + () = &mut notified => {} + () = tokio::time::sleep_until(deadline) => break, + } + } + poll_from(self, id) + } + pub fn kill(&self, id: &str) -> Result { kill_from(self, id) } @@ -408,6 +442,7 @@ fn kill_from(registry: &BackgroundRegistry, id: &str) -> Result { if let Some(pgid) = proc.pgid { crate::tools::kill_group(pgid); } + proc.changed.notify_waiters(); Ok(format!("[{id}] killed (`{}`)", proc.command)) } @@ -556,6 +591,7 @@ async fn drive( inner.reaped = true; drop(inner); proc.reaped.notify_waiters(); + proc.changed.notify_waiters(); } /// Append every line from one pipe into the shared buffer, enforcing the size @@ -584,6 +620,8 @@ async fn pump(pipe: Option, proc: &BgProc) { inner.output.drain(..cut); inner.read_offset = inner.read_offset.saturating_sub(cut); } + drop(inner); + proc.changed.notify_waiters(); } } @@ -741,6 +779,77 @@ mod tests { assert!(kill("bg_does_not_exist").is_err()); } + #[tokio::test] + async fn poll_wait_blocks_until_new_output_arrives() { + let _guard = TEST_LOCK.lock().await; + let registry = BackgroundRegistry::default(); + let runner = crate::ProcessRunner::from_current_dir().unwrap(); + let id = registry + .spawn(&runner, "sleep 0.4; echo late-line; sleep 600") + .unwrap(); + + let started = std::time::Instant::now(); + let out = registry + .poll_wait(&id, Duration::from_secs(10)) + .await + .unwrap(); + + assert!( + out.contains("late-line"), + "the wait should return the fresh output: {out:?}" + ); + assert!( + started.elapsed() < Duration::from_secs(8), + "must wake on output, not sleep out the full wait: {:?}", + started.elapsed() + ); + registry.kill(&id).unwrap(); + } + + #[tokio::test] + async fn poll_wait_times_out_to_idle_status_on_a_quiet_process() { + let _guard = TEST_LOCK.lock().await; + let registry = BackgroundRegistry::default(); + let runner = crate::ProcessRunner::from_current_dir().unwrap(); + let id = registry.spawn(&runner, "sleep 600").unwrap(); + + let started = std::time::Instant::now(); + let out = registry + .poll_wait(&id, Duration::from_millis(200)) + .await + .unwrap(); + + assert!( + out.contains("running — no new output"), + "a timed-out wait reports genuine idleness: {out:?}" + ); + assert!(started.elapsed() >= Duration::from_millis(180)); + registry.kill(&id).unwrap(); + } + + #[tokio::test] + async fn poll_wait_wakes_promptly_when_the_process_is_killed() { + let _guard = TEST_LOCK.lock().await; + let registry = std::sync::Arc::new(BackgroundRegistry::default()); + let runner = crate::ProcessRunner::from_current_dir().unwrap(); + let id = registry.spawn(&runner, "sleep 600").unwrap(); + + let waiter = { + let registry = registry.clone(); + let id = id.clone(); + tokio::spawn(async move { registry.poll_wait(&id, Duration::from_secs(30)).await }) + }; + tokio::time::sleep(Duration::from_millis(100)).await; + registry.kill(&id).unwrap(); + + let out = tokio::time::timeout(Duration::from_secs(5), waiter) + .await + .expect("kill must wake the waiter") + .unwrap() + .unwrap(); + assert!(out.contains("killed"), "got: {out:?}"); + } + #[tokio::test] async fn adopt_keeps_child_running_and_seeds_output() { let _guard = TEST_LOCK.lock().await; diff --git a/crates/hi-tools/src/catalog.rs b/crates/hi-tools/src/catalog.rs index c865d75..0683a31 100644 --- a/crates/hi-tools/src/catalog.rs +++ b/crates/hi-tools/src/catalog.rs @@ -126,7 +126,7 @@ fn build_tool_specs() -> Vec { }, ToolSpec { name: "bash".into(), - description: "Run a shell command via `sh -c` in the current working directory and return combined stdout/stderr. stdin is closed, so commands never block on input. A foreground command still running at its timeout is moved to the background (kept running, not killed) and returns a handle id — read its output with bash_output and stop it with bash_kill. For a process you know upfront is long-lived or blocking (a dev server, a file watcher, `tail -f`), set run_in_background:true to get the handle immediately. For a slow but finite build or test suite, raise `timeout` so it finishes in the foreground.".into(), + description: "Run a shell command via `sh -c` in the current working directory and return combined stdout/stderr. stdin is closed, so commands never block on input. A foreground command still running at its timeout is moved to the background (kept running, not killed) and returns a handle id — read its output with bash_output (pass wait_secs to block for new output instead of polling) and stop it with bash_kill. For a process you know upfront is long-lived or blocking (a dev server, a file watcher, `tail -f`), set run_in_background:true to get the handle immediately. For a slow but finite build or test suite, raise `timeout` so it finishes in the foreground. For very long background work (a big download, a multi-hour job), chain the follow-up steps into the command itself (`fetch && convert`) so nothing has to babysit it.".into(), parameters: json!({ "type": "object", "properties": { @@ -139,11 +139,12 @@ fn build_tool_specs() -> Vec { }, ToolSpec { name: "bash_output".into(), - description: "Read new output (stdout+stderr) from a background process started by `bash` with run_in_background, since the last read. Also reports whether it is still running, exited (with code), or was killed. Returns immediately. Do not tight-poll while it reports running with no new output — sleep meaningfully between checks, do other work, or for a finite build/test raise `bash` timeout and run it in the foreground instead.".into(), + description: "Read new output (stdout+stderr) from a background process started by `bash` with run_in_background, since the last read. Also reports whether it is still running, exited (with code), or was killed. Never tight-poll: pass wait_secs (e.g. 300, max 600) to block until the process produces new output or exits — one waiting call replaces a poll loop. For work expected to outlast the turn (large downloads, long jobs), chain follow-up steps into the background command itself (`cmd && next`), report the current status, and stop instead of babysitting it.".into(), parameters: json!({ "type": "object", "properties": { - "id": { "type": "string", "description": "The background process handle returned by bash (e.g. `bg_1`)." } + "id": { "type": "string", "description": "The background process handle returned by bash (e.g. `bg_1`)." }, + "wait_secs": { "type": "integer", "description": "Optional: block up to this many seconds (max 600) until the process emits new output or exits, then return. 0 or omitted returns immediately. Prefer a single generous wait over repeated instant polls." } }, "required": ["id"] }), diff --git a/crates/hi-tools/src/lib.rs b/crates/hi-tools/src/lib.rs index 94d23fb..da80b05 100644 --- a/crates/hi-tools/src/lib.rs +++ b/crates/hi-tools/src/lib.rs @@ -673,6 +673,57 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[tokio::test] + async fn bash_output_wait_secs_blocks_until_output_instead_of_polling() { + let dir = unique_test_dir("hi-background-wait-secs"); + let state = dir.join(".hi/state"); + std::fs::create_dir_all(&state).unwrap(); + let lsp = std::sync::Arc::new(hi_lsp::LspManager::new(&dir).unwrap()); + let background = crate::BackgroundRegistry::default(); + let cache = std::sync::Mutex::new(crate::ReadCache::new()); + let repo_map = std::sync::Mutex::new(crate::RepoMapCache::new()); + + let started = crate::execute_in_runtime( + &dir, + &state, + &lsp, + &background, + &cache, + &repo_map, + "bash", + r#"{"command":"sleep 0.4; echo waited-for-this; sleep 600","run_in_background":true}"#, + ) + .await; + let id = started.background.as_ref().unwrap().id.clone(); + + let clock = std::time::Instant::now(); + let polled = crate::execute_in_runtime( + &dir, + &state, + &lsp, + &background, + &cache, + &repo_map, + "bash_output", + &serde_json::json!({ "id": id, "wait_secs": 10 }).to_string(), + ) + .await; + + assert_eq!(polled.status, crate::ToolStatus::Succeeded); + assert!( + polled.content.contains("waited-for-this"), + "one waiting call should return the fresh output: {}", + polled.content + ); + assert!( + clock.elapsed() < std::time::Duration::from_secs(8), + "the wait must wake on output, not sleep out its budget: {:?}", + clock.elapsed() + ); + let _ = background.kill(&id); + let _ = std::fs::remove_dir_all(dir); + } + #[tokio::test] async fn background_bash_reports_and_seals_terminal_file_effects() { let dir = unique_test_dir("hi-background-effects"); diff --git a/crates/hi-tools/src/tools/mod.rs b/crates/hi-tools/src/tools/mod.rs index 9773973..b701838 100644 --- a/crates/hi-tools/src/tools/mod.rs +++ b/crates/hi-tools/src/tools/mod.rs @@ -849,9 +849,22 @@ async fn run( #[derive(Deserialize)] struct Args { id: String, + #[serde(default)] + wait_secs: u64, } let args: Args = parse(arguments)?; - let result = resources.background.poll(&args.id)?; + // Blocking wait: one tool call covers minutes of quiet process + // time instead of a model-round-per-poll loop. Capped so a + // forgotten wait cannot park a turn for more than 10 minutes. + let wait_secs = args.wait_secs.min(600); + let result = if wait_secs > 0 { + resources + .background + .poll_wait(&args.id, std::time::Duration::from_secs(wait_secs)) + .await? + } else { + resources.background.poll(&args.id)? + }; let background = resources.background.outcome(&args.id)?; if let Ok(mut cache) = resources.read_cache.lock() { cache.clear(); diff --git a/crates/hi-tui/src/app/commands.rs b/crates/hi-tui/src/app/commands.rs index 1a5515e..5ab1c7a 100644 --- a/crates/hi-tui/src/app/commands.rs +++ b/crates/hi-tui/src/app/commands.rs @@ -1732,7 +1732,7 @@ impl crate::App { ConfigArg::MaxStepsAuto => { agent.set_max_steps_auto(); self.push(Line::styled( - "step limit → auto (intent-aware; applies next turn)".to_string(), + "step limit → off (auto = off; applies next turn)".to_string(), dim(), )); } diff --git a/crates/hi-tui/src/tests.rs b/crates/hi-tui/src/tests.rs index 813cef4..5ffe2b0 100644 --- a/crates/hi-tui/src/tests.rs +++ b/crates/hi-tui/src/tests.rs @@ -443,7 +443,7 @@ async fn config_command_sets_disables_and_restores_automatic_step_limit() { )); let mut agent = hi_agent::Agent::new(provider, hi_agent::AgentConfig::default()).unwrap(); let mut app = test_app("openai", "gpt-4o"); - assert_eq!(agent.max_steps_setting(), "auto"); + assert_eq!(agent.max_steps_setting(), "off", "uncapped by default"); app.handle_command(&mut agent, hi_agent::Command::Config("steps 350".into())) .await; @@ -453,10 +453,12 @@ async fn config_command_sets_disables_and_restores_automatic_step_limit() { .await; assert_eq!(agent.max_steps_setting(), "off"); + app.handle_command(&mut agent, hi_agent::Command::Config("steps 350".into())) + .await; app.handle_command(&mut agent, hi_agent::Command::Config("steps auto".into())) .await; - assert_eq!(agent.max_steps_setting(), "auto"); - assert!(app.transcript_text().contains("step limit → auto")); + assert_eq!(agent.max_steps_setting(), "off", "auto is an alias for off"); + assert!(app.transcript_text().contains("step limit → off (auto = off")); } #[test] diff --git a/docs/0.2-migration.md b/docs/0.2-migration.md index 3004710..0f2d107 100644 --- a/docs/0.2-migration.md +++ b/docs/0.2-migration.md @@ -57,9 +57,11 @@ removed. Use: --tool-set dynamic|minimal|full ``` -When `--max-steps` is omitted, the automatic caps are 80 model calls for a -clearly read-only turn, 120 for recognized implementation work, and 200 for a -general or ambiguous turn. Ambiguous turns still receive mutation safeguards. +When `--max-steps` is omitted there is no per-turn model-call cap (the 0.2 +automatic 80/120/200 intent caps were removed): stalled or repeating turns are +ended by the no-progress budgets instead. When a cap is set and hit, the turn +gets one final tool-free wrap-up round before stopping. Ambiguous turns still +receive mutation safeguards. One-shot exit codes are now stable: