From a496c243f4112fe99b0043bd02c0b95eecad04db Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 1 Apr 2026 00:34:20 -0400 Subject: [PATCH] Remove auto dry-run when LLM providers are missing Previously, workflows silently fell back to dry-run mode when no LLM providers were configured or client init failed. This caused command-only workflows to skip execution entirely. Now missing LLM providers produce a hard error when the graph has LLM nodes, and are ignored when it doesn't. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/tests/it/cmd/run.rs | 17 ++----- .../fabro-workflow/src/pipeline/initialize.rs | 46 +++++++++---------- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index 60fb64374..967623dcb 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -517,17 +517,6 @@ fn json_run_implies_auto_approve_for_human_gates() { "working_directory": "[TEMP_DIR]" } }, - { - "id": "[EVENT_ID]", - "ts": "[TIMESTAMP]", - "run_id": "[ULID]", - "event": "run.notice", - "properties": { - "level": "warn", - "code": "dry_run_no_llm", - "message": "No LLM providers configured. Running in dry-run mode." - } - }, { "id": "[EVENT_ID]", "ts": "[TIMESTAMP]", @@ -568,7 +557,7 @@ fn json_run_implies_auto_approve_for_human_gates() { "preferred_label": null, "suggested_next_ids": [], "usage": null, - "notes": "[Simulated] start", + "notes": null, "files_touched": [] } }, @@ -630,7 +619,7 @@ fn json_run_implies_auto_approve_for_human_gates() { "ship" ], "usage": null, - "notes": "[Simulated] approve", + "notes": null, "files_touched": [] } }, @@ -695,7 +684,7 @@ fn json_run_implies_auto_approve_for_human_gates() { "preferred_label": null, "suggested_next_ids": [], "usage": null, - "notes": "[Simulated] Command skipped: echo shipped", + "notes": "Script completed: echo shipped", "files_touched": [] } }, diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index 8a1bcb73f..ffe293f45 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use std::time::Instant; use fabro_agent::Sandbox; +use fabro_graphviz::graph; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_llm::client::Client; use fabro_sandbox::{ @@ -259,23 +260,27 @@ async fn build_registry( spec: &LlmSpec, interviewer: Arc, sandbox_env: &HashMap, - emitter: &EventEmitter, + graph: &graph::Graph, ) -> Result<(Arc, Option, bool), FabroError> { - let build_dry_run = || Arc::new(default_registry(Arc::clone(&interviewer), || None)); + let build_no_backend = || Arc::new(default_registry(Arc::clone(&interviewer), || None)); if spec.dry_run { - return Ok((build_dry_run(), None, true)); + return Ok((build_no_backend(), None, true)); } + let graph_needs_llm = graph + .nodes + .values() + .any(|n| graph::is_llm_handler_type(n.handler_type())); + match Client::from_env().await { Ok(client) if client.provider_names().is_empty() => { - emit_run_notice( - emitter, - RunNoticeLevel::Warn, - "dry_run_no_llm", - "No LLM providers configured. Running in dry-run mode.", - ); - Ok((build_dry_run(), None, true)) + if graph_needs_llm { + return Err(FabroError::Precondition( + "No LLM providers configured. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate.".to_string(), + )); + } + Ok((build_no_backend(), None, false)) } Ok(client) => { let env = sandbox_env.clone(); @@ -293,13 +298,12 @@ async fn build_registry( Ok((registry, Some(client), false)) } Err(e) => { - emit_run_notice( - emitter, - RunNoticeLevel::Warn, - "dry_run_llm_init_failed", - format!("Failed to initialize LLM client: {e}. Running in dry-run mode."), - ); - Ok((build_dry_run(), None, true)) + if graph_needs_llm { + return Err(FabroError::Precondition(format!( + "Failed to initialize LLM client: {e}. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate.", + ))); + } + Ok((build_no_backend(), None, false)) } } } @@ -527,13 +531,7 @@ pub async fn initialize( // A caller-supplied registry owns execution behavior for its handlers. (registry, None, options.dry_run) } else { - build_registry( - &options.llm, - Arc::clone(&options.interviewer), - &env, - &options.emitter, - ) - .await? + build_registry(&options.llm, Arc::clone(&options.interviewer), &env, &graph).await? }; if effective_dry_run { options.dry_run = true;