From 4f4f576439f56f3742ea89f374e2efe3bb5ff15a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 11 May 2026 16:25:30 -0400 Subject: [PATCH] refactor: collapse codergen run arguments Introduce CodergenRunRequest so multi-turn backend calls use the same request-object shape as one-shot calls and stop growing the trait parameter list. --- .../fabro-cli/tests/it/workflow/real_cli.rs | 22 +-- .../fabro-workflow/src/handler/agent.rs | 170 ++++------------- .../fabro-workflow/src/handler/fan_in.rs | 28 +-- .../fabro-workflow/src/handler/llm/acp.rs | 118 ++++++------ .../fabro-workflow/src/handler/llm/api.rs | 25 ++- .../fabro-workflow/src/handler/llm/cli.rs | 134 ++++---------- .../fabro-workflow/src/handler/prompt.rs | 42 +---- .../tests/it/daytona_integration.rs | 22 +-- .../fabro-workflow/tests/it/integration.rs | 173 ++++++------------ 9 files changed, 238 insertions(+), 496 deletions(-) diff --git a/lib/crates/fabro-cli/tests/it/workflow/real_cli.rs b/lib/crates/fabro-cli/tests/it/workflow/real_cli.rs index efa322731..601ffab65 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/real_cli.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/real_cli.rs @@ -5,7 +5,7 @@ use fabro_graphviz::graph::{AttrValue, Node}; use fabro_llm::provider::Provider; use fabro_workflow::context::Context; use fabro_workflow::event::Emitter; -use fabro_workflow::handler::agent::{CodergenBackend, CodergenResult}; +use fabro_workflow::handler::agent::{CodergenBackend, CodergenResult, CodergenRunRequest}; use fabro_workflow::handler::llm::cli::AgentCliBackend; /// Run a real CLI tool via LocalSandbox and verify the full flow. @@ -26,16 +26,16 @@ async fn run_real_cli_test(provider: Provider, model: &str) { let context = Context::new(); let emitter = Arc::new(Emitter::default()); let result = backend - .run( - &node, - "What is 2+2? Reply with just the number.", - &context, - None, - &emitter, - &env, - None, - tokio_util::sync::CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "What is 2+2? Reply with just the number.", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &env, + tool_hooks: None, + cancel_token: tokio_util::sync::CancellationToken::new(), + }) .await .unwrap_or_else(|_| panic!("CLI backend ({provider}/{model}) should succeed")); diff --git a/lib/crates/fabro-workflow/src/handler/agent.rs b/lib/crates/fabro-workflow/src/handler/agent.rs index c5a2561d2..376d45431 100644 --- a/lib/crates/fabro-workflow/src/handler/agent.rs +++ b/lib/crates/fabro-workflow/src/handler/agent.rs @@ -28,6 +28,17 @@ pub enum CodergenResult { Full(Outcome), } +pub struct CodergenRunRequest<'a> { + pub node: &'a Node, + pub prompt: &'a str, + pub context: &'a Context, + pub thread_id: Option<&'a str>, + pub emitter: &'a Arc, + pub sandbox: &'a Arc, + pub tool_hooks: Option>, + pub cancel_token: CancellationToken, +} + pub struct OneShotRequest<'a> { pub node: &'a Node, pub prompt: &'a str, @@ -39,24 +50,10 @@ pub struct OneShotRequest<'a> { } /// Backend interface for LLM execution in codergen nodes. -#[allow( - clippy::too_many_arguments, - reason = "Codergen run mode needs the node, prompt, context, and runtime handles separately." -)] #[async_trait] pub trait CodergenBackend: Send + Sync { /// Run a multi-turn agent loop (the default codergen mode). - async fn run( - &self, - node: &Node, - prompt: &str, - context: &Context, - thread_id: Option<&str>, - emitter: &Arc, - sandbox: &Arc, - tool_hooks: Option>, - cancel_token: CancellationToken, - ) -> Result; + async fn run(&self, request: CodergenRunRequest<'_>) -> Result; /// Run a single LLM call with no tools (one_shot mode). async fn one_shot(&self, _request: OneShotRequest<'_>) -> Result { @@ -304,16 +301,16 @@ impl Handler for AgentHandler { let (response_text, stage_usage, backend_files_touched, last_file_touched) = if let Some(backend) = &self.backend { let result = backend - .run( + .run(CodergenRunRequest { node, - &prompt, + prompt: &prompt, context, - thread_id.as_deref(), - &services.run.emitter, - &services.run.sandbox, + thread_id: thread_id.as_deref(), + emitter: &services.run.emitter, + sandbox: &services.run.sandbox, tool_hooks, - services.run.cancel_token(), - ) + cancel_token: services.run.cancel_token(), + }) .await; match result { Ok(CodergenResult::Full(outcome)) => return Ok(outcome), @@ -432,7 +429,6 @@ mod tests { use tempfile::TempDir; use super::*; - use crate::event::Emitter; fn make_services() -> EngineServices { EngineServices::test_default() @@ -644,25 +640,13 @@ mod tests { #[tokio::test] async fn codergen_handler_prefers_response_text_over_status_json() { - use std::sync::Arc; - // Backend returns response text with routing directives — status.json // in the sandbox should be ignored. struct DirectiveBackend; #[async_trait] impl CodergenBackend for DirectiveBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { Ok(CodergenResult::Text { text: r#"Done. {"outcome": "succeeded", "preferred_next_label": "approve"}"# @@ -707,23 +691,11 @@ mod tests { #[tokio::test] async fn codergen_handler_extracts_status_from_last_file_touched() { - use std::sync::Arc; - struct LastFileBackend; #[async_trait] impl CodergenBackend for LastFileBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { Ok(CodergenResult::Text { text: "Done writing results.".to_string(), usage: None, @@ -775,21 +747,11 @@ mod tests { #[async_trait] impl CodergenBackend for ProviderEventBackend { - async fn run( - &self, - node: &Node, - _prompt: &str, - context: &Context, - _thread_id: Option<&str>, - emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { - let scope = StageScope::for_handler(context, &node.id); - emitter.emit_scoped( + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + let scope = StageScope::for_handler(request.context, &request.node.id); + request.emitter.emit_scoped( &crate::event::Event::AgentSessionActivated { - node_id: node.id.clone(), + node_id: request.node.id.clone(), visit: scope.visit, session_id: "session_123".to_string(), thread_id: None, @@ -886,18 +848,9 @@ mod tests { #[async_trait] impl CodergenBackend for ThreadCapturingBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { - *self.captured_thread_id.lock().unwrap() = Some(thread_id.map(String::from)); + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + *self.captured_thread_id.lock().unwrap() = + Some(request.thread_id.map(String::from)); Ok(CodergenResult::Text { text: "ok".to_string(), usage: None, @@ -939,18 +892,9 @@ mod tests { #[async_trait] impl CodergenBackend for ThreadCapturingBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { - *self.captured_thread_id.lock().unwrap() = Some(thread_id.map(String::from)); + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + *self.captured_thread_id.lock().unwrap() = + Some(request.thread_id.map(String::from)); Ok(CodergenResult::Text { text: "ok".to_string(), usage: None, @@ -987,17 +931,7 @@ mod tests { #[async_trait] impl CodergenBackend for FailingBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { Err(Error::handler("Request timed out".to_string())) } } @@ -1135,17 +1069,7 @@ Some text in between. #[async_trait] impl CodergenBackend for ValidationFailBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { Err(Error::Validation("bad config".to_string())) } } @@ -1176,18 +1100,8 @@ Some text in between. #[async_trait] impl CodergenBackend for PromptCapturingBackend { - async fn run( - &self, - _node: &Node, - prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { - *self.captured_prompt.lock().unwrap() = Some(prompt.to_string()); + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + *self.captured_prompt.lock().unwrap() = Some(request.prompt.to_string()); Ok(CodergenResult::Text { text: "ok".to_string(), usage: None, @@ -1246,18 +1160,8 @@ Some text in between. #[async_trait] impl CodergenBackend for PromptCapturingBackend { - async fn run( - &self, - _node: &Node, - prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { - *self.captured_prompt.lock().unwrap() = Some(prompt.to_string()); + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + *self.captured_prompt.lock().unwrap() = Some(request.prompt.to_string()); Ok(CodergenResult::Text { text: "ok".to_string(), usage: None, diff --git a/lib/crates/fabro-workflow/src/handler/fan_in.rs b/lib/crates/fabro-workflow/src/handler/fan_in.rs index 11a79f7b0..455b27b26 100644 --- a/lib/crates/fabro-workflow/src/handler/fan_in.rs +++ b/lib/crates/fabro-workflow/src/handler/fan_in.rs @@ -6,7 +6,7 @@ use fabro_agent::Sandbox; use fabro_graphviz::graph::{Graph, Node}; use tokio_util::sync::CancellationToken; -use super::agent::{CodergenBackend, CodergenResult}; +use super::agent::{CodergenBackend, CodergenResult, CodergenRunRequest}; use super::{EngineServices, Handler}; use crate::context::{Context, keys}; use crate::error::Error; @@ -260,16 +260,16 @@ async fn llm_evaluate( // Fan-in evaluation runs outside a thread context, so pass None match backend - .run( - &eval_node, - &full_prompt, + .run(CodergenRunRequest { + node: &eval_node, + prompt: &full_prompt, context, - None, + thread_id: None, emitter, sandbox, - None, + tool_hooks: None, cancel_token, - ) + }) .await { Ok(CodergenResult::Full(outcome)) => { @@ -469,23 +469,13 @@ mod tests { async fn fan_in_with_backend_llm_eval() { use tempfile::TempDir; - use crate::handler::agent::CodergenBackend; + use crate::handler::agent::{CodergenBackend, CodergenRunRequest}; struct MockBackend; #[async_trait] impl CodergenBackend for MockBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { // Return text that contains the ID "branch_b" Ok(CodergenResult::Text { text: "The best candidate is branch_b".to_string(), diff --git a/lib/crates/fabro-workflow/src/handler/llm/acp.rs b/lib/crates/fabro-workflow/src/handler/llm/acp.rs index 03197c6e0..5df192f39 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/acp.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/acp.rs @@ -12,11 +12,10 @@ use fabro_model::Provider; use fabro_util::time::elapsed_ms; use tokio_util::sync::CancellationToken; -use super::super::agent::{CodergenBackend, CodergenResult, OneShotRequest}; +use super::super::agent::{CodergenBackend, CodergenResult, CodergenRunRequest, OneShotRequest}; use super::changed_files; use super::cli::AgentCli; use super::launch_env::{AgentLaunchEnvRequest, resolve_agent_launch_env}; -use crate::context::Context; use crate::error::Error; use crate::event::{Emitter, Event, StageScope}; @@ -199,25 +198,15 @@ impl AgentAcpBackend { #[async_trait] impl CodergenBackend for AgentAcpBackend { - async fn run( - &self, - node: &Node, - prompt: &str, - context: &Context, - _thread_id: Option<&str>, - emitter: &Arc, - sandbox: &Arc, - _tool_hooks: Option>, - cancel_token: CancellationToken, - ) -> Result { - let stage_scope = StageScope::for_handler(context, &node.id); + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + let stage_scope = StageScope::for_handler(request.context, &request.node.id); self.run_turn( - node, - prompt.to_string(), - emitter, + request.node, + request.prompt.to_string(), + request.emitter, &stage_scope, - sandbox, - cancel_token, + request.sandbox, + request.cancel_token, ) .await } @@ -294,7 +283,9 @@ mod tests { use super::AgentAcpBackend; use crate::context::Context; use crate::event::{Emitter, StageScope}; - use crate::handler::agent::{CodergenBackend, CodergenResult, OneShotRequest}; + use crate::handler::agent::{ + CodergenBackend, CodergenResult, CodergenRunRequest, OneShotRequest, + }; #[tokio::test] async fn acp_backend_run_sends_prompt_and_returns_text() { @@ -329,17 +320,19 @@ mod tests { HashMap::from([("ACP_MODE".to_string(), "write_file".to_string())]), ); let sandbox: Arc = Arc::new(LocalSandbox::new(tempdir.path().to_path_buf())); + let emitter = Arc::new(Emitter::default()); + let context = Context::new(); let result = backend - .run( - &node, - "write hello", - &Context::new(), - None, - &Arc::new(Emitter::default()), - &sandbox, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "write hello", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await .unwrap(); @@ -441,17 +434,19 @@ mod tests { HashMap::from([("ACP_STOP_REASON".to_string(), "cancelled".to_string())]), ); let sandbox: Arc = Arc::new(LocalSandbox::new(tempdir.path().to_path_buf())); + let emitter = Arc::new(Emitter::default()); + let context = Context::new(); let result = backend - .run( - &node, - "cancel", - &Context::new(), - None, - &Arc::new(Emitter::default()), - &sandbox, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "cancel", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await; let Err(err) = result else { panic!("expected cancellation error"); @@ -497,17 +492,18 @@ mod tests { move |event| events.lock().unwrap().push(event.clone()) }); + let context = Context::new(); backend - .run( - &node, - "write hello", - &Context::new(), - None, - &emitter, - &sandbox, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "write hello", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await .unwrap(); @@ -540,17 +536,19 @@ mod tests { .insert("backend".to_string(), AttrValue::String("acp".to_string())); let backend = AgentAcpBackend::new_from_env("fake-acp".to_string(), Provider::OpenAi); + let emitter = Arc::new(Emitter::default()); + let context = Context::new(); let result = backend - .run( - &node, - "write hello", - &Context::new(), - None, - &Arc::new(Emitter::default()), - &sandbox_dyn, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "write hello", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox_dyn, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await; let Err(err) = result else { panic!("ACP without acp_command should fail"); diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index 51aae4d42..a8cdfab03 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -19,10 +19,10 @@ use tokio::sync::Mutex as TokioMutex; use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; -use super::super::agent::{CodergenBackend, CodergenResult, OneShotRequest}; +use super::super::agent::{CodergenBackend, CodergenResult, CodergenRunRequest, OneShotRequest}; use super::activation_lease::{ActivationLease, ActivationLeaseOptions}; +use crate::context::WorkflowContext; use crate::context::keys::Fidelity; -use crate::context::{Context, WorkflowContext}; use crate::error::Error; use crate::event::{Emitter, Event, StageScope}; use crate::outcome::billed_model_usage_from_llm; @@ -616,17 +616,16 @@ impl CodergenBackend for AgentApiBackend { }) } - async fn run( - &self, - node: &Node, - prompt: &str, - context: &Context, - thread_id: Option<&str>, - emitter: &Arc, - sandbox: &Arc, - tool_hooks: Option>, - cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + let node = request.node; + let prompt = request.prompt; + let context = request.context; + let thread_id = request.thread_id; + let emitter = request.emitter; + let sandbox = request.sandbox; + let tool_hooks = request.tool_hooks; + let cancel_token = request.cancel_token; + let actual_model = node.model().unwrap_or(&self.model).to_string(); let _actual_provider = node .provider() diff --git a/lib/crates/fabro-workflow/src/handler/llm/cli.rs b/lib/crates/fabro-workflow/src/handler/llm/cli.rs index f721f3cac..67ad67788 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/cli.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/cli.rs @@ -38,11 +38,10 @@ fn cli_failure_detail(stdout: &str, stderr: &str, command: &str) -> String { } } -use super::super::agent::{CodergenBackend, CodergenResult, OneShotRequest}; +use super::super::agent::{CodergenBackend, CodergenResult, CodergenRunRequest, OneShotRequest}; use super::acp::AgentAcpBackend; use super::changed_files; use super::launch_env::{AgentLaunchEnvRequest, resolve_agent_launch_env}; -use crate::context::Context; use crate::error::Error; use crate::event::{Emitter, Event, StageScope}; use crate::outcome::billed_model_usage_from_llm; @@ -390,17 +389,14 @@ impl AgentCliBackend { #[async_trait] impl CodergenBackend for AgentCliBackend { - async fn run( - &self, - node: &Node, - prompt: &str, - context: &Context, - _thread_id: Option<&str>, - emitter: &Arc, - sandbox: &Arc, - _tool_hooks: Option>, - cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + let node = request.node; + let prompt = request.prompt; + let context = request.context; + let emitter = request.emitter; + let sandbox = request.sandbox; + let cancel_token = request.cancel_token; + // 1. Snapshot git state before the CLI run let files_before = changed_files::detect_changed_files(sandbox).await; @@ -722,60 +718,11 @@ fn unsupported_backend_error(raw: &str) -> Error { #[async_trait] impl CodergenBackend for BackendRouter { - async fn run( - &self, - node: &Node, - prompt: &str, - context: &Context, - thread_id: Option<&str>, - emitter: &Arc, - sandbox: &Arc, - tool_hooks: Option>, - cancel_token: CancellationToken, - ) -> Result { - match Self::select_backend(node)? { - LlmBackend::Api => { - self.api - .run( - node, - prompt, - context, - thread_id, - emitter, - sandbox, - tool_hooks, - cancel_token, - ) - .await - } - LlmBackend::Cli => { - self.cli - .run( - node, - prompt, - context, - thread_id, - emitter, - sandbox, - tool_hooks, - cancel_token, - ) - .await - } - LlmBackend::Acp => { - self.acp - .run( - node, - prompt, - context, - thread_id, - emitter, - sandbox, - tool_hooks, - cancel_token, - ) - .await - } + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + match Self::select_backend(request.node)? { + LlmBackend::Api => self.api.run(request).await, + LlmBackend::Cli => self.cli.run(request).await, + LlmBackend::Acp => self.acp.run(request).await, } } @@ -800,6 +747,7 @@ mod tests { use fabro_graphviz::graph::AttrValue; use super::*; + use crate::context::Context; // -- AgentCli -- @@ -1348,17 +1296,7 @@ mod tests { #[async_trait] impl CodergenBackend for StubBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { Ok(CodergenResult::Text { text: "stub".to_string(), usage: None, @@ -1518,16 +1456,16 @@ mod tests { let events = collect_events(&emitter); let result = backend - .run( - &node, - "Do something", - &context, - None, - &emitter, - &sandbox, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "Do something", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await; let Err(err) = result else { @@ -1571,16 +1509,16 @@ mod tests { let events = collect_events(&emitter); let result = backend - .run( - &node, - "Do something slow", - &context, - None, - &emitter, - &sandbox, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "Do something slow", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await; let Err(err) = result else { diff --git a/lib/crates/fabro-workflow/src/handler/prompt.rs b/lib/crates/fabro-workflow/src/handler/prompt.rs index 7d440087b..c65744741 100644 --- a/lib/crates/fabro-workflow/src/handler/prompt.rs +++ b/lib/crates/fabro-workflow/src/handler/prompt.rs @@ -210,10 +210,10 @@ mod tests { use fabro_types::fixtures; use object_store::memory::InMemory; use tempfile::TempDir; - use tokio_util::sync::CancellationToken; use super::*; use crate::event::Emitter; + use crate::handler::agent::CodergenRunRequest; fn make_services() -> EngineServices { EngineServices::test_default() @@ -311,23 +311,11 @@ mod tests { #[tokio::test] async fn prompt_handler_dispatches_to_backend_one_shot() { - use fabro_agent::Sandbox; - struct OneShotBackend; #[async_trait] impl CodergenBackend for OneShotBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { panic!("run() should not be called for prompt handler"); } @@ -370,23 +358,11 @@ mod tests { #[tokio::test] async fn prompt_handler_projects_provider_used_from_prompt_events() { - use fabro_agent::Sandbox; - struct ProviderOneShotBackend; #[async_trait] impl CodergenBackend for ProviderOneShotBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { panic!("run() should not be called for prompt handler"); } @@ -432,17 +408,7 @@ mod tests { #[async_trait] impl CodergenBackend for OneShotCapturingBackend { - async fn run( - &self, - _node: &Node, - _prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: CancellationToken, - ) -> Result { + async fn run(&self, _request: CodergenRunRequest<'_>) -> Result { panic!("run() should not be called for prompt handler"); } diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index 50e5bd39c..289613a27 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -995,7 +995,7 @@ async fn daytona_parallel_git_branching_e2e() { // CLI Backend on Daytona — real CLI tools via exec_command // --------------------------------------------------------------------------- -use fabro_workflow::handler::agent::{CodergenBackend, CodergenResult}; +use fabro_workflow::handler::agent::{CodergenBackend, CodergenResult, CodergenRunRequest}; use fabro_workflow::handler::llm::AgentCliBackend; /// Helper: run a real CLI backend test on Daytona. @@ -1072,16 +1072,16 @@ async fn run_daytona_cli_test(provider: Provider, model: &str, install_command: let emitter = Arc::new(Emitter::default()); let result = backend - .run( - &node, - "What is 2+2? Reply with just the number.", - &context, - None, - &emitter, - &env, - None, - CancellationToken::new(), - ) + .run(CodergenRunRequest { + node: &node, + prompt: "What is 2+2? Reply with just the number.", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &env, + tool_hooks: None, + cancel_token: CancellationToken::new(), + }) .await; match result { diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 2d50a638c..8376c27ae 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -38,7 +38,9 @@ use fabro_validate::{Severity, validate, validate_or_raise}; use fabro_workflow::context::Context; use fabro_workflow::error::{Error, FailureSignatureExt}; use fabro_workflow::event::{Emitter, Event}; -use fabro_workflow::handler::agent::{AgentHandler, CodergenBackend, CodergenResult}; +use fabro_workflow::handler::agent::{ + AgentHandler, CodergenBackend, CodergenResult, CodergenRunRequest, +}; use fabro_workflow::handler::command::CommandHandler; use fabro_workflow::handler::conditional::ConditionalHandler; use fabro_workflow::handler::exit::ExitHandler; @@ -65,6 +67,25 @@ fn local_env() -> Arc { )) } +fn codergen_run_request<'a>( + node: &'a Node, + prompt: &'a str, + context: &'a Context, + emitter: &'a Arc, + sandbox: &'a Arc, +) -> CodergenRunRequest<'a> { + CodergenRunRequest { + node, + prompt, + context, + thread_id: None, + emitter, + sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + } +} + fn test_run_id(label: &str) -> RunId { let mut hasher = DefaultHasher::new(); label.hash(&mut hasher); @@ -1606,22 +1627,12 @@ struct MockCodergenBackend; #[async_trait::async_trait] impl CodergenBackend for MockCodergenBackend { - async fn run( - &self, - node: &Node, - prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: tokio_util::sync::CancellationToken, - ) -> Result { + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { Ok(CodergenResult::Text { text: format!( "Response for {}: processed prompt '{}'", - node.id, - &prompt[..prompt.len().min(50)] + request.node.id, + &request.prompt[..request.prompt.len().min(50)] ), usage: None, files_touched: Vec::new(), @@ -6283,10 +6294,9 @@ mod real_llm { use fabro_llm::providers::OpenAiAdapter; use fabro_llm::types::{Message, Request}; use fabro_types::WorkflowSettings; - use fabro_workflow::context::Context; use fabro_workflow::error::Error; use fabro_workflow::handler::agent::{ - AgentHandler, CodergenBackend, CodergenResult, OneShotRequest, + AgentHandler, CodergenBackend, CodergenResult, CodergenRunRequest, OneShotRequest, }; use tokio_util::sync::CancellationToken; @@ -6298,18 +6308,8 @@ mod real_llm { #[async_trait] impl CodergenBackend for LlmCodergenBackend { - async fn run( - &self, - _node: &Node, - prompt: &str, - _context: &Context, - _thread_id: Option<&str>, - _emitter: &Arc, - _sandbox: &Arc, - _tool_hooks: Option>, - _cancel_token: tokio_util::sync::CancellationToken, - ) -> Result { - self.complete(prompt).await + async fn run(&self, request: CodergenRunRequest<'_>) -> Result { + self.complete(request.prompt).await } async fn one_shot(&self, request: OneShotRequest<'_>) -> Result { @@ -9804,16 +9804,13 @@ async fn cli_backend_run_writes_prompt_and_calls_exec() { let emitter = Arc::new(Emitter::default()); let result = backend - .run( + .run(codergen_run_request( &node, "Fix the authentication bug", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await .expect("CLI backend should succeed"); @@ -9878,16 +9875,13 @@ async fn cli_backend_run_detects_changed_files() { let emitter = Arc::new(Emitter::default()); let result = backend - .run( + .run(codergen_run_request( &node, "Add a new feature", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await .expect("CLI backend should succeed"); @@ -9912,16 +9906,13 @@ async fn cli_backend_run_with_codex_provider() { let emitter = Arc::new(Emitter::default()); let result = backend - .run( + .run(codergen_run_request( &node, "Build the API", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await .expect("CLI backend should succeed"); @@ -10080,16 +10071,13 @@ async fn cli_backend_run_fails_on_nonzero_exit() { let _ = env; // unused, just for the above struct let result = backend - .run( + .run(codergen_run_request( &node, "do something", &context, - None, &emitter, &failing_env, - None, - CancellationToken::new(), - ) + )) .await; let err = match result { @@ -10118,16 +10106,13 @@ async fn cli_backend_run_fails_on_unparseable_output() { let emitter = Arc::new(Emitter::default()); let result = backend - .run( + .run(codergen_run_request( &node, "do something", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await; let err = match result { @@ -10160,16 +10145,9 @@ async fn cli_backend_run_uses_node_model_override() { let emitter = Arc::new(Emitter::default()); backend - .run( - &node, - "test", - &context, - None, - &emitter, - &env, - None, - CancellationToken::new(), - ) + .run(codergen_run_request( + &node, "test", &context, &emitter, &env, + )) .await .expect("should succeed"); @@ -10210,16 +10188,9 @@ async fn cli_backend_run_uses_node_provider_override() { let emitter = Arc::new(Emitter::default()); backend - .run( - &node, - "test", - &context, - None, - &emitter, - &env, - None, - CancellationToken::new(), - ) + .run(codergen_run_request( + &node, "test", &context, &emitter, &env, + )) .await .expect("should succeed"); @@ -10244,16 +10215,9 @@ async fn cli_backend_run_returns_text_and_usage() { let emitter = Arc::new(Emitter::default()); let result = backend - .run( - &node, - "test", - &context, - None, - &emitter, - &env, - None, - CancellationToken::new(), - ) + .run(codergen_run_request( + &node, "test", &context, &emitter, &env, + )) .await .expect("should succeed"); @@ -10297,16 +10261,13 @@ async fn backend_router_delegates_to_cli_for_cli_node() { let emitter = Arc::new(Emitter::default()); let result = router - .run( + .run(codergen_run_request( &node, "Fix the bug", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await .expect("router should succeed"); @@ -10340,16 +10301,13 @@ async fn backend_router_delegates_to_api_for_normal_node() { let emitter = Arc::new(Emitter::default()); let result = router - .run( + .run(codergen_run_request( &node, "Plan the work", &context, - None, &emitter, &env, - None, - CancellationToken::new(), - ) + )) .await .expect("router should succeed"); @@ -10386,16 +10344,9 @@ async fn backend_router_delegates_to_cli_for_backend_attr() { let emitter = Arc::new(Emitter::default()); let result = router - .run( - &node, - "Build it", - &context, - None, - &emitter, - &env, - None, - CancellationToken::new(), - ) + .run(codergen_run_request( + &node, "Build it", &context, &emitter, &env, + )) .await .expect("router should succeed"); @@ -10448,17 +10399,13 @@ async fn backend_router_delegates_to_acp_for_acp_node() { )), ); + let context = Context::new(); + let emitter = Arc::new(Emitter::default()); + let result = router - .run( - &node, - "Build it", - &Context::new(), - None, - &Arc::new(Emitter::default()), - &env, - None, - CancellationToken::new(), - ) + .run(codergen_run_request( + &node, "Build it", &context, &emitter, &env, + )) .await .expect("router should succeed");