mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-12 23:02:41 +00:00
fix(workflow): constrain last-file routing fallback
This commit is contained in:
parent
e75c3dcd37
commit
02437fb18a
4 changed files with 195 additions and 45 deletions
|
|
@ -95,6 +95,8 @@ Agent nodes can provide routing directives through fallback files. Fabro checks
|
|||
|
||||
This fallback chain applies to normal routing extraction and to `output_schema="routing"`. For validated routing, Fabro only advances to the next source when the current source has no JSON object or no object with recognized routing fields. If the current source contains malformed routing JSON or valid JSON with wrong routing field types, validation fails and Fabro starts the repair loop instead.
|
||||
|
||||
The last-file fallback only reads `.json` and `.md` files (case-insensitive), and the routing JSON must be the final standalone object in the file, followed only by whitespace. Fabro ignores other file types and routing JSON followed by any other content. These restrictions do not apply to the dedicated `status.json` fallback.
|
||||
|
||||
Prompt nodes do not use file fallbacks; they validate or extract routing directives from the response text only.
|
||||
|
||||
If no source provides routing directives, the transition falls through to condition matching, unconditional edges, or weight-based tiebreaking as described in [Transitions](/workflows/transitions).
|
||||
|
|
|
|||
|
|
@ -235,7 +235,7 @@ audit [
|
|||
- On validation failure, Fabro sends validation feedback to the same active context before failing: prompt nodes keep the prior assistant response in the message list, and API-backed agent nodes repair in the same live session.
|
||||
- `output_retries` defaults to `2` and controls only these corrective structured-output turns. Negative values are treated as `0`. It is not the same as `max_retries` and does not consume workflow retry attempts.
|
||||
- Custom schema output is stored in context at `output.{node_id}`. Routing schema output updates routing fields and any `context_updates`.
|
||||
- Agent routing fallbacks still apply to `output_schema="routing"`: response text first, then `status.json`, then the last file touched by the agent. Custom schemas and prompt nodes validate response text only.
|
||||
- Agent routing fallbacks still apply to `output_schema="routing"`: response text first, then `status.json`, then the last file touched by the agent. The last-file fallback only accepts `.json` and `.md` files (case-insensitive) whose final standalone JSON object contains the routing directive; only whitespace may follow it. Custom schemas and prompt nodes validate response text only.
|
||||
- `backend="acp"` with `output_schema` is unsupported in this release.
|
||||
|
||||
### Command nodes
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ use crate::event::{Emitter, Event, StageScope};
|
|||
use crate::interview_runtime::WorkflowAgentQuestionRuntime;
|
||||
use crate::outcome::{BilledModelUsage, Outcome, OutcomeExt};
|
||||
|
||||
const LAST_FILE_ROUTING_EXTENSIONS: &[&str] = &["json", "md"];
|
||||
|
||||
/// Result from a `CodergenBackend` invocation.
|
||||
#[allow(
|
||||
clippy::large_enum_variant,
|
||||
|
|
@ -169,8 +171,8 @@ pub(crate) async fn validate_agent_output_sources(
|
|||
}
|
||||
|
||||
if let Some(path) = last_file_touched {
|
||||
if let Some(contents) = read_sandbox_file(sandbox, path).await {
|
||||
return structured_output::validate_response_text(schema, &contents);
|
||||
if let Some(routing_json) = read_last_file_routing_json(sandbox, path).await {
|
||||
return structured_output::validate_response_text(schema, &routing_json);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -181,6 +183,19 @@ async fn read_sandbox_file(sandbox: &Arc<dyn Sandbox>, path: &str) -> Option<Str
|
|||
sandbox.read_file_text(path).await.ok()
|
||||
}
|
||||
|
||||
async fn read_last_file_routing_json(sandbox: &Arc<dyn Sandbox>, path: &str) -> Option<String> {
|
||||
let extension = Path::new(path).extension()?.to_str()?;
|
||||
if !LAST_FILE_ROUTING_EXTENSIONS
|
||||
.iter()
|
||||
.any(|allowed| extension.eq_ignore_ascii_case(allowed))
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let contents = read_sandbox_file(sandbox, path).await?;
|
||||
structured_output::terminal_json_object(&contents).map(str::to_owned)
|
||||
}
|
||||
|
||||
/// Truncate a string to at most `max_chars` characters (char-boundary safe).
|
||||
pub(crate) fn truncate(s: &str, max_chars: usize) -> &str {
|
||||
if s.len() <= max_chars {
|
||||
|
|
@ -382,7 +397,7 @@ impl Handler for AgentHandler {
|
|||
} else {
|
||||
// 7b. Parse routing directives from response text, falling back to
|
||||
// status.json written by the agent into the sandbox CWD, then to
|
||||
// the last file the agent wrote.
|
||||
// a terminal JSON object in an eligible last-written file.
|
||||
let found_in_response = extract_status_fields(&response_text, &mut outcome);
|
||||
if !found_in_response {
|
||||
let mut found_in_status_json = false;
|
||||
|
|
@ -393,9 +408,10 @@ impl Handler for AgentHandler {
|
|||
}
|
||||
if !found_in_status_json {
|
||||
if let Some(ref path) = last_file_touched {
|
||||
if let Some(contents) = read_sandbox_file(&services.run.sandbox, path).await
|
||||
if let Some(routing_json) =
|
||||
read_last_file_routing_json(&services.run.sandbox, path).await
|
||||
{
|
||||
extract_status_fields(&contents, &mut outcome);
|
||||
extract_status_fields(&routing_json, &mut outcome);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -506,6 +522,68 @@ mod tests {
|
|||
context
|
||||
}
|
||||
|
||||
struct LastFileBackend {
|
||||
path: String,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl CodergenBackend for LastFileBackend {
|
||||
async fn run(&self, _request: CodergenRunRequest<'_>) -> Result<CodergenResult, Error> {
|
||||
Ok(CodergenResult::Text {
|
||||
text: "Done writing results.".to_string(),
|
||||
usage: None,
|
||||
files_touched: vec![self.path.clone()],
|
||||
last_file_touched: Some(self.path.clone()),
|
||||
timing: StageTiming::default(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async fn execute_with_last_file(path: &str, contents: &str) -> Outcome {
|
||||
let sandbox_dir = TempDir::new().unwrap();
|
||||
std::fs::write(sandbox_dir.path().join(path), contents).unwrap();
|
||||
|
||||
let handler = AgentHandler::new(Some(Box::new(LastFileBackend {
|
||||
path: path.to_string(),
|
||||
})));
|
||||
let node = Node::new("step");
|
||||
let context = test_context();
|
||||
let graph = Graph::new("test");
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
let mut services = EngineServices::test_default();
|
||||
services.run =
|
||||
services
|
||||
.run
|
||||
.with_sandbox(std::sync::Arc::new(fabro_agent::LocalSandbox::new(
|
||||
sandbox_dir.path().to_path_buf(),
|
||||
)));
|
||||
|
||||
handler
|
||||
.execute(&node, &context, &graph, tmp.path(), &services)
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
async fn validate_routing_with_last_file(
|
||||
path: &str,
|
||||
contents: &str,
|
||||
) -> Result<ValidatedStructuredOutput, StructuredOutputError> {
|
||||
let sandbox_dir = TempDir::new().unwrap();
|
||||
std::fs::write(sandbox_dir.path().join(path), contents).unwrap();
|
||||
let sandbox: Arc<dyn Sandbox> = Arc::new(fabro_agent::LocalSandbox::new(
|
||||
sandbox_dir.path().to_path_buf(),
|
||||
));
|
||||
|
||||
validate_agent_output_sources(
|
||||
&OutputSchemaKind::Routing,
|
||||
"Done writing results.",
|
||||
&sandbox,
|
||||
Some(path),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_simulate() {
|
||||
let handler = AgentHandler::new(None);
|
||||
|
|
@ -731,49 +809,13 @@ mod tests {
|
|||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_extracts_status_from_last_file_touched() {
|
||||
struct LastFileBackend;
|
||||
|
||||
#[async_trait]
|
||||
impl CodergenBackend for LastFileBackend {
|
||||
async fn run(&self, _request: CodergenRunRequest<'_>) -> Result<CodergenResult, Error> {
|
||||
Ok(CodergenResult::Text {
|
||||
text: "Done writing results.".to_string(),
|
||||
usage: None,
|
||||
files_touched: vec!["results.md".to_string()],
|
||||
last_file_touched: Some("results.md".to_string()),
|
||||
timing: StageTiming::default(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
let sandbox_dir = TempDir::new().unwrap();
|
||||
// Write status fields into the file the agent "touched" — no status.json
|
||||
std::fs::write(
|
||||
sandbox_dir.path().join("results.md"),
|
||||
let outcome = execute_with_last_file(
|
||||
"results.md",
|
||||
r#"# Results
|
||||
{"context_updates": {"verified": "true"}}
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let handler = AgentHandler::new(Some(Box::new(LastFileBackend)));
|
||||
let node = Node::new("step");
|
||||
let context = test_context();
|
||||
let graph = Graph::new("test");
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
let mut services = EngineServices::test_default();
|
||||
services.run =
|
||||
services
|
||||
.run
|
||||
.with_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)
|
||||
.await
|
||||
.unwrap();
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.status, crate::outcome::StageOutcome::Succeeded);
|
||||
assert_eq!(
|
||||
|
|
@ -782,6 +824,33 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_ignores_nonterminal_status_in_last_markdown_file() {
|
||||
let outcome = execute_with_last_file(
|
||||
"results.md",
|
||||
r#"{"outcome":"failed","failure_reason":"tests failed"}
|
||||
|
||||
All checks passed.
|
||||
"#,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.status, crate::outcome::StageOutcome::Succeeded);
|
||||
assert!(outcome.failure.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_ignores_terminal_status_in_disallowed_last_file() {
|
||||
let outcome = execute_with_last_file(
|
||||
"command.rs",
|
||||
r#"{"outcome":"failed","failure_reason":"tests failed"}"#,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.status, crate::outcome::StageOutcome::Succeeded);
|
||||
assert!(outcome.failure.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_output_schema_routing_uses_status_json_fallback_when_response_has_no_json()
|
||||
{
|
||||
|
|
@ -819,6 +888,51 @@ mod tests {
|
|||
assert_eq!(outcome.preferred_label.as_deref(), Some("review"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validated_routing_accepts_terminal_status_in_json_file_case_insensitively() {
|
||||
let validated = validate_routing_with_last_file(
|
||||
"results.JSON",
|
||||
"# Results\n\n{\"preferred_next_label\":\"review\"}\n",
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
validated.value,
|
||||
serde_json::json!({"preferred_next_label": "review"}),
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validated_routing_ignores_nonterminal_status_in_last_markdown_file() {
|
||||
let error = validate_routing_with_last_file(
|
||||
"results.md",
|
||||
"{\"outcome\":\"failed\",\"failure_reason\":\"tests failed\"}\nAll checks passed.",
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(
|
||||
error.kind(),
|
||||
structured_output::StructuredOutputErrorKind::NoJsonObject,
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validated_routing_ignores_terminal_status_in_disallowed_last_file() {
|
||||
let error = validate_routing_with_last_file(
|
||||
"command.rs",
|
||||
r#"{"outcome":"failed","failure_reason":"tests failed"}"#,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(
|
||||
error.kind(),
|
||||
structured_output::StructuredOutputErrorKind::NoJsonObject,
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codergen_handler_output_schema_routing_rejects_malformed_response_before_status_json_fallback()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -264,6 +264,15 @@ fn find_json_objects(text: &str) -> Vec<&str> {
|
|||
results
|
||||
}
|
||||
|
||||
/// Return the outermost balanced JSON object that ends the text, ignoring
|
||||
/// trailing whitespace.
|
||||
pub(crate) fn terminal_json_object(text: &str) -> Option<&str> {
|
||||
let trimmed = text.trim_end();
|
||||
find_json_objects(trimmed)
|
||||
.into_iter()
|
||||
.find(|candidate| trimmed.ends_with(candidate))
|
||||
}
|
||||
|
||||
pub(crate) fn extract_status_fields(text: &str, outcome: &mut Outcome) -> bool {
|
||||
let candidates = find_json_objects(text);
|
||||
|
||||
|
|
@ -493,6 +502,31 @@ mod tests {
|
|||
assert!(error.messages()[0].contains("recognized routing field"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_json_object_accepts_final_object_after_prose() {
|
||||
let object =
|
||||
terminal_json_object("# Results\n\n{\"context_updates\":{\"verified\":true}}\n\n");
|
||||
|
||||
assert_eq!(object, Some(r#"{"context_updates":{"verified":true}}"#),);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_json_object_returns_outermost_nested_object() {
|
||||
let object = terminal_json_object(r#"Results: {"context_updates":{"verified":true}}"#);
|
||||
|
||||
assert_eq!(object, Some(r#"{"context_updates":{"verified":true}}"#),);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_json_object_rejects_object_followed_by_content() {
|
||||
assert_eq!(
|
||||
terminal_json_object(
|
||||
"{\"outcome\":\"failed\",\"failure_reason\":\"tests failed\"}\nMore details",
|
||||
),
|
||||
None,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn routing_json_with_wrong_field_type_is_invalid() {
|
||||
let error =
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue