From 73d6839ee9a1d24a8ca95c620b3178b7ccb54fc5 Mon Sep 17 00:00:00 2001 From: Ryan Ward Date: Fri, 11 Sep 2026 05:45:19 -0500 Subject: [PATCH] Remove default agent turn cap and preserve queued tool status Allow new provider runs to continue without an arbitrary lifetime turn budget while preserving explicit limits, checkpoint compatibility, and bounded retries. Keep pending tool proposals visibly queued until execution status arrives, and show cancellation only when supported by actual state. Validation: formatting, inline test-layout checks, both presubmit Clippy commands, 325 targeted regressions, and 54 core tests after final cleanup passed. --- app/src/ai/blocklist/block/model/helper.rs | 27 +++++++++++ .../ai/blocklist/block/view_impl/output.rs | 17 ++++--- .../inline_action/requested_command.rs | 18 ++++---- crates/galaxy_agent_core/src/provider_run.rs | 38 +++++++++------ .../src/provider_run_tests.rs | 46 ++++++++++++++++++- 5 files changed, 113 insertions(+), 33 deletions(-) diff --git a/app/src/ai/blocklist/block/model/helper.rs b/app/src/ai/blocklist/block/model/helper.rs index 3d6344eb..908383f8 100644 --- a/app/src/ai/blocklist/block/model/helper.rs +++ b/app/src/ai/blocklist/block/model/helper.rs @@ -6,6 +6,7 @@ use crate::ai::agent::{ AIAgentAction, AIAgentActionId, AIAgentActionType, AIAgentInput, AIAgentOutputMessageType, SummarizationType, }; +use crate::ai::blocklist::action_model::AIActionStatus; use crate::ai::blocklist::BlocklistAIActionModel; use crate::BlocklistAIHistoryModel; @@ -14,6 +15,12 @@ use crate::BlocklistAIHistoryModel; // These are defined within a separate trait rather than default implementations of `AIBlockModel` // so implementations cannot errantly override them. pub trait AIBlockModelHelper { + fn action_status_for_display( + &self, + action_model: &BlocklistAIActionModel, + action_id: &AIAgentActionId, + app: &AppContext, + ) -> Option; fn is_first_action_in_output(&self, action_id: &AIAgentActionId, app: &AppContext) -> bool; fn conversation<'a>(&self, app: &'a AppContext) -> Option<&'a AIConversation>; @@ -41,6 +48,26 @@ pub trait AIBlockModelHelper { } impl AIBlockModelHelper for T { + fn action_status_for_display( + &self, + action_model: &BlocklistAIActionModel, + action_id: &AIAgentActionId, + app: &AppContext, + ) -> Option { + let conversation = self.conversation(app)?; + let status = action_model.get_action_status(conversation.id(), action_id); + // Proposals appear before the full batch is handed to the executor. A + // finished output message is not evidence that an unstarted tool was cancelled. + status.or_else(|| { + (!conversation.status().is_done() + && self.is_latest_visible_exchange_in_root_task(app) + && self.status(app).output_to_render().is_some_and(|output| { + output.get().actions().any(|action| action.id == *action_id) + })) + .then_some(AIActionStatus::Queued) + }) + } + fn is_first_action_in_output(&self, action_id: &AIAgentActionId, app: &AppContext) -> bool { self.status(app).output_to_render().is_some_and(|output| { output diff --git a/app/src/ai/blocklist/block/view_impl/output.rs b/app/src/ai/blocklist/block/view_impl/output.rs index 0a1e775d..b6e85f2b 100644 --- a/app/src/ai/blocklist/block/view_impl/output.rs +++ b/app/src/ai/blocklist/block/view_impl/output.rs @@ -3702,17 +3702,11 @@ pub fn action_icon( app: &AppContext, ) -> galaxyui::elements::Icon { let appearance = Appearance::as_ref(app); - let status = ai_block_model - .conversation_id(app) - .and_then(|conversation_id| { - action_model - .as_ref(app) - .get_action_status(conversation_id, action_id) - }); + let status = ai_block_model.action_status_for_display(action_model.as_ref(app), action_id, app); match status { Some(status) => match status { AIActionStatus::Preprocessing => icons::gray_circle_icon(appearance), - AIActionStatus::Queued => icons::gray_stop_icon(appearance), + AIActionStatus::Queued => icons::gray_clock_icon(appearance), AIActionStatus::Blocked => icons::yellow_stop_icon(appearance), AIActionStatus::RunningAsync => icons::yellow_running_icon(appearance), AIActionStatus::Finished(result) => { @@ -3741,8 +3735,13 @@ pub fn action_icon( } else { icons::gray_circle_icon(appearance) } - } else { + } else if ai_block_model + .conversation(app) + .is_some_and(|conversation| conversation.status().is_cancelled()) + { inline_action_icons::cancelled_icon(appearance) + } else { + icons::gray_circle_icon(appearance) } } } diff --git a/app/src/ai/blocklist/inline_action/requested_command.rs b/app/src/ai/blocklist/inline_action/requested_command.rs index f716f860..9bab45b1 100644 --- a/app/src/ai/blocklist/inline_action/requested_command.rs +++ b/app/src/ai/blocklist/inline_action/requested_command.rs @@ -1171,10 +1171,11 @@ impl RequestedCommandView { app: &AppContext, ) -> Box { let appearance = Appearance::as_ref(app); - let action_status = self - .action_model - .as_ref(app) - .get_action_status(self.client_ids.conversation_id, &self.action_id); + let action_status = self.block_model.action_status_for_display( + self.action_model.as_ref(app), + &self.action_id, + app, + ); let mut title: Cow<'static, str>; let mut font_override = None; @@ -1541,10 +1542,11 @@ impl View for RequestedCommandView { fn render(&self, app: &AppContext) -> Box { let appearance = Appearance::as_ref(app); let theme = appearance.theme(); - let action_status = self - .action_model - .as_ref(app) - .get_action_status(self.client_ids.conversation_id, &self.action_id); + let action_status = self.block_model.action_status_for_display( + self.action_model.as_ref(app), + &self.action_id, + app, + ); let is_last_output_message_in_output = self .block_model diff --git a/crates/galaxy_agent_core/src/provider_run.rs b/crates/galaxy_agent_core/src/provider_run.rs index e773c0d1..0346c289 100644 --- a/crates/galaxy_agent_core/src/provider_run.rs +++ b/crates/galaxy_agent_core/src/provider_run.rs @@ -90,14 +90,15 @@ impl From<&str> for ProviderRequestProfile { #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct ProviderRunLimits { - pub max_model_turns: u32, + /// Optional lifetime budget. Interactive runs continue until completion or cancellation. + pub max_model_turns: Option, pub max_model_retries_per_turn: u32, } impl Default for ProviderRunLimits { fn default() -> Self { Self { - max_model_turns: 100, + max_model_turns: None, // Three retries means the initial call plus three delayed retries; // the fourth failure is surfaced to the user. max_model_retries_per_turn: 3, @@ -453,7 +454,7 @@ impl ProviderRun { profile: impl Into, mut limits: ProviderRunLimits, ) -> Self { - limits.max_model_turns = limits.max_model_turns.max(1); + limits.max_model_turns = limits.max_model_turns.map(|limit| limit.max(1)); Self { id: id.into(), epoch: RunEpoch::default(), @@ -562,13 +563,15 @@ impl ProviderRun { if self.profile.as_str().is_empty() { return Err(invalid("request profile must not be empty".to_string())); } - if self.limits.max_model_turns == 0 { + if self.limits.max_model_turns == Some(0) { return Err(invalid("model-turn limit must be at least one".to_string())); } - if self.model_turns > self.limits.max_model_turns { + if let Some(limit) = self.limits.max_model_turns + && self.model_turns > limit + { return Err(invalid(format!( - "model-turn counter {} exceeds limit {}", - self.model_turns, self.limits.max_model_turns + "model-turn counter {} exceeds limit {limit}", + self.model_turns ))); } if self.epoch.get() < u64::from(self.model_turns) { @@ -595,7 +598,7 @@ impl ProviderRun { match &self.state { ProviderRunState::ReadyToCallModel => {} ProviderRunState::AwaitingModel { call } => { - if self.model_turns >= self.limits.max_model_turns { + if self.model_turn_limit_reached() { return Err(invalid( "awaiting a model call after reaching the model-turn limit".to_string(), )); @@ -724,9 +727,7 @@ impl ProviderRun { "retry-limit failure lacks a recoverable source".to_string(), )); } - ProviderRunFailureKind::TurnLimitExceeded - if self.model_turns < self.limits.max_model_turns => - { + ProviderRunFailureKind::TurnLimitExceeded if !self.model_turn_limit_reached() => { return Err(invalid( "turn-limit failure occurred before reaching the limit".to_string(), )); @@ -864,17 +865,26 @@ impl ProviderRun { matches!(self.state, ProviderRunState::ReadyToCallModel).then(|| self.current_work_id()) } + fn model_turn_limit_reached(&self) -> bool { + self.limits + .max_model_turns + .is_some_and(|limit| self.model_turns >= limit) + } + pub fn next_step(&mut self) -> Result, ProviderRunProtocolError> { loop { match self.state.clone() { ProviderRunState::ReadyToCallModel => { - if self.model_turns >= self.limits.max_model_turns { + if let Some(limit) = self + .limits + .max_model_turns + .filter(|limit| self.model_turns >= *limit) + { self.state = ProviderRunState::Failed { failure: ProviderRunFailure { kind: ProviderRunFailureKind::TurnLimitExceeded, message: format!( - "provider run reached its {} model-turn limit", - self.limits.max_model_turns + "provider run reached its {limit} model-turn limit" ), source: None, }, diff --git a/crates/galaxy_agent_core/src/provider_run_tests.rs b/crates/galaxy_agent_core/src/provider_run_tests.rs index c2df7b1b..368d7c23 100644 --- a/crates/galaxy_agent_core/src/provider_run_tests.rs +++ b/crates/galaxy_agent_core/src/provider_run_tests.rs @@ -154,7 +154,7 @@ fn stale_model_completion_is_rejected_without_mutation() { #[test] fn model_retries_reuse_work_identity_and_stop_at_the_budget() { let mut run = run_with_limits(ProviderRunLimits { - max_model_turns: 5, + max_model_turns: Some(5), max_model_retries_per_turn: 1, }); let call = next_model_call(&mut run); @@ -752,7 +752,7 @@ fn failure_while_tools_are_pending_records_correlated_errors_before_terminal_sta #[test] fn turn_limit_cannot_finish_successfully_after_tools_require_another_model_turn() { let mut run = run_with_limits(ProviderRunLimits { - max_model_turns: 1, + max_model_turns: Some(1), max_model_retries_per_turn: 0, }); let batch = accept_tool_turn( @@ -1384,3 +1384,45 @@ fn restore_normalization_commits_a_fully_resolved_batch() { } if tool_use_id == "resolved" )); } + +#[test] +fn default_run_continues_past_one_hundred_tool_turns_and_restores() { + let mut run = run(); + for index in 0..150 { + let id = format!("poll-{index}"); + let batch = accept_tool_turn( + &mut run, + tool_turn( + vec![tool_call(&id, "read_shell_command_output")], + &["read_shell_command_output"], + ), + ); + run.complete_tool(&batch.work_id, successful_result(&id, "still running")) + .unwrap(); + run.commit_tool_batch(&batch.work_id).unwrap(); + if index == 99 { + run = serde_json::from_str(&serde_json::to_string(&run).unwrap()).unwrap(); + run.validate_restored_state().unwrap(); + } + } + assert_eq!(run.model_turns(), 150); + let call = next_model_call(&mut run); + run.accept_model_turn(&call.work_id, text_turn("Finished")) + .unwrap(); + assert_eq!(run.next_step().unwrap(), None); + assert!(matches!( + run.state(), + ProviderRunState::AwaitingDriver { .. } + )); +} + +#[test] +fn serialized_numeric_turn_budgets_remain_explicit_limits() { + let limits: ProviderRunLimits = serde_json::from_value(json!({ + "max_model_turns": 100, + "max_model_retries_per_turn": 3 + })) + .unwrap(); + assert_eq!(limits.max_model_turns, Some(100)); + assert_eq!(ProviderRunLimits::default().max_model_turns, None); +}