From 538560f8523599531da618c837cf77b318fb501b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 23 Mar 2026 13:23:23 -0400 Subject: [PATCH] Move subagent_manager from setter to Session::new() constructor parameter Replaces set_subagent_manager() with an Option parameter on the constructor so the dependency is explicit at creation time. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-agent/README.md | 2 +- lib/crates/fabro-agent/src/cli.rs | 10 ++- lib/crates/fabro-agent/src/session.rs | 62 +++++++++++-------- lib/crates/fabro-agent/src/subagent.rs | 2 +- lib/crates/fabro-agent/src/test_support.rs | 8 +-- lib/crates/fabro-agent/src/v4a_patch.rs | 3 +- lib/crates/fabro-agent/tests/parity_matrix.rs | 5 +- lib/crates/fabro-retro/src/retro_agent.rs | 8 ++- lib/crates/fabro-workflows/src/backend/api.rs | 10 ++- 9 files changed, 70 insertions(+), 40 deletions(-) diff --git a/lib/crates/fabro-agent/README.md b/lib/crates/fabro-agent/README.md index d9c76f7df..dd5728de2 100644 --- a/lib/crates/fabro-agent/README.md +++ b/lib/crates/fabro-agent/README.md @@ -139,7 +139,7 @@ let config = SessionConfig { }; // 5. Create and initialize the session -let mut session = Session::new(client, profile, env, config); +let mut session = Session::new(client, profile, env, config, None); session.initialize().await; // 6. Subscribe to events (for UI rendering) diff --git a/lib/crates/fabro-agent/src/cli.rs b/lib/crates/fabro-agent/src/cli.rs index 1654b0630..4945e129b 100644 --- a/lib/crates/fabro-agent/src/cli.rs +++ b/lib/crates/fabro-agent/src/cli.rs @@ -466,13 +466,19 @@ pub async fn run_with_args_and_client( tool_hooks: factory_hooks.clone(), ..SessionConfig::default() }, + None, ) }); profile.register_subagent_tools(manager, factory, 0); let profile: Arc = Arc::from(profile); - let mut session = Session::new(client, profile, env, config); - session.set_subagent_manager(manager_for_callback.clone()); + let mut session = Session::new( + client, + profile, + env, + config, + Some(manager_for_callback.clone()), + ); // Wire subagent event callback to parent session's emitter manager_for_callback diff --git a/lib/crates/fabro-agent/src/session.rs b/lib/crates/fabro-agent/src/session.rs index 26f2f7f8f..420659d4c 100644 --- a/lib/crates/fabro-agent/src/session.rs +++ b/lib/crates/fabro-agent/src/session.rs @@ -54,6 +54,7 @@ impl Session { provider_profile: Arc, sandbox: Arc, config: SessionConfig, + subagent_manager: Option>>, ) -> Self { Self { id: uuid::Uuid::new_v4().to_string(), @@ -74,7 +75,7 @@ impl Session { system_prompt: String::new(), file_tracker: FileTracker::default(), tool_env: None, - subagent_manager: None, + subagent_manager, } } @@ -82,13 +83,6 @@ impl Session { self.tool_env = Some(env); } - pub fn set_subagent_manager( - &mut self, - manager: Arc>, - ) { - self.subagent_manager = Some(manager); - } - /// Initialize session by discovering project docs and capturing environment context. /// Call before `process_input`. pub async fn initialize(&mut self) { @@ -1032,10 +1026,23 @@ mod tests { } async fn make_session_with_provider(provider: Arc) -> Session { + make_session_with_provider_and_manager(provider, None).await + } + + async fn make_session_with_provider_and_manager( + provider: Arc, + subagent_manager: Option>>, + ) -> Session { let client = make_client(provider).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockSandbox::default()); - Session::new(client, profile, env, SessionConfig::default()) + Session::new( + client, + profile, + env, + SessionConfig::default(), + subagent_manager, + ) } // --- Tests --- @@ -1399,7 +1406,7 @@ mod tests { enable_loop_detection: false, ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); // Wire the session's cancel_token to our shared one session.cancel_token = cancel_token; @@ -1430,7 +1437,7 @@ mod tests { let client = make_client(error_provider).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockSandbox::default()); - let mut session = Session::new(client, profile, env, SessionConfig::default()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); let result = session.process_input("Hello").await; assert!(result.is_err()); @@ -1509,7 +1516,7 @@ mod tests { let client = make_client(provider).await; let profile = Arc::new(TestProfile::with_tools(registry)); let env = Arc::new(MockSandbox::default()); - let mut session = Session::new(client, profile, env, SessionConfig::default()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); let mut rx = session.subscribe(); session.process_input("Use echo three times").await.unwrap(); @@ -1560,7 +1567,7 @@ mod tests { let registry = ToolRegistry::new(); let profile = Arc::new(TestProfile::with_context_window(registry, 100)); let env = Arc::new(MockSandbox::default()); - let mut session = Session::new(client, profile, env, SessionConfig::default()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); let mut rx = session.subscribe(); session.process_input(&large_input).await.unwrap(); @@ -1586,7 +1593,7 @@ mod tests { 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()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); // Default reasoning_effort is None session.set_reasoning_effort(Some(fabro_llm::types::ReasoningEffort::High)); @@ -1612,7 +1619,7 @@ mod tests { // Large context window so short input stays well under 80% let profile = Arc::new(TestProfile::with_context_window(registry, 200_000)); let env = Arc::new(MockSandbox::default()); - let mut session = Session::new(client, profile, env, SessionConfig::default()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); let mut rx = session.subscribe(); session.process_input("Hi").await.unwrap(); @@ -1745,7 +1752,7 @@ mod tests { user_instructions: Some("Always use TDD".into()), ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); session.initialize().await; session.process_input("test").await.unwrap(); @@ -1769,7 +1776,7 @@ mod tests { 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()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); // Intentionally skip initialize(): system prompt remains empty. session.process_input("test").await.unwrap(); @@ -2000,7 +2007,7 @@ mod tests { 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()); + let mut session = Session::new(client, profile, env, SessionConfig::default(), None); let result = session.process_input("Hello").await; assert!(matches!( @@ -2178,7 +2185,7 @@ mod tests { compaction_preserve_turns: 1, ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); let mut rx = session.subscribe(); session.process_input(&large_input).await.unwrap(); @@ -2220,7 +2227,7 @@ mod tests { enable_context_compaction: false, ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); let mut rx = session.subscribe(); session.process_input(&large_input).await.unwrap(); @@ -2304,7 +2311,7 @@ mod tests { compaction_preserve_turns: 1, ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); let mut rx = session.subscribe(); // Should not return an error even though compaction fails @@ -2409,7 +2416,7 @@ mod tests { ..Default::default() }; - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); let mut rx = session.subscribe(); // First call: tool call executes, files get tracked, no compaction yet @@ -2502,7 +2509,7 @@ mod tests { let client = make_client(provider).await; let profile: Arc = Arc::new(TestProfile::new()); let env: Arc = Arc::new(MockSandbox::default()); - let mut session = Session::new(client, profile, env, config); + let mut session = Session::new(client, profile, env, config, None); // Subscribe to events before initialize let mut rx = session.subscribe(); @@ -2650,9 +2657,14 @@ mod tests { async fn close_cleans_up_subagents_before_emitting_session_ended() { use crate::subagent::SubAgentManager; - let mut session = make_session(vec![text_response("done")]).await; let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let provider = Arc::new(ScriptedStreamProvider::new(vec![ + ScriptedStreamCall::Response(text_response("done")), + ])); + let mut session = + make_session_with_provider_and_manager(provider, Some(manager.clone())).await; + // Wire the manager's event callback to the session's emitter manager .lock() @@ -2663,8 +2675,6 @@ mod tests { let child = make_session(vec![text_response("child done")]).await; let agent_id = manager.lock().await.spawn(child, "task".into(), 0).unwrap(); - session.set_subagent_manager(manager.clone()); - // Collect events let mut rx = session.subscribe(); session.close(); diff --git a/lib/crates/fabro-agent/src/subagent.rs b/lib/crates/fabro-agent/src/subagent.rs index dd9d74ca2..c9cd09ca3 100644 --- a/lib/crates/fabro-agent/src/subagent.rs +++ b/lib/crates/fabro-agent/src/subagent.rs @@ -414,7 +414,7 @@ mod tests { 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 session = Session::new(client, profile, env, SessionConfig::default(), None); let agent_id = manager.spawn(session, "Do something".into(), 0).unwrap(); let _ = manager.wait(&agent_id).await.unwrap(); diff --git a/lib/crates/fabro-agent/src/test_support.rs b/lib/crates/fabro-agent/src/test_support.rs index be701c1e2..62b814bcd 100644 --- a/lib/crates/fabro-agent/src/test_support.rs +++ b/lib/crates/fabro-agent/src/test_support.rs @@ -196,7 +196,7 @@ pub async fn make_session(responses: Vec) -> Session { let client = make_client(provider).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockSandbox::default()); - Session::new(client, profile, env, SessionConfig::default()) + Session::new(client, profile, env, SessionConfig::default(), None) } pub async fn make_session_with_tools(responses: Vec, registry: ToolRegistry) -> Session { @@ -204,7 +204,7 @@ pub async fn make_session_with_tools(responses: Vec, registry: ToolReg let client = make_client(provider).await; let profile = Arc::new(TestProfile::with_tools(registry)); let env = Arc::new(MockSandbox::default()); - Session::new(client, profile, env, SessionConfig::default()) + Session::new(client, profile, env, SessionConfig::default(), None) } pub async fn make_session_with_config(responses: Vec, config: SessionConfig) -> Session { @@ -212,7 +212,7 @@ pub async fn make_session_with_config(responses: Vec, config: SessionC let client = make_client(provider).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockSandbox::default()); - Session::new(client, profile, env, config) + Session::new(client, profile, env, config, None) } pub async fn make_session_with_tools_and_config( @@ -224,7 +224,7 @@ pub async fn make_session_with_tools_and_config( let client = make_client(provider).await; let profile = Arc::new(TestProfile::with_tools(registry)); let env = Arc::new(MockSandbox::default()); - Session::new(client, profile, env, config) + Session::new(client, profile, env, config, None) } pub fn tool_call_response( diff --git a/lib/crates/fabro-agent/src/v4a_patch.rs b/lib/crates/fabro-agent/src/v4a_patch.rs index 320d49389..d90c4901d 100644 --- a/lib/crates/fabro-agent/src/v4a_patch.rs +++ b/lib/crates/fabro-agent/src/v4a_patch.rs @@ -1452,7 +1452,8 @@ def farewell(name): let provider = Arc::new(MockLlmProvider::new(responses)); let client = make_client(provider).await; let profile = Arc::new(TestProfile::with_tools(registry)); - let mut session = Session::new(client, profile, env.clone(), SessionConfig::default()); + let mut session = + Session::new(client, profile, env.clone(), SessionConfig::default(), None); session.initialize().await; session .process_input("Update the greeting functions") diff --git a/lib/crates/fabro-agent/tests/parity_matrix.rs b/lib/crates/fabro-agent/tests/parity_matrix.rs index 4c43d4c33..270a78ca2 100644 --- a/lib/crates/fabro-agent/tests/parity_matrix.rs +++ b/lib/crates/fabro-agent/tests/parity_matrix.rs @@ -95,6 +95,7 @@ async fn make_session(provider: Provider, model: &str, cwd: &Path) -> Session { sub_profile, sub_env, SessionConfig::default(), + None, ) }); profile.register_subagent_tools(manager, factory, 0); @@ -104,7 +105,7 @@ async fn make_session(provider: Provider, model: &str, cwd: &Path) -> Session { max_turns: 20, ..SessionConfig::default() }; - Session::new(client, profile, env, config) + Session::new(client, profile, env, config, None) } async fn make_session_with_config( @@ -117,7 +118,7 @@ async fn make_session_with_config( let client = Client::from_env().await.expect("Client::from_env failed"); let profile: Arc = Arc::from(build_profile(provider, model, &client)); let env = Arc::new(LocalSandbox::new(cwd.to_path_buf())); - Session::new(client, profile, env, config) + Session::new(client, profile, env, config, None) } macro_rules! provider_test { diff --git a/lib/crates/fabro-retro/src/retro_agent.rs b/lib/crates/fabro-retro/src/retro_agent.rs index de4cbe2ef..896091845 100644 --- a/lib/crates/fabro-retro/src/retro_agent.rs +++ b/lib/crates/fabro-retro/src/retro_agent.rs @@ -163,7 +163,13 @@ pub async fn run_retro_agent( ..SessionConfig::default() }; - let mut session = Session::new(llm_client.clone(), profile, Arc::clone(sandbox), config); + let mut session = Session::new( + llm_client.clone(), + profile, + Arc::clone(sandbox), + config, + None, + ); // Set up event writer before initialize (which emits SessionStarted) let retro_dir = run_dir.join("retro"); diff --git a/lib/crates/fabro-workflows/src/backend/api.rs b/lib/crates/fabro-workflows/src/backend/api.rs index 1a004bfe5..519223855 100644 --- a/lib/crates/fabro-workflows/src/backend/api.rs +++ b/lib/crates/fabro-workflows/src/backend/api.rs @@ -231,6 +231,7 @@ impl AgentApiBackend { child_profile, Arc::clone(&factory_env), SessionConfig::default(), + None, ); if !factory_tool_env.is_empty() { session.set_tool_env(factory_tool_env.clone()); @@ -241,8 +242,13 @@ impl AgentApiBackend { profile.register_subagent_tools(manager, factory, 0); let profile: Arc = Arc::from(profile); - let mut session = Session::new(client, profile, Arc::clone(sandbox), config); - session.set_subagent_manager(manager_for_callback.clone()); + let mut session = Session::new( + client, + profile, + Arc::clone(sandbox), + config, + Some(manager_for_callback.clone()), + ); if !env.is_empty() { session.set_tool_env(env.clone()); }