From 6280a822eb6ae0196b3f8d5ae7cef64999809b61 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 8 Mar 2026 09:59:56 -0400 Subject: [PATCH] Fix subagent/system prompt empty handling with TDD --- crates/arc-agent/src/session.rs | 36 +++++++++++++++++++++-- crates/arc-agent/src/subagent.rs | 34 +++++++++++++++++++++ crates/arc-llm/src/providers/anthropic.rs | 36 ++++++++++++++++++++--- crates/arc-llm/src/providers/common.rs | 20 ++++++++++++- 4 files changed, 119 insertions(+), 7 deletions(-) diff --git a/crates/arc-agent/src/session.rs b/crates/arc-agent/src/session.rs index 4bb7c5649..a9050a0f2 100644 --- a/crates/arc-agent/src/session.rs +++ b/crates/arc-agent/src/session.rs @@ -677,7 +677,10 @@ impl Session { } fn build_request(&self) -> Request { - let mut messages = vec![Message::system(self.system_prompt.clone())]; + let mut messages = Vec::new(); + if !self.system_prompt.trim().is_empty() { + messages.push(Message::system(self.system_prompt.clone())); + } messages.extend(self.history.convert_to_messages()); let tools = self.provider_profile.tools(); @@ -722,7 +725,7 @@ mod tests { use crate::tool_registry::{RegisteredTool, ToolRegistry}; use arc_llm::error::ProviderErrorDetail; use arc_llm::provider::{ProviderAdapter, StreamEventStream}; - use arc_llm::types::{Response, ToolDefinition}; + use arc_llm::types::{Response, Role, ToolDefinition}; use std::sync::atomic::{AtomicUsize, Ordering}; // --- Tests --- @@ -1446,6 +1449,35 @@ mod tests { ); } + #[tokio::test] + async fn request_omits_system_message_when_prompt_empty() { + let provider = Arc::new(CapturingLlmProvider::new()); + let provider_ref = provider.clone(); + let client = make_client(provider as Arc).await; + let profile = Arc::new(TestProfile::new()); + let env = Arc::new(MockSandbox::default()); + let mut session = Session::new(client, profile, env, SessionConfig::default()); + + // Intentionally skip initialize(): system prompt remains empty. + session.process_input("test").await.unwrap(); + + let captured = provider_ref.captured_request.lock().unwrap(); + let request = captured + .as_ref() + .expect("request should have been captured"); + assert!( + request + .messages + .iter() + .all(|message| message.role != Role::System), + "request should not contain an empty system message" + ); + assert!( + matches!(request.messages.first(), Some(message) if message.role == Role::User), + "first request message should be user input" + ); + } + #[tokio::test] async fn tool_approval_denies_tool() { let mut registry = ToolRegistry::new(); diff --git a/crates/arc-agent/src/subagent.rs b/crates/arc-agent/src/subagent.rs index caf566330..2a2395924 100644 --- a/crates/arc-agent/src/subagent.rs +++ b/crates/arc-agent/src/subagent.rs @@ -100,6 +100,7 @@ impl SubAgentManager { let task_prompt_for_spawn = task_prompt.clone(); let task = tokio::spawn(async move { + session.initialize().await; session.process_input(&task_prompt_for_spawn).await?; let turns = session.history().turns(); let last_text = turns.iter().rev().find_map(|t| match t { @@ -369,7 +370,10 @@ pub fn make_close_agent_tool(manager: Arc>) #[cfg(test)] mod tests { use super::*; + use crate::config::SessionConfig; use crate::test_support::*; + use arc_llm::provider::ProviderAdapter; + use arc_llm::types::Role; // --- Tests --- @@ -391,6 +395,36 @@ mod tests { assert!(manager.get(&agent_id).is_some()); } + #[tokio::test] + async fn spawn_initializes_session_before_processing_input() { + let mut manager = SubAgentManager::new(3); + + let provider = Arc::new(CapturingLlmProvider::new()); + let provider_ref = provider.clone(); + let client = make_client(provider as Arc).await; + let profile = Arc::new(TestProfile::new()); + let env = Arc::new(MockSandbox::default()); + let session = Session::new(client, profile, env, SessionConfig::default()); + + let agent_id = manager.spawn(session, "Do something".into(), 0).unwrap(); + let _ = manager.wait(&agent_id).await.unwrap(); + + let captured = provider_ref.captured_request.lock().unwrap(); + let request = captured + .as_ref() + .expect("request should have been captured"); + let system_message = request + .messages + .iter() + .find(|message| message.role == Role::System) + .expect("subagent request should include system message"); + + assert!( + !system_message.text().trim().is_empty(), + "subagent system prompt should not be empty" + ); + } + #[tokio::test] async fn depth_limit_enforced() { let mut manager = SubAgentManager::new(2); diff --git a/crates/arc-llm/src/providers/anthropic.rs b/crates/arc-llm/src/providers/anthropic.rs index db7d204aa..716a3fadf 100644 --- a/crates/arc-llm/src/providers/anthropic.rs +++ b/crates/arc-llm/src/providers/anthropic.rs @@ -1044,11 +1044,13 @@ fn build_api_request( let auto_cache = is_auto_cache_enabled(request.provider_options.as_ref()); - let mut system_value = system.map(|s| { - if auto_cache { - system_with_cache_control(&s) + let mut system_value = system.and_then(|s| { + if s.trim().is_empty() { + None + } else if auto_cache { + Some(system_with_cache_control(&s)) } else { - serde_json::Value::String(s) + Some(serde_json::Value::String(s)) } }); @@ -1581,6 +1583,32 @@ mod tests { assert_eq!(arr[0]["cache_control"]["type"], "ephemeral"); } + #[test] + fn build_api_request_omits_whitespace_only_system_prompt() { + let adapter = Adapter::new("test-key"); + let request = Request { + model: "claude-sonnet-4-20250514".to_string(), + messages: vec![Message::system(" \n\t"), Message::user("Hello")], + provider: Some("anthropic".to_string()), + tools: None, + tool_choice: None, + response_format: None, + temperature: None, + top_p: None, + max_tokens: Some(128), + stop_sequences: None, + reasoning_effort: None, + metadata: None, + provider_options: None, + }; + + let (api_request, _req_builder) = build_api_request(&adapter, &request, false); + assert!( + api_request.system.is_none(), + "whitespace-only system prompts should be omitted" + ); + } + fn make_request_with_format(format: crate::types::ResponseFormat) -> Request { Request { model: "claude-sonnet-4-20250514".to_string(), diff --git a/crates/arc-llm/src/providers/common.rs b/crates/arc-llm/src/providers/common.rs index 3a299b417..471ca6f1d 100644 --- a/crates/arc-llm/src/providers/common.rs +++ b/crates/arc-llm/src/providers/common.rs @@ -42,7 +42,10 @@ pub fn extract_system_prompt(messages: &[Message]) -> (Option, Vec<&Mess let mut other = Vec::new(); for msg in messages { if msg.role == Role::System || msg.role == Role::Developer { - system_parts.push(msg.text()); + let text = msg.text(); + if !text.trim().is_empty() { + system_parts.push(text); + } } else { other.push(msg); } @@ -471,6 +474,21 @@ mod tests { assert_eq!(other.len(), 1); } + #[test] + fn extract_system_prompt_ignores_whitespace_system_and_developer() { + let dev = Message { + role: Role::Developer, + content: vec![ContentPart::text(" \n\t ")], + name: None, + tool_call_id: None, + }; + let msgs = vec![Message::system(" "), dev, Message::user("hi")]; + let (sys, other) = extract_system_prompt(&msgs); + assert_eq!(sys, None); + assert_eq!(other.len(), 1); + assert_eq!(other[0].role, Role::User); + } + #[test] fn extract_system_prompt_empty() { let msgs: Vec = vec![];