diff --git a/Cargo.lock b/Cargo.lock index cb7206de0..de3ec254c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3049,7 +3049,6 @@ dependencies = [ "chrono", "dirs", "fabro-auth", - "fabro-checkpoint", "fabro-client", "fabro-config", "fabro-dump", @@ -3085,13 +3084,11 @@ dependencies = [ "pebble-agent", "pebble-coding-agent", "rand 0.9.4", - "regex", "sandbox-driver", "scopeguard", "serde", "serde_json", "sha2 0.10.9", - "strum 0.28.0", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/lib/components/fabro-llm/src/error.rs b/lib/components/fabro-llm/src/error.rs deleted file mode 100644 index 3b0104da6..000000000 --- a/lib/components/fabro-llm/src/error.rs +++ /dev/null @@ -1,61 +0,0 @@ -//! The one failure-classification rule that is Fabro's own. -//! -//! Retry, auth, cancellation, and failover questions are answered by the -//! lithos `Error` and `ErrorData` themselves. What stays here is the loop and -//! restart detector's signature format, which names Fabro's own categories. - -use lithos_llm::catalog::ProviderId; -use lithos_llm::types::{ErrorData, ErrorKind}; - -/// A stable `category|provider|detail` string for loop and restart detection. -/// -/// The category is `api_canceled` for a cancelled call, `api_transient` for a -/// failure the provider may be asked to repeat, and `api_deterministic` for -/// everything else; the detail is the error kind's stored spelling. -#[must_use] -pub fn failure_signature_hint(error: &ErrorData) -> String { - let provider = error.provider().map_or("unknown", ProviderId::as_str); - let category = if error.is_cancelled() { - "api_canceled" - } else if error.is_retryable() { - "api_transient" - } else { - "api_deterministic" - }; - let kind: ErrorKind = error.kind(); - format!("{category}|{provider}|{}", kind.as_str()) -} - -#[cfg(test)] -mod tests { - use lithos_llm::types::{Error, RetryClassification}; - - use super::*; - - fn error(kind: ErrorKind) -> ErrorData { - Error::new(kind, "boom") - .with_provider(ProviderId::new("openai")) - .data() - } - - #[test] - fn signatures_name_category_provider_and_kind() { - assert_eq!( - failure_signature_hint(&error(ErrorKind::InvalidRequest)), - "api_deterministic|openai|invalid_request" - ); - assert_eq!( - failure_signature_hint( - &Error::new(ErrorKind::RateLimit, "boom") - .with_provider(ProviderId::new("openai")) - .with_retry(RetryClassification::Safe) - .data() - ), - "api_transient|openai|rate_limit" - ); - assert_eq!( - failure_signature_hint(&error(ErrorKind::Cancelled)), - "api_canceled|openai|cancelled" - ); - } -} diff --git a/lib/components/fabro-llm/src/lib.rs b/lib/components/fabro-llm/src/lib.rs index 27199a2ed..7d6d6c0fd 100644 --- a/lib/components/fabro-llm/src/lib.rs +++ b/lib/components/fabro-llm/src/lib.rs @@ -12,8 +12,7 @@ //! - model and provider probes ([`probe`]), and the API views of the catalog //! ([`api`]); //! - the `fabro exec` gateway adapter that speaks to a Fabro server -//! ([`gateway`]); -//! - the failure signature loop detection reads ([`error`]). +//! ([`gateway`]). //! //! Local-file inlining, structured output, readable-reasoning normalization, //! and the retry, auth, and failover predicates are lithos-llm's own. @@ -21,7 +20,6 @@ pub mod api; pub mod catalog; pub mod client; -pub mod error; pub mod gateway; pub mod probe; pub mod selection; @@ -33,7 +31,6 @@ pub use client::{ ClientOptions, FabroClient, LlmSetupError, RetryListener, RetryNotice, build_client, build_offline_client, configured_providers, }; -pub use error::failure_signature_hint; pub use lithos_llm::client::{Client, ClientBuild}; pub use lithos_llm::middleware::{CallContext, CancellationToken, RetryPolicy, RetryStage}; pub use lithos_llm::resolver::ModelSelectionError as RouteSelectionError; diff --git a/lib/components/fabro-workflow/Cargo.toml b/lib/components/fabro-workflow/Cargo.toml index e3f175212..3e78e1523 100644 --- a/lib/components/fabro-workflow/Cargo.toml +++ b/lib/components/fabro-workflow/Cargo.toml @@ -34,7 +34,6 @@ fabro-template = { path = "../../foundation/fabro-template" } fabro-tool = { path = "../fabro-tool" } fabro-util = { path = "../../foundation/fabro-util" } fabro-redact.workspace = true -fabro-checkpoint = { path = "../fabro-checkpoint" } fabro-llm = { path = "../fabro-llm" } fabro-store = { path = "../fabro-store" } fabro-static.workspace = true @@ -42,7 +41,6 @@ fabro-types = { path = "../../foundation/fabro-types" } lithos-llm = { workspace = true, features = ["runtime"] } fabro-http.workspace = true thiserror.workspace = true -strum.workspace = true serde.workspace = true serde_json.workspace = true jsonschema.workspace = true @@ -56,7 +54,6 @@ async-trait.workspace = true futures.workspace = true chrono = { workspace = true, features = ["serde"] } dirs = "6" -regex.workspace = true scopeguard = "1" md5.workspace = true hex.workspace = true diff --git a/lib/components/fabro-workflow/src/error.rs b/lib/components/fabro-workflow/src/error.rs index 657c94b23..eb7b0ed61 100644 --- a/lib/components/fabro-workflow/src/error.rs +++ b/lib/components/fabro-workflow/src/error.rs @@ -1,102 +1,15 @@ use std::fmt; -use std::sync::{Arc, LazyLock}; +use std::sync::Arc; use fabro_graphviz::Error as GraphvizError; -use fabro_llm::{ErrorData, ErrorKind, ModelSelectionError, failure_signature_hint}; use fabro_template::TemplateError; use fabro_types::diagnostic::Diagnostic; -pub use fabro_types::failure_signature::FailureSignature; -pub use fabro_types::outcome::FailureCategory; -use fabro_types::settings::{AmbiguousModelRef, ResolveError}; -use fabro_types::{ExecOutputTail, FailureReason, RunFailure}; -use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes}; -use regex::Regex; +use fabro_types::settings::ResolveError; +use fabro_util::error::{SharedError, collect_chain, render_with_causes}; use thiserror::Error as ThisError; -use crate::outcome::{FailureDetail, Outcome, StageOutcome}; - -/// Classify an LLM error into a `FailureCategory` based on its structure. -#[must_use] -pub fn classify_sdk_error(err: &ErrorData) -> FailureCategory { - match err.kind() { - ErrorKind::RateLimit - | ErrorKind::Server - | ErrorKind::Network - | ErrorKind::Timeout - | ErrorKind::StreamDecode => FailureCategory::TransientInfra, - ErrorKind::ContextLength | ErrorKind::QuotaExceeded => FailureCategory::BudgetExhausted, - ErrorKind::Cancelled => FailureCategory::Canceled, - // Configuration, model selection, auth, access, not-found, invalid - // request, content filter, provider, decode, resource limit, and - // middleware failures are deterministic. `ErrorKind` is - // non-exhaustive: a category added by a newer lithos never enables - // automatic retry either. - _ => FailureCategory::Deterministic, - } -} - -const TRANSIENT_INFRA_HINTS: &[&str] = &[ - "timeout", - "timed out", - "rate limit", - "rate limited", - "connection refused", - "connection reset", - "500", - "502", - "503", - "504", - "context deadline exceeded", - "could not resolve host", - "could not resolve hostname", - "temporary failure", - "network is unreachable", - "broken pipe", - "tls handshake timeout", - "i/o timeout", - "no route to host", - "temporarily unavailable", - "try again", - "too many requests", - "service unavailable", - "gateway timeout", - "econnrefused", - "econnreset", - "dial tcp", - "transport is closing", - "stream disconnected", - "stream closed before", - "index.crates.io", - "download of config.json failed", - "toolchain_or_dependency_registry_unavailable", - "toolchain dependency resolution blocked by network", - "toolchain_workspace_io", - "cross-device link", - "invalid cross-device link", - "os error 18", - "state change in progress", - "sandbox stop still in progress", -]; - -const BUDGET_EXHAUSTED_HINTS: &[&str] = &[ - "turn limit", - "token limit", - "context length", - "budget", - "quota exceeded", - "max_tokens", - "max tokens", - "context window exceeded", - "budget exhausted", - "token limit exceeded", -]; - -const STRUCTURAL_HINTS: &[&str] = &[ - "write_scope_violation", - "write scope violation", - "scope violation", -]; - +/// A template error shared across clones of the workflow error that carries +/// it, so the miette diagnostic and the source chain survive cloning. #[derive(Debug, Clone)] pub struct SharedTemplateError(Arc); @@ -114,153 +27,38 @@ impl SharedTemplateError { impl fmt::Display for SharedTemplateError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, formatter) + fmt::Display::fmt(&*self.0, formatter) } } impl std::error::Error for SharedTemplateError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - self.0.source() + std::error::Error::source(&*self.0) } } impl miette::Diagnostic for SharedTemplateError { fn code<'a>(&'a self) -> Option> { - miette::Diagnostic::code(self.inner()) + miette::Diagnostic::code(&*self.0) } fn help<'a>(&'a self) -> Option> { - miette::Diagnostic::help(self.inner()) + miette::Diagnostic::help(&*self.0) } fn source_code(&self) -> Option<&dyn miette::SourceCode> { - miette::Diagnostic::source_code(self.inner()) + miette::Diagnostic::source_code(&*self.0) } fn labels(&self) -> Option + '_>> { - miette::Diagnostic::labels(self.inner()) + miette::Diagnostic::labels(&*self.0) } fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - miette::Diagnostic::diagnostic_source(self.inner()) + miette::Diagnostic::diagnostic_source(&*self.0) } } -/// Matches git SHAs and other long hex blobs. -static HEX_RE: LazyLock = - LazyLock::new(|| Regex::new(r"\b[0-9a-f]{7,64}\b").expect("hardcoded regex should compile")); - -/// Classify a failure reason string using heuristics. -/// -/// This is the fallback when structured error information is not available -/// (e.g. for `Handler(String)` or `Engine(String)` errors). -#[must_use] -pub fn classify_failure_reason(reason: &str) -> FailureCategory { - // Mask commit SHAs first. They are hex, so one contains "500" or "503" - // often enough to matter, which would read as a transient infra hint. The - // bare status codes those hints look for are too short to be masked. - let lowered = reason.to_lowercase(); - let lower = HEX_RE.replace_all(&lowered, ""); - - if lower.contains("interrupt") - || (lower.contains("cancel") - && !lower.contains("cancelling due to test failure") - && !lower.contains("canceling due to test failure")) - { - return FailureCategory::Canceled; - } - - if TRANSIENT_INFRA_HINTS - .iter() - .any(|hint| lower.contains(hint)) - { - return FailureCategory::TransientInfra; - } - - if BUDGET_EXHAUSTED_HINTS - .iter() - .any(|hint| lower.contains(hint)) - { - return FailureCategory::BudgetExhausted; - } - - if STRUCTURAL_HINTS.iter().any(|hint| lower.contains(hint)) { - return FailureCategory::Structural; - } - - FailureCategory::Deterministic -} - -/// Normalize a failure reason for stable signature grouping. -/// -/// Replaces variable data (hex strings, digits) with placeholders so that -/// semantically identical errors produce the same signature regardless of -/// line numbers, commit hashes, or timestamps. -pub fn normalize_failure_reason(reason: &str) -> String { - static DIGITS_RE: LazyLock = - LazyLock::new(|| Regex::new(r"\b\d+\b").expect("hardcoded regex should compile")); - static COMMA_SPACE_RE: LazyLock = - LazyLock::new(|| Regex::new(r",\s+").expect("hardcoded regex should compile")); - static WHITESPACE_RE: LazyLock = - LazyLock::new(|| Regex::new(r"\s+").expect("hardcoded regex should compile")); - - let s = reason.trim().to_lowercase(); - if s.is_empty() { - return String::new(); - } - let s = HEX_RE.replace_all(&s, ""); - let s = DIGITS_RE.replace_all(&s, ""); - let s = COMMA_SPACE_RE.replace_all(&s, ","); - let s = WHITESPACE_RE.replace_all(&s, " "); - let s = s.trim(); - if s.len() > 240 { - s[..s.floor_char_boundary(240)].to_string() - } else { - s.to_string() - } -} - -pub trait FailureSignatureExt { - fn new( - node_id: &str, - failure_class: FailureCategory, - signature_hint: Option<&str>, - failure_reason: Option<&str>, - ) -> Self; -} - -impl FailureSignatureExt for FailureSignature { - fn new( - node_id: &str, - failure_class: FailureCategory, - signature_hint: Option<&str>, - failure_reason: Option<&str>, - ) -> Self { - let reason = signature_hint - .map(normalize_failure_reason) - .filter(|s| !s.is_empty()) - .or_else(|| failure_reason.map(normalize_failure_reason)) - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| "unknown".to_string()); - Self(format!("{}|{}|{}", node_id.trim(), failure_class, reason)) - } -} - -/// Pipeline stage that produced an [`Error::Stage`]. -/// -/// The three stages share a failure shape — a message, an eagerly classified -/// [`FailureCategory`], an optional command output tail, and an optional -/// source — and differ only in where they run and whether a retry is possible. -#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display)] -pub enum ErrorStage { - /// A node handler failed. Retryable: the engine can re-run the node. - Handler, - /// The engine itself failed while driving the graph. Retryable. - Engine, - /// The publish stage failed. Terminal: publish runs once, after execution. - Publish, -} - #[derive(ThisError, Debug, Clone)] pub enum Error { #[error("Parse error: {0}")] @@ -280,12 +78,6 @@ pub enum Error { source: ResolveError, }, - #[error("Model selection failed: {0}")] - ModelSelection(#[from] ModelSelectionError), - - #[error("Model reference failed: {0}")] - ModelReference(#[from] AmbiguousModelRef), - #[error("{message}")] Template { message: String, @@ -293,26 +85,15 @@ pub enum Error { source: SharedTemplateError, }, - #[error("{stage} error: {message}")] - Stage { - stage: ErrorStage, - message: String, - failure_class: FailureCategory, - exec_output_tail: Option, - /// Structured context lines appended after the source chain in - /// `causes()` — e.g. one line per push attempt on a publish push - /// failure. - extra_causes: Vec, + /// Fabro's own platform work around a run failed: a store call, a + /// serialization, a spawned task, a Git command. + #[error("Engine error: {message}")] + Engine { + message: String, #[source] - source: Option, + source: Option, }, - #[error("LLM error: {0}")] - Llm(Box), - - #[error("Checkpoint error: {0}")] - Checkpoint(String), - #[error("Stylesheet error: {0}")] Stylesheet(String), @@ -325,75 +106,11 @@ pub enum Error { #[error("Run not found: {0}")] RunNotFound(String), - #[error("Unsupported operation: {0}")] - Unsupported(String), - - #[error("{0}")] - OutputSchemaValidation(String), - #[error("Pipeline cancelled")] Cancelled, } impl Error { - /// Smart constructor for Handler errors. Classifies the failure reason - /// eagerly. - /// Build a stage error, classifying the message eagerly. - fn stage( - stage: ErrorStage, - message: impl Into, - exec_output_tail: Option, - ) -> Self { - let message = message.into(); - let failure_class = classify_failure_reason(&message); - Self::Stage { - stage, - message, - failure_class, - exec_output_tail, - extra_causes: Vec::new(), - source: None, - } - } - - /// Build a stage error from a source, classifying the rendered chain so - /// hints buried in the causes still reach [`Self::failure_category`]. - fn stage_with_source( - stage: ErrorStage, - message: impl Into, - source: impl Into, - exec_output_tail: Option, - ) -> Self { - Self::stage_with_source_details(stage, message, source, None, exec_output_tail, Vec::new()) - } - - fn stage_with_source_details( - stage: ErrorStage, - message: impl Into, - source: impl Into, - failure_class: Option, - exec_output_tail: Option, - extra_causes: Vec, - ) -> Self { - let message = message.into(); - let source = SharedError::new(source.into()); - let failure_class = failure_class.unwrap_or_else(|| { - classify_failure_reason(&render_with_causes(&message, &collect_chain(&source))) - }); - Self::Stage { - stage, - message, - failure_class, - exec_output_tail, - extra_causes, - source: Some(source), - } - } - - pub fn handler(message: impl Into) -> Self { - Self::stage(ErrorStage::Handler, message, None) - } - pub fn template(message: impl Into, source: TemplateError) -> Self { Self::Template { message: message.into(), @@ -401,106 +118,35 @@ impl Error { } } - pub fn handler_with_exec_output_tail( - message: impl Into, - exec_output_tail: Option, - ) -> Self { - Self::stage(ErrorStage::Handler, message, exec_output_tail) - } - - pub fn handler_with_source( - message: impl Into, - source: impl Into, - ) -> Self { - Self::handler_with_source_and_exec_output_tail(message, source, None) - } - - pub fn handler_with_source_and_exec_output_tail( - message: impl Into, - source: impl Into, - exec_output_tail: Option, - ) -> Self { - Self::stage_with_source(ErrorStage::Handler, message, source, exec_output_tail) - } - - pub fn handler_with_anyhow(message: impl Into, source: anyhow::Error) -> Self { - Self::handler_with_source(message, source) - } - pub fn engine(message: impl Into) -> Self { - Self::stage(ErrorStage::Engine, message, None) + Self::Engine { + message: message.into(), + source: None, + } } pub fn engine_with_source( message: impl Into, source: impl Into, ) -> Self { - Self::stage_with_source(ErrorStage::Engine, message, source, None) + Self::Engine { + message: message.into(), + source: Some(SharedError::new(source.into())), + } } pub fn engine_with_anyhow(message: impl Into, source: anyhow::Error) -> Self { Self::engine_with_source(message, source) } - /// Build an error for the required publish stage. - pub fn publish(message: impl Into) -> Self { - Self::stage(ErrorStage::Publish, message, None) - } - - pub fn publish_with_source( - message: impl Into, - source: impl Into, - ) -> Self { - Self::publish_with_source_and_exec_output_tail(message, source, None) - } - - pub fn publish_with_source_and_exec_output_tail( - message: impl Into, - source: impl Into, - exec_output_tail: Option, - ) -> Self { - Self::stage_with_source(ErrorStage::Publish, message, source, exec_output_tail) - } - - /// Build a publish error with an explicitly determined failure category, - /// for callers that know more than message sniffing can recover — e.g. - /// exhausted push retries whose attempts all classified as transient. - /// `extra_causes` lines land after the source chain in the failure - /// detail (one line per push attempt). - pub fn publish_with_source_and_class( - message: impl Into, - source: impl Into, - failure_class: FailureCategory, - exec_output_tail: Option, - extra_causes: Vec, - ) -> Self { - Self::stage_with_source_details( - ErrorStage::Publish, - message, - source, - Some(failure_class), - exec_output_tail, - extra_causes, - ) - } - #[must_use] pub fn causes(&self) -> Vec { match self { - Self::Stage { - source, - extra_causes, - .. - } => { - let mut causes = source - .as_ref() - .map_or_else(Vec::new, |source| collect_chain(source)); - causes.extend(extra_causes.iter().cloned()); - causes - } + Self::Engine { source, .. } => source + .as_ref() + .map_or_else(Vec::new, |source| collect_chain(source)), Self::Template { source, .. } => collect_chain(source), Self::ScriptInterpolation { source, .. } => collect_chain(source), - Self::Llm(err) => collect_causes(err), _ => Vec::new(), } } @@ -509,116 +155,6 @@ impl Error { pub fn display_with_causes(&self) -> String { render_with_causes(&self.to_string(), &self.causes()) } - - /// Whether this error category is retryable (transient) or terminal. - /// - /// Retryable: Handler and Engine stages (the engine can re-run the node), - /// I/O, and LLM errors the SDK marks retryable. Terminal: the Publish - /// stage (it runs once, after execution), Parse, Validation, - /// OutputSchemaValidation, Stylesheet, Checkpoint, and Cancelled. - #[must_use] - pub fn is_retryable(&self) -> bool { - match self { - Self::Io(_) => true, - Self::Stage { stage, .. } => { - matches!(stage, ErrorStage::Handler | ErrorStage::Engine) - } - Self::Llm(sdk_err) => sdk_err.is_retryable(), - Self::Parse(_) - | Self::Validation(_) - | Self::ValidationFailed { .. } - | Self::ScriptInterpolation { .. } - | Self::ModelSelection(_) - | Self::ModelReference(_) - | Self::Template { .. } - | Self::Stylesheet(_) - | Self::Checkpoint(_) - | Self::Precondition(_) - | Self::RunNotFound(_) - | Self::Unsupported(_) - | Self::OutputSchemaValidation(_) - | Self::Cancelled => false, - } - } - - /// Classify this error into a `FailureCategory`. - #[must_use] - pub fn failure_category(&self) -> FailureCategory { - match self { - Self::Cancelled => FailureCategory::Canceled, - Self::Llm(sdk_err) => classify_sdk_error(sdk_err), - Self::Io(_) => FailureCategory::TransientInfra, - Self::Parse(_) - | Self::Validation(_) - | Self::ValidationFailed { .. } - | Self::ScriptInterpolation { .. } - | Self::ModelSelection(_) - | Self::ModelReference(_) - | Self::Template { .. } - | Self::Stylesheet(_) - | Self::Checkpoint(_) - | Self::Unsupported(_) - | Self::OutputSchemaValidation(_) => FailureCategory::Deterministic, - Self::Precondition(_) | Self::RunNotFound(_) => FailureCategory::Structural, - Self::Stage { failure_class, .. } => *failure_class, - } - } - - /// The terminal [`FailureReason`] this error maps to on a run. - #[must_use] - pub fn failure_reason(&self) -> FailureReason { - match self { - Self::Cancelled => FailureReason::Cancelled, - Self::Stage { - stage: ErrorStage::Publish, - .. - } => FailureReason::PublishFailed, - _ => FailureReason::WorkflowError, - } - } - - /// Return a stable failure signature hint when structured error info is - /// available. - #[must_use] - pub fn failure_signature_hint(&self) -> Option { - match self { - Self::Llm(sdk_err) => Some(FailureSignature(failure_signature_hint(sdk_err))), - _ => None, - } - } - - #[must_use] - pub fn to_failure_detail(&self) -> FailureDetail { - let (message, explicit_exec_output_tail) = match self { - Self::Stage { - message, - exec_output_tail, - .. - } => (message.clone(), exec_output_tail.clone()), - _ => (self.to_string(), None), - }; - FailureDetail { - message, - causes: self.causes(), - category: self.failure_category(), - system_actor: None, - signature: self.failure_signature_hint(), - exec_output_tail: explicit_exec_output_tail - .or_else(|| fabro_sandbox::default_redacted_output_tail(self)), - } - } - - /// Build a fail `Outcome` with structured `FailureDetail`. - pub fn to_fail_outcome(&self) -> Outcome { - let failure = self.to_failure_detail(); - Outcome { - status: StageOutcome::Failed { - retry_requested: false, - }, - failure: Some(failure), - ..Outcome::success() - } - } } impl miette::Diagnostic for Error { @@ -658,43 +194,12 @@ impl miette::Diagnostic for Error { } } -#[must_use] -pub fn run_failure_from_error(error: &Error, reason: FailureReason) -> RunFailure { - RunFailure { - reason, - detail: error.to_failure_detail(), - } -} - -#[must_use] -pub fn run_failure_from_outcome_failure( - failure: &FailureDetail, - reason: FailureReason, -) -> RunFailure { - RunFailure { - reason, - detail: failure.clone(), - } -} - impl From for Error { fn from(err: std::io::Error) -> Self { Self::Io(err.to_string()) } } -impl From for Error { - fn from(err: ErrorData) -> Self { - Self::Llm(Box::new(err)) - } -} - -impl From for Error { - fn from(err: fabro_llm::Error) -> Self { - Self::from(ErrorData::from(err)) - } -} - impl From for Error { fn from(e: GraphvizError) -> Self { match e { @@ -704,38 +209,12 @@ impl From for Error { } } -impl From for Error { - fn from(err: fabro_template::TemplateError) -> Self { - let rendered = collect_chain(&err).join(": "); - Self::template(format!("template expansion failed: {rendered}"), err) - } -} - pub type Result = std::result::Result; #[cfg(test)] mod tests { - use fabro_llm::RetryClassification; - use super::*; - /// A stored LLM error of `kind` from the `openai` provider. - fn sdk_error(kind: ErrorKind, message: &str) -> ErrorData { - ErrorData::from( - fabro_llm::Error::new(kind, message) - .with_provider(lithos_llm::catalog::builtin::openai()), - ) - } - - /// A transient failure the provider may be asked to repeat. - fn transient_error(kind: ErrorKind, message: &str) -> ErrorData { - ErrorData::from( - fabro_llm::Error::new(kind, message) - .with_provider(lithos_llm::catalog::builtin::openai()) - .with_retry(RetryClassification::Safe), - ) - } - #[derive(Debug)] struct TestCause(&'static str); @@ -844,31 +323,6 @@ mod tests { err.display_with_causes(), "Engine error: Failed to initialize sandbox\n caused by: Failed to pull Docker image buildpack-deps:noble\n caused by: connection refused" ); - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - } - - #[test] - fn engine_error_with_sandbox_state_change_cause_classifies_transient() { - let source = TestOuterError { - message: "Failed to start Daytona sandbox", - source: TestCause("Sandbox state change in progress"), - }; - let err = Error::engine_with_source("Pipeline lifecycle operation failed", source); - - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - assert!(err.is_retryable()); - } - - #[test] - fn handler_error_display() { - let err = Error::handler("LLM call failed"); - assert_eq!(err.to_string(), "Handler error: LLM call failed"); - } - - #[test] - fn checkpoint_error_display() { - let err = Error::Checkpoint("file not found".to_string()); - assert_eq!(err.to_string(), "Checkpoint error: file not found"); } #[test] @@ -900,1067 +354,6 @@ mod tests { assert_eq!(err.to_string(), "Pipeline cancelled"); } - #[test] - fn cancelled_is_not_retryable() { - assert!(!Error::Cancelled.is_retryable()); - } - - #[test] - fn is_retryable_terminal_errors() { - assert!(!Error::Parse("bad".to_string()).is_retryable()); - assert!(!Error::Validation("bad".to_string()).is_retryable()); - assert!( - !Error::ValidationFailed { - diagnostics: vec![], - } - .is_retryable() - ); - assert!(!Error::Stylesheet("bad".to_string()).is_retryable()); - assert!(!Error::Checkpoint("bad".to_string()).is_retryable()); - } - - #[test] - fn is_retryable_transient_errors() { - assert!(Error::handler("timeout").is_retryable()); - assert!(Error::engine("transient").is_retryable()); - assert!(Error::Io("connection reset".to_string()).is_retryable()); - } - - // --- FailureCategory Display/FromStr/serde tests --- - - #[test] - fn failure_class_display_all_values() { - assert_eq!( - FailureCategory::TransientInfra.to_string(), - "transient_infra" - ); - assert_eq!(FailureCategory::Deterministic.to_string(), "deterministic"); - assert_eq!( - FailureCategory::BudgetExhausted.to_string(), - "budget_exhausted" - ); - assert_eq!( - FailureCategory::CompilationLoop.to_string(), - "compilation_loop" - ); - assert_eq!(FailureCategory::Canceled.to_string(), "canceled"); - assert_eq!(FailureCategory::Structural.to_string(), "structural"); - } - - #[test] - fn failure_class_from_str_all_values() { - assert_eq!( - "transient_infra".parse::().unwrap(), - FailureCategory::TransientInfra - ); - assert_eq!( - "deterministic".parse::().unwrap(), - FailureCategory::Deterministic - ); - assert_eq!( - "budget_exhausted".parse::().unwrap(), - FailureCategory::BudgetExhausted - ); - assert_eq!( - "compilation_loop".parse::().unwrap(), - FailureCategory::CompilationLoop - ); - assert_eq!( - "canceled".parse::().unwrap(), - FailureCategory::Canceled - ); - assert_eq!( - "structural".parse::().unwrap(), - FailureCategory::Structural - ); - } - - #[test] - fn failure_class_from_str_invalid() { - assert_eq!( - "unknown".parse::().unwrap(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_from_str_alias_retryable() { - assert_eq!( - "retryable".parse::().unwrap(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_from_str_alias_transient() { - assert_eq!( - "transient".parse::().unwrap(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_from_str_alias_permanent() { - assert_eq!( - "permanent".parse::().unwrap(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_from_str_alias_cancelled_british() { - assert_eq!( - "cancelled".parse::().unwrap(), - FailureCategory::Canceled - ); - } - - #[test] - fn failure_class_from_str_alias_budget() { - assert_eq!( - "budget".parse::().unwrap(), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn failure_class_from_str_alias_compile_loop() { - assert_eq!( - "compile_loop".parse::().unwrap(), - FailureCategory::CompilationLoop - ); - } - - #[test] - fn failure_class_from_str_alias_scope_violation() { - assert_eq!( - "scope_violation".parse::().unwrap(), - FailureCategory::Structural - ); - } - - #[test] - fn failure_class_from_str_unknown_defaults_deterministic() { - assert_eq!( - "garbage_xyz".parse::().unwrap(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_from_str_case_insensitive() { - assert_eq!( - "TRANSIENT_INFRA".parse::().unwrap(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_from_str_trims_whitespace() { - assert_eq!( - " transient_infra ".parse::().unwrap(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_from_str_empty_defaults_deterministic() { - assert_eq!( - "".parse::().unwrap(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_serde_roundtrip() { - let values = [ - FailureCategory::TransientInfra, - FailureCategory::Deterministic, - FailureCategory::BudgetExhausted, - FailureCategory::CompilationLoop, - FailureCategory::Canceled, - FailureCategory::Structural, - ]; - for fc in values { - let json = serde_json::to_string(&fc).unwrap(); - let parsed: FailureCategory = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed, fc); - } - } - - // --- Llm variant tests --- - - #[test] - fn llm_error_display() { - let sdk_err = transient_error(ErrorKind::Network, "connection refused"); - let err = Error::from(sdk_err); - assert_eq!(err.to_string(), "LLM error: connection refused"); - } - - #[test] - fn llm_error_retryable_delegates_to_sdk() { - let retryable = Error::from(transient_error(ErrorKind::Network, "timeout")); - assert!(retryable.is_retryable()); - - let non_retryable = Error::from(sdk_error(ErrorKind::Configuration, "bad config")); - assert!(!non_retryable.is_retryable()); - } - - #[test] - fn llm_error_from_sdk_error() { - let sdk_err = transient_error(ErrorKind::StreamDecode, "broken pipe"); - let err = Error::from(sdk_err); - assert!(matches!(err, Error::Llm(_))); - } - - // --- failure_class() method tests --- - - #[test] - fn failure_class_cancelled() { - assert_eq!( - Error::Cancelled.failure_category(), - FailureCategory::Canceled - ); - } - - #[test] - fn failure_class_io() { - assert_eq!( - Error::Io("disk full".into()).failure_category(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_parse() { - assert_eq!( - Error::Parse("bad syntax".into()).failure_category(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_handler_with_timeout() { - assert_eq!( - Error::handler("request timed out").failure_category(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_class_handler_deterministic() { - assert_eq!( - Error::handler("invalid configuration").failure_category(), - FailureCategory::Deterministic - ); - } - - #[test] - fn failure_class_llm_rate_limit() { - let err = Error::from(transient_error(ErrorKind::RateLimit, "too fast")); - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - } - - #[test] - fn failure_class_llm_context_length() { - let err = Error::from(sdk_error(ErrorKind::ContextLength, "too long")); - assert_eq!(err.failure_category(), FailureCategory::BudgetExhausted); - } - - #[test] - fn failure_class_llm_auth() { - let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); - assert_eq!(err.failure_category(), FailureCategory::Deterministic); - } - - #[test] - fn failure_class_llm_abort() { - let err = Error::from(sdk_error(ErrorKind::Cancelled, "user cancelled")); - assert_eq!(err.failure_category(), FailureCategory::Canceled); - } - - #[test] - fn failure_class_llm_timeout() { - let err = Error::from(transient_error(ErrorKind::Timeout, "timed out")); - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - } - - // --- classify_sdk_error tests --- - - #[test] - fn classify_sdk_rate_limit() { - let err = transient_error(ErrorKind::RateLimit, "too fast"); - assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); - } - - #[test] - fn classify_sdk_server() { - let err = transient_error(ErrorKind::Server, "500"); - assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); - } - - #[test] - fn classify_sdk_context_length() { - let err = sdk_error(ErrorKind::ContextLength, "too long"); - assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); - } - - #[test] - fn classify_sdk_quota_exceeded() { - let err = sdk_error(ErrorKind::QuotaExceeded, "out of quota"); - assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); - } - - #[test] - fn classify_sdk_auth() { - let err = sdk_error(ErrorKind::Authentication, "bad key"); - assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); - } - - #[test] - fn classify_sdk_request_timeout() { - let err = transient_error(ErrorKind::Timeout, "timed out"); - assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); - } - - #[test] - fn classify_sdk_abort() { - let err = sdk_error(ErrorKind::Cancelled, "cancelled"); - assert_eq!(classify_sdk_error(&err), FailureCategory::Canceled); - } - - #[test] - fn classify_sdk_invalid_tool_call() { - let err = sdk_error(ErrorKind::InvalidRequest, "bad tool"); - assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); - } - - #[test] - fn classify_sdk_invalid_request() { - let err = sdk_error(ErrorKind::InvalidRequest, "unsupported reasoning effort"); - assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); - } - - // --- hints count guards --- - - #[test] - fn transient_infra_hints_count() { - assert_eq!(TRANSIENT_INFRA_HINTS.len(), 40); - } - - #[test] - fn budget_exhausted_hints_count() { - assert_eq!(BUDGET_EXHAUSTED_HINTS.len(), 10); - } - - #[test] - fn structural_hints_count() { - assert_eq!(STRUCTURAL_HINTS.len(), 3); - } - - // --- classify_failure_reason regression tests --- - - // Canceled - - #[test] - fn classify_reason_cancel() { - assert_eq!( - classify_failure_reason("operation cancelled by user"), - FailureCategory::Canceled - ); - } - - #[test] - fn classify_reason_nextest_canceling_due_to_test_failure_is_deterministic() { - assert_eq!( - classify_failure_reason( - "Script failed with exit code: 100\n\nCancelling due to test failure: 7 tests still running" - ), - FailureCategory::Deterministic - ); - } - - #[test] - fn classify_reason_abort() { - assert_eq!( - classify_failure_reason("interrupted by signal"), - FailureCategory::Canceled - ); - } - - // Budget exhausted - - #[test] - fn classify_reason_turn_limit() { - assert_eq!( - classify_failure_reason("exceeded turn limit of 10"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_token_limit() { - assert_eq!( - classify_failure_reason("token limit reached"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_context_length() { - assert_eq!( - classify_failure_reason("context length exceeded"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_budget() { - assert_eq!( - classify_failure_reason("budget exceeded for run"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_quota_exceeded() { - assert_eq!( - classify_failure_reason("quota exceeded"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_max_tokens() { - assert_eq!( - classify_failure_reason("max_tokens exceeded"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_max_tokens_space() { - assert_eq!( - classify_failure_reason("max tokens reached"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_context_window_exceeded() { - assert_eq!( - classify_failure_reason("context window exceeded"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_budget_exhausted() { - assert_eq!( - classify_failure_reason("budget exhausted for this session"), - FailureCategory::BudgetExhausted - ); - } - - #[test] - fn classify_reason_token_limit_exceeded() { - assert_eq!( - classify_failure_reason("token limit exceeded"), - FailureCategory::BudgetExhausted - ); - } - - // Structural - - #[test] - fn classify_reason_scope_violation() { - assert_eq!( - classify_failure_reason("scope violation detected"), - FailureCategory::Structural - ); - } - - // Transient infra - - #[test] - fn classify_reason_timeout() { - assert_eq!( - classify_failure_reason("request timed out after 30s"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_rate_limit() { - assert_eq!( - classify_failure_reason("rate limited by provider"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_connection_refused() { - assert_eq!( - classify_failure_reason("connection refused"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_connection_reset() { - assert_eq!( - classify_failure_reason("connection reset by peer"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_sandbox_state_change_in_progress() { - assert_eq!( - classify_failure_reason( - "Pipeline lifecycle operation failed: failed to activate sandbox after node \ - attempt survey: Failed to start Daytona sandbox: Sandbox state change in progress" - ), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_sandbox_stop_still_in_progress() { - assert_eq!( - classify_failure_reason("Daytona sandbox stop still in progress after 120s"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_500() { - assert_eq!( - classify_failure_reason("HTTP 500 Internal Server Error"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_502() { - assert_eq!( - classify_failure_reason("HTTP 502 Bad Gateway"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_503() { - assert_eq!( - classify_failure_reason("HTTP 503 Service Unavailable"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_504() { - assert_eq!( - classify_failure_reason("HTTP 504 Gateway Timeout"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_context_deadline_exceeded() { - assert_eq!( - classify_failure_reason("context deadline exceeded"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_could_not_resolve_host() { - assert_eq!( - classify_failure_reason("could not resolve host api.example.com"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_could_not_resolve_hostname() { - assert_eq!( - classify_failure_reason("could not resolve hostname"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_temporary_failure() { - assert_eq!( - classify_failure_reason("temporary failure"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_temporary_failure_in_name_resolution() { - assert_eq!( - classify_failure_reason("temporary failure in name resolution"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_network_is_unreachable() { - assert_eq!( - classify_failure_reason("network is unreachable"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_broken_pipe() { - assert_eq!( - classify_failure_reason("broken pipe"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_tls_handshake_timeout() { - assert_eq!( - classify_failure_reason("tls handshake timeout"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_io_timeout() { - assert_eq!( - classify_failure_reason("i/o timeout"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_no_route_to_host() { - assert_eq!( - classify_failure_reason("no route to host"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_temporarily_unavailable() { - assert_eq!( - classify_failure_reason("resource temporarily unavailable"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_try_again() { - assert_eq!( - classify_failure_reason("try again later"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_too_many_requests() { - assert_eq!( - classify_failure_reason("too many requests"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_service_unavailable() { - assert_eq!( - classify_failure_reason("service unavailable"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_gateway_timeout() { - assert_eq!( - classify_failure_reason("gateway timeout"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_econnrefused() { - assert_eq!( - classify_failure_reason("ECONNREFUSED"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_econnreset() { - assert_eq!( - classify_failure_reason("ECONNRESET"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_dial_tcp() { - assert_eq!( - classify_failure_reason("dial tcp 10.0.0.1:443: connect: connection refused"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_transport_is_closing() { - assert_eq!( - classify_failure_reason("transport is closing"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_stream_disconnected() { - assert_eq!( - classify_failure_reason("stream disconnected"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_stream_closed_before() { - assert_eq!( - classify_failure_reason("stream closed before completion"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_index_crates_io() { - assert_eq!( - classify_failure_reason("failed to fetch index.crates.io"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_download_config_json_failed() { - assert_eq!( - classify_failure_reason("download of config.json failed"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_toolchain_registry_unavailable() { - assert_eq!( - classify_failure_reason("toolchain_or_dependency_registry_unavailable"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_toolchain_dependency_network() { - assert_eq!( - classify_failure_reason("toolchain dependency resolution blocked by network"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_toolchain_workspace_io() { - assert_eq!( - classify_failure_reason("toolchain_workspace_io"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_cross_device_link() { - assert_eq!( - classify_failure_reason("cross-device link"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_invalid_cross_device_link() { - assert_eq!( - classify_failure_reason("invalid cross-device link"), - FailureCategory::TransientInfra - ); - } - - #[test] - fn classify_reason_os_error_18() { - assert_eq!( - classify_failure_reason("os error 18"), - FailureCategory::TransientInfra - ); - } - - // Structural - - #[test] - fn classify_reason_write_scope_violation_underscore() { - assert_eq!( - classify_failure_reason("write_scope_violation detected"), - FailureCategory::Structural - ); - } - - #[test] - fn classify_reason_write_scope_violation_space() { - assert_eq!( - classify_failure_reason("write scope violation detected"), - FailureCategory::Structural - ); - } - - // Default deterministic - - #[test] - fn classify_reason_default_deterministic() { - assert_eq!( - classify_failure_reason("invalid configuration parameter"), - FailureCategory::Deterministic - ); - } - - // --- normalize_failure_reason tests --- - - #[test] - fn normalize_empty_and_whitespace_returns_empty() { - assert_eq!(normalize_failure_reason(""), ""); - assert_eq!(normalize_failure_reason(" "), ""); - assert_eq!(normalize_failure_reason("\n\t"), ""); - } - - #[test] - fn normalize_lowercases_and_trims() { - assert_eq!(normalize_failure_reason(" Hello World "), "hello world"); - } - - #[test] - fn normalize_replaces_hex_strings() { - assert_eq!( - normalize_failure_reason("commit abc123def0"), - "commit " - ); - // Short hex (< 7 chars) not replaced - assert_eq!(normalize_failure_reason("value abcdef"), "value abcdef"); - } - - #[test] - fn normalize_replaces_digit_sequences() { - assert_eq!(normalize_failure_reason("line 42"), "line "); - assert_eq!(normalize_failure_reason("error 0"), "error "); - } - - #[test] - fn normalize_collapses_comma_space_and_whitespace() { - assert_eq!(normalize_failure_reason("a, b, c"), "a,b,c"); - assert_eq!(normalize_failure_reason("a b"), "a b"); - } - - #[test] - fn normalize_truncates_to_240_chars() { - let long = "a".repeat(300); - let result = normalize_failure_reason(&long); - assert_eq!(result.len(), 240); - } - - #[test] - fn normalize_truncation_respects_utf8_boundaries() { - // Build a string of 2-byte chars ("é" is 2 bytes in UTF-8) that crosses - // the 240 byte boundary mid-character. - let input = "é".repeat(200); // 400 bytes, each char is 2 bytes - let result = normalize_failure_reason(&input); - assert!(result.len() <= 240); - // Must be valid UTF-8 (String guarantees this, but verify length is even - // since every char is 2 bytes) - assert_eq!(result.len() % 2, 0); - - // Also test with a mix: 239 ASCII bytes + a 2-byte char - let input2 = format!("{}{}", "a".repeat(239), "é"); - let result2 = normalize_failure_reason(&input2); - assert!(result2.len() <= 240); - // Should truncate to 239 (dropping the 2-byte char that would push to 241) - assert_eq!(result2.len(), 239); - } - - #[test] - fn normalize_combined_example() { - assert_eq!( - normalize_failure_reason("Error at line 42 in abc123def"), - "error at line in " - ); - } - - // --- FailureSignature tests --- - - #[test] - fn failure_signature_format() { - let sig = FailureSignature::new( - "verify", - FailureCategory::Deterministic, - None, - Some("test failed"), - ); - assert_eq!(sig.to_string(), "verify|deterministic|test failed"); - } - - #[test] - fn failure_signature_display() { - let sig = FailureSignature::new( - "build", - FailureCategory::Structural, - None, - Some("scope violation"), - ); - assert_eq!(format!("{sig}"), "build|structural|scope violation"); - } - - #[test] - fn failure_signature_hint_takes_priority() { - let sig = FailureSignature::new( - "verify", - FailureCategory::Deterministic, - Some("custom hint"), - Some("raw reason"), - ); - assert_eq!(sig.to_string(), "verify|deterministic|custom hint"); - } - - #[test] - fn failure_signature_missing_reason_falls_back_to_unknown() { - let sig = FailureSignature::new("node", FailureCategory::Deterministic, None, None); - assert_eq!(sig.to_string(), "node|deterministic|unknown"); - } - - #[test] - fn failure_signature_equality_and_hash() { - let sig1 = FailureSignature::new( - "verify", - FailureCategory::Deterministic, - None, - Some("test failed"), - ); - let sig2 = FailureSignature::new( - "verify", - FailureCategory::Deterministic, - None, - Some("test failed"), - ); - assert_eq!(sig1, sig2); - - let mut map = std::collections::HashMap::new(); - map.insert(sig1.clone(), 1); - assert_eq!(map.get(&sig2), Some(&1)); - } - - // --- is_signature_tracked tests --- - - #[test] - fn is_signature_tracked_deterministic_and_structural() { - assert!(FailureCategory::Deterministic.is_signature_tracked()); - assert!(FailureCategory::Structural.is_signature_tracked()); - } - - #[test] - fn is_signature_tracked_false_for_others() { - assert!(!FailureCategory::TransientInfra.is_signature_tracked()); - assert!(!FailureCategory::BudgetExhausted.is_signature_tracked()); - assert!(!FailureCategory::Canceled.is_signature_tracked()); - assert!(!FailureCategory::CompilationLoop.is_signature_tracked()); - } - - // --- failure_signature_hint tests --- - - #[test] - fn failure_signature_hint_llm_returns_some() { - let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); - assert_eq!( - err.failure_signature_hint(), - Some(FailureSignature( - "api_deterministic|openai|authentication".to_string() - )) - ); - } - - #[test] - fn failure_signature_hint_handler_returns_none() { - let err = Error::handler("something failed"); - assert_eq!(err.failure_signature_hint(), None); - } - - #[test] - fn failure_signature_hint_engine_returns_none() { - let err = Error::engine("engine error"); - assert_eq!(err.failure_signature_hint(), None); - } - - // --- to_fail_outcome tests --- - - #[test] - fn to_fail_outcome_llm_has_class_and_signature() { - let err = Error::from(sdk_error(ErrorKind::Authentication, "bad key")); - let outcome = err.to_fail_outcome(); - assert_eq!(outcome.status, crate::outcome::StageOutcome::Failed { - retry_requested: false, - }); - let failure = outcome.failure.as_ref().unwrap(); - assert_eq!(failure.category, FailureCategory::Deterministic); - assert_eq!( - failure.signature.as_deref(), - Some("api_deterministic|openai|authentication") - ); - } - - #[test] - fn to_fail_outcome_handler_has_class_but_no_signature() { - let err = Error::handler("connection refused"); - let outcome = err.to_fail_outcome(); - assert_eq!(outcome.status, crate::outcome::StageOutcome::Failed { - retry_requested: false, - }); - let failure = outcome.failure.as_ref().unwrap(); - assert_eq!(failure.category, FailureCategory::TransientInfra); - assert!(failure.signature.is_none()); - } - - #[test] - fn to_fail_outcome_no_context_updates() { - let err = Error::from(transient_error(ErrorKind::Network, "refused")); - let outcome = err.to_fail_outcome(); - assert!(outcome.context_updates.is_empty()); - } - - // --- Phase 2: Eager classification tests --- - - #[test] - fn handler_eager_classification() { - let err = Error::handler("connection refused"); - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - } - - #[test] - fn handler_eager_classification_survives_clone() { - let err = Error::handler("connection refused"); - let cloned = err.clone(); - assert_eq!(cloned.failure_category(), FailureCategory::TransientInfra); - } - - #[test] - fn handler_smart_constructor_preserves_message() { - let err = Error::handler("some error"); - assert!(err.to_string().contains("some error")); - } - - #[test] - fn engine_eager_classification() { - let err = Error::engine("rate limit exceeded"); - assert_eq!(err.failure_category(), FailureCategory::TransientInfra); - } - #[test] fn error_clone_preserves_display_for_all_variants() { let errors: Vec = vec![ @@ -1979,114 +372,15 @@ mod tests { }], }, Error::engine("engine err"), - Error::publish("publish err"), - Error::handler("handler err"), - Error::from(transient_error(ErrorKind::Network, "refused")), - Error::Checkpoint("cp err".into()), + Error::engine_with_source("engine err", TestCause("cause")), Error::Stylesheet("style err".into()), Error::Io("io err".into()), + Error::Precondition("precondition".into()), + Error::RunNotFound("run".into()), Error::Cancelled, ]; for err in errors { assert_eq!(err.to_string(), err.clone().to_string()); } } - - #[test] - fn handler_display_unchanged() { - assert_eq!( - Error::handler("LLM call failed").to_string(), - "Handler error: LLM call failed" - ); - } - - #[test] - fn engine_display_unchanged() { - assert_eq!( - Error::engine("no outgoing edge").to_string(), - "Engine error: no outgoing edge" - ); - } - - /// Publish runs once, after execution, so no caller can retry it — even - /// when the message looks transient. The failure category is still - /// classified for reporting. - #[test] - fn publish_errors_are_terminal() { - assert!(!Error::publish("connection timed out").is_retryable()); - assert!(!Error::publish("permission denied").is_retryable()); - assert_eq!( - Error::publish("connection timed out").failure_category(), - FailureCategory::TransientInfra - ); - } - - #[test] - fn failure_reason_distinguishes_publish_and_cancelled() { - assert_eq!( - Error::publish("nope").failure_reason(), - FailureReason::PublishFailed - ); - assert_eq!(Error::Cancelled.failure_reason(), FailureReason::Cancelled); - assert_eq!( - Error::engine("boom").failure_reason(), - FailureReason::WorkflowError - ); - } - - #[test] - fn failure_class_stability() { - let messages = [ - "connection refused", - "timeout", - "rate limit", - "context length exceeded", - "cancel", - "invalid configuration", - "write_scope_violation", - ]; - for msg in messages { - assert_eq!( - Error::handler(msg).failure_category(), - classify_failure_reason(msg), - "mismatch for message: {msg}" - ); - } - } - - /// Commit SHAs are hex, so they contain digit runs like "503" often enough - /// to matter. Masking them keeps a deterministic failure from being - /// reported as transient just because of the SHA it names. - #[test] - fn commit_shas_do_not_trip_transient_infra_hints() { - let sha = "a503b1c9d4e2f7a8b6c3d0e1f2a3b4c5d6e7f8a9"; - assert_eq!( - classify_failure_reason(&format!("failed to push final commit {sha} to branch 'x'")), - FailureCategory::Deterministic - ); - // A real status code is still a transient hint. - assert_eq!( - classify_failure_reason("push rejected with 503"), - FailureCategory::TransientInfra - ); - } - - // --- E2E error pipeline tests --- - - #[test] - fn e2e_handler_retryable_checks() { - assert!(Error::handler("timeout").is_retryable()); - assert!(Error::handler("auth error").is_retryable()); - } - - #[test] - fn e2e_run_failure_projection_uses_handler_error_shape() { - let err = Error::handler("connection refused"); - let failure = run_failure_from_error(&err, FailureReason::WorkflowError); - - assert_eq!(failure.detail.message, "connection refused"); - assert_eq!(failure.detail.causes, Vec::::new()); - assert_eq!(failure.reason, FailureReason::WorkflowError); - assert_eq!(failure.detail.category, FailureCategory::TransientInfra); - } } diff --git a/lib/components/fabro-workflow/src/git.rs b/lib/components/fabro-workflow/src/git.rs index accd87785..444fe5ecd 100644 --- a/lib/components/fabro-workflow/src/git.rs +++ b/lib/components/fabro-workflow/src/git.rs @@ -1,17 +1,11 @@ use std::path::Path; use std::process::Command; -pub use fabro_checkpoint::author::GitAuthor; use fabro_redact::DisplaySafeUrl; -use fabro_types::{DirtyStatus, GitContext, WorkflowSettings}; -use tokio::task::{JoinError, spawn_blocking}; -use tokio::time::timeout; +use fabro_types::{DirtyStatus, GitContext}; use crate::error::{Error, Result}; -/// Branch prefix for workflow run branches (e.g. `fabro/run/{run_id}`). -pub const RUN_BRANCH_PREFIX: &str = "fabro/run/"; - /// A local checkout could not be inspected without changing it. #[derive(Debug, thiserror::Error)] pub enum GitObservationError { @@ -112,16 +106,6 @@ fn sanitized_origin_url(value: &str) -> String { fabro_github::normalize_repo_origin_url(url.as_str()) } -pub fn git_author_from_settings(settings: &WorkflowSettings) -> GitAuthor { - settings - .run - .git - .author - .clone() - .map(|author| GitAuthor::from(&author)) - .unwrap_or_default() -} - fn git_error(msg: impl Into) -> Error { Error::engine(msg.into()) } @@ -138,24 +122,12 @@ fn git_cmd(dir: &Path) -> Command { cmd } -/// Assert the working directory is a clean git repo (no uncommitted changes). -pub fn ensure_clean(repo: &Path) -> Result<()> { - tracing::debug!(path = %repo.display(), "Checking git cleanliness"); - let output = git_cmd(repo) +/// Whether the working directory is a git repo with no uncommitted changes. +fn working_tree_is_clean(repo: &Path) -> bool { + git_cmd(repo) .args(["status", "--porcelain"]) .output() - .map_err(|e| Error::engine_with_source("git status failed", e))?; - - if !output.status.success() { - return Err(git_error("not a git repository")); - } - - let stdout = String::from_utf8_lossy(&output.stdout); - if !stdout.trim().is_empty() { - return Err(git_error("working directory has uncommitted changes")); - } - - Ok(()) + .is_ok_and(|output| output.status.success() && output.stdout.trim_ascii().is_empty()) } /// Return the SHA of HEAD. @@ -172,50 +144,6 @@ pub fn head_sha(repo: &Path) -> Result { Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) } -/// Run a `git push` command and check for success. -fn run_git_push(cmd: &mut Command) -> Result<()> { - let output = cmd - .output() - .map_err(|e| Error::engine_with_source("git push failed", e))?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(git_error(format!("git push failed: {stderr}"))); - } - Ok(()) -} - -/// Push a local ref to an explicit remote URL. -/// -/// Uses a URL (not a named remote) so the host repo's remote config is -/// untouched. Disables credential helpers so only the inline URL credentials -/// are used. -pub fn push_ref(repo: &Path, url: &str, refname: &str) -> Result<()> { - let redacted_url = if let Some(at_pos) = url.find('@') { - format!("https://***@{}", &url[at_pos + 1..]) - } else { - url.to_string() - }; - tracing::info!( - repo_dir = %repo.display(), - url = %redacted_url, - refname, - "Pushing ref to remote" - ); - run_git_push(git_cmd(repo).args(["-c", "credential.helper=", "push", url, refname])) -} - -/// Push a local branch to the named remote using the user's configured -/// credentials. -pub fn push_branch(repo: &Path, remote: &str, branch: &str) -> Result<()> { - tracing::info!( - repo_dir = %repo.display(), - remote, - branch, - "Pushing branch to remote" - ); - run_git_push(git_cmd(repo).args(["push", remote, branch])) -} - /// Push a local branch to the named remote without allowing Git to prompt. pub fn push_branch_noninteractive(repo: &Path, remote: &str, branch: &str) -> Result<()> { tracing::info!( @@ -224,11 +152,16 @@ pub fn push_branch_noninteractive(repo: &Path, remote: &str, branch: &str) -> Re branch, "Pushing branch to remote without terminal prompts" ); - run_git_push( - git_cmd(repo) - .env("GIT_TERMINAL_PROMPT", "0") - .args(["push", remote, branch]), - ) + let output = git_cmd(repo) + .env("GIT_TERMINAL_PROMPT", "0") + .args(["push", remote, branch]) + .output() + .map_err(|e| Error::engine_with_source("git push failed", e))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(git_error(format!("git push failed: {stderr}"))); + } + Ok(()) } /// Read the exact commit currently advertised for a remote branch without @@ -265,48 +198,6 @@ pub fn remote_branch_sha_noninteractive( Ok(None) } -/// Error from [`blocking_push_with_timeout`]. -pub enum BlockingPushError { - /// The git push itself failed. - Push(Error), - /// The spawned blocking task panicked. - Panicked(JoinError), - /// The push did not complete within the timeout. - TimedOut, -} - -impl std::fmt::Display for BlockingPushError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Push(e) => write!(f, "{e}"), - Self::Panicked(e) => write!(f, "task panicked: {e}"), - Self::TimedOut => write!(f, "timed out"), - } - } -} - -/// Run a blocking git-push function with a timeout, flattening the -/// triple-nested Result. -pub async fn blocking_push_with_timeout( - timeout_secs: u64, - f: F, -) -> std::result::Result<(), BlockingPushError> -where - F: FnOnce() -> Result<()> + Send + 'static, -{ - match timeout( - std::time::Duration::from_secs(timeout_secs), - spawn_blocking(f), - ) - .await - { - Ok(Ok(Ok(()))) => Ok(()), - Ok(Ok(Err(e))) => Err(BlockingPushError::Push(e)), - Ok(Err(e)) => Err(BlockingPushError::Panicked(e)), - Err(_) => Err(BlockingPushError::TimedOut), - } -} - /// Returns true if the local branch has commits not yet on the remote. /// On any git error (no remote ref, detached HEAD, etc.), returns true /// so the caller falls back to pushing. @@ -354,7 +245,7 @@ impl std::fmt::Display for GitSyncStatus { /// Determine the sync status of the repository relative to a remote. pub fn sync_status(repo: &Path, remote: &str, branch: Option<&str>) -> GitSyncStatus { - if ensure_clean(repo).is_err() { + if !working_tree_is_clean(repo) { return GitSyncStatus::Dirty; } match branch { @@ -479,26 +370,27 @@ mod tests { } #[test] - fn ensure_clean_on_clean_repo() { - let dir = tempfile::tempdir().unwrap(); - init_repo(dir.path()); - assert!(ensure_clean(dir.path()).is_ok()); - } - - #[test] - fn ensure_clean_fails_with_dirty_file() { + fn sync_status_is_dirty_with_uncommitted_changes() { let dir = tempfile::tempdir().unwrap(); init_repo(dir.path()); + assert_ne!( + sync_status(dir.path(), "origin", None), + GitSyncStatus::Dirty + ); fs::write(dir.path().join("dirty.txt"), "hello").unwrap(); - let err = ensure_clean(dir.path()).unwrap_err(); - assert!(err.to_string().contains("uncommitted changes")); + assert_eq!( + sync_status(dir.path(), "origin", None), + GitSyncStatus::Dirty + ); } #[test] - fn ensure_clean_fails_on_non_repo() { + fn sync_status_is_dirty_on_non_repo() { let dir = tempfile::tempdir().unwrap(); - let err = ensure_clean(dir.path()).unwrap_err(); - assert!(err.to_string().contains("not a git repository")); + assert_eq!( + sync_status(dir.path(), "origin", None), + GitSyncStatus::Dirty + ); } #[test] @@ -511,10 +403,10 @@ mod tests { } #[test] - fn push_branch_fails_for_nonexistent_remote() { + fn push_branch_noninteractive_fails_for_nonexistent_remote() { let dir = tempfile::tempdir().unwrap(); init_repo(dir.path()); - let result = push_branch(dir.path(), "nonexistent", "main"); + let result = push_branch_noninteractive(dir.path(), "nonexistent", "main"); assert!(result.is_err()); } diff --git a/lib/components/fabro-workflow/src/git_identity.rs b/lib/components/fabro-workflow/src/git_identity.rs deleted file mode 100644 index 24c29573e..000000000 --- a/lib/components/fabro-workflow/src/git_identity.rs +++ /dev/null @@ -1,329 +0,0 @@ -//! One Git author and committer identity per run. -//! -//! The run resolves its identity once, after its GitHub credentials are -//! selected and before anything can commit, then uses it everywhere: engine -//! checkpoints and metadata commits read it through -//! [`git_author_from_settings`](crate::git::git_author_from_settings), -//! and every workflow command, prepare step, native agent shell tool, and ACP -//! agent launch receives it as the four `GIT_AUTHOR_*` / `GIT_COMMITTER_*` -//! variables so plain `git commit` inside the sandbox agrees with the engine. -//! -//! Resolution order: an explicit, complete `run.git.author`; the run's GitHub -//! App bot account; the authenticated user of the run's GitHub PAT; the -//! generic Fabro identity. A partial `run.git.author` overlays the fields it -//! supplies on whichever identity the credentials resolve to. Only the run's -//! selected credentials are consulted: a lookup failure for them is a setup -//! error, never a silent change of author. - -use std::collections::HashMap; -use std::sync::Arc; -use std::time::Duration; - -use anyhow::Context as _; -use fabro_github::token_source::InstallationTokenSource; -use fabro_github::{GitHubCredentials, identity}; -use fabro_types::settings::run::GitAuthorSettings; -use fabro_types::{GitIdentity, GitIdentitySource, WorkflowSettings}; -use tokio::time::timeout; - -use crate::error::Error; - -/// Environment variables Git reads for the author and committer. -pub const GIT_IDENTITY_ENV_KEYS: [&str; 4] = [ - "GIT_AUTHOR_NAME", - "GIT_AUTHOR_EMAIL", - "GIT_COMMITTER_NAME", - "GIT_COMMITTER_EMAIL", -]; - -/// Upper bound on one identity lookup against the GitHub API. -const LOOKUP_TIMEOUT: Duration = Duration::from_secs(30); - -/// The outcome of resolving a run's identity. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ResolvedGitIdentity { - pub identity: GitIdentity, - /// Set when the selected credentials were a standalone installation - /// token whose App bot account cannot be determined; the identity fell - /// back to the generic Fabro identity (plus any explicit fields). - pub warning: Option, -} - -/// The explicit `run.git.author` fields, trimmed; empty values count as unset. -fn explicit_fields(settings: &WorkflowSettings) -> (Option, Option) { - let author: Option<&GitAuthorSettings> = settings.run.git.author.as_ref(); - let field = |value: Option<&String>| { - value - .map(|value| value.trim()) - .filter(|value| !value.is_empty()) - .map(str::to_string) - }; - ( - field(author.and_then(|author| author.name.as_ref())), - field(author.and_then(|author| author.email.as_ref())), - ) -} - -/// Overlay explicit fields on a resolved identity. The source stays that of -/// the resolved identity unless both fields are explicit. -fn overlay(mut identity: GitIdentity, name: Option, email: Option) -> GitIdentity { - if name.is_some() && email.is_some() { - identity.source = GitIdentitySource::Explicit; - } - if let Some(name) = name { - identity.name = name; - } - if let Some(email) = email { - identity.email = email; - } - identity -} - -/// Resolve the run's Git identity from its settings and selected credentials. -/// -/// `github_token` is the run's managed token source, used only as the bearer -/// for the App bot-account lookup (that endpoint rejects App JWTs). -pub async fn resolve_git_identity( - settings: &WorkflowSettings, - credentials: Option<&GitHubCredentials>, - github_token: Option<&Arc>, -) -> Result { - let (name, email) = explicit_fields(settings); - if let (Some(name), Some(email)) = (name.clone(), email.clone()) { - return Ok(ResolvedGitIdentity { - identity: GitIdentity { - name, - email, - source: GitIdentitySource::Explicit, - }, - warning: None, - }); - } - - let (credential_identity, warning) = match credentials { - None => (GitIdentity::fabro_default(), None), - Some(GitHubCredentials::Installation(_)) => ( - GitIdentity::fabro_default(), - Some( - "The run's GitHub credential is a standalone installation token whose App bot \ - account cannot be determined; commits use the generic Fabro identity." - .to_string(), - ), - ), - Some(credentials) => ( - lookup_credential_identity(credentials, github_token) - .await - .map_err(|err| { - Error::engine_with_anyhow("Failed to resolve the run's Git identity", err) - })?, - None, - ), - }; - - Ok(ResolvedGitIdentity { - identity: overlay(credential_identity, name, email), - warning, - }) -} - -async fn lookup_credential_identity( - credentials: &GitHubCredentials, - github_token: Option<&Arc>, -) -> anyhow::Result { - let client = fabro_http::http_client() - .map_err(anyhow::Error::new) - .context("building HTTP client for GitHub identity lookup")?; - let base_url = fabro_github::github_api_base_url(); - let lookup = async { - match credentials { - GitHubCredentials::App(app) => { - let bearer = match github_token { - Some(source) => Some( - source - .resolve() - .await - .context("resolving the GitHub token for the App bot lookup")?, - ), - None => None, - }; - let account = identity::lookup_app_bot_identity( - &client, - app, - &base_url, - bearer.as_ref().map(|token| token.token.expose()), - ) - .await?; - Ok::<_, anyhow::Error>(GitIdentity { - email: account.noreply_email(), - name: account.login, - source: GitIdentitySource::GithubApp, - }) - } - GitHubCredentials::Pat(token) => { - let account = identity::lookup_token_identity(&client, token, &base_url).await?; - Ok(GitIdentity { - email: account.noreply_email(), - name: account.login, - source: GitIdentitySource::GithubPat, - }) - } - GitHubCredentials::Installation(_) => { - unreachable!("installation tokens never reach the credential lookup") - } - } - }; - timeout(LOOKUP_TIMEOUT, lookup) - .await - .context("GitHub identity lookup timed out")? -} - -/// The four Git environment variables for `identity`. -#[must_use] -pub fn git_identity_env(identity: &GitIdentity) -> [(&'static str, String); 4] { - [ - ("GIT_AUTHOR_NAME", identity.name.clone()), - ("GIT_AUTHOR_EMAIL", identity.email.clone()), - ("GIT_COMMITTER_NAME", identity.name.clone()), - ("GIT_COMMITTER_EMAIL", identity.email.clone()), - ] -} - -/// Set the identity variables on `env`, replacing any existing values so the -/// run's identity wins over inherited host variables and conflicting run or -/// step environment entries. -pub fn apply_git_identity_env(env: &mut HashMap, identity: &GitIdentity) { - for (key, value) in git_identity_env(identity) { - env.insert(key.to_string(), value); - } -} - -#[cfg(test)] -mod tests { - use fabro_types::settings::run::GitAuthorSettings; - - use super::*; - - fn settings(name: Option<&str>, email: Option<&str>) -> WorkflowSettings { - let mut settings = WorkflowSettings::default(); - settings.run.git.author = Some(GitAuthorSettings { - name: name.map(str::to_string), - email: email.map(str::to_string), - }); - settings - } - - fn installation() -> GitHubCredentials { - GitHubCredentials::Installation(fabro_github::InstallationToken { - token: "ghs_token".to_string(), - expires_at: chrono::Utc::now() + chrono::Duration::hours(1), - }) - } - - #[tokio::test] - async fn no_credentials_use_the_generic_identity_without_a_lookup() { - let resolved = resolve_git_identity(&WorkflowSettings::default(), None, None) - .await - .unwrap(); - assert_eq!(resolved.identity, GitIdentity::fabro_default()); - assert_eq!(resolved.identity.source, GitIdentitySource::Default); - assert!(resolved.warning.is_none()); - } - - #[tokio::test] - async fn complete_explicit_author_skips_credential_lookup() { - // A PAT lookup would need the network; a complete explicit author - // must never get that far. - let creds = GitHubCredentials::Pat("ghp_never_used".to_string()); - let resolved = resolve_git_identity( - &settings(Some("Release Bot"), Some("release@example.com")), - Some(&creds), - None, - ) - .await - .unwrap(); - assert_eq!(resolved.identity, GitIdentity { - name: "Release Bot".to_string(), - email: "release@example.com".to_string(), - source: GitIdentitySource::Explicit, - }); - } - - #[tokio::test] - async fn partial_explicit_author_overlays_the_resolved_identity() { - let resolved = resolve_git_identity(&settings(Some("Only Name"), None), None, None) - .await - .unwrap(); - assert_eq!(resolved.identity, GitIdentity { - name: "Only Name".to_string(), - email: GitIdentity::DEFAULT_EMAIL.to_string(), - source: GitIdentitySource::Default, - }); - - let resolved = resolve_git_identity(&settings(None, Some("only@example.com")), None, None) - .await - .unwrap(); - assert_eq!(resolved.identity.name, GitIdentity::DEFAULT_NAME); - assert_eq!(resolved.identity.email, "only@example.com"); - } - - #[tokio::test] - async fn blank_explicit_fields_count_as_unset() { - let resolved = resolve_git_identity(&settings(Some(" "), Some("")), None, None) - .await - .unwrap(); - assert_eq!(resolved.identity, GitIdentity::fabro_default()); - } - - #[tokio::test] - async fn standalone_installation_token_falls_back_with_a_warning() { - let resolved = - resolve_git_identity(&WorkflowSettings::default(), Some(&installation()), None) - .await - .unwrap(); - assert_eq!(resolved.identity, GitIdentity::fabro_default()); - let warning = resolved.warning.expect("fallback should warn"); - assert!( - warning.contains("standalone installation token"), - "{warning}" - ); - } - - #[tokio::test] - async fn standalone_installation_token_keeps_explicit_fields() { - let resolved = resolve_git_identity( - &settings(None, Some("pinned@example.com")), - Some(&installation()), - None, - ) - .await - .unwrap(); - assert_eq!(resolved.identity.name, GitIdentity::DEFAULT_NAME); - assert_eq!(resolved.identity.email, "pinned@example.com"); - assert_eq!(resolved.identity.source, GitIdentitySource::Default); - assert!(resolved.warning.is_some()); - } - - #[test] - fn identity_env_replaces_conflicting_entries() { - let identity = GitIdentity { - name: "fabro-bot[bot]".to_string(), - email: "7+fabro-bot[bot]@users.noreply.github.com".to_string(), - source: GitIdentitySource::GithubApp, - }; - let mut env = HashMap::from([ - ("GIT_AUTHOR_NAME".to_string(), "someone else".to_string()), - ( - "GIT_COMMITTER_EMAIL".to_string(), - "x@example.com".to_string(), - ), - ("KEEP".to_string(), "1".to_string()), - ]); - apply_git_identity_env(&mut env, &identity); - assert_eq!(env["GIT_AUTHOR_NAME"], "fabro-bot[bot]"); - assert_eq!(env["GIT_AUTHOR_EMAIL"], identity.email); - assert_eq!(env["GIT_COMMITTER_NAME"], "fabro-bot[bot]"); - assert_eq!(env["GIT_COMMITTER_EMAIL"], identity.email); - assert_eq!(env["KEEP"], "1"); - assert_eq!(env.len(), 5); - } -} diff --git a/lib/components/fabro-workflow/src/lib.rs b/lib/components/fabro-workflow/src/lib.rs index 499612196..5d1907fb1 100644 --- a/lib/components/fabro-workflow/src/lib.rs +++ b/lib/components/fabro-workflow/src/lib.rs @@ -5,10 +5,10 @@ //! what Fabro itself owns: the create-time compile of the Fabro graph the //! read side displays (`pipeline`, `transforms`, `operations`), the run //! records and status vocabulary (`records`, `run_status`), the Git -//! helpers a run's platform effects use (`git`, `git_identity`, -//! `sandbox_git`), pull request creation (`pull_request`), the run tools an -//! agent session calls (`run_tools`, `services`), the built-in web search -//! backend (`web_search`). +//! helpers a run's platform effects use (`git`, `sandbox_git`), pull +//! request creation (`pull_request`), the run tools an agent session calls +//! (`run_tools`, `services`), the built-in web search backend +//! (`web_search`). #![cfg_attr( test, @@ -30,7 +30,6 @@ pub mod error; pub mod file_resolver; pub mod git; -pub mod git_identity; pub mod operations; pub mod outcome; pub mod pipeline; @@ -39,7 +38,7 @@ pub mod records; pub mod run_lookup; pub mod usage_rollup; -pub use error::{Error, FailureCategory, FailureSignature, FailureSignatureExt, Result}; +pub use error::{Error, Result}; pub use fabro_types::ManifestPath; pub use usage_rollup::{ ProjectionUsageByModel, ProjectionUsageRollup, ProjectionUsageStage,