From 3a28bc02499b67f565a5be0d734f515509aea130 Mon Sep 17 00:00:00 2001 From: Fabro Date: Thu, 23 Jul 2026 18:51:58 +0000 Subject: [PATCH] fabro(01KY7WQ92JWT90307EBQY6P2HV): simplify_sol (failed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fabro-Run: 01KY7WQ92JWT90307EBQY6P2HV Fabro-Completed: 7 ⚒️ Generated with [Fabro](https://fabro.sh) --- .../src/rules/inert_attribute.rs | 24 +++++++++++++++++++ .../fabro-workflow/src/handler/command.rs | 9 +++---- .../src/handler/structured_output.rs | 18 +++++--------- .../fabro-workflow/tests/it/integration.rs | 20 ++++------------ 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/lib/crates/fabro-validate/src/rules/inert_attribute.rs b/lib/crates/fabro-validate/src/rules/inert_attribute.rs index 7d303dcdf..6c547d26f 100644 --- a/lib/crates/fabro-validate/src/rules/inert_attribute.rs +++ b/lib/crates/fabro-validate/src/rules/inert_attribute.rs @@ -20,6 +20,7 @@ const HANDLER_SPECIFIC_ATTRS: &[(&str, &[&str])] = &[ ("duration", &["wait"]), ("join_policy", &["parallel"]), ("max_parallel", &["parallel"]), + ("output_retries", &["agent", "prompt"]), ("output_schema", &["agent", "prompt", "command"]), ("prompt", &["agent", "prompt", "parallel.fan_in"]), ]; @@ -151,6 +152,21 @@ mod tests { assert_eq!(d.len(), 2); } + #[test] + fn warns_on_output_retries_on_command_node() { + let mut g = minimal_graph(); + g.nodes.insert( + "run".to_string(), + node_with_attr("run", "parallelogram", "output_retries", "2"), + ); + + let d = Rule.apply(&g); + + assert_eq!(d.len(), 1); + assert!(d[0].message.contains("'output_retries'")); + assert!(d[0].message.contains("agent, prompt")); + } + #[test] fn accepts_attrs_on_their_own_handler_types() { let mut g = minimal_graph(); @@ -170,6 +186,10 @@ mod tests { "work".to_string(), node_with_attr("work", "box", "prompt", "do things"), ); + g.nodes.insert( + "review".to_string(), + node_with_attr("review", "box", "output_retries", "2"), + ); g.nodes.insert( "fork".to_string(), node_with_attr("fork", "component", "join_policy", "wait_all"), @@ -178,6 +198,10 @@ mod tests { "spec".to_string(), node_with_attr("spec", "tab", "output_schema", "routing"), ); + g.nodes.insert( + "prompt".to_string(), + node_with_attr("prompt", "tab", "output_retries", "2"), + ); assert!(Rule.apply(&g).is_empty()); } diff --git a/lib/crates/fabro-workflow/src/handler/command.rs b/lib/crates/fabro-workflow/src/handler/command.rs index 0b744d988..54fe0ff20 100644 --- a/lib/crates/fabro-workflow/src/handler/command.rs +++ b/lib/crates/fabro-workflow/src/handler/command.rs @@ -220,10 +220,11 @@ fn schema_validation_failure_reason( error: &StructuredOutputError, output_text: &str, ) -> String { - let mut reason = format!( - "Script output failed output_schema validation: {script}\n{}", - error.bulleted_messages() - ); + let mut reason = format!("Script output failed output_schema validation: {script}"); + for message in error.messages() { + reason.push_str("\n- "); + reason.push_str(message); + } append_output_tail(&mut reason, output_text); reason } diff --git a/lib/crates/fabro-workflow/src/handler/structured_output.rs b/lib/crates/fabro-workflow/src/handler/structured_output.rs index d28a82636..15669462a 100644 --- a/lib/crates/fabro-workflow/src/handler/structured_output.rs +++ b/lib/crates/fabro-workflow/src/handler/structured_output.rs @@ -72,22 +72,11 @@ impl StructuredOutputError { self.kind } - #[cfg(test)] #[must_use] pub(crate) fn messages(&self) -> &[String] { &self.messages } - /// One `- {message}` bullet line per validator error, joined by newlines. - #[must_use] - pub(crate) fn bulleted_messages(&self) -> String { - self.messages - .iter() - .map(|message| format!("- {message}")) - .collect::>() - .join("\n") - } - #[must_use] pub(crate) fn allows_routing_fallback(&self) -> bool { matches!( @@ -108,7 +97,12 @@ impl StructuredOutputError { "Return a single JSON object that satisfies the configured JSON Schema.".to_string() } }; - let errors = self.bulleted_messages(); + let errors = self + .messages + .iter() + .map(|message| format!("- {message}")) + .collect::>() + .join("\n"); format!( "Your previous response did not satisfy the node's output_schema.\n\n\ Validation errors:\n{errors}\n\n\ diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index dedc58df9..5d95ab4e9 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -33,7 +33,7 @@ use fabro_interview::{ use fabro_model::catalog::{LlmCatalogSettings, ProviderCatalogSettings}; use fabro_model::{Catalog, ProviderId}; use fabro_store::{ArtifactKey, ArtifactStore, Database}; -use fabro_types::{RunEvent, RunId, StageId, WorkflowSettings, parse_blob_ref}; +use fabro_types::{EventBody, RunEvent, RunId, StageId, WorkflowSettings, parse_blob_ref}; use fabro_validate::{Severity, validate, validate_or_raise}; use fabro_workflow::context::Context; use fabro_workflow::error::{Error, FailureSignatureExt}; @@ -1964,20 +1964,8 @@ async fn command_schema_validation_failure_does_not_consume_retries() { registry.register("exit", Box::new(ExitHandler)); registry.register("command", Box::new(CommandHandler)); let engine = WorkflowRunner::new(registry, Arc::new(emitter), local_env()); - let run_options = RunOptions { - settings: WorkflowSettings::default(), - run_dir: dir.path().to_path_buf(), - cancel_token: CancellationToken::new(), - run_id: test_run_id("command-schema-no-retry"), - labels: std::collections::HashMap::new(), - workflow_slug: None, - github_app: None, - base_branch: None, - display_base_sha: None, - pre_run_git: None, - fork_source_ref: None, - git: None, - }; + let mut run_options = make_run_options(dir.path()); + run_options.run_id = test_run_id("command-schema-no-retry"); let (outcome, state) = engine .run_with_state(&graph, &run_options) @@ -2014,7 +2002,7 @@ async fn command_schema_validation_failure_does_not_consume_retries() { .lock() .unwrap() .iter() - .filter(|event| event.event_name() == "command.started") + .filter(|event| matches!(event.body, EventBody::CommandStarted(_))) .count(); assert_eq!(command_starts, 1, "command should execute exactly once"); }