scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-11 22:53:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Fix dry-run bug in API server and deduplicate test helpers

Browse source 
The API server set RunConfig.dry_run but never called
engine.set_dry_run(), so command/script nodes executed for real
during API-served dry runs. Add the missing call.

Also extract EngineServices::test_default() to replace 11 identical
make_services() bodies and 8 inline struct constructions across
handler test modules (-254/+57 lines).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-15 12:28:54 -04:00
parent ef21be5a6d
commit d25fc56b10
No known key found for this signature in database
14 changed files with 57 additions and 254 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
lib/crates/fabro-api/src/server.rs
View file

@ -581,6 +581,9 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
Arc::clone(&interviewer) as Arc<dyn Interviewer>,
sandbox,
);
if state.dry_run {
engine.set_dry_run(true);
}
// Wire up hook runner from server config
if !state.hooks.is_empty() {

1
lib/crates/fabro-workflows/src/cli/run.rs
View file

@ -1136,7 +1136,6 @@ pub async fn run_command(
if dry_run_mode {
engine.set_dry_run(true);
}
// Wire up hook runner from run config or run defaults
{
let hooks = run_cfg

59
lib/crates/fabro-workflows/src/handler/agent.rs
View file

@ -346,22 +346,10 @@ mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::AttrValue;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
use tempfile::TempDir;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]
@ -489,17 +477,10 @@ mod tests {
let graph = Graph::new("test");
let tmp = TempDir::new().unwrap();
let services = EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)
@ -554,17 +535,10 @@ mod tests {
let graph = Graph::new("test");
let tmp = TempDir::new().unwrap();
let services = EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)
@ -620,17 +594,10 @@ mod tests {
let graph = Graph::new("test");
let tmp = TempDir::new().unwrap();
let services = EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.sandbox = std::sync::Arc::new(fabro_agent::LocalSandbox::new(
sandbox_dir.path().to_path_buf(),
));
let outcome = handler
.execute(&node, &context, &graph, tmp.path(), &services)

27
lib/crates/fabro-workflows/src/handler/command.rs
View file

@ -163,25 +163,12 @@ impl Handler for CommandHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::AttrValue;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
use crate::outcome::StageStatus;
use std::time::Duration;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]
@ -711,15 +698,9 @@ mod tests {
}
fn make_spy_services(sandbox: std::sync::Arc<SpySandbox>) -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox,
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
let mut services = EngineServices::test_default();
services.sandbox = sandbox;
services
}
#[tokio::test]

16
lib/crates/fabro-workflows/src/handler/conditional.rs
View file

@ -32,22 +32,8 @@ impl Handler for ConditionalHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

16
lib/crates/fabro-workflows/src/handler/exit.rs
View file

@ -29,22 +29,8 @@ impl Handler for ExitHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

15
lib/crates/fabro-workflows/src/handler/fan_in.rs
View file

@ -284,23 +284,10 @@ async fn llm_evaluate(
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
use crate::outcome::StageStatus;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

15
lib/crates/fabro-workflows/src/handler/human.rs
View file

@ -284,25 +284,12 @@ fn answer_text(answer: &Answer) -> String {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::{AttrValue, Edge};
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
use crate::interviewer::auto_approve::AutoApproveInterviewer;
use crate::interviewer::recording::RecordingInterviewer;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
fn build_graph_with_human_gate() -> Graph {

80
lib/crates/fabro-workflows/src/handler/manager_loop.rs
View file

@ -255,27 +255,18 @@ impl Handler for SubWorkflowHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::AttrValue;
use crate::handler::exit::ExitHandler;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
let mut services = EngineServices::test_default();
let mut registry = HandlerRegistry::new(Box::new(StartHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
}
services.registry = std::sync::Arc::new(registry);
services
}
fn child_dot_succeeds() -> &'static str {
@ -399,17 +390,8 @@ mod tests {
let mut registry = HandlerRegistry::new(Box::new(ContextEchoHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let services = EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.registry = std::sync::Arc::new(registry);
let handler = SubWorkflowHandler;
let mut node = Node::new("manager");
@ -509,17 +491,8 @@ mod tests {
let mut registry = HandlerRegistry::new(Box::new(SlowHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let services = EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.registry = std::sync::Arc::new(registry);
let handler = SubWorkflowHandler;
let mut node = Node::new("manager");
@ -571,17 +544,8 @@ mod tests {
let mut registry = HandlerRegistry::new(Box::new(SlowHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let services = EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.registry = std::sync::Arc::new(registry);
let handler = SubWorkflowHandler;
let mut node = Node::new("manager");
@ -739,17 +703,8 @@ mod tests {
let mut registry = HandlerRegistry::new(Box::new(ContextEchoHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let services = EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.registry = std::sync::Arc::new(registry);
let handler = SubWorkflowHandler;
let mut node = Node::new("manager");
@ -824,17 +779,8 @@ mod tests {
let mut registry = HandlerRegistry::new(Box::new(PreambleEchoHandler));
registry.register("start", Box::new(StartHandler));
registry.register("exit", Box::new(ExitHandler));
let services = EngineServices {
registry: std::sync::Arc::new(registry),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
};
let mut services = EngineServices::test_default();
services.registry = std::sync::Arc::new(registry);
let handler = SubWorkflowHandler;
let mut node = Node::new("manager");

16
lib/crates/fabro-workflows/src/handler/mod.rs
View file

@ -52,6 +52,22 @@ impl EngineServices {
pub fn set_git_state(&self, state: Option<Arc<GitState>>) {
*self.git_state.write().unwrap() = state;
}
/// Test-only default: empty registry, no hooks, local sandbox at cwd.
#[cfg(test)]
pub fn test_default() -> Self {
Self {
registry: Arc::new(HandlerRegistry::new(Box::new(start::StartHandler))),
emitter: Arc::new(EventEmitter::new()),
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: HashMap::new(),
dry_run: false,
}
}
}
/// The handler interface for node execution.

16
lib/crates/fabro-workflows/src/handler/parallel.rs
View file

@ -728,24 +728,10 @@ fn find_join_node(results: &[BranchResult], graph: &Graph) -> Option<String> {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::{AttrValue, Edge};
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
let registry = HandlerRegistry::new(Box::new(StartHandler));
EngineServices {
registry: Arc::new(registry),
emitter: Arc::new(EventEmitter::new()),
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

15
lib/crates/fabro-workflows/src/handler/prompt.rs
View file

@ -147,25 +147,12 @@ impl Handler for PromptHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::graph::AttrValue;
use crate::handler::start::StartHandler;
use crate::handler::HandlerRegistry;
use std::sync::Arc;
use tempfile::TempDir;
fn make_services() -> EngineServices {
EngineServices {
registry: Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: Arc::new(EventEmitter::new()),
sandbox: Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

15
lib/crates/fabro-workflows/src/handler/start.rs
View file

@ -29,21 +29,8 @@ impl Handler for StartHandler {
#[cfg(test)]
mod tests {
use super::*;
use crate::event::EventEmitter;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(StartHandler))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]

17
lib/crates/fabro-workflows/src/handler/wait.rs
View file

@ -42,23 +42,8 @@ mod tests {
use std::time::Duration;
use super::*;
use crate::event::EventEmitter;
use crate::handler::HandlerRegistry;
fn make_services() -> EngineServices {
EngineServices {
registry: std::sync::Arc::new(HandlerRegistry::new(Box::new(
crate::handler::start::StartHandler,
))),
emitter: std::sync::Arc::new(EventEmitter::new()),
sandbox: std::sync::Arc::new(fabro_agent::LocalSandbox::new(
std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")),
)),
git_state: std::sync::RwLock::new(None),
hook_runner: None,
env: std::collections::HashMap::new(),
dry_run: false,
}
EngineServices::test_default()
}
#[tokio::test]