diff --git a/AGENTS.md b/AGENTS.md index 9915ff23..868a84aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,6 +128,8 @@ Key invariants: - The stream emits a `UserQuery` proto message at the start of each response for conversation title - Progressive summary (if present) is prepended to the messages array as a user/assistant pair in `translator.rs` - Loop prevention guardrail in `controller.rs` detects repeated tool failures (3+ identical) and injects corrective instructions +- Direct-provider long-running shell follow-ups create unlinked CLI tasks under the root task with an empty subagent tool-call ID; `TaskStore` linearization must include their exchanges chronologically even though no parent `Subagent` output references them +- Orchestrated child conversations are leaf workers by default: nested `RunAgents` and legacy `StartAgent` calls must be rejected before autonomous or permission bypasses, and child requests must not advertise delegation tools ### Platform Setup - `./script/bootstrap` - Platform-specific setup plus common agent skill installation from `skills-lock.json`; prompts for project/global when an install or update is needed unless a target flag or environment override is provided. diff --git a/app/src/ai/agent/api/impl.rs b/app/src/ai/agent/api/impl.rs index eef5cd8c..e567a094 100644 --- a/app/src/ai/agent/api/impl.rs +++ b/app/src/ai/agent/api/impl.rs @@ -19,11 +19,16 @@ pub async fn generate_multi_agent_output( cancellation_rx: futures::channel::oneshot::Receiver<()>, ) -> Result { let supported_tools_override = params.supported_tools_override.take(); - let supported_tools = supported_tools_override + let mut supported_tools = supported_tools_override .clone() .unwrap_or_else(|| get_supported_tools(¶ms)); - let supported_cli_agent_tools = + remove_orchestration_tools_if_disabled(&mut supported_tools, params.orchestration_enabled); + let mut supported_cli_agent_tools = supported_tools_override.unwrap_or_else(|| get_supported_cli_agent_tools(¶ms)); + remove_orchestration_tools_if_disabled( + &mut supported_cli_agent_tools, + params.orchestration_enabled, + ); if params.should_redact_secrets { redaction::redact_inputs(&mut params.input); } @@ -254,6 +259,21 @@ pub async fn generate_multi_agent_output( } } +fn remove_orchestration_tools_if_disabled( + supported_tools: &mut Vec, + orchestration_enabled: bool, +) { + if orchestration_enabled { + return; + } + supported_tools.retain(|tool| { + !matches!( + tool, + api::ToolType::Subagent | api::ToolType::RunAgents | api::ToolType::StartAgentV2 + ) + }); +} + fn get_supported_tools(params: &RequestParams) -> Vec { let mut supported_tools = vec![ api::ToolType::Grep, @@ -262,7 +282,6 @@ fn get_supported_tools(params: &RequestParams) -> Vec { api::ToolType::ReadMcpResource, api::ToolType::CallMcpTool, api::ToolType::RunShellCommand, - api::ToolType::Subagent, api::ToolType::WriteToLongRunningShellCommand, api::ToolType::ReadShellCommandOutput, api::ToolType::ReadDocuments, @@ -270,6 +289,10 @@ fn get_supported_tools(params: &RequestParams) -> Vec { api::ToolType::EditDocuments, ]; + if params.orchestration_enabled { + supported_tools.push(api::ToolType::Subagent); + } + if FeatureFlag::ConversationsAsContext.is_enabled() { supported_tools.push(api::ToolType::FetchConversation); } diff --git a/app/src/ai/agent/api/impl_tests.rs b/app/src/ai/agent/api/impl_tests.rs index 25a8c80a..d924e4a9 100644 --- a/app/src/ai/agent/api/impl_tests.rs +++ b/app/src/ai/agent/api/impl_tests.rs @@ -2,7 +2,9 @@ use galaxy_core::features::FeatureFlag; use galaxy_core::HostId; use warp_multi_agent_api as api; -use super::{get_supported_cli_agent_tools, get_supported_tools}; +use super::{ + get_supported_cli_agent_tools, get_supported_tools, remove_orchestration_tools_if_disabled, +}; use crate::ai::agent::api::RequestParams; use crate::ai::blocklist::SessionContext; use crate::ai::llms::LLMId; @@ -77,6 +79,40 @@ fn supported_tools_expose_local_subagents_without_hosted_orchestration_tools() { assert!(!supported_tools.contains(&api::ToolType::StartAgentV2)); } +#[test] +fn supported_tools_omit_subagents_when_orchestration_is_disabled() { + let params = request_params_with_ask_user_question_enabled(false); + let supported_tools = get_supported_tools(¶ms); + + assert!(!supported_tools.contains(&api::ToolType::Subagent)); +} + +#[test] +fn supported_tool_override_cannot_restore_leaf_orchestration_tools() { + let mut supported_tools = vec![ + api::ToolType::Grep, + api::ToolType::Subagent, + api::ToolType::RunAgents, + api::ToolType::StartAgentV2, + ]; + + remove_orchestration_tools_if_disabled(&mut supported_tools, false); + + assert_eq!(supported_tools, vec![api::ToolType::Grep]); +} + +#[test] +fn enabled_orchestration_preserves_supported_tool_override() { + let mut supported_tools = vec![api::ToolType::Grep, api::ToolType::Subagent]; + + remove_orchestration_tools_if_disabled(&mut supported_tools, true); + + assert_eq!( + supported_tools, + vec![api::ToolType::Grep, api::ToolType::Subagent] + ); +} + #[test] fn supported_tools_omit_hosted_only_capabilities() { let params = request_params_with_ask_user_question_enabled(false); diff --git a/app/src/ai/agent/task_store.rs b/app/src/ai/agent/task_store.rs index 428e8227..64834a7b 100644 --- a/app/src/ai/agent/task_store.rs +++ b/app/src/ai/agent/task_store.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use warp_multi_agent_api as api; @@ -360,6 +360,59 @@ impl TaskStore { append_refs_for_task(tasks, &mut refs, root_task); } + let indexed_task_ids = refs + .iter() + .map(|exchange_ref| exchange_ref.task_id.clone()) + .collect::>(); + let mut direct_cli_tasks = tasks + .values() + .filter(|task| { + !indexed_task_ids.contains(task.id()) + && task.parent_id().as_ref() == Some(root_task_id) + && task.is_cli_subagent() + && task + .subagent_params() + .is_some_and(|params| params.tool_call_id.is_empty()) + && task.exchanges().next().is_some() + }) + .collect::>(); + direct_cli_tasks.sort_by(|left, right| { + left.exchanges() + .next() + .map(|exchange| exchange.start_time) + .cmp(&right.exchanges().next().map(|exchange| exchange.start_time)) + .then_with(|| left.id().to_string().cmp(&right.id().to_string())) + }); + + // Direct providers synthesize CLI monitor tasks without a parent Subagent message. + // Place each task as one chronological block so its exchanges remain reachable without + // changing the DFS order of server-linked subtasks. + for task in direct_cli_tasks { + let first_start_time = task + .exchanges() + .next() + .expect("direct CLI task was filtered to contain an exchange") + .start_time; + let insertion_index = refs + .iter() + .position(|exchange_ref| { + tasks + .get(&exchange_ref.task_id) + .and_then(|task| task.exchanges().nth(exchange_ref.exchange_index)) + .is_some_and(|exchange| exchange.start_time > first_start_time) + }) + .unwrap_or(refs.len()); + let task_id = task.id().clone(); + let task_refs = task + .exchanges() + .enumerate() + .map(|(exchange_index, _)| ExchangeRef { + task_id: task_id.clone(), + exchange_index, + }) + .collect::>(); + refs.splice(insertion_index..insertion_index, task_refs); + } refs } } diff --git a/app/src/ai/agent/task_store_tests.rs b/app/src/ai/agent/task_store_tests.rs index af4ea066..0a33ae90 100644 --- a/app/src/ai/agent/task_store_tests.rs +++ b/app/src/ai/agent/task_store_tests.rs @@ -146,6 +146,46 @@ fn test_insert_subtask() { assert!(store.contains(&subtask_id)); } +#[test] +fn test_unlinked_direct_cli_task_is_linearized_chronologically() { + let base_time = Local::now(); + let mut root_task = Task::new_optimistic_root(); + let root_task_id = root_task.id().clone(); + + let mut before_cli = create_test_exchange(); + before_cli.start_time = base_time; + let before_cli_id = before_cli.id; + root_task.append_exchange(before_cli); + + let mut after_cli = create_test_exchange(); + after_cli.start_time = base_time + chrono::Duration::seconds(2); + let after_cli_id = after_cli.id; + root_task.append_exchange(after_cli); + + let mut cli_task = + Task::new_optimistic_cli_agent_subtask(BlockId::new(), Some(root_task_id.to_string())); + let mut cli_exchange = create_test_exchange(); + cli_exchange.start_time = base_time + chrono::Duration::seconds(1); + let cli_exchange_id = cli_exchange.id; + cli_task.append_exchange(cli_exchange); + + let mut store = TaskStore::with_root_task(root_task); + store.insert(cli_task); + + let exchange_ids = store + .all_exchanges() + .map(|exchange| exchange.id) + .collect::>(); + assert_eq!( + exchange_ids, + vec![before_cli_id, cli_exchange_id, after_cli_id] + ); + assert_eq!( + store.latest_exchange().map(|exchange| exchange.id), + Some(after_cli_id) + ); +} + #[test] fn test_remove_task() { let task = create_test_task_with_exchanges(3); diff --git a/app/src/ai/bedrock/request_translator.rs b/app/src/ai/bedrock/request_translator.rs index 99171ec2..168f9009 100644 --- a/app/src/ai/bedrock/request_translator.rs +++ b/app/src/ai/bedrock/request_translator.rs @@ -1655,7 +1655,7 @@ pub(crate) fn tool_name_is_supported(name: &str, supported: &HashSet has(ToolType::ReadDocuments), "create_plan" | "create_notebook" => has(ToolType::CreateDocuments), "edit_plan" | "edit_notebook" => has(ToolType::EditDocuments), - "start_agent" => has(ToolType::Subagent) || has(ToolType::StartAgentV2), + "run_agents" | "start_agent" => has(ToolType::Subagent) || has(ToolType::StartAgentV2), "ask_user_question" => has(ToolType::AskUserQuestion), "read_skill" => has(ToolType::ReadSkill), "fetch_conversation" => has(ToolType::FetchConversation), @@ -1910,6 +1910,54 @@ pub fn default_tool_definitions() -> Vec { "required": ["diffs"] }), }, + ToolDefinition { + name: "run_agents".to_string(), + description: "Start one or more child agents that share run-wide configuration. Use independent, uniquely named tasks and do not launch duplicate agents for follow-up work. Omitted run-wide fields use the embedded local child runtime and inherit the parent model.".to_string(), + input_schema: serde_json::json!({ + "type": "object", + "properties": { + "summary": { "type": "string", "description": "Brief explanation of why child agents help with this task" }, + "base_prompt": { "type": "string", "default": "", "description": "Instructions prepended to every child prompt" }, + "skills": { + "type": "array", + "items": { + "type": "object", + "properties": { + "skill": { "type": "string" }, + "reference_type": { "type": "string", "enum": ["path", "bundled"] } + }, + "required": ["skill", "reference_type"] + } + }, + "model_id": { "type": "string", "default": "", "description": "Optional child model override; empty inherits the parent model" }, + "harness_type": { "type": "string", "default": "", "description": "Optional harness identifier; empty selects the embedded local child runtime" }, + "execution_mode": { + "type": "object", + "properties": { + "type": { "type": "string", "enum": ["local", "remote"], "default": "local" }, + "environment_id": { "type": "string", "default": "" }, + "worker_host": { "type": "string", "default": "" }, + "computer_use_enabled": { "type": "boolean", "default": false } + } + }, + "agent_run_configs": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "properties": { + "name": { "type": "string", "description": "Unique child name" }, + "prompt": { "type": "string", "default": "", "description": "Child-specific instructions" }, + "title": { "type": "string", "default": "", "description": "Optional display title" } + }, + "required": ["name", "prompt"] + } + }, + "plan_id": { "type": "string", "default": "", "description": "Optional associated plan document ID" } + }, + "required": ["summary", "agent_run_configs"] + }), + }, ToolDefinition { name: "start_agent".to_string(), description: "Start a sub-agent to handle a specific task autonomously. Use for delegating independent work that can run in parallel. The agent gets its own conversation context and tool access. IMPORTANT: Only use this for the initial investigation or when genuinely new research is needed. Do NOT re-spawn agents for follow-up questions if you already have their output in context — just answer from the information you already have.".to_string(), diff --git a/app/src/ai/bedrock/request_translator_tests.rs b/app/src/ai/bedrock/request_translator_tests.rs index 57485fcc..9ff97746 100644 --- a/app/src/ai/bedrock/request_translator_tests.rs +++ b/app/src/ai/bedrock/request_translator_tests.rs @@ -99,6 +99,7 @@ fn advertised_tools_follow_client_capabilities_and_include_local_subagents() { vec![ "run_shell_command", "read_files", + "run_agents", "start_agent", "recall_tool_history" ] diff --git a/app/src/ai/blocklist/action_model/execute.rs b/app/src/ai/blocklist/action_model/execute.rs index ea15e7b2..f315ee42 100644 --- a/app/src/ai/blocklist/action_model/execute.rs +++ b/app/src/ai/blocklist/action_model/execute.rs @@ -106,6 +106,27 @@ use crate::util::image::{ use crate::util::openable_file_type::is_binary_file; use crate::BlocklistAIHistoryModel; +const CHILD_AGENT_DELEGATION_DENIAL_REASON: &str = + "Child agents are leaf workers and cannot launch additional agents. Complete the assigned task directly or report the blocker to the lead agent."; +const CHILD_AGENT_LEAF_INSTRUCTIONS: &str = r#"You are a leaf worker launched by a lead agent. +- Complete the assigned task directly and stay within its stated scope. +- Do not launch, delegate to, or create additional agents. +- Report blockers and completion to the lead through the available coordination channel."#; + +pub(super) fn child_agent_delegation_denial_reason( + conversation_id: AIConversationId, + ctx: &AppContext, +) -> Option { + BlocklistAIHistoryModel::as_ref(ctx) + .conversation(&conversation_id) + .is_some_and(|conversation| conversation.is_child_agent_conversation()) + .then(|| CHILD_AGENT_DELEGATION_DENIAL_REASON.to_string()) +} + +pub(super) fn compose_leaf_agent_prompt(task_prompt: &str) -> String { + format!("{CHILD_AGENT_LEAF_INSTRUCTIONS}\n\nAssigned task:\n{task_prompt}") +} + /// Types of actions that can be executed in parallel. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum ParallelExecutionPolicy { diff --git a/app/src/ai/blocklist/action_model/execute/run_agents.rs b/app/src/ai/blocklist/action_model/execute/run_agents.rs index 963cd508..59bed9af 100644 --- a/app/src/ai/blocklist/action_model/execute/run_agents.rs +++ b/app/src/ai/blocklist/action_model/execute/run_agents.rs @@ -2,7 +2,7 @@ //! //! Fans out per-child via [`super::start_agent::StartAgentExecutor::dispatch`] //! and aggregates the outcomes into a single `RunAgentsResult`. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::time::Duration; use ai::agent::action::{RunAgentsAgentRunConfig, RunAgentsExecutionMode, RunAgentsRequest}; @@ -20,7 +20,10 @@ use warp_cli::agent::Harness; use warpui::{Entity, EntityId, ModelContext, ModelHandle, SingletonEntity}; use super::start_agent::{StartAgentExecutor, StartAgentOutcome}; -use super::{ActionExecution, AnyActionExecution, ExecuteActionInput, PreprocessActionInput}; +use super::{ + child_agent_delegation_denial_reason, ActionExecution, AnyActionExecution, ExecuteActionInput, + PreprocessActionInput, +}; use crate::ai::agent::conversation::AIConversationId; use crate::ai::agent::{ AIAgentAction, AIAgentActionId, AIAgentActionResultType, AIAgentActionType, AIAgentInput, @@ -413,6 +416,9 @@ impl RunAgentsExecutor { let AIAgentActionType::RunAgents(request) = &input.action.action else { return false; }; + if child_agent_delegation_denial_reason(input.conversation_id, ctx).is_some() { + return true; + } if AppExecutionMode::as_ref(ctx).is_autonomous() { return true; } @@ -476,9 +482,9 @@ fn resolve_request_from_approved_config( /// Normalizes the request and returns a denial reason when launch is blocked. /// -/// Autonomous agents always run: their calls may still inherit approved plan -/// config fields and default auth secrets, but they bypass interactive policy -/// denials because they cannot present a confirmation card. +/// Root autonomous agents bypass interactive policy denials because they cannot +/// present a confirmation card. Child-agent delegation is rejected before that +/// bypass, while allowed root calls still inherit approved config and auth fields. fn prepare_request_for_execution( request: &mut RunAgentsRequest, parent_conversation_id: AIConversationId, @@ -486,6 +492,10 @@ fn prepare_request_for_execution( launched_agents: &HashMap>, ctx: &ModelContext, ) -> Option { + if let Some(reason) = child_agent_delegation_denial_reason(parent_conversation_id, ctx) { + return Some(reason); + } + let status = resolve_request_from_approved_config(request, parent_conversation_id, ctx); populate_default_auth_secret_for_execution(request, ctx); if let Some(reason) = @@ -544,8 +554,11 @@ fn duplicate_launched_agents_reason( let duplicates = requested_agents .iter() - .map(|(normalized_name, _)| existing_agents.get(normalized_name)) - .collect::>>()?; + .filter_map(|(normalized_name, _)| existing_agents.get(normalized_name)) + .collect::>(); + if duplicates.is_empty() { + return None; + } let duplicate_list = duplicates .iter() .map(|agent| format!("{} ({})", agent.name, agent.agent_id)) @@ -696,6 +709,20 @@ fn validate_request(request: &RunAgentsRequest) -> Result<(), String> { if request.agent_run_configs.is_empty() { return Err("orchestrate: empty agent_run_configs".to_string()); } + + let mut normalized_names = HashSet::new(); + for config in &request.agent_run_configs { + let Some(normalized_name) = normalize_agent_name(&config.name) else { + return Err("orchestrate: agent names must not be empty".to_string()); + }; + if !normalized_names.insert(normalized_name) { + return Err(format!( + "orchestrate: duplicate agent name '{}' in the same batch", + config.name.trim() + )); + } + } + if matches!(request.execution_mode, RunAgentsExecutionMode::Local) { if let Some(harness) = Harness::parse_local_child_harness(&request.harness_type) { if let Some(message) = local_harness_product_disabled_message(harness) { diff --git a/app/src/ai/blocklist/action_model/execute/run_agents_tests.rs b/app/src/ai/blocklist/action_model/execute/run_agents_tests.rs index ca75ffb5..7d8a72e0 100644 --- a/app/src/ai/blocklist/action_model/execute/run_agents_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/run_agents_tests.rs @@ -91,6 +91,15 @@ fn persist_plan_config_with_harness( }); } +fn mark_conversation_as_child(app: &mut App, conversation_id: AIConversationId) { + BlocklistAIHistoryModel::handle(app).update(app, |history, _ctx| { + history + .conversation_mut(&conversation_id) + .expect("conversation should exist") + .set_parent_agent_id("parent-agent".to_string()); + }); +} + #[test] fn should_autoexecute_duplicate_launched_agent_denial() { App::test((), |mut app| async move { @@ -162,6 +171,159 @@ fn execute_denies_duplicate_launched_agent() { }); } +#[test] +fn execute_denies_run_agents_from_child_conversation() { + App::test((), |mut app| async move { + let state = initialize_run_agents_test(&mut app, ExecutionMode::App); + mark_conversation_as_child(&mut app, state.conversation_id); + let action = remote_run_agents_action("oz"); + + let should_autoexecute = state.executor.update(&mut app, |executor, ctx| { + executor.should_autoexecute( + ExecuteActionInput { + action: &action, + conversation_id: state.conversation_id, + }, + ctx, + ) + }); + assert!( + should_autoexecute, + "the denial should not require user approval" + ); + + let execution = state.executor.update(&mut app, |executor, ctx| { + executor + .execute( + ExecuteActionInput { + action: &action, + conversation_id: state.conversation_id, + }, + ctx, + ) + .into() + }); + + assert!(matches!( + execution, + AnyActionExecution::Sync(AIAgentActionResultType::RunAgents( + RunAgentsResult::Denied { reason } + )) if reason.contains("leaf workers") + )); + }); +} + +#[test] +fn autonomous_mode_still_denies_run_agents_from_child_conversation() { + App::test((), |mut app| async move { + let state = initialize_run_agents_test(&mut app, ExecutionMode::Sdk); + mark_conversation_as_child(&mut app, state.conversation_id); + let action = remote_run_agents_action("oz"); + + let execution = state.executor.update(&mut app, |executor, ctx| { + executor + .execute( + ExecuteActionInput { + action: &action, + conversation_id: state.conversation_id, + }, + ctx, + ) + .into() + }); + + assert!(matches!( + execution, + AnyActionExecution::Sync(AIAgentActionResultType::RunAgents( + RunAgentsResult::Denied { reason } + )) if reason.contains("leaf workers") + )); + }); +} + +#[test] +fn execute_denies_mixed_batch_containing_launched_agent() { + App::test((), |mut app| async move { + let state = initialize_run_agents_test(&mut app, ExecutionMode::App); + state.executor.update(&mut app, |executor, _ctx| { + executor.record_launched_agents( + state.conversation_id, + &[RunAgentsAgentOutcome { + name: "child".to_string(), + kind: RunAgentsAgentOutcomeKind::Launched { + agent_id: "agent-123".to_string(), + }, + }], + ); + }); + let mut action = remote_run_agents_action("oz"); + let AIAgentActionType::RunAgents(request) = &mut action.action else { + panic!("expected run_agents action"); + }; + request.agent_run_configs.push(RunAgentsAgentRunConfig { + name: "new-child".to_string(), + prompt: "Do separate work".to_string(), + title: String::new(), + }); + + let execution = state.executor.update(&mut app, |executor, ctx| { + executor + .execute( + ExecuteActionInput { + action: &action, + conversation_id: state.conversation_id, + }, + ctx, + ) + .into() + }); + + assert!(matches!( + execution, + AnyActionExecution::Sync(AIAgentActionResultType::RunAgents( + RunAgentsResult::Denied { reason } + )) if reason.contains("child (agent-123)") + )); + }); +} + +#[test] +fn validate_request_rejects_blank_and_duplicate_agent_names() { + let AIAgentActionType::RunAgents(mut request) = remote_run_agents_action("oz").action else { + panic!("expected run_agents action"); + }; + request.agent_run_configs[0].name = " ".to_string(); + assert_eq!( + validate_request(&request), + Err("orchestrate: agent names must not be empty".to_string()) + ); + + request.agent_run_configs[0].name = "Child".to_string(); + request.agent_run_configs.push(RunAgentsAgentRunConfig { + name: " child ".to_string(), + prompt: "Do separate work".to_string(), + title: String::new(), + }); + assert_eq!( + validate_request(&request), + Err("orchestrate: duplicate agent name 'child' in the same batch".to_string()) + ); +} + +#[test] +fn validate_request_allows_unique_sibling_names() { + let AIAgentActionType::RunAgents(mut request) = remote_run_agents_action("oz").action else { + panic!("expected run_agents action"); + }; + request.agent_run_configs.push(RunAgentsAgentRunConfig { + name: "second-child".to_string(), + prompt: "Do separate work".to_string(), + title: String::new(), + }); + + assert_eq!(validate_request(&request), Ok(())); +} + fn initialize_run_agents_test(app: &mut App, mode: ExecutionMode) -> RunAgentsTestState { initialize_settings_for_tests_with_mode(app, mode, false); let global_resource_handles = GlobalResourceHandles::mock(app); diff --git a/app/src/ai/blocklist/action_model/execute/start_agent.rs b/app/src/ai/blocklist/action_model/execute/start_agent.rs index 3a13f662..0eb8adba 100644 --- a/app/src/ai/blocklist/action_model/execute/start_agent.rs +++ b/app/src/ai/blocklist/action_model/execute/start_agent.rs @@ -6,7 +6,10 @@ use galaxy_cli::agent::Harness; use galaxyui::{Entity, ModelContext, ModelHandle, SingletonEntity}; use shell_words::split as split_shell_words; -use super::{ActionExecution, AnyActionExecution, ExecuteActionInput, PreprocessActionInput}; +use super::{ + child_agent_delegation_denial_reason, compose_leaf_agent_prompt, ActionExecution, + AnyActionExecution, ExecuteActionInput, PreprocessActionInput, +}; use crate::ai::agent::conversation::{AIConversation, AIConversationId, ConversationStatus}; use crate::ai::agent::{ AIAgentAction, AIAgentActionId, AIAgentActionResultType, AIAgentActionType, LifecycleEventType, @@ -400,12 +403,19 @@ impl StartAgentExecutor { return ActionExecution::InvalidAction; }; - let prompt = prompt.clone(); let version = *version; - let action_id = input.action.id.clone(); let parent_conversation_id = input.conversation_id; + if let Some(error) = child_agent_delegation_denial_reason(parent_conversation_id, ctx) { + return ActionExecution::Sync(AIAgentActionResultType::StartAgent( + StartAgentResult::Error { error, version }, + )); + } + + let prompt = prompt.clone(); + let action_id = input.action.id.clone(); let (prompt, execution_mode) = normalize_legacy_local_child_harness_command(prompt, execution_mode.clone()); + let prompt = compose_leaf_agent_prompt(&prompt); let (execution_mode, parent_run_id) = match execution_mode { StartAgentExecutionMode::Local { harness_type: None, @@ -597,9 +607,15 @@ impl StartAgentExecutor { parent_run_id: Option, ctx: &mut ModelContext, ) -> async_channel::Receiver { + let (sender, receiver) = async_channel::bounded(1); + if let Some(error) = child_agent_delegation_denial_reason(parent_conversation_id, ctx) { + let _ = sender.try_send(StartAgentOutcome::Error(error)); + return receiver; + } + let (prompt, execution_mode) = normalize_legacy_local_child_harness_command(prompt, execution_mode); - let (sender, receiver) = async_channel::bounded(1); + let prompt = compose_leaf_agent_prompt(&prompt); let request_id = self.next_request_id(); self.pending.insert( request_id, diff --git a/app/src/ai/blocklist/action_model/execute/start_agent_tests.rs b/app/src/ai/blocklist/action_model/execute/start_agent_tests.rs index 41ff6663..95bebb6b 100644 --- a/app/src/ai/blocklist/action_model/execute/start_agent_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/start_agent_tests.rs @@ -28,6 +28,28 @@ impl Entity for CapturedDirectProviderChildLinks { type Event = (); } +#[derive(Default)] +struct CapturedStartAgentPrompts(Vec); + +impl Entity for CapturedStartAgentPrompts { + type Event = (); +} + +fn capture_start_agent_prompts( + app: &mut App, + executor: &ModelHandle, +) -> ModelHandle { + let captured = app.add_model(|_| CapturedStartAgentPrompts::default()); + captured.update(app, |_, ctx| { + ctx.subscribe_to_model(executor, |captured, _, event, _ctx| { + if let StartAgentExecutorEvent::CreateAgent(request) = event { + captured.0.push(request.prompt.clone()); + } + }); + }); + captured +} + fn capture_direct_provider_child_links( app: &mut App, executor: &ModelHandle, @@ -79,6 +101,126 @@ fn build_start_agent_action_with_prompt( } } +#[test] +fn execute_wraps_child_prompt_with_leaf_worker_contract() { + App::test((), |mut app| async move { + let terminal_view_id = EntityId::new(); + let history_model = app.add_singleton_model(|_| BlocklistAIHistoryModel::new_for_test()); + let executor = app.add_model(StartAgentExecutor::new); + let captured = capture_start_agent_prompts(&mut app, &executor); + let root_conversation_id = history_model.update(&mut app, |history, ctx| { + history.start_new_conversation(terminal_view_id, false, false, false, ctx) + }); + let action = build_start_agent_action( + StartAgentVersion::V1, + StartAgentExecutionMode::local_with_defaults(), + ); + + let execution = executor.update(&mut app, |executor, ctx| { + executor + .execute( + ExecuteActionInput { + action: &action, + conversation_id: root_conversation_id, + }, + ctx, + ) + .into() + }); + assert!(matches!(execution, AnyActionExecution::Async { .. })); + captured.read(&app, |captured, _ctx| { + assert_eq!(captured.0.len(), 1); + assert!(captured.0[0].contains("You are a leaf worker")); + assert!( + captured.0[0].contains("Do not launch, delegate to, or create additional agents") + ); + assert!(captured.0[0].ends_with("Assigned task:\nInvestigate the failure")); + }); + }); +} + +#[test] +fn execute_denies_start_agent_from_child_conversation() { + App::test((), |mut app| async move { + let terminal_view_id = EntityId::new(); + let history_model = app.add_singleton_model(|_| BlocklistAIHistoryModel::new_for_test()); + let executor = app.add_model(StartAgentExecutor::new); + let child_conversation_id = history_model.update(&mut app, |history, ctx| { + let conversation_id = + history.start_new_conversation(terminal_view_id, false, false, false, ctx); + history + .conversation_mut(&conversation_id) + .expect("conversation should exist") + .set_parent_agent_id("parent-agent".to_string()); + conversation_id + }); + let action = build_start_agent_action( + StartAgentVersion::V1, + StartAgentExecutionMode::local_with_defaults(), + ); + + let execution = executor.update(&mut app, |executor, ctx| { + executor + .execute( + ExecuteActionInput { + action: &action, + conversation_id: child_conversation_id, + }, + ctx, + ) + .into() + }); + + assert!(matches!( + execution, + AnyActionExecution::Sync(AIAgentActionResultType::StartAgent( + StartAgentResult::Error { error, .. } + )) if error.contains("leaf workers") + )); + executor.read(&app, |executor, _ctx| { + assert!(executor.pending.is_empty()); + }); + }); +} + +#[test] +fn dispatch_denies_child_conversation_defense_in_depth() { + App::test((), |mut app| async move { + let terminal_view_id = EntityId::new(); + let history_model = app.add_singleton_model(|_| BlocklistAIHistoryModel::new_for_test()); + let executor = app.add_model(StartAgentExecutor::new); + let child_conversation_id = history_model.update(&mut app, |history, ctx| { + let conversation_id = + history.start_new_conversation(terminal_view_id, false, false, false, ctx); + history + .conversation_mut(&conversation_id) + .expect("conversation should exist") + .set_parent_agent_id("parent-agent".to_string()); + conversation_id + }); + + let receiver = executor.update(&mut app, |executor, ctx| { + executor.dispatch( + "grandchild".to_string(), + "Do more work".to_string(), + StartAgentExecutionMode::local_with_defaults(), + None, + child_conversation_id, + None, + ctx, + ) + }); + + assert!(matches!( + receiver.try_recv(), + Ok(StartAgentOutcome::Error(error)) if error.contains("leaf workers") + )); + executor.read(&app, |executor, _ctx| { + assert!(executor.pending.is_empty()); + }); + }); +} + #[test] fn legacy_local_codex_command_prompt_normalizes_to_local_harness() { let (prompt, execution_mode) = normalize_legacy_local_child_harness_command( diff --git a/app/src/ai/blocklist/controller.rs b/app/src/ai/blocklist/controller.rs index 7fcfbfeb..382d16df 100644 --- a/app/src/ai/blocklist/controller.rs +++ b/app/src/ai/blocklist/controller.rs @@ -3044,6 +3044,13 @@ impl BlocklistAIController { }); request_params.parent_agent_id = parent_agent_id; request_params.agent_name = agent_name; + if history_model + .as_ref(ctx) + .conversation(&conversation_id) + .is_some_and(|conversation| conversation.is_child_agent_conversation()) + { + request_params.orchestration_enabled = false; + } request_params.message_history = bedrock_history; request_params.tool_result_archive = bedrock_tool_result_archive; request_params.progressive_summary = bedrock_progressive_summary; diff --git a/app/src/ai/llms_tests.rs b/app/src/ai/llms_tests.rs index 89fa9d98..5976d3ad 100644 --- a/app/src/ai/llms_tests.rs +++ b/app/src/ai/llms_tests.rs @@ -560,9 +560,31 @@ fn disabled_providers_do_not_leave_models_in_the_runtime_inventory() { } #[test] -fn chatgpt_reasoning_modes_route_to_the_base_model_with_effort_metadata() { +fn chatgpt_reasoning_modes_route_with_catalog_context_metadata() { App::test((), |mut app| async move { initialize_settings_for_tests(&mut app); + let provider = crate::settings::ai::default_chatgpt_provider(); + let configured_model = |model_id: &str| { + provider + .models + .iter() + .find(|model| model.model_id == model_id) + .expect("ChatGPT model should be configured") + }; + + let gpt_54 = configured_model("gpt-5.4"); + assert_eq!(gpt_54.context_size, 1_000_000); + assert_eq!(gpt_54.max_input_tokens, Some(950_000)); + let gpt_56_sol = configured_model("gpt-5.6-sol"); + assert_eq!(gpt_56_sol.context_size, 272_000); + assert_eq!(gpt_56_sol.max_input_tokens, Some(258_400)); + let codex_spark = configured_model("gpt-5.3-codex-spark"); + assert_eq!(codex_spark.context_size, 128_000); + assert_eq!(codex_spark.max_input_tokens, Some(121_600)); + let uncached_model = configured_model("gpt-5.4-pro"); + assert_eq!(uncached_model.context_size, 200_000); + assert_eq!(uncached_model.max_input_tokens, None); + AISettings::handle(&app).update(&mut app, |settings, ctx| { settings .bedrock_enabled @@ -582,34 +604,56 @@ fn chatgpt_reasoning_modes_route_to_the_base_model_with_effort_metadata() { .expect("OpenAI model setting should update"); settings .openai_providers - .set_value(vec![crate::settings::ai::default_chatgpt_provider()], ctx) + .set_value(vec![provider], ctx) .expect("OpenAI provider setting should update"); }); let mut preferences = empty_preferences(); app.read(|ctx| preferences.inject_openai_models(ctx)); + let model_info = |model_id: &str| { + preferences + .models_by_feature + .agent_mode + .choices + .iter() + .find(|model| model.id.as_str() == model_id) + .expect("ChatGPT model should be available") + }; + let assert_fixed_context = |model_id: &str, expected: u32| { + let context_window = &model_info(model_id).context_window; + assert!(!context_window.is_configurable); + assert_eq!(context_window.min, expected); + assert_eq!(context_window.max, expected); + assert_eq!(context_window.default_max, expected); + }; + let mode_id = "gpt-5.4::reasoning::high"; - let mode = preferences - .models_by_feature - .agent_mode - .choices - .iter() - .find(|model| model.id.as_str() == mode_id) - .expect("GPT-5.4 high mode should be available"); + let mode = model_info(mode_id); assert_eq!(mode.reasoning_level.as_deref(), Some("high")); + assert_fixed_context(mode_id, 950_000); let routing = preferences .openai_client_config_for_model(mode_id) .expect("reasoning mode should have a routing entry"); assert_eq!(routing.model.as_deref(), Some("gpt-5.4")); assert_eq!(routing.reasoning_effort.as_deref(), Some("high")); + assert_eq!(routing.max_input_tokens, Some(950_000)); let ultra_id = "gpt-5.6-sol::reasoning::ultra"; + assert_fixed_context(ultra_id, 258_400); let ultra_routing = preferences .openai_client_config_for_model(ultra_id) .expect("GPT-5.6 Sol ultra mode should have a routing entry"); assert_eq!(ultra_routing.model.as_deref(), Some("gpt-5.6-sol")); assert_eq!(ultra_routing.reasoning_effort.as_deref(), Some("ultra")); + assert_eq!(ultra_routing.max_input_tokens, Some(258_400)); + + let spark_id = "gpt-5.3-codex-spark"; + assert_fixed_context(spark_id, 121_600); + let spark_routing = preferences + .openai_client_config_for_model(spark_id) + .expect("GPT-5.3 Codex Spark should have a routing entry"); + assert_eq!(spark_routing.max_input_tokens, Some(121_600)); }); } diff --git a/app/src/ai/runtime/rig_request_tests.rs b/app/src/ai/runtime/rig_request_tests.rs index a3077bb0..8bf9ae11 100644 --- a/app/src/ai/runtime/rig_request_tests.rs +++ b/app/src/ai/runtime/rig_request_tests.rs @@ -283,6 +283,25 @@ fn bedrock_rig_turn_uses_bedrock_history_invariants_without_a_proto_round_trip() assert_eq!(prepared.request.messages, prepared.persistent_messages); } +#[test] +fn modern_and_legacy_orchestration_tools_follow_subagent_capabilities() { + for capability in [ToolType::Subagent, ToolType::StartAgentV2] { + let (tools, _) = tool_definitions(&[capability], None); + let names = tools + .iter() + .map(|tool| tool.name.as_str()) + .collect::>(); + + assert!(names.contains(&"run_agents")); + assert!(names.contains(&"start_agent")); + } + + let (leaf_tools, _) = tool_definitions(&[ToolType::Grep], None); + assert!(!leaf_tools + .iter() + .any(|tool| matches!(tool.name.as_str(), "run_agents" | "start_agent"))); +} + #[test] #[allow(deprecated)] fn grouped_mcp_tool_names_use_the_installation_id_not_the_display_name() { diff --git a/app/src/ai/runtime/rig_tool.rs b/app/src/ai/runtime/rig_tool.rs index b603bd52..73ce56cc 100644 --- a/app/src/ai/runtime/rig_tool.rs +++ b/app/src/ai/runtime/rig_tool.rs @@ -12,8 +12,9 @@ use crate::ai::agent::{ AIAgentAction, AIAgentActionType, AIAgentPtyWriteMode, AskUserQuestionItem, AskUserQuestionOption, AskUserQuestionType, CreateDocumentsRequest, DocumentDiff, DocumentToCreate, EditDocumentsRequest, FileEdit, FileLocations, ReadDocumentsRequest, - ReadFilesRequest, ReadSkillRequest, SearchCodebaseRequest, ShellCommandDelay, - StartAgentExecutionMode, StartAgentVersion, + ReadFilesRequest, ReadSkillRequest, RunAgentsAgentRunConfig, RunAgentsExecutionMode, + RunAgentsRequest, SearchCodebaseRequest, ShellCommandDelay, StartAgentExecutionMode, + StartAgentVersion, }; use crate::ai::document::ai_document_model::AIDocumentId; @@ -140,6 +141,27 @@ pub(super) fn action_from_tool_call( .collect(), }) } + "run_agents" => AIAgentActionType::RunAgents(RunAgentsRequest { + summary: string(input, "summary"), + base_prompt: string(input, "base_prompt"), + skills: skill_references(input, skill_path_origin), + model_id: string(input, "model_id"), + harness_type: string(input, "harness_type"), + execution_mode: run_agents_execution_mode(input), + agent_run_configs: input + .get("agent_run_configs") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .map(|config| RunAgentsAgentRunConfig { + name: string(config, "name"), + prompt: string(config, "prompt"), + title: string(config, "title"), + }) + .collect(), + plan_id: string(input, "plan_id"), + harness_auth_secret_name: None, + }), "start_agent" => AIAgentActionType::StartAgent { version: StartAgentVersion::V1, name: string(input, "name"), @@ -263,6 +285,52 @@ fn uuid(input: &serde_json::Value, key: &str) -> Option { .and_then(|value| Uuid::parse_str(value).ok()) } +fn skill_references( + input: &serde_json::Value, + skill_path_origin: &SkillPathOrigin, +) -> Vec { + input + .get("skills") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter_map(|skill| { + let reference = string(skill, "skill"); + if reference.is_empty() { + return None; + } + match skill + .get("reference_type") + .and_then(serde_json::Value::as_str) + { + Some("bundled") => Some(SkillReference::BundledSkillId(reference)), + Some("path") | Some(_) | None => skill_path_origin + .location_for_path(reference) + .ok() + .map(SkillReference::Path), + } + }) + .collect() +} + +fn run_agents_execution_mode(input: &serde_json::Value) -> RunAgentsExecutionMode { + let Some(execution_mode) = input.get("execution_mode") else { + return RunAgentsExecutionMode::Local; + }; + let mode_type = execution_mode + .get("type") + .and_then(serde_json::Value::as_str) + .or_else(|| execution_mode.as_str()); + match mode_type { + Some("remote") => RunAgentsExecutionMode::Remote { + environment_id: string(execution_mode, "environment_id"), + worker_host: string(execution_mode, "worker_host"), + computer_use_enabled: boolean(execution_mode, "computer_use_enabled"), + }, + Some("local") | Some(_) | None => RunAgentsExecutionMode::Local, + } +} + fn file_location(file: &serde_json::Value) -> Option { if let Some(name) = file.as_str() { return Some(FileLocations { diff --git a/app/src/ai/runtime/rig_tool_tests.rs b/app/src/ai/runtime/rig_tool_tests.rs index 7c285ee6..6fdd0faf 100644 --- a/app/src/ai/runtime/rig_tool_tests.rs +++ b/app/src/ai/runtime/rig_tool_tests.rs @@ -6,7 +6,7 @@ use ai::skills::{SkillPathOrigin, SkillReference}; use galaxy_agent_core::ToolCall; use super::{action_from_tool_call, MCPToolTarget}; -use crate::ai::agent::{AIAgentActionType, FileEdit}; +use crate::ai::agent::{AIAgentActionType, FileEdit, RunAgentsExecutionMode}; fn call(name: &str, arguments: serde_json::Value) -> ToolCall { ToolCall { @@ -164,6 +164,110 @@ fn local_skill_paths_preserve_the_session_origin() { )); } +#[test] +fn run_agents_calls_decode_to_local_domain_requests_with_safe_defaults() { + let action = action_from_tool_call( + "task-1", + &call( + "run_agents", + serde_json::json!({ + "summary": "Parallel investigation", + "base_prompt": "Inspect before changing files.", + "agent_run_configs": [ + { + "name": "runtime", + "prompt": "Inspect runtime behavior", + "title": "Runtime investigator" + }, + { + "name": "tests", + "prompt": "Design focused tests" + } + ] + }), + ), + &SkillPathOrigin::Local, + &HashMap::new(), + ) + .unwrap(); + + let AIAgentActionType::RunAgents(request) = action.action else { + panic!("expected run-agents action"); + }; + assert_eq!(request.summary, "Parallel investigation"); + assert_eq!(request.base_prompt, "Inspect before changing files."); + assert!(request.skills.is_empty()); + assert!(request.model_id.is_empty()); + assert!(request.harness_type.is_empty()); + assert_eq!(request.execution_mode, RunAgentsExecutionMode::Local); + assert!(request.plan_id.is_empty()); + assert!(request.harness_auth_secret_name.is_none()); + assert_eq!(request.agent_run_configs.len(), 2); + assert_eq!(request.agent_run_configs[0].name, "runtime"); + assert_eq!( + request.agent_run_configs[0].prompt, + "Inspect runtime behavior" + ); + assert_eq!(request.agent_run_configs[0].title, "Runtime investigator"); + assert_eq!(request.agent_run_configs[1].name, "tests"); + assert_eq!(request.agent_run_configs[1].prompt, "Design focused tests"); + assert!(request.agent_run_configs[1].title.is_empty()); +} + +#[test] +fn run_agents_calls_preserve_remote_config_and_skills() { + let action = action_from_tool_call( + "task-1", + &call( + "run_agents", + serde_json::json!({ + "summary": "Remote investigation", + "model_id": "remote-model", + "harness_type": "codex", + "execution_mode": { + "type": "remote", + "environment_id": "env-1", + "worker_host": "worker.example", + "computer_use_enabled": true + }, + "skills": [ + {"skill": "galaxyctrl", "reference_type": "bundled"}, + {"skill": "/repo/SKILL.md", "reference_type": "path"} + ], + "agent_run_configs": [{"name": "remote", "prompt": "Inspect"}], + "plan_id": "plan-1" + }), + ), + &SkillPathOrigin::Local, + &HashMap::new(), + ) + .unwrap(); + + let AIAgentActionType::RunAgents(request) = action.action else { + panic!("expected run-agents action"); + }; + assert_eq!(request.model_id, "remote-model"); + assert_eq!(request.harness_type, "codex"); + assert_eq!(request.plan_id, "plan-1"); + assert_eq!( + request.skills, + vec![ + SkillReference::BundledSkillId("galaxyctrl".to_string()), + SkillReference::Path(galaxy_util::local_or_remote_path::LocalOrRemotePath::Local( + PathBuf::from("/repo/SKILL.md") + )), + ] + ); + assert_eq!( + request.execution_mode, + RunAgentsExecutionMode::Remote { + environment_id: "env-1".to_string(), + worker_host: "worker.example".to_string(), + computer_use_enabled: true, + } + ); +} + #[test] fn unknown_tools_are_rejected_before_the_permission_boundary() { let error = action_from_tool_call( diff --git a/app/src/integration_testing/agent_mode/assertions.rs b/app/src/integration_testing/agent_mode/assertions.rs index 4cb5ee60..11b69673 100644 --- a/app/src/integration_testing/agent_mode/assertions.rs +++ b/app/src/integration_testing/agent_mode/assertions.rs @@ -92,6 +92,99 @@ pub fn assert_latest_exchange_text( }) } +/// Asserts that the active conversation created exactly one hidden leaf child +/// and that the child completed with the expected identity and output. +pub fn assert_single_hidden_child_agent_succeeds( + expected_agent_name: &'static str, + expected_output: &'static str, +) -> AssertionCallback { + Box::new(move |app, window_id| { + let terminal_view = terminal_view(app, window_id, 0, 0); + BlocklistAIHistoryModel::handle(app).update(app, |history_model, _| { + let Some(parent) = history_model.active_conversation(terminal_view.id()) else { + return AssertionOutcome::failure("No active parent conversation".to_owned()); + }; + let parent_id = parent.id(); + let children = history_model.child_conversations_of(parent_id); + let child = match children.as_slice() { + [] => { + return AssertionOutcome::failure( + "Waiting for the hidden child conversation".to_owned(), + ); + } + [child] => *child, + _ => { + return AssertionOutcome::immediate_failure(format!( + "Expected exactly one child conversation, found {}", + children.len() + )); + } + }; + + match child.status() { + ConversationStatus::Success => {} + ConversationStatus::InProgress + | ConversationStatus::TransientError + | ConversationStatus::WaitingForEvents => { + return AssertionOutcome::failure(format!( + "Waiting for child agent to succeed; current status: {:?}", + child.status() + )); + } + ConversationStatus::Blocked { .. } + | ConversationStatus::Error + | ConversationStatus::Cancelled => { + return AssertionOutcome::immediate_failure(format!( + "Child agent finished unsuccessfully: {:?}", + child.status() + )); + } + } + + if child.agent_name() != Some(expected_agent_name) { + return AssertionOutcome::immediate_failure(format!( + "Expected child name {expected_agent_name:?}, found {:?}", + child.agent_name() + )); + } + if child.parent_conversation_id() != Some(parent_id) { + return AssertionOutcome::immediate_failure(format!( + "Child {:?} was not linked to active parent {parent_id:?}", + child.id() + )); + } + if !child.is_child_agent_conversation() || !child.should_exclude_from_navigation() { + return AssertionOutcome::immediate_failure( + "Child conversation was not hidden from normal navigation".to_owned(), + ); + } + let grandchildren = history_model.child_conversations_of(child.id()); + if !grandchildren.is_empty() { + return AssertionOutcome::immediate_failure(format!( + "Leaf child unexpectedly created {} grandchildren", + grandchildren.len() + )); + } + + let output = child + .all_exchanges() + .into_iter() + .filter_map(|exchange| exchange.output_status.output()) + .map(|output| output.get().format_for_copy(None)) + .filter(|text| !text.is_empty()) + .collect::>() + .join("\n\n"); + if !output.contains(expected_output) { + return AssertionOutcome::immediate_failure(format!( + "Child output did not contain {expected_output:?}: {output}" + )); + } + + AssertionOutcome::Success + }) + }) +} + // Make an assertion on the action requested in the exchange at exchange_index. /// This is private because `AIAgentActionType` is not public outside the warp app crate /// for use within agent mode evals, so they can't write the `ActionAssertion` directly. @@ -180,22 +273,41 @@ pub fn assert_any_exchange_text( Box::new(move |app, window_id| { let terminal_view = terminal_view(app, window_id, 0, 0); BlocklistAIHistoryModel::handle(app).update(app, |history_model, _| { - let exchange_count = get_exchange_count(terminal_view.id(), history_model); - (0..exchange_count) - .map(|exchange_index| { - exchange_succeeds_with_expected_output( - Some(Box::new(assertion.clone())), - None, - ConversationTarget::Active, - terminal_view.id(), - exchange_index, - history_model, - ) - }) - .find(|outcome| matches!(outcome, AssertionOutcome::Success)) - .unwrap_or(AssertionOutcome::failure( - "No exchanges match assertion".to_owned(), - )) + let Some(conversation) = history_model.active_conversation(terminal_view.id()) else { + return AssertionOutcome::failure("No active conversation".to_owned()); + }; + let mut output_texts = Vec::with_capacity(conversation.exchange_count()); + for exchange in conversation.all_exchanges() { + let AIAgentOutputStatus::Finished { finished_output } = &exchange.output_status + else { + return AssertionOutcome::failure(format!( + "Exchange {:?} is not finished", + exchange.id + )); + }; + match finished_output { + FinishedAIAgentOutput::Success { output } => { + let text = output.get().format_for_copy(None); + if assertion(&text) { + return AssertionOutcome::Success; + } + output_texts.push(text); + } + FinishedAIAgentOutput::Error { error, .. } => { + return AssertionOutcome::immediate_failure(format!( + "Exchange failed with error: {error:?}" + )); + } + FinishedAIAgentOutput::Cancelled { .. } => { + return AssertionOutcome::immediate_failure( + "Exchange was cancelled".to_owned(), + ); + } + } + } + AssertionOutcome::failure(format!( + "No exchanges match assertion. Exchange outputs: {output_texts:?}" + )) }) }) } diff --git a/app/src/integration_testing/agent_mode/step.rs b/app/src/integration_testing/agent_mode/step.rs index e7738a1d..485ae0f5 100644 --- a/app/src/integration_testing/agent_mode/step.rs +++ b/app/src/integration_testing/agent_mode/step.rs @@ -9,7 +9,7 @@ use galaxyui::{async_assert, SingletonEntity}; use prost::Message; use crate::ai::execution_profiles::profiles::AIExecutionProfilesModel; -use crate::ai::execution_profiles::ActionPermission; +use crate::ai::execution_profiles::{ActionPermission, RunAgentsPermission}; use crate::ai::llms::{LLMId, LLMPreferences}; use crate::ai::mcp::{ JsonTemplate, TemplatableMCPServer, TemplatableMCPServerInstallation, @@ -284,6 +284,20 @@ pub fn set_execution_profile_auto_execute() -> TestStep { ) } +/// Sets the execution profile to auto-run child agents. +pub fn set_execution_profile_auto_run_agents() -> TestStep { + TestStep::new("Set execution profile to auto-run child agents").add_named_assertion( + "Update execution profile", + |app, _window_id| { + AIExecutionProfilesModel::handle(app).update(app, |profiles, ctx| { + let default_profile_id = *profiles.default_profile(ctx).id(); + profiles.set_run_agents(default_profile_id, RunAgentsPermission::AlwaysAllow, ctx); + }); + async_assert!(true, "Successfully updated execution profile") + }, + ) +} + /// Sets the execution profile to auto-apply code diffs. pub fn set_execution_profile_auto_apply_code_diffs() -> TestStep { TestStep::new("Set execution profile to auto-apply code diffs").add_named_assertion( diff --git a/app/src/integration_testing/terminal/assertion.rs b/app/src/integration_testing/terminal/assertion.rs index 76d66fe0..6879af7e 100644 --- a/app/src/integration_testing/terminal/assertion.rs +++ b/app/src/integration_testing/terminal/assertion.rs @@ -807,8 +807,12 @@ pub fn assert_active_session_local_path(expected_path: &'static str) -> Assertio } pub fn assert_input_is_focused() -> AssertionCallback { - Box::new(|app, window_id| { - let terminal_view = single_terminal_view_for_tab(app, window_id, 0); + assert_input_is_focused_for_pane(0, 0) +} + +pub fn assert_input_is_focused_for_pane(tab_index: usize, pane_index: usize) -> AssertionCallback { + Box::new(move |app, window_id| { + let terminal_view = terminal_view(app, window_id, tab_index, pane_index); terminal_view.read(app, |view, ctx| { let is_input_focused = view.input().as_ref(ctx).editor().as_ref(ctx).is_focused(); async_assert!(is_input_focused) diff --git a/app/src/pane_group/pane/local_harness_launch_tests.rs b/app/src/pane_group/pane/local_harness_launch_tests.rs index 3b50b8e4..0b252536 100644 --- a/app/src/pane_group/pane/local_harness_launch_tests.rs +++ b/app/src/pane_group/pane/local_harness_launch_tests.rs @@ -167,6 +167,15 @@ fn build_local_codex_child_command_quotes_the_prompt() { ); } +#[test] +fn local_harness_commands_preserve_leaf_worker_contract() { + let prompt = "You are a leaf worker. Do not launch additional agents."; + + assert!(local_claude_child_prompt(prompt).contains(prompt)); + assert!(build_local_codex_child_command(prompt).contains(prompt)); + assert!(build_local_opencode_child_command(prompt).contains(prompt)); +} + #[test] fn local_child_task_config_records_supported_third_party_harnesses() { for harness in [Harness::Claude, Harness::OpenCode, Harness::Codex] { diff --git a/app/src/settings/ai.rs b/app/src/settings/ai.rs index 286eea03..14565291 100644 --- a/app/src/settings/ai.rs +++ b/app/src/settings/ai.rs @@ -1033,58 +1033,94 @@ const INITIAL_RIG_MODEL_ID: &str = "codex-gpt-5.6-sol-xhigh"; fn default_chatgpt_models() -> Vec { // The ChatGPT OAuth backend does not expose a model-listing capability through Rig, - // so keep this catalog small and explicit. Reasoning variants are expanded into - // selectable LLM entries when the provider is injected into the runtime inventory. + // so keep this catalog small and explicit. Context limits come from Codex model + // metadata; models absent from that catalog retain the generic fallback. [ ( "gpt-5.6-sol", "GPT-5.6 Sol", vec!["low", "medium", "high", "xhigh", "max", "ultra"], + 272_000, + Some(258_400), ), ( "gpt-5.6-terra", "GPT-5.6 Terra", vec!["low", "medium", "high", "xhigh", "max", "ultra"], + 272_000, + Some(258_400), ), ( "gpt-5.6-luna", "GPT-5.6 Luna", vec!["low", "medium", "high", "xhigh", "max", "ultra"], + 272_000, + Some(258_400), + ), + ( + "gpt-5.4", + "GPT-5.4", + vec!["low", "medium", "high", "xhigh"], + 1_000_000, + Some(950_000), ), - ("gpt-5.4", "GPT-5.4", vec!["low", "medium", "high", "xhigh"]), ( "gpt-5.4-pro", "GPT-5.4 Pro", vec!["medium", "high", "xhigh"], + default_context_size(), + None, ), ( "gpt-5.3-codex", "GPT-5.3 Codex", vec!["low", "medium", "high", "xhigh"], + default_context_size(), + None, + ), + ( + "gpt-5.3-codex-spark", + "GPT-5.3 Codex Spark", + vec![], + 128_000, + Some(121_600), + ), + ( + "gpt-5.3-instant", + "GPT-5.3 Instant", + vec![], + default_context_size(), + None, + ), + ( + "gpt-5.3-chat-latest", + "GPT-5.3 Chat Latest", + vec![], + default_context_size(), + None, ), - ("gpt-5.3-codex-spark", "GPT-5.3 Codex Spark", vec![]), - ("gpt-5.3-instant", "GPT-5.3 Instant", vec![]), - ("gpt-5.3-chat-latest", "GPT-5.3 Chat Latest", vec![]), ] .into_iter() .map( - |(model_id, display_name, reasoning_efforts)| OpenAIModelConfig { - model_id: model_id.to_string(), - display_name: display_name.to_string(), - // ChatGPT's subscription backend accepts image input for its chat - // models, but it does not expose a public capability discovery - // endpoint. Keep this explicit catalog in sync with that contract - // so the model picker does not hide vision context. - vision_supported: true, - context_size: default_context_size(), - max_input_tokens: None, - max_output_tokens: None, - provider: Some("openai".to_string()), - use_rig: true, - supports_system_messages: Some(true), - capability_overrides: HashMap::new(), - reasoning_efforts: reasoning_efforts.into_iter().map(str::to_string).collect(), - enabled: true, + |(model_id, display_name, reasoning_efforts, context_size, max_input_tokens)| { + OpenAIModelConfig { + model_id: model_id.to_string(), + display_name: display_name.to_string(), + // ChatGPT's subscription backend accepts image input for its chat + // models, but it does not expose a public capability discovery + // endpoint. Keep this explicit catalog in sync with that contract + // so the model picker does not hide vision context. + vision_supported: true, + context_size, + max_input_tokens, + max_output_tokens: None, + provider: Some("openai".to_string()), + use_rig: true, + supports_system_messages: Some(true), + capability_overrides: HashMap::new(), + reasoning_efforts: reasoning_efforts.into_iter().map(str::to_string).collect(), + enabled: true, + } }, ) .collect() diff --git a/app/src/settings_view/provider_setup_modal.rs b/app/src/settings_view/provider_setup_modal.rs index 6ab20451..6d43fa73 100644 --- a/app/src/settings_view/provider_setup_modal.rs +++ b/app/src/settings_view/provider_setup_modal.rs @@ -1145,7 +1145,7 @@ impl ProviderSetupModalBody { if let ChatGPTAuthState::Failed(error) = &state { children.push( Text::new(error.clone(), appearance.monospace_font_family(), 11.) - .with_color(appearance.theme().ui_error_color().into()) + .with_color(appearance.theme().ui_error_color()) .soft_wrap(true) .finish(), ); diff --git a/crates/ai/src/agent/action_result/mod.rs b/crates/ai/src/agent/action_result/mod.rs index 6e493028..375050b5 100644 --- a/crates/ai/src/agent/action_result/mod.rs +++ b/crates/ai/src/agent/action_result/mod.rs @@ -294,8 +294,8 @@ impl AIAgentActionResultType { | Self::StartAgent(_) | Self::SendMessageToAgent(_) | Self::AskUserQuestion(_) - | Self::RunAgents(_) | Self::WaitForEvents(_) => self.to_string(), + Self::RunAgents(result) => result.model_content(), } } } @@ -1629,6 +1629,69 @@ pub enum RunAgentsAgentOutcomeKind { Failed { error: String }, } +impl RunAgentsResult { + fn model_content(&self) -> String { + let value = match self { + Self::Launched { + model_id, + harness_type, + execution_mode, + agents, + } => { + let execution_mode = match execution_mode { + RunAgentsLaunchedExecutionMode::Local => serde_json::json!({ + "type": "local", + }), + RunAgentsLaunchedExecutionMode::Remote { + environment_id, + worker_host, + computer_use_enabled, + } => serde_json::json!({ + "type": "remote", + "environment_id": environment_id, + "worker_host": worker_host, + "computer_use_enabled": computer_use_enabled, + }), + }; + let agents = agents + .iter() + .map(|agent| match &agent.kind { + RunAgentsAgentOutcomeKind::Launched { agent_id } => serde_json::json!({ + "name": agent.name, + "status": "launched", + "agent_id": agent_id, + }), + RunAgentsAgentOutcomeKind::Failed { error } => serde_json::json!({ + "name": agent.name, + "status": "failed", + "error": error, + }), + }) + .collect::>(); + serde_json::json!({ + "status": "launched", + "model_id": model_id, + "harness_type": harness_type, + "execution_mode": execution_mode, + "agents": agents, + }) + } + Self::Denied { reason } => serde_json::json!({ + "status": "denied", + "reason": reason, + }), + Self::Failure { error } => serde_json::json!({ + "status": "failure", + "error": error, + }), + Self::Cancelled => serde_json::json!({ + "status": "cancelled", + }), + }; + value.to_string() + } +} + impl Display for RunAgentsResult { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/ai/src/agent/action_result/mod_tests.rs b/crates/ai/src/agent/action_result/mod_tests.rs index 59622c89..2765139c 100644 --- a/crates/ai/src/agent/action_result/mod_tests.rs +++ b/crates/ai/src/agent/action_result/mod_tests.rs @@ -1,4 +1,7 @@ -use super::{StartAgentResult, StartAgentVersion}; +use super::{ + AIAgentActionResultType, RunAgentsAgentOutcome, RunAgentsAgentOutcomeKind, + RunAgentsLaunchedExecutionMode, RunAgentsResult, StartAgentResult, StartAgentVersion, +}; #[test] fn deserializes_legacy_start_agent_success_without_version_as_v1() { @@ -42,3 +45,90 @@ fn deserializes_legacy_start_agent_cancelled_without_version_as_v1() { } ); } + +#[test] +fn run_agents_model_content_contains_resolved_config_and_agent_outcomes() { + let result = AIAgentActionResultType::RunAgents(RunAgentsResult::Launched { + model_id: "resolved-model".to_string(), + harness_type: "oz".to_string(), + execution_mode: RunAgentsLaunchedExecutionMode::Remote { + environment_id: "env-1".to_string(), + worker_host: "worker.example".to_string(), + computer_use_enabled: true, + }, + agents: vec![ + RunAgentsAgentOutcome { + name: "research".to_string(), + kind: RunAgentsAgentOutcomeKind::Launched { + agent_id: "agent-1".to_string(), + }, + }, + RunAgentsAgentOutcome { + name: "tests".to_string(), + kind: RunAgentsAgentOutcomeKind::Failed { + error: "capacity exhausted".to_string(), + }, + }, + ], + }); + + let content: serde_json::Value = serde_json::from_str(&result.model_content()) + .expect("run-agents model content should be valid JSON"); + assert_eq!( + content, + serde_json::json!({ + "status": "launched", + "model_id": "resolved-model", + "harness_type": "oz", + "execution_mode": { + "type": "remote", + "environment_id": "env-1", + "worker_host": "worker.example", + "computer_use_enabled": true, + }, + "agents": [ + { + "name": "research", + "status": "launched", + "agent_id": "agent-1", + }, + { + "name": "tests", + "status": "failed", + "error": "capacity exhausted", + }, + ], + }) + ); + assert_eq!( + result.to_string(), + "Orchestrate launched (1/2 agents started)" + ); +} + +#[test] +fn run_agents_model_content_serializes_terminal_non_launch_outcomes() { + for (result, expected) in [ + ( + RunAgentsResult::Denied { + reason: "not approved".to_string(), + }, + serde_json::json!({ "status": "denied", "reason": "not approved" }), + ), + ( + RunAgentsResult::Failure { + error: "invalid request".to_string(), + }, + serde_json::json!({ "status": "failure", "error": "invalid request" }), + ), + ( + RunAgentsResult::Cancelled, + serde_json::json!({ "status": "cancelled" }), + ), + ] { + let result = AIAgentActionResultType::RunAgents(result); + let content: serde_json::Value = serde_json::from_str(&result.model_content()) + .expect("run-agents model content should be valid JSON"); + assert_eq!(content, expected); + } +} diff --git a/crates/galaxy_agent_rig/tests/chatgpt_live.rs b/crates/galaxy_agent_rig/tests/chatgpt_live.rs new file mode 100644 index 00000000..d321456d --- /dev/null +++ b/crates/galaxy_agent_rig/tests/chatgpt_live.rs @@ -0,0 +1,69 @@ +use std::fs::File; +use std::io::BufReader; +use std::path::{Path, PathBuf}; + +use rig_core::client::CompletionClient; +use rig_core::completion::CompletionModel; +use rig_core::providers::chatgpt::{self, ChatGPTAuth}; + +fn codex_auth_path() -> PathBuf { + if let Some(codex_home) = std::env::var_os("CODEX_HOME") { + return PathBuf::from(codex_home).join("auth.json"); + } + + let home = std::env::var_os("HOME").expect("HOME must be set to locate ~/.codex/auth.json"); + PathBuf::from(home).join(".codex").join("auth.json") +} + +fn load_codex_auth(path: &Path) -> ChatGPTAuth { + let file = File::open(path) + .unwrap_or_else(|error| panic!("failed to open {}: {error}", path.display())); + let document: serde_json::Value = serde_json::from_reader(BufReader::new(file)) + .unwrap_or_else(|error| panic!("failed to parse {}: {error}", path.display())); + let tokens = document + .get("tokens") + .unwrap_or_else(|| panic!("{} does not contain a tokens object", path.display())); + let access_token = tokens + .get("access_token") + .and_then(serde_json::Value::as_str) + .filter(|token| !token.is_empty()) + .unwrap_or_else(|| panic!("{} does not contain an access token", path.display())); + let account_id = tokens + .get("account_id") + .and_then(serde_json::Value::as_str) + .filter(|account_id| !account_id.is_empty()) + .map(str::to_string); + + ChatGPTAuth::AccessToken { + access_token: access_token.to_string(), + account_id, + } +} + +#[tokio::test(flavor = "current_thread")] +#[ignore = "makes a live ChatGPT request using local Codex credentials"] +async fn live_chatgpt_backend_via_rig_records_full_response() { + let auth_path = codex_auth_path(); + let client = chatgpt::Client::builder() + .api_key(load_codex_auth(&auth_path)) + .allow_device_flow(false) + .build() + .expect("Rig ChatGPT client should build"); + let model_id = + std::env::var("GALAXY_CHATGPT_LIVE_MODEL").unwrap_or_else(|_| chatgpt::GPT_5_4.to_string()); + let prompt = std::env::var("GALAXY_CHATGPT_LIVE_PROMPT").unwrap_or_else(|_| { + "Reply with exactly two short sentences explaining what a live backend smoke test verifies." + .to_string() + }); + let model = client.completion_model(&model_id); + let request = model.completion_request(prompt).build(); + + let response = model + .completion(request) + .await + .expect("live ChatGPT completion should succeed"); + let recorded = serde_json::to_string_pretty(&response) + .expect("the normalized Rig response should serialize"); + + println!("CHATGPT_LIVE_RESPONSE_BEGIN\n{recorded}\nCHATGPT_LIVE_RESPONSE_END"); +} diff --git a/crates/integration/src/bin/integration.rs b/crates/integration/src/bin/integration.rs index 00637d25..8503921a 100644 --- a/crates/integration/src/bin/integration.rs +++ b/crates/integration/src/bin/integration.rs @@ -435,10 +435,12 @@ fn register_tests() -> HashMap<&'static str, BoxedBuilderFn> { register_test!(test_rig_read_tool_round_trip); register_test!(test_rig_shell_tool_success_round_trip); register_test!(test_rig_shell_tool_failure_round_trip); + register_test!(test_rig_shell_long_running_round_trip); register_test!(test_rig_shell_tool_permission_denial); register_test!(test_rig_edit_tool_round_trip); register_test!(test_rig_in_flight_cancellation); register_test!(test_rig_mcp_tool_round_trip); + register_test!(test_rig_local_run_agents_round_trip); register_test!(test_git_prompt_chips); // These tests are only invoked manually, and not included in the diff --git a/crates/integration/src/test/rig_runtime.rs b/crates/integration/src/test/rig_runtime.rs index 775053f0..ee8f6b7a 100644 --- a/crates/integration/src/test/rig_runtime.rs +++ b/crates/integration/src/test/rig_runtime.rs @@ -1,7 +1,7 @@ use std::io::{ErrorKind, Read, Write}; use std::net::{SocketAddr, TcpListener, TcpStream}; use std::path::Path; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::time::Duration; @@ -9,10 +9,12 @@ use std::time::Duration; use galaxyui_core::async_assert; use warp::features::FeatureFlag; use warp::integration_testing::agent_mode::{ - assert_latest_exchange_text, assert_task_is_cancelled, enter_agent_view, + assert_any_exchange_text, assert_latest_exchange_text, + assert_single_hidden_child_agent_succeeds, assert_task_is_cancelled, enter_agent_view, set_execution_profile_auto_apply_code_diffs, set_execution_profile_auto_execute, - set_execution_profile_auto_execute_mcp_tools, set_execution_profile_no_auto_execute, - set_preferred_agent_mode_llm, start_ephemeral_mcp_server_for_testing, submit_ai_query, + set_execution_profile_auto_execute_mcp_tools, set_execution_profile_auto_run_agents, + set_execution_profile_no_auto_execute, set_preferred_agent_mode_llm, + start_ephemeral_mcp_server_for_testing, submit_ai_query, submit_ai_query_and_wait_until_blocked, submit_ai_query_and_wait_until_done, wait_until_mcp_server_is_active_for_testing, ConversationTarget, }; @@ -35,6 +37,11 @@ const SHELL_SUCCESS_FINAL_TEXT: &str = "Rig shell success round trip completed." const SHELL_FAILURE_OUTPUT: &str = "rig-shell-failure-output"; const SHELL_FAILURE_FINAL_TEXT: &str = "Rig shell failure round trip completed."; const SHELL_DENIED_FINAL_TEXT: &str = "Rig shell denial was preserved."; +const LONG_RUNNING_CALL_ID: &str = "rig-long-running-call"; +const LONG_RUNNING_POLL_CALL_ID: &str = "rig-long-running-poll-call"; +const LONG_RUNNING_START_OUTPUT: &str = "rig-long-running-start"; +const LONG_RUNNING_COMPLETE_OUTPUT: &str = "rig-long-running-complete"; +const LONG_RUNNING_FINAL_TEXT: &str = "Rig long-running shell round trip completed."; const EDIT_CALL_ID: &str = "rig-edit-call"; const EDIT_INITIAL_CONTENT: &str = "before Rig edit\n"; const EDIT_UPDATED_CONTENT: &str = "after Rig edit\n"; @@ -46,6 +53,12 @@ const MCP_SERVER_NAME: &str = "rig-integration"; const MCP_TOOL_NAME: &str = "mcp__11111111-1111-4111-8111-111111111111__echo"; const MCP_INPUT: &str = "hello from Rig"; const MCP_FINAL_TEXT: &str = "Rig MCP round trip completed."; +const RUN_AGENTS_CALL_ID: &str = "rig-run-agents-call"; +const RUN_AGENTS_CHILD_NAME: &str = "rig-child"; +const RUN_AGENTS_CHILD_PROMPT: &str = + "Return the deterministic Rig child completion marker without calling tools."; +const RUN_AGENTS_CHILD_OUTPUT: &str = "Rig child agent completed."; +const RUN_AGENTS_FINAL_TEXT: &str = "Rig local orchestration round trip completed."; #[derive(Clone)] enum MockScenario { @@ -54,6 +67,7 @@ enum MockScenario { }, ShellSuccess, ShellFailure, + ShellLongRunning, ShellDenied { marker_path: Arc>, }, @@ -65,6 +79,7 @@ enum MockScenario { stream_cancelled: Arc, }, Mcp, + LocalRunAgents, } pub fn test_rig_read_tool_round_trip() -> Builder { @@ -124,6 +139,25 @@ pub fn test_rig_shell_tool_failure_round_trip() -> Builder { ) } +pub fn test_rig_shell_long_running_round_trip() -> Builder { + rig_builder(MockScenario::ShellLongRunning) + .with_step(wait_until_bootstrapped_single_pane_for_tab(0)) + .with_step(set_preferred_agent_mode_llm(MODEL_ID)) + .with_step(set_execution_profile_auto_execute()) + .with_step(enter_agent_view()) + .with_step(submit_ai_query_and_wait_until_done( + "Run the requested long-running shell monitor check.", + Duration::from_secs(60), + )) + .with_step( + new_step_with_default_assertions("Assert Rig long-running shell reached Agent Mode") + .add_named_assertion( + "Final response follows the completed shell poll", + assert_any_exchange_text(|text| text.contains(LONG_RUNNING_FINAL_TEXT)), + ), + ) +} + pub fn test_rig_shell_tool_permission_denial() -> Builder { let marker_path = Arc::new(Mutex::new(String::new())); rig_builder(MockScenario::ShellDenied { marker_path }) @@ -258,6 +292,32 @@ pub fn test_rig_mcp_tool_round_trip() -> Builder { ) } +pub fn test_rig_local_run_agents_round_trip() -> Builder { + rig_builder(MockScenario::LocalRunAgents) + .with_step(wait_until_bootstrapped_single_pane_for_tab(0)) + .with_step(set_preferred_agent_mode_llm(MODEL_ID)) + .with_step(set_execution_profile_auto_run_agents()) + .with_step(enter_agent_view()) + .with_step(submit_ai_query_and_wait_until_done( + "Launch the deterministic local child agent.", + Duration::from_secs(90), + )) + .with_step( + new_step_with_default_assertions("Assert Rig local orchestration reached Agent Mode") + .add_named_assertion( + "Final response follows the structured run-agents result", + assert_latest_exchange_text(|text| text.contains(RUN_AGENTS_FINAL_TEXT)), + ) + .add_named_assertion( + "Hidden child completed as a leaf worker", + assert_single_hidden_child_agent_succeeds( + RUN_AGENTS_CHILD_NAME, + RUN_AGENTS_CHILD_OUTPUT, + ), + ), + ) +} + fn rig_builder(scenario: MockScenario) -> Builder { FeatureFlag::AgentView.set_enabled(true); FeatureFlag::MCPGroupedServerContext.set_enabled(true); @@ -293,8 +353,10 @@ fn rig_builder(scenario: MockScenario) -> Builder { } MockScenario::ShellSuccess | MockScenario::ShellFailure + | MockScenario::ShellLongRunning | MockScenario::Cancellation { .. } - | MockScenario::Mcp => {} + | MockScenario::Mcp + | MockScenario::LocalRunAgents => {} } }) .with_cleanup(move |_utils| { @@ -337,12 +399,11 @@ fn start_mock_provider( listener .set_nonblocking(true) .expect("should make mock provider nonblocking"); - let request_count = AtomicUsize::new(0); let thread = thread::spawn(move || { while !stop.load(Ordering::SeqCst) { match listener.accept() { Ok((mut stream, _)) => { - serve_request(&mut stream, &scenario, &request_count, &stop); + serve_request(&mut stream, &scenario, &stop); } Err(error) if error.kind() == ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(10)); @@ -354,12 +415,7 @@ fn start_mock_provider( (address, thread) } -fn serve_request( - stream: &mut TcpStream, - scenario: &MockScenario, - request_count: &AtomicUsize, - stop: &AtomicBool, -) { +fn serve_request(stream: &mut TcpStream, scenario: &MockScenario, stop: &AtomicBool) { stream .set_read_timeout(Some(Duration::from_secs(5))) .expect("should set request timeout"); @@ -377,27 +433,137 @@ fn serve_request( request_line.contains("/chat/completions"), "unexpected mock provider request: {request_line}" ); - let turn = request_count.fetch_add(1, Ordering::SeqCst); + let request_body = parse_request_body(&request); + if matches!(scenario, MockScenario::ShellLongRunning) { + let body = long_running_shell_sse(&request_body); + write_response(stream, "text/event-stream", &body); + return; + } + if matches!(scenario, MockScenario::LocalRunAgents) { + let body = local_run_agents_sse(&request_body); + write_response(stream, "text/event-stream", &body); + return; + } if let MockScenario::Cancellation { stream_started, stream_cancelled, } = scenario { - assert_eq!(turn, 0, "unexpected extra cancellation chat request"); + assert!( + !request_has_tool_result(&request_body), + "cancellation scenario should not issue a follow-up tool result" + ); write_cancellable_response(stream, stream_started, stream_cancelled, stop); return; } - let body = match turn { - 0 => tool_call_sse(scenario), - 1 => { - assert_follow_up_request(scenario, &request); - final_text_sse(final_text(scenario)) - } - _ => panic!("unexpected extra chat completion request"), + + let call_id = scenario_call_id(scenario).expect("non-cancellation scenario should call a tool"); + let body = if let Some(content) = tool_result_content(&request_body, call_id) { + assert!( + !content.is_empty(), + "follow-up tool result should contain model-facing content" + ); + assert_follow_up_request(scenario, &request); + final_text_sse(final_text(scenario)) + } else { + assert!( + !request_has_tool_result(&request_body), + "unexpected tool result in initial scenario request" + ); + let expected_tool = scenario_tool_name(scenario) + .expect("non-cancellation scenario should advertise its tool"); + assert!( + advertised_tool_names(&request_body).contains(&expected_tool), + "initial request should advertise {expected_tool}" + ); + tool_call_sse(scenario) }; write_response(stream, "text/event-stream", &body); } +fn parse_request_body(request: &str) -> serde_json::Value { + let (_, body) = request + .split_once("\r\n\r\n") + .expect("provider request should contain an HTTP body"); + serde_json::from_str(body).expect("provider request body should be valid JSON") +} + +fn request_has_tool_result(request: &serde_json::Value) -> bool { + request + .get("messages") + .and_then(serde_json::Value::as_array) + .is_some_and(|messages| { + messages.iter().any(|message| { + message.get("role").and_then(serde_json::Value::as_str) == Some("tool") + && message.get("tool_call_id").is_some() + }) + }) +} + +fn tool_result_content(request: &serde_json::Value, call_id: &str) -> Option { + request + .get("messages")? + .as_array()? + .iter() + .find(|message| { + message.get("role").and_then(serde_json::Value::as_str) == Some("tool") + && message + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + == Some(call_id) + }) + .and_then(|message| message.get("content")) + .map(|content| { + content + .as_str() + .map(ToOwned::to_owned) + .unwrap_or_else(|| content.to_string()) + }) +} + +fn scenario_call_id(scenario: &MockScenario) -> Option<&'static str> { + match scenario { + MockScenario::Read { .. } => Some(READ_CALL_ID), + MockScenario::ShellSuccess + | MockScenario::ShellFailure + | MockScenario::ShellDenied { .. } => Some(SHELL_CALL_ID), + MockScenario::ShellLongRunning => Some(LONG_RUNNING_CALL_ID), + MockScenario::Edit { .. } => Some(EDIT_CALL_ID), + MockScenario::Cancellation { .. } => None, + MockScenario::Mcp => Some(MCP_CALL_ID), + MockScenario::LocalRunAgents => Some(RUN_AGENTS_CALL_ID), + } +} + +fn scenario_tool_name(scenario: &MockScenario) -> Option<&'static str> { + match scenario { + MockScenario::Read { .. } => Some("read_files"), + MockScenario::ShellSuccess + | MockScenario::ShellFailure + | MockScenario::ShellLongRunning + | MockScenario::ShellDenied { .. } => Some("run_shell_command"), + MockScenario::Edit { .. } => Some("apply_file_diffs"), + MockScenario::Cancellation { .. } => None, + MockScenario::Mcp => Some(MCP_TOOL_NAME), + MockScenario::LocalRunAgents => Some("run_agents"), + } +} + +fn advertised_tool_names(request: &serde_json::Value) -> Vec<&str> { + request + .get("tools") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter_map(|tool| { + tool.get("function") + .and_then(|function| function.get("name")) + .or_else(|| tool.get("name")) + .and_then(serde_json::Value::as_str) + }) + .collect() +} + fn read_request(stream: &mut TcpStream) -> String { let mut request = Vec::new(); let mut chunk = [0; 8 * 1024]; @@ -453,6 +619,9 @@ fn tool_call_sse(scenario: &MockScenario) -> String { "run_shell_command", shell_arguments("(printf '%s\\n' 'rig-shell-failure-output' >&2; exit 7)"), ), + MockScenario::ShellLongRunning => { + unreachable!("long-running shell requests use request-aware routing") + } MockScenario::ShellDenied { marker_path } => { let marker_path = marker_path.lock().expect("marker path lock").clone(); ( @@ -486,7 +655,14 @@ fn tool_call_sse(scenario: &MockScenario) -> String { MCP_TOOL_NAME, serde_json::json!({"text": MCP_INPUT}), ), + MockScenario::LocalRunAgents => { + unreachable!("local orchestration requests use request-aware routing") + } }; + tool_call_sse_for(call_id, tool_name, arguments) +} + +fn tool_call_sse_for(call_id: &str, tool_name: &str, arguments: serde_json::Value) -> String { let tool_delta = serde_json::json!({ "id": "rig-integration-1", "model": MODEL_ID, @@ -519,13 +695,205 @@ fn tool_call_sse(scenario: &MockScenario) -> String { format!("data: {tool_delta}\n\ndata: {tool_stop}\n\ndata: {usage}\n\ndata: [DONE]\n\n") } +fn long_running_shell_sse(request: &serde_json::Value) -> String { + if let Some(poll_result) = tool_result_content(request, LONG_RUNNING_POLL_CALL_ID) { + assert!( + poll_result.contains(LONG_RUNNING_COMPLETE_OUTPUT), + "completed poll should contain the command's final output" + ); + assert!( + poll_result.contains("exit code 0"), + "completed poll should contain the successful exit code" + ); + let initial_result = tool_result_content(request, LONG_RUNNING_CALL_ID) + .expect("completed poll request should preserve the initial running snapshot"); + let command_id = command_id_from_result(&initial_result); + let poll_arguments = tool_call_arguments(request, LONG_RUNNING_POLL_CALL_ID) + .expect("completed poll request should preserve the polling tool call"); + assert_eq!( + poll_arguments + .get("command_id") + .and_then(serde_json::Value::as_str), + Some(command_id.as_str()), + "poll must reuse the dynamic command ID returned by Galaxy" + ); + return final_text_sse(LONG_RUNNING_FINAL_TEXT); + } + + if let Some(initial_result) = tool_result_content(request, LONG_RUNNING_CALL_ID) { + assert!( + initial_result.contains("Command is still running"), + "non-blocking command should first return a running snapshot" + ); + assert!( + initial_result.contains(LONG_RUNNING_START_OUTPUT), + "running snapshot should contain real intermediate output" + ); + let command_id = command_id_from_result(&initial_result); + assert_ne!( + command_id, LONG_RUNNING_CALL_ID, + "the command ID should be the real terminal block ID, not the tool call ID" + ); + assert!( + advertised_tool_names(request).contains(&"read_shell_command_output"), + "running snapshot follow-up should advertise the shell polling tool" + ); + return tool_call_sse_for( + LONG_RUNNING_POLL_CALL_ID, + "read_shell_command_output", + serde_json::json!({"command_id": command_id, "wait_seconds": 5}), + ); + } + + assert!( + !request_has_tool_result(request), + "initial long-running shell request should not contain tool results" + ); + assert!( + advertised_tool_names(request).contains(&"run_shell_command"), + "initial long-running request should advertise the shell tool" + ); + let command = format!( + "printf '%s\\n' '{LONG_RUNNING_START_OUTPUT}'; sleep 4; printf '%s\\n' '{LONG_RUNNING_COMPLETE_OUTPUT}'" + ); + tool_call_sse_for( + LONG_RUNNING_CALL_ID, + "run_shell_command", + shell_arguments_with_wait(&command, false), + ) +} + +fn local_run_agents_sse(request: &serde_json::Value) -> String { + if let Some(result) = tool_result_content(request, RUN_AGENTS_CALL_ID) { + let result: serde_json::Value = + serde_json::from_str(&result).expect("run-agents result should be structured JSON"); + assert_eq!( + result.get("status").and_then(serde_json::Value::as_str), + Some("launched") + ); + assert_eq!( + result + .pointer("/execution_mode/type") + .and_then(serde_json::Value::as_str), + Some("local") + ); + let agents = result + .get("agents") + .and_then(serde_json::Value::as_array) + .expect("run-agents result should contain agent outcomes"); + assert_eq!(agents.len(), 1, "exactly one child should be launched"); + assert_eq!( + agents[0].get("name").and_then(serde_json::Value::as_str), + Some(RUN_AGENTS_CHILD_NAME) + ); + assert_eq!( + agents[0].get("status").and_then(serde_json::Value::as_str), + Some("launched") + ); + assert!( + agents[0] + .get("agent_id") + .and_then(serde_json::Value::as_str) + .is_some_and(|id| !id.is_empty()), + "launched child should return its real conversation ID" + ); + return final_text_sse(RUN_AGENTS_FINAL_TEXT); + } + + if request_messages_contain(request, RUN_AGENTS_CHILD_PROMPT) { + assert!( + !request_has_tool_result(request), + "child's initial request should not contain tool results" + ); + let tools = advertised_tool_names(request); + for delegation_tool in [ + "run_agents", + "start_agent", + "send_message_to_agent", + "wait_for_events", + ] { + assert!( + !tools.contains(&delegation_tool), + "leaf child must not advertise {delegation_tool}" + ); + } + return final_text_sse(RUN_AGENTS_CHILD_OUTPUT); + } + + assert!( + !request_has_tool_result(request), + "root's initial orchestration request should not contain tool results" + ); + let tools = advertised_tool_names(request); + assert!( + tools.contains(&"run_agents"), + "root request should advertise modern run_agents" + ); + assert!( + tools.contains(&"start_agent"), + "root request should retain legacy start_agent compatibility" + ); + tool_call_sse_for( + RUN_AGENTS_CALL_ID, + "run_agents", + serde_json::json!({ + "summary": "Launch one deterministic local child", + "base_prompt": "Complete the assigned task directly.", + "agent_run_configs": [{ + "name": RUN_AGENTS_CHILD_NAME, + "prompt": RUN_AGENTS_CHILD_PROMPT, + "title": "Rig child agent" + }] + }), + ) +} + +fn request_messages_contain(request: &serde_json::Value, expected: &str) -> bool { + request + .get("messages") + .is_some_and(|messages| messages.to_string().contains(expected)) +} + +fn command_id_from_result(result: &str) -> String { + result + .lines() + .find_map(|line| line.strip_prefix("Command ID: ")) + .map(str::trim) + .filter(|id| !id.is_empty()) + .map(ToOwned::to_owned) + .expect("long-running result should include a dynamic command ID") +} + +fn tool_call_arguments(request: &serde_json::Value, call_id: &str) -> Option { + request + .get("messages")? + .as_array()? + .iter() + .filter(|message| { + message.get("role").and_then(serde_json::Value::as_str) == Some("assistant") + }) + .filter_map(|message| message.get("tool_calls")?.as_array()) + .flatten() + .find(|call| call.get("id").and_then(serde_json::Value::as_str) == Some(call_id)) + .and_then(|call| call.get("function")) + .and_then(|function| function.get("arguments")) + .and_then(|arguments| match arguments { + serde_json::Value::String(arguments) => serde_json::from_str(arguments).ok(), + arguments => Some(arguments.clone()), + }) +} + fn shell_arguments(command: &str) -> serde_json::Value { + shell_arguments_with_wait(command, true) +} + +fn shell_arguments_with_wait(command: &str, wait_until_complete: bool) -> serde_json::Value { serde_json::json!({ "command": command, "is_read_only": false, "is_risky": false, "uses_pager": false, - "wait_until_complete": true, + "wait_until_complete": wait_until_complete, }) } @@ -573,6 +941,9 @@ fn assert_follow_up_request(scenario: &MockScenario, request: &str) { "failed shell result should remain an explicit model error" ); } + MockScenario::ShellLongRunning => { + unreachable!("long-running shell follow-ups use request-aware routing") + } MockScenario::ShellDenied { marker_path } => { assert!( request.contains(SHELL_CALL_ID), @@ -615,6 +986,9 @@ fn assert_follow_up_request(scenario: &MockScenario, request: &str) { "follow-up request should contain the real MCP tool result" ); } + MockScenario::LocalRunAgents => { + unreachable!("local orchestration follow-ups use request-aware routing") + } } } @@ -623,12 +997,14 @@ fn final_text(scenario: &MockScenario) -> &'static str { MockScenario::Read { .. } => READ_FINAL_TEXT, MockScenario::ShellSuccess => SHELL_SUCCESS_FINAL_TEXT, MockScenario::ShellFailure => SHELL_FAILURE_FINAL_TEXT, + MockScenario::ShellLongRunning => LONG_RUNNING_FINAL_TEXT, MockScenario::ShellDenied { .. } => SHELL_DENIED_FINAL_TEXT, MockScenario::Edit { .. } => EDIT_FINAL_TEXT, MockScenario::Cancellation { .. } => { unreachable!("cancellation streams do not produce final text") } MockScenario::Mcp => MCP_FINAL_TEXT, + MockScenario::LocalRunAgents => RUN_AGENTS_FINAL_TEXT, } } @@ -638,15 +1014,21 @@ fn final_text_sse(final_text: &str) -> String { "model": MODEL_ID, "choices": [{ "delta": {"content": final_text, "tool_calls": []}, - "finish_reason": "stop", + "finish_reason": null, }], "usage": null, }); + let stop = serde_json::json!({ + "id": "rig-integration-2", + "model": MODEL_ID, + "choices": [{"delta": {"tool_calls": []}, "finish_reason": "stop"}], + "usage": null, + }); let usage = serde_json::json!({ "choices": [], "usage": {"prompt_tokens": 30, "completion_tokens": 6, "total_tokens": 36}, }); - format!("data: {text}\n\ndata: {usage}\n\ndata: [DONE]\n\n") + format!("data: {text}\n\ndata: {stop}\n\ndata: {usage}\n\ndata: [DONE]\n\n") } fn write_response(stream: &mut TcpStream, content_type: &str, body: &str) { diff --git a/crates/integration/tests/integration/ui_tests.rs b/crates/integration/tests/integration/ui_tests.rs index 58f8b25e..539f5d15 100644 --- a/crates/integration/tests/integration/ui_tests.rs +++ b/crates/integration/tests/integration/ui_tests.rs @@ -314,10 +314,12 @@ integration_tests! { test_rig_read_tool_round_trip, test_rig_shell_tool_success_round_trip, test_rig_shell_tool_failure_round_trip, + test_rig_shell_long_running_round_trip, test_rig_shell_tool_permission_denial, test_rig_edit_tool_round_trip, test_rig_in_flight_cancellation, test_rig_mcp_tool_round_trip, + test_rig_local_run_agents_round_trip, test_rule_creation, test_rule_update,