From 6297b200f771007a22929988716c3b8fbbbed8e8 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Wed, 13 May 2026 09:32:28 -0700 Subject: [PATCH] refactor(run): add rich failure contract (#256) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Terminal run failures now use a first-class `RunFailure` contract so downstream consumers receive structured diagnostics instead of flat `error` / `causes` / `reason` fields. The wire shape keeps concise public messages, source-chain causes, classification, optional actor/signature data, and redacted exec output tail in one nested value. Refs fabro-sh/fabro#198 ## What Changed - Added `fabro_types::RunFailure` and changed `run.failed` to emit `properties.failure` with `final_git_commit_sha` for failed-run commit state. - Replaced `Conclusion.failure_reason` with `Conclusion.failure` while leaving stage-level `StageCompletion.failure_reason` untouched. - Updated workflow internals to preserve owned error source chains until terminal event projection, then convert them into `RunFailure.causes`. - Updated store, server, CLI, OpenAPI, and generated TypeScript client consumers to use the nested failure object. - Added serialization, OpenAPI replacement, projection, and lifecycle coverage for the new contract. ## Validation - `cargo nextest run -p fabro-api -p fabro-types -p fabro-workflow -p fabro-store -p fabro-server -p fabro-cli` - `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` - `cargo +nightly-2026-04-14 fmt --check --all` - `cd apps/fabro-web && bun run typecheck` - `cd apps/fabro-web && bun test` - `git diff --check` --- [![Compound Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 via [Codex](https://openai.com/codex) --- ...26-05-13-rich-run-failure-contract-plan.md | 81 ++++++ docs/public/api-reference/fabro-api.yaml | 67 ++++- lib/crates/fabro-api/build.rs | 5 + lib/crates/fabro-api/src/lib.rs | 18 +- .../fabro-api/tests/run_failure_round_trip.rs | 108 ++++++++ .../fabro-cli/src/commands/run/events.rs | 7 +- .../fabro-cli/src/commands/run/output.rs | 6 +- .../fabro-cli/src/commands/run/runner.rs | 48 ++-- lib/crates/fabro-cli/src/commands/run/wait.rs | 17 +- lib/crates/fabro-dump/src/lib.rs | 2 +- lib/crates/fabro-server/src/run_files.rs | 2 +- lib/crates/fabro-server/src/server.rs | 233 +++++++----------- lib/crates/fabro-server/src/server/tests.rs | 2 +- .../tests/it/scenario/lifecycle.rs | 6 +- lib/crates/fabro-store/src/run_state.rs | 115 ++++++--- lib/crates/fabro-store/src/slate/mod.rs | 14 +- lib/crates/fabro-types/src/conclusion.rs | 4 +- lib/crates/fabro-types/src/lib.rs | 2 + lib/crates/fabro-types/src/run_event/mod.rs | 7 +- lib/crates/fabro-types/src/run_event/run.rs | 21 +- lib/crates/fabro-types/src/run_failure.rs | 18 ++ .../fabro-types/tests/run_failure_serde.rs | 135 ++++++++++ lib/crates/fabro-workflow/src/artifact.rs | 32 +-- lib/crates/fabro-workflow/src/command_log.rs | 8 +- .../fabro-workflow/src/devcontainer_bridge.rs | 2 +- lib/crates/fabro-workflow/src/error.rs | 133 +++++----- .../fabro-workflow/src/event/convert.rs | 97 +++++--- lib/crates/fabro-workflow/src/event/events.rs | 58 ++++- lib/crates/fabro-workflow/src/git.rs | 12 +- .../fabro-workflow/src/handler/agent.rs | 2 +- .../fabro-workflow/src/handler/command.rs | 4 +- .../fabro-workflow/src/handler/llm/acp.rs | 6 +- .../fabro-workflow/src/handler/llm/api.rs | 4 +- .../fabro-workflow/src/handler/llm/cli.rs | 11 +- .../src/handler/llm/launch_env.rs | 6 +- .../fabro-workflow/src/handler/parallel.rs | 10 +- .../fabro-workflow/src/lifecycle/git.rs | 2 +- .../fabro-workflow/src/operations/archive.rs | 22 +- .../fabro-workflow/src/operations/create.rs | 2 +- .../fabro-workflow/src/operations/start.rs | 74 +++--- .../fabro-workflow/src/pipeline/finalize.rs | 72 +++--- .../fabro-workflow/src/pipeline/initialize.rs | 24 +- .../src/pipeline/pull_request.rs | 2 +- .../fabro-workflow/tests/it/integration.rs | 2 +- .../src/.openapi-generator/FILES | 3 + .../fabro-api-client/src/api/models-api.ts | 2 +- .../fabro-api-client/src/models/conclusion.ts | 5 +- .../src/models/exec-output-tail.ts | 26 ++ .../src/models/failure-category.ts | 33 +++ .../fabro-api-client/src/models/index.ts | 3 + .../fabro-api-client/src/models/model.ts | 6 +- .../src/models/run-failure.ts | 46 ++++ 52 files changed, 1140 insertions(+), 487 deletions(-) create mode 100644 docs/plans/2026-05-13-rich-run-failure-contract-plan.md create mode 100644 lib/crates/fabro-api/tests/run_failure_round_trip.rs create mode 100644 lib/crates/fabro-types/src/run_failure.rs create mode 100644 lib/crates/fabro-types/tests/run_failure_serde.rs create mode 100644 lib/packages/fabro-api-client/src/models/exec-output-tail.ts create mode 100644 lib/packages/fabro-api-client/src/models/failure-category.ts create mode 100644 lib/packages/fabro-api-client/src/models/run-failure.ts diff --git a/docs/plans/2026-05-13-rich-run-failure-contract-plan.md b/docs/plans/2026-05-13-rich-run-failure-contract-plan.md new file mode 100644 index 000000000..70c6f1858 --- /dev/null +++ b/docs/plans/2026-05-13-rich-run-failure-contract-plan.md @@ -0,0 +1,81 @@ +--- +title: "refactor: Rich run failure contract" +type: refactor +status: completed +date: 2026-05-13 +origin: https://github.com/fabro-sh/fabro/issues/198 +--- + +# refactor: Rich run failure contract + +## Summary + +Refactor terminal run failures around a first-class `RunFailure` value object. Preserve rich internal errors and source chains until terminal event projection, then serialize a structured failure payload with message, causes, classification, optional signature, actor, and redacted exec output tail. + +Assumption: this is greenfield. Do not preserve the old flat `run.failed.properties.error` / `causes` / `reason` contract or old `Conclusion.failure_reason` shape. + +## Key Changes + +- Add `fabro_types::RunFailure` with this wire shape: + - `message: String` + - `causes: Vec`, omitted when empty + - `reason: FailureReason` + - `category: FailureCategory` + - `system_actor: Option` + - `signature: Option` + - `exec_output_tail: Option` +- Change `RunFailedProps` to contain: + - `failure: RunFailure` + - `duration_ms` + - `final_git_commit_sha: Option`, replacing failed-run `git_commit_sha` + - `final_patch`, `diff_summary`, and optional `billing` +- Change `Conclusion` from `failure_reason: Option` to `failure: Option`. Keep `StageCompletion.failure_reason` unchanged; stage-level failure text is separate from terminal run failure diagnostics. +- Update OpenAPI `Conclusion` schemas and regenerate `lib/packages/fabro-api-client` models. `RunEvent.properties` is still generic, but docs/examples should show nested `failure`. + +## Implementation + +- In `fabro-workflow`, remove `fabro_workflow::Error` from `Event::WorkflowRunFailed`; the event carries `failure: RunFailure`. +- Add workflow-local projection helpers: + - `run_failure_from_error(error, reason)` for rich `Error` values. + - `run_failure_from_outcome_failure(failure_detail, reason)` for failed `Outcome` values without an error. +- Preserve source chains in `fabro_workflow::Error` before projection: + - Replace rendered `causes: Vec` fields on `Engine` / `Handler` with `source: Option`. + - Make `engine_with_source` / `handler_with_source` take owned `impl Into`, not borrowed `&dyn Error`. + - Remove `Serialize` / `Deserialize` from `fabro_workflow::Error`; it is internal error transport, not durable wire data. + - Keep `Error::causes()` and `display_with_causes()` as boundary helpers backed by `source()`. +- Populate `RunFailure` as follows: + - `message`: concise public message; for `Engine` / `Handler`, use the stored message without the `"Engine error:"` / `"Handler error:"` prefix. + - `causes`: collected source chain strings. + - `category`: `error.failure_category()` or `FailureDetail.category`. + - `signature`: existing `FailureDetail.signature` or `error.failure_signature_hint()`, wrapped as `FailureSignature`. + - `system_actor`: from `FailureDetail.system_actor`; `None` for generic errors. + - `exec_output_tail`: `fabro_sandbox::default_redacted_output_tail(error)`. +- Update consumers to read the new nested shape: + - run projection status uses `props.failure.reason` + - conclusion stores `Some(props.failure.clone())` + - server managed-run summaries use `props.failure.message` + - CLI progress/output renders `failure.message` plus `failure.causes` where full diagnostics are appropriate + - event tracing logs failure metadata and tail byte/truncation metadata only, never tail contents + +## Test Plan + +- `fabro-types` serialization tests: + - `run.failed` serializes with nested `properties.failure`. + - no top-level `error`, `causes`, `reason`, or `git_commit_sha` remains. + - `exec_output_tail` and empty `causes` are omitted when absent. + - `Conclusion` serializes `failure: RunFailure`. +- `fabro-workflow` projection tests: + - owned source chains survive through `Error::engine_with_source` / `handler_with_source`. + - a nested sandbox exec error produces `failure.exec_output_tail`. + - raw stdout/stderr never appear in `failure.message`. + - secrets in exec output are redacted in the serialized event payload. + - failed `Outcome` maps its `FailureDetail` category, actor, and signature into `RunFailure`. +- Projection/consumer tests: + - run state projects failed status from `props.failure.reason`. + - conclusion renders full message plus causes for CLI output. + - server managed-run failure summaries continue showing a concise message. + - existing `run.failed` fixtures and inline snapshots are updated intentionally. +- Verification commands: + - `cargo nextest run -p fabro-types -p fabro-workflow -p fabro-store -p fabro-server -p fabro-cli` + - `cd apps/fabro-web && bun run typecheck` + - regenerate API client after OpenAPI edits, then typecheck affected TypeScript. diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 6c1eda3ba..47097a334 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -4904,6 +4904,67 @@ components: - bootstrap_failed - sandbox_init_failed + FailureCategory: + description: Product-level classification for grouping and retry policy. + type: string + enum: + - transient_infra + - deterministic + - budget_exhausted + - compilation_loop + - canceled + - structural + + FailureSignature: + description: Stable normalized signature for grouping related failures. + type: string + + ExecOutputTail: + description: Redacted tail of command stdout/stderr captured for diagnostics. + type: object + properties: + stdout: + type: ["string", "null"] + stderr: + type: ["string", "null"] + stdout_truncated: + type: boolean + default: false + stderr_truncated: + type: boolean + default: false + + RunFailure: + description: Rich terminal run failure diagnostics. + type: object + required: + - message + - reason + - category + properties: + message: + type: string + causes: + type: array + items: + type: string + reason: + $ref: "#/components/schemas/FailureReason" + category: + $ref: "#/components/schemas/FailureCategory" + system_actor: + oneOf: + - $ref: "#/components/schemas/SystemActorKind" + - type: "null" + signature: + oneOf: + - $ref: "#/components/schemas/FailureSignature" + - type: "null" + exec_output_tail: + oneOf: + - $ref: "#/components/schemas/ExecOutputTail" + - type: "null" + RunManifest: description: Self-contained workflow run manifest. type: object @@ -6402,8 +6463,10 @@ components: type: integer format: uint64 minimum: 0 - failure_reason: - type: ["string", "null"] + failure: + oneOf: + - $ref: "#/components/schemas/RunFailure" + - type: "null" final_git_commit_sha: type: ["string", "null"] stages: diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index e81126d4e..ff492c02a 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -189,6 +189,9 @@ fn main() { ("RunStatus", "fabro_types::status::RunStatus", &[]), ("SuccessReason", "fabro_types::status::SuccessReason", &[]), ("FailureReason", "fabro_types::status::FailureReason", &[]), + ("FailureCategory", "fabro_types::FailureCategory", &[]), + ("FailureSignature", "fabro_types::FailureSignature", &[]), + ("RunFailure", "fabro_types::RunFailure", &[]), ("BlockedReason", "fabro_types::status::BlockedReason", &[]), ( "RunControlAction", @@ -338,6 +341,7 @@ fn main() { ("SystemActorKind", "fabro_types::SystemActorKind", &[]), ("QuestionType", "fabro_types::QuestionType", &[]), ("StageCompletion", "fabro_types::StageCompletion", &[]), + ("Conclusion", "fabro_types::Conclusion", &[]), ("StageOutcome", "fabro_types::StageOutcome", &[]), ("StageHandler", "fabro_types::StageHandler", &[]), ("StageState", "fabro_types::StageState", &[]), @@ -356,6 +360,7 @@ fn main() { &[], ), ("BilledTokenCounts", "fabro_types::BilledTokenCounts", &[]), + ("ExecOutputTail", "fabro_types::ExecOutputTail", &[]), ("ProviderId", "fabro_model::ProviderId", &[]), ("Model", "fabro_model::Model", &[]), ("ModelLimits", "fabro_model::ModelLimits", &[]), diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index e5a6e62a7..035f3deb6 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -29,15 +29,15 @@ pub mod types { BlockedReason, FailureReason, RunControlAction, RunStatus, SuccessReason, }; pub use fabro_types::{ - AuthMethod, BilledTokenCounts, CommandTermination, DiffStats, DiffSummary, DirtyStatus, - EventEnvelope, GitContext, IdpIdentity, InterviewOption, InterviewQuestionRecord, - PendingInterviewRecord, PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, - QuestionType, RepositoryRef, Run, RunClientProvenance, RunEvent, RunProjection, - RunProvenance, RunSandbox, RunSandboxRuntime, RunServerProvenance, SandboxDetails, - SandboxProvider, SandboxResources, SandboxService, SandboxServiceListResponse, - SandboxState, SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, - StageCompletion, StageHandler, StageOutcome, StageProjection, StageState, SystemActorKind, - UserPrincipal, WorkflowSettings, + AuthMethod, BilledTokenCounts, CommandTermination, Conclusion, DiffStats, DiffSummary, + DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, FailureSignature, GitContext, + IdpIdentity, InterviewOption, InterviewQuestionRecord, PendingInterviewRecord, + PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, QuestionType, RepositoryRef, + Run, RunClientProvenance, RunEvent, RunFailure, RunProjection, RunProvenance, RunSandbox, + RunSandboxRuntime, RunServerProvenance, SandboxDetails, SandboxProvider, SandboxResources, + SandboxService, SandboxServiceListResponse, SandboxState, SandboxTimestamps, + SecretMetadata, SecretType, ServerSettings, StageCompletion, StageHandler, StageOutcome, + StageProjection, StageState, SystemActorKind, UserPrincipal, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/run_failure_round_trip.rs b/lib/crates/fabro-api/tests/run_failure_round_trip.rs new file mode 100644 index 000000000..418688c09 --- /dev/null +++ b/lib/crates/fabro-api/tests/run_failure_round_trip.rs @@ -0,0 +1,108 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::{ + Conclusion as ApiConclusion, ExecOutputTail as ApiExecOutputTail, + FailureCategory as ApiFailureCategory, FailureSignature as ApiFailureSignature, + RunFailure as ApiRunFailure, +}; +use fabro_types::{ + Conclusion, ExecOutputTail, FailureCategory, FailureReason, FailureSignature, RunFailure, + StageOutcome, +}; +use serde::Serialize; +use serde_json::{Value, json}; + +#[test] +fn run_failure_family_reuses_domain_types() { + assert_same_type::(); + assert_same_type::(); + assert_same_type::(); + assert_same_type::(); + assert_same_type::(); +} + +#[test] +fn run_failure_json_matches_openapi_shape() { + assert_json( + RunFailure { + message: "Failed to initialize sandbox".to_string(), + causes: vec!["connection refused".to_string()], + reason: FailureReason::SandboxInitFailed, + category: FailureCategory::TransientInfra, + system_actor: None, + signature: Some(FailureSignature("init|transient_infra|docker".to_string())), + exec_output_tail: Some(ExecOutputTail { + stdout: None, + stderr: Some("last stderr line".to_string()), + stdout_truncated: false, + stderr_truncated: true, + }), + }, + json!({ + "message": "Failed to initialize sandbox", + "causes": ["connection refused"], + "reason": "sandbox_init_failed", + "category": "transient_infra", + "signature": "init|transient_infra|docker", + "exec_output_tail": { + "stderr": "last stderr line", + "stderr_truncated": true + } + }), + ); +} + +#[test] +fn conclusion_json_uses_failure_object() { + assert_json( + Conclusion { + timestamp: chrono::DateTime::parse_from_rfc3339("2026-05-13T12:00:00Z") + .unwrap() + .with_timezone(&chrono::Utc), + status: StageOutcome::Failed { + retry_requested: false, + }, + duration_ms: 42, + failure: Some(RunFailure { + message: "boom".to_string(), + causes: Vec::new(), + reason: FailureReason::WorkflowError, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }), + final_git_commit_sha: None, + stages: Vec::new(), + billing: None, + total_retries: 0, + diff: Default::default(), + }, + json!({ + "timestamp": "2026-05-13T12:00:00Z", + "status": "failed", + "duration_ms": 42, + "failure": { + "message": "boom", + "reason": "workflow_error", + "category": "deterministic" + }, + "total_retries": 0, + "diff": {} + }), + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should reuse {}", + type_name::(), + type_name::() + ); +} + +fn assert_json(value: T, expected: Value) { + assert_eq!(serde_json::to_value(value).unwrap(), expected); +} diff --git a/lib/crates/fabro-cli/src/commands/run/events.rs b/lib/crates/fabro-cli/src/commands/run/events.rs index e87da50e9..b6cc09f1c 100644 --- a/lib/crates/fabro-cli/src/commands/run/events.rs +++ b/lib/crates/fabro-cli/src/commands/run/events.rs @@ -454,7 +454,10 @@ fn format_event_pretty_value(envelope: &serde_json::Value, styles: &Styles) -> O Some(lines.join("\n")) } "run.failed" => { - let error = prop_str_field(envelope, "error").unwrap_or("unknown error"); + let error = prop_field(envelope, "failure") + .and_then(|failure| failure.get("message")) + .and_then(serde_json::Value::as_str) + .unwrap_or("unknown error"); Some(format!( "{} {} {}", styles.dim.apply_to(&ts), @@ -1260,7 +1263,7 @@ mod tests { #[test] fn pretty_workflow_run_failed() { let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.failed","properties":{"error":"sandbox timeout"}}"#; + let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.failed","properties":{"failure":{"message":"sandbox timeout","reason":"workflow_error","category":"deterministic"}}}"#; let result = format_event_pretty(line, &styles).unwrap(); assert!(result.contains("Failed"), "got: {result}"); assert!(result.contains("sandbox timeout"), "got: {result}"); diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index d23aa2704..9278dd9b4 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -7,6 +7,7 @@ use cli_table::{Cell, CellStruct, Style, Table}; use fabro_api::types; use fabro_types::{PullRequestRecord, RunBlobId, RunId, parse_blob_ref}; use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; +use fabro_util::error::render_with_causes; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use fabro_util::text::strip_goal_decoration; @@ -242,8 +243,9 @@ pub(crate) fn print_run_conclusion( } } - if let Some(ref failure) = conclusion.failure_reason { - fabro_util::printerr!(printer, "Failure: {}", styles.red.apply_to(failure)); + if let Some(ref failure) = conclusion.failure { + let rendered = render_with_causes(&failure.message, &failure.causes); + fabro_util::printerr!(printer, "Failure: {}", styles.red.apply_to(rendered)); } if pushed_branch.is_some() || pr_url.is_some() { diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index ca20d7a47..4b31c923b 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -508,7 +508,7 @@ fn worker_title_phase_for_event(body: &EventBody) -> Option { } EventBody::RunPaused(_) => Some(WorkerTitlePhase::Paused), EventBody::RunCompleted(_) => Some(WorkerTitlePhase::Succeeded), - EventBody::RunFailed(props) => Some(if props.reason == FailureReason::Cancelled { + EventBody::RunFailed(props) => Some(if props.failure.reason == FailureReason::Cancelled { WorkerTitlePhase::Cancelled } else { WorkerTitlePhase::Failed @@ -651,8 +651,8 @@ mod tests { RunFailedProps, RunStatusTransitionProps, }; use fabro_types::{ - AuthMethod, EventBody, FailureReason, IdpIdentity, Principal, QuestionType, SuccessReason, - fixtures, + AuthMethod, EventBody, FailureCategory, FailureReason, IdpIdentity, Principal, + QuestionType, RunFailure, SuccessReason, fixtures, }; use fabro_vault::{SecretType, Vault}; use fabro_workflow::event::RunEventSink; @@ -777,25 +777,39 @@ mod tests { ); assert_eq!( worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps { - error: "cancelled".to_string(), - causes: Vec::new(), - duration_ms: 10, - reason: FailureReason::Cancelled, - git_commit_sha: None, - final_patch: None, - diff_summary: None, + failure: RunFailure { + message: "cancelled".to_string(), + causes: Vec::new(), + reason: FailureReason::Cancelled, + category: FailureCategory::Canceled, + system_actor: None, + signature: None, + exec_output_tail: None, + }, + duration_ms: 10, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, })), Some(WorkerTitlePhase::Cancelled) ); assert_eq!( worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps { - error: "boom".to_string(), - causes: Vec::new(), - duration_ms: 10, - reason: FailureReason::Terminated, - git_commit_sha: None, - final_patch: None, - diff_summary: None, + failure: RunFailure { + message: "boom".to_string(), + causes: Vec::new(), + reason: FailureReason::Terminated, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }, + duration_ms: 10, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, })), Some(WorkerTitlePhase::Failed) ); diff --git a/lib/crates/fabro-cli/src/commands/run/wait.rs b/lib/crates/fabro-cli/src/commands/run/wait.rs index a2d5bf148..6bde50d83 100644 --- a/lib/crates/fabro-cli/src/commands/run/wait.rs +++ b/lib/crates/fabro-cli/src/commands/run/wait.rs @@ -135,7 +135,8 @@ fn print_human_output( #[cfg(test)] mod tests { use fabro_types::{ - BilledTokenCounts, RunDiff, RunStatus, StageOutcome, SuccessReason, fixtures, + BilledTokenCounts, FailureCategory, FailureReason, RunDiff, RunFailure, RunStatus, + StageOutcome, SuccessReason, fixtures, }; use fabro_workflow::records::Conclusion; @@ -152,7 +153,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 12345, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: vec![], billing: Some(BilledTokenCounts { @@ -211,7 +212,15 @@ mod tests { retry_requested: false, }, duration_ms: 500, - failure_reason: Some("error".into()), + failure: Some(RunFailure { + message: "error".into(), + causes: Vec::new(), + reason: FailureReason::WorkflowError, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }), final_git_commit_sha: None, stages: vec![], billing: None, @@ -237,7 +246,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 8000, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: vec![], billing: Some(BilledTokenCounts { diff --git a/lib/crates/fabro-dump/src/lib.rs b/lib/crates/fabro-dump/src/lib.rs index 06505dca1..8fed39198 100644 --- a/lib/crates/fabro-dump/src/lib.rs +++ b/lib/crates/fabro-dump/src/lib.rs @@ -549,7 +549,7 @@ mod tests { .unwrap(), status: StageOutcome::Succeeded, duration_ms: 5, - failure_reason: None, + failure: None, final_git_commit_sha: Some("abc123".to_string()), stages: Vec::new(), billing: None, diff --git a/lib/crates/fabro-server/src/run_files.rs b/lib/crates/fabro-server/src/run_files.rs index 8dac630a1..d93c02d69 100644 --- a/lib/crates/fabro-server/src/run_files.rs +++ b/lib/crates/fabro-server/src/run_files.rs @@ -2391,7 +2391,7 @@ index 1111111..2222222 160000 timestamp: chrono::Utc::now(), status: fabro_types::StageOutcome::Succeeded, duration_ms: 1, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index aaba95dac..3a41c0959 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -2082,19 +2082,10 @@ pub(crate) async fn reconcile_incomplete_runs_on_startup( summary.lifecycle.pending_control, "Fabro server restarted before the run reached a terminal state.".to_string(), ); - workflow_event::append_event( - &run_store, - &summary.id, - &workflow_event::Event::WorkflowRunFailed { - error, - duration_ms: 0, - reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await?; + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &error, 0, reason, None, None, None, None, + ); + workflow_event::append_event(&run_store, &summary.id, &failure_event).await?; reconciled += 1; } @@ -2136,19 +2127,10 @@ async fn persist_shutdown_run_failures( run_state.pending_control, "Fabro server shut down before the run reached a terminal state.".to_string(), ); - workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error, - duration_ms: 0, - reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await?; + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &error, 0, reason, None, None, None, None, + ); + workflow_event::append_event(&run_store, &run_id, &failure_event).await?; } Ok(()) @@ -2216,19 +2198,16 @@ async fn persist_cancelled_run_status(state: &AppState, run_id: RunId) -> anyhow return Ok(()); } - workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::Cancelled, - duration_ms: 0, - reason: FailureReason::Cancelled, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::Cancelled, + 0, + FailureReason::Cancelled, + None, + None, + None, + None, + ); + workflow_event::append_event(&run_store, &run_id, &failure_event).await } async fn finish_cancelled_run_before_execution(state: &Arc, run_id: RunId) { @@ -2255,19 +2234,17 @@ async fn fail_run_before_execution( ) { match state.store.open_run(&run_id).await { Ok(run_store) => { - if let Err(err) = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(message.clone()), - duration_ms: 0, - reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine(message.clone()), + 0, + reason, + None, + None, + None, + None, + ); + if let Err(err) = + workflow_event::append_event(&run_store, &run_id, &failure_event).await { error!(run_id = %run_id, error = %err, "Failed to persist run failure status"); } @@ -2427,9 +2404,9 @@ fn update_live_run_from_event(state: &AppState, run_id: RunId, event: &RunEvent) } EventBody::RunFailed(props) => { managed_run.status = RunStatus::Failed { - reason: props.reason, + reason: props.failure.reason, }; - managed_run.error = Some(props.error.clone()); + managed_run.error = Some(props.failure.message.clone()); managed_run.active_api_stages.clear(); managed_run.active_non_steerable_agent_stages.clear(); } @@ -2540,21 +2517,11 @@ async fn append_worker_exit_failure( state.pending_control, format!("Worker exited before emitting a terminal run event: {wait_status}"), ); + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &error, 0, reason, None, None, None, None, + ); - if let Err(err) = workflow_event::append_event( - run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error, - duration_ms: 0, - reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await - { + if let Err(err) = workflow_event::append_event(run_store, &run_id, &failure_event).await { tracing::warn!(run_id = %run_id, error = %err, "Failed to append worker exit failure"); } } @@ -3202,25 +3169,18 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { Ok(child) => child, Err(err) => { tracing::error!(run_id = %run_id, error = %err, "Failed to spawn worker"); - let _ = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(err.to_string()), - duration_ms: 0, - reason: FailureReason::LaunchFailed, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await; - fail_managed_run( - &state, - run_id, + let message = format!("Failed to spawn worker: {err}"); + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine_with_anyhow("Failed to spawn worker", err), + 0, FailureReason::LaunchFailed, - format!("Failed to spawn worker: {err}"), + None, + None, + None, + None, ); + let _ = workflow_event::append_event(&run_store, &run_id, &failure_event).await; + fail_managed_run(&state, run_id, FailureReason::LaunchFailed, message); state.scheduler_notify.notify_one(); return; } @@ -3230,19 +3190,16 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { let message = "Worker process did not report a PID".to_string(); tracing::error!(run_id = %run_id, "{message}"); let _ = child.start_kill(); - let _ = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(message.clone()), - duration_ms: 0, - reason: FailureReason::LaunchFailed, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await; + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine(message.clone()), + 0, + FailureReason::LaunchFailed, + None, + None, + None, + None, + ); + let _ = workflow_event::append_event(&run_store, &run_id, &failure_event).await; fail_managed_run(&state, run_id, FailureReason::LaunchFailed, message); state.scheduler_notify.notify_one(); return; @@ -3261,19 +3218,16 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { let message = "Worker stdin pipe was unavailable".to_string(); tracing::error!(run_id = %run_id, "{message}"); let _ = child.start_kill(); - let _ = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(message.clone()), - duration_ms: 0, - reason: FailureReason::LaunchFailed, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await; + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine(message.clone()), + 0, + FailureReason::LaunchFailed, + None, + None, + None, + None, + ); + let _ = workflow_event::append_event(&run_store, &run_id, &failure_event).await; fail_managed_run(&state, run_id, FailureReason::LaunchFailed, message); state.scheduler_notify.notify_one(); return; @@ -3283,19 +3237,16 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { let message = "Worker stderr pipe was unavailable".to_string(); tracing::error!(run_id = %run_id, "{message}"); let _ = child.start_kill(); - let _ = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(message.clone()), - duration_ms: 0, - reason: FailureReason::LaunchFailed, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await; + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine(message.clone()), + 0, + FailureReason::LaunchFailed, + None, + None, + None, + None, + ); + let _ = workflow_event::append_event(&run_store, &run_id, &failure_event).await; fail_managed_run(&state, run_id, FailureReason::LaunchFailed, message); state.scheduler_notify.notify_one(); return; @@ -3316,26 +3267,19 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { Ok(status) => status, Err(err) => { tracing::error!(run_id = %run_id, error = %err, "Failed while waiting on worker"); + let message = format!("Worker wait failed: {err}"); let _ = child.start_kill(); - let _ = workflow_event::append_event( - &run_store, - &run_id, - &workflow_event::Event::WorkflowRunFailed { - error: WorkflowError::engine(err.to_string()), - duration_ms: 0, - reason: FailureReason::Terminated, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }, - ) - .await; - fail_managed_run( - &state, - run_id, + let failure_event = workflow_event::Event::workflow_run_failed_from_error( + &WorkflowError::engine_with_source("Worker wait failed", err), + 0, FailureReason::Terminated, - format!("Worker wait failed: {err}"), + None, + None, + None, + None, ); + let _ = workflow_event::append_event(&run_store, &run_id, &failure_event).await; + fail_managed_run(&state, run_id, FailureReason::Terminated, message); state.scheduler_notify.notify_one(); return; } @@ -3408,7 +3352,12 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { managed_run.error = final_state .conclusion .as_ref() - .and_then(|conclusion| conclusion.failure_reason.clone()) + .and_then(|conclusion| { + conclusion + .failure + .as_ref() + .map(|failure| failure.message.clone()) + }) .or_else(|| managed_run.error.clone()); managed_run.checkpoint = final_state.current_checkpoint().cloned(); managed_run.run_dir = Some(run_dir); diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index b072e7509..00d3a2090 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -2332,7 +2332,7 @@ async fn persist_cancelled_run_status_ignores_already_terminal_runs() { assert!(!run_store.list_events().await.unwrap().iter().any(|event| { matches!( event.event.body, - EventBody::RunFailed(ref props) if props.reason == FailureReason::Cancelled + EventBody::RunFailed(ref props) if props.failure.reason == FailureReason::Cancelled ) })); } diff --git a/lib/crates/fabro-server/tests/it/scenario/lifecycle.rs b/lib/crates/fabro-server/tests/it/scenario/lifecycle.rs index d4c5c1583..e67a5e2fc 100644 --- a/lib/crates/fabro-server/tests/it/scenario/lifecycle.rs +++ b/lib/crates/fabro-server/tests/it/scenario/lifecycle.rs @@ -350,10 +350,12 @@ async fn cancel_at_human_gate_persists_cancelled_terminal_event() { .filter(|&event| event["event"] == "run.failed") .map(|event| { ( - event["properties"]["reason"] + event["properties"]["failure"]["reason"] + .as_str() + .map(ToOwned::to_owned), + event["properties"]["failure"]["message"] .as_str() .map(ToOwned::to_owned), - event["properties"]["error"].as_str().map(ToOwned::to_owned), ) }) .collect::>(); diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 176b32c5a..973b94aec 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -16,7 +16,6 @@ use fabro_types::{ RunStatus, RunSummary, RunTimestamps, SandboxProvider, StageCompletion, StageHandler, StageId, StageOutcome, StageProjection, StageState, StartRecord, WorkflowRef, first_event_seq, }; -use fabro_util::error::render_with_causes; use serde_json::Value; use crate::{Error, EventEnvelope, Result}; @@ -149,7 +148,7 @@ impl RunProjectionReducer for RunProjection { EventBody::RunFailed(props) => { self.try_apply_status( RunStatus::Failed { - reason: props.reason, + reason: props.failure.reason, }, ts, )?; @@ -779,7 +778,7 @@ fn conclusion_from_completed( status: StageOutcome::from_str(&props.status) .map_err(|err| Error::InvalidEvent(format!("invalid completed stage status: {err}")))?, duration_ms: props.duration_ms, - failure_reason: None, + failure: None, final_git_commit_sha: props.final_git_commit_sha.clone(), stages: Vec::new(), billing: props.billing.clone(), @@ -798,10 +797,10 @@ fn conclusion_from_failed(props: &RunFailedProps, timestamp: DateTime) -> C retry_requested: false, }, duration_ms: props.duration_ms, - failure_reason: Some(render_with_causes(&props.error, &props.causes)), - final_git_commit_sha: props.git_commit_sha.clone(), + failure: Some(props.failure.clone()), + final_git_commit_sha: props.final_git_commit_sha.clone(), stages: Vec::new(), - billing: None, + billing: props.billing.clone(), total_retries: 0, diff: RunDiff { patch: props.final_patch.clone(), @@ -2181,13 +2180,20 @@ mod tests { .apply_event(&test_event( 1, EventBody::RunFailed(RunFailedProps { - error: "boom".to_string(), - causes: Vec::new(), - duration_ms: 42, - reason: FailureReason::WorkflowError, - git_commit_sha: Some("abc123".to_string()), - final_patch: Some(patch.to_string()), - diff_summary: None, + failure: fabro_types::RunFailure { + message: "boom".to_string(), + causes: Vec::new(), + reason: FailureReason::WorkflowError, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }, + duration_ms: 42, + final_git_commit_sha: Some("abc123".to_string()), + final_patch: Some(patch.to_string()), + diff_summary: None, + billing: None, }), None, )) @@ -2294,9 +2300,12 @@ mod tests { 3, "run.failed", &json!({ - "error": "boom", + "failure": { + "message": "boom", + "reason": "workflow_error", + "category": "deterministic" + }, "duration_ms": 42, - "reason": "workflow_error", "diff_summary": { "files_changed": 5, "additions": 20, @@ -2323,27 +2332,71 @@ mod tests { .apply_event(&test_event( 1, EventBody::RunFailed(RunFailedProps { - error: "Engine error: Failed to initialize sandbox".to_string(), - causes: vec![ - "Failed to pull Docker image buildpack-deps:noble".to_string(), - "connection refused".to_string(), - ], - duration_ms: 42, - reason: FailureReason::WorkflowError, - git_commit_sha: None, - final_patch: None, - diff_summary: None, + failure: fabro_types::RunFailure { + message: "Failed to initialize sandbox".to_string(), + causes: vec![ + "Failed to pull Docker image buildpack-deps:noble".to_string(), + "connection refused".to_string(), + ], + reason: FailureReason::WorkflowError, + category: FailureCategory::TransientInfra, + system_actor: None, + signature: None, + exec_output_tail: None, + }, + duration_ms: 42, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, }), None, )) .unwrap(); - assert_eq!( - state.conclusion.unwrap().failure_reason.as_deref(), - Some( - "Engine error: Failed to initialize sandbox\n caused by: Failed to pull Docker image buildpack-deps:noble\n caused by: connection refused" - ) - ); + let failure = state.conclusion.unwrap().failure.unwrap(); + assert_eq!(failure.message, "Failed to initialize sandbox"); + assert_eq!(failure.causes, vec![ + "Failed to pull Docker image buildpack-deps:noble".to_string(), + "connection refused".to_string(), + ]); + } + + #[test] + fn run_failed_projection_uses_nested_failure_reason_and_conclusion() { + let mut state = running_projection(); + let failure = fabro_types::RunFailure { + message: "Failed to initialize sandbox".to_string(), + causes: vec!["connection refused".to_string()], + reason: FailureReason::SandboxInitFailed, + category: FailureCategory::TransientInfra, + system_actor: Some(fabro_types::SystemActorKind::Engine), + signature: Some(fabro_types::FailureSignature( + "init|transient_infra|docker".to_string(), + )), + exec_output_tail: None, + }; + state + .apply_event(&test_event( + 1, + EventBody::RunFailed(RunFailedProps { + failure: failure.clone(), + duration_ms: 42, + final_git_commit_sha: Some("abc123".to_string()), + final_patch: None, + diff_summary: None, + billing: None, + }), + None, + )) + .unwrap(); + + assert_eq!(state.status, RunStatus::Failed { + reason: FailureReason::SandboxInitFailed, + }); + let conclusion = state.conclusion.unwrap(); + assert_eq!(conclusion.failure, Some(failure)); + assert_eq!(conclusion.final_git_commit_sha.as_deref(), Some("abc123")); } #[test] diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index 7e90f2603..d94762152 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -724,9 +724,12 @@ mod tests { "2026-03-27T12:00:07Z", "run.failed", &serde_json::json!({ - "error": "cancelled", + "failure": { + "message": "cancelled", + "reason": "cancelled", + "category": "canceled" + }, "duration_ms": 1, - "reason": "cancelled", }), )) .await @@ -1122,9 +1125,12 @@ mod tests { "2026-03-27T12:00:04Z", "run.failed", &serde_json::json!({ - "error": "workflow failed", + "failure": { + "message": "workflow failed", + "reason": "workflow_error", + "category": "deterministic" + }, "duration_ms": 1, - "reason": "workflow_error", }), )) .await diff --git a/lib/crates/fabro-types/src/conclusion.rs b/lib/crates/fabro-types/src/conclusion.rs index 50d1eaa4d..9cced24bb 100644 --- a/lib/crates/fabro-types/src/conclusion.rs +++ b/lib/crates/fabro-types/src/conclusion.rs @@ -2,7 +2,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use crate::outcome::StageOutcome; -use crate::{BilledTokenCounts, RunDiff}; +use crate::{BilledTokenCounts, RunDiff, RunFailure}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct StageSummary { @@ -20,7 +20,7 @@ pub struct Conclusion { pub status: StageOutcome, pub duration_ms: u64, #[serde(default, skip_serializing_if = "Option::is_none")] - pub failure_reason: Option, + pub failure: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub final_git_commit_sha: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index e4da87ec6..7b11683b5 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -21,6 +21,7 @@ pub mod repository; pub mod run; pub mod run_blob_id; pub mod run_event; +pub mod run_failure; pub mod run_id; pub mod run_projection; pub mod run_sandbox; @@ -78,6 +79,7 @@ pub use run_event::{ EventBody, ExecOutputTail, InterviewOption, MetadataSnapshotFailureKind, MetadataSnapshotPhase, RunEvent, RunNoticeCode, RunNoticeLevel, SessionCapability, }; +pub use run_failure::RunFailure; pub use run_id::{RunId, fixtures}; pub use run_projection::{ CheckpointRecord, PendingInterviewRecord, RunProjection, StageProjection, first_event_seq, diff --git a/lib/crates/fabro-types/src/run_event/mod.rs b/lib/crates/fabro-types/src/run_event/mod.rs index b473ad6b4..2c46231ed 100644 --- a/lib/crates/fabro-types/src/run_event/mod.rs +++ b/lib/crates/fabro-types/src/run_event/mod.rs @@ -1086,9 +1086,12 @@ mod tests { ( "run.failed", json!({ - "error": "boom", + "failure": { + "message": "boom", + "reason": "workflow_error", + "category": "deterministic" + }, "duration_ms": 42, - "reason": "workflow_error", "diff_summary": { "files_changed": 2, "additions": 10, diff --git a/lib/crates/fabro-types/src/run_event/run.rs b/lib/crates/fabro-types/src/run_event/run.rs index 3b932883e..123d89c39 100644 --- a/lib/crates/fabro-types/src/run_event/run.rs +++ b/lib/crates/fabro-types/src/run_event/run.rs @@ -3,9 +3,9 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; use super::{BilledTokenCounts, ExecOutputTail, RunNoticeLevel}; -use crate::status::{BlockedReason, FailureReason, SuccessReason}; +use crate::status::{BlockedReason, SuccessReason}; use crate::{ - DiffSummary, ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunId, + DiffSummary, ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunFailure, RunId, RunProvenance, WorkflowSettings, }; @@ -151,19 +151,16 @@ pub struct RunCompletedProps { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunFailedProps { - pub error: String, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub causes: Vec, - pub duration_ms: u64, - pub reason: FailureReason, + pub failure: RunFailure, + pub duration_ms: u64, #[serde(default, skip_serializing_if = "Option::is_none")] - pub git_commit_sha: Option, - // Optional unified-patch text captured at run end. Additive for back-compat: - // pre-change events replay with `final_patch: None` via serde default. + pub final_git_commit_sha: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub final_patch: Option, + pub final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub diff_summary: Option, + pub diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub billing: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_failure.rs b/lib/crates/fabro-types/src/run_failure.rs new file mode 100644 index 000000000..f519063b5 --- /dev/null +++ b/lib/crates/fabro-types/src/run_failure.rs @@ -0,0 +1,18 @@ +use serde::{Deserialize, Serialize}; + +use crate::{ExecOutputTail, FailureCategory, FailureReason, FailureSignature, SystemActorKind}; + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct RunFailure { + pub message: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub causes: Vec, + pub reason: FailureReason, + pub category: FailureCategory, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub system_actor: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub signature: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub exec_output_tail: Option, +} diff --git a/lib/crates/fabro-types/tests/run_failure_serde.rs b/lib/crates/fabro-types/tests/run_failure_serde.rs new file mode 100644 index 000000000..a5bf63270 --- /dev/null +++ b/lib/crates/fabro-types/tests/run_failure_serde.rs @@ -0,0 +1,135 @@ +use fabro_types::run_event::run::RunFailedProps; +use fabro_types::{ + Conclusion, EventBody, ExecOutputTail, FailureCategory, FailureReason, FailureSignature, + RunDiff, RunFailure, StageOutcome, SystemActorKind, +}; +use serde_json::json; + +#[test] +fn run_failed_serializes_nested_failure_contract() { + let body = EventBody::RunFailed(RunFailedProps { + failure: RunFailure { + message: "Failed to initialize sandbox".to_string(), + causes: vec![ + "Failed to pull Docker image buildpack-deps:noble".to_string(), + "connection refused".to_string(), + ], + reason: FailureReason::SandboxInitFailed, + category: FailureCategory::TransientInfra, + system_actor: Some(SystemActorKind::Engine), + signature: Some(FailureSignature( + "init|transient_infra|docker-pull".to_string(), + )), + exec_output_tail: Some(ExecOutputTail { + stdout: Some("last stdout line".to_string()), + stderr: Some("last stderr line".to_string()), + stdout_truncated: false, + stderr_truncated: true, + }), + }, + duration_ms: 42, + final_git_commit_sha: Some("abc123".to_string()), + final_patch: Some("diff --git a/file b/file".to_string()), + diff_summary: None, + billing: None, + }); + + let value = serde_json::to_value(&body).expect("run.failed body should serialize"); + + assert_eq!(value["event"], "run.failed"); + assert_eq!( + value["properties"], + json!({ + "failure": { + "message": "Failed to initialize sandbox", + "causes": [ + "Failed to pull Docker image buildpack-deps:noble", + "connection refused" + ], + "reason": "sandbox_init_failed", + "category": "transient_infra", + "system_actor": "engine", + "signature": "init|transient_infra|docker-pull", + "exec_output_tail": { + "stdout": "last stdout line", + "stderr": "last stderr line", + "stderr_truncated": true + } + }, + "duration_ms": 42, + "final_git_commit_sha": "abc123", + "final_patch": "diff --git a/file b/file" + }) + ); + assert!(value["properties"].get("error").is_none()); + assert!(value["properties"].get("causes").is_none()); + assert!(value["properties"].get("reason").is_none()); + assert!(value["properties"].get("git_commit_sha").is_none()); +} + +#[test] +fn run_failed_omits_empty_failure_optional_fields() { + let body = EventBody::RunFailed(RunFailedProps { + failure: RunFailure { + message: "boom".to_string(), + causes: Vec::new(), + reason: FailureReason::WorkflowError, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }, + duration_ms: 1, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, + }); + + let value = serde_json::to_value(&body).expect("run.failed body should serialize"); + + assert_eq!( + value["properties"], + json!({ + "failure": { + "message": "boom", + "reason": "workflow_error", + "category": "deterministic" + }, + "duration_ms": 1 + }) + ); +} + +#[test] +fn conclusion_serializes_rich_failure() { + let conclusion = Conclusion { + timestamp: chrono::DateTime::parse_from_rfc3339("2026-05-13T12:00:00Z") + .unwrap() + .with_timezone(&chrono::Utc), + status: StageOutcome::Failed { + retry_requested: false, + }, + duration_ms: 42, + failure: Some(RunFailure { + message: "run failed".to_string(), + causes: vec!["leaf cause".to_string()], + reason: FailureReason::WorkflowError, + category: FailureCategory::Deterministic, + system_actor: None, + signature: None, + exec_output_tail: None, + }), + final_git_commit_sha: None, + stages: Vec::new(), + billing: None, + total_retries: 0, + diff: RunDiff::default(), + }; + + let value = serde_json::to_value(&conclusion).expect("conclusion should serialize"); + + assert_eq!(value["failure"]["message"], "run failed"); + assert_eq!(value["failure"]["causes"], json!(["leaf cause"])); + assert!(value.get("failure_reason").is_none()); +} diff --git a/lib/crates/fabro-workflow/src/artifact.rs b/lib/crates/fabro-workflow/src/artifact.rs index c991bf78c..510a37ab5 100644 --- a/lib/crates/fabro-workflow/src/artifact.rs +++ b/lib/crates/fabro-workflow/src/artifact.rs @@ -36,13 +36,13 @@ pub async fn offload_large_values( ) -> Result<()> { for value in updates.values_mut() { let bytes = serde_json::to_vec(&*value) - .map_err(|e| Error::engine_with_source("artifact serialize failed", &e))?; + .map_err(|e| Error::engine_with_source("artifact serialize failed", e))?; if bytes.len() > BLOB_OFFLOAD_THRESHOLD { let blob_id = run_store .write_blob(&bytes) .await - .map_err(|e| Error::engine_with_anyhow("artifact blob write failed", &e))?; + .map_err(|e| Error::engine_with_anyhow("artifact blob write failed", e))?; *value = Value::String(format_blob_ref(&blob_id)); } } @@ -171,10 +171,10 @@ pub async fn resolve_text_or_blob_ref_str( let bytes = run_store .read_blob(&blob_id) .await - .map_err(|e| Error::engine_with_anyhow("text blob read failed", &e))? + .map_err(|e| Error::engine_with_anyhow("text blob read failed", e))? .ok_or_else(|| Error::engine(format!("text blob missing: {blob_id}")))?; serde_json::from_slice::(&bytes) - .map_err(|e| Error::engine_with_source("text blob was not a JSON string", &e)) + .map_err(|e| Error::engine_with_source("text blob was not a JSON string", e)) } /// Sync artifact files to a remote sandbox. @@ -204,13 +204,13 @@ pub async fn sync_artifacts_to_env( Err(e) => { return Err(Error::engine_with_source( "failed to check artifact existence", - &e, + e, )); } } let content = fs::read_to_string(&local_path).await.map_err(|e| { - Error::engine_with_source(format!("failed to read local artifact {local_path}"), &e) + Error::engine_with_source(format!("failed to read local artifact {local_path}"), e) })?; let filename = std::path::Path::new(&local_path) @@ -222,7 +222,7 @@ pub async fn sync_artifacts_to_env( env.write_file(&remote_path, &content) .await - .map_err(|e| Error::engine_with_source("failed to write artifact to remote env", &e))?; + .map_err(|e| Error::engine_with_source("failed to write artifact to remote env", e))?; *value = Value::String(format!("{ARTIFACT_POINTER_PREFIX}{remote_path}")); } @@ -309,7 +309,7 @@ async fn materialize_blob_ref( let bytes = run_store .read_blob(blob_id) .await - .map_err(|e| Error::engine_with_anyhow("artifact blob read failed", &e))? + .map_err(|e| Error::engine_with_anyhow("artifact blob read failed", e))? .ok_or_else(|| Error::engine(format!("artifact blob missing: {blob_id}")))?; if is_local_execution(env, run_dir).await? { @@ -334,12 +334,12 @@ async fn materialize_blob_ref( if !env .file_exists(&remote_path) .await - .map_err(|e| Error::engine_with_source("failed to check blob existence", &e))? + .map_err(|e| Error::engine_with_source("failed to check blob existence", e))? { let content = String::from_utf8(bytes.to_vec()) - .map_err(|e| Error::engine_with_source("artifact blob was not valid UTF-8 JSON", &e))?; + .map_err(|e| Error::engine_with_source("artifact blob was not valid UTF-8 JSON", e))?; env.write_file(&remote_path, &content).await.map_err(|e| { - Error::engine_with_source("failed to write artifact blob to sandbox", &e) + Error::engine_with_source("failed to write artifact blob to sandbox", e) })?; } @@ -354,13 +354,13 @@ async fn resolve_explicit_file_ref(value: &str, env: &dyn Sandbox) -> Result Result Result Result { env.file_exists(&run_dir.to_string_lossy()) .await - .map_err(|e| Error::engine_with_source("failed to inspect sandbox locality", &e)) + .map_err(|e| Error::engine_with_source("failed to inspect sandbox locality", e)) } fn local_materialized_blob_path(run_dir: &Path, blob_id: &RunBlobId) -> PathBuf { diff --git a/lib/crates/fabro-workflow/src/command_log.rs b/lib/crates/fabro-workflow/src/command_log.rs index 0c6519660..67c4373e1 100644 --- a/lib/crates/fabro-workflow/src/command_log.rs +++ b/lib/crates/fabro-workflow/src/command_log.rs @@ -115,10 +115,10 @@ pub async fn read_json_string_blob( let bytes = run_store .read_blob(&blob_id) .await - .map_err(|err| Error::engine_with_anyhow("command log blob read failed", &err))? + .map_err(|err| Error::engine_with_anyhow("command log blob read failed", err))? .ok_or_else(|| Error::engine(format!("command log blob missing: {blob_id}")))?; let text = serde_json::from_slice::(&bytes) - .map_err(|err| Error::engine_with_source("command log blob was not a JSON string", &err))?; + .map_err(|err| Error::engine_with_source("command log blob was not a JSON string", err))?; Ok(Some(text)) } @@ -154,10 +154,10 @@ async fn remove_if_exists(path: &Path) -> Result<()> { async fn write_json_string_blob(run_store: &RunStoreHandle, text: &str) -> Result { let value = Value::String(text.to_string()); let bytes = serde_json::to_vec(&value) - .map_err(|err| Error::engine_with_source("command log JSON serialization failed", &err))?; + .map_err(|err| Error::engine_with_source("command log JSON serialization failed", err))?; let blob_id = run_store .write_blob(&bytes) .await - .map_err(|err| Error::engine_with_anyhow("command log blob write failed", &err))?; + .map_err(|err| Error::engine_with_anyhow("command log blob write failed", err))?; Ok(format_blob_ref(&blob_id)) } diff --git a/lib/crates/fabro-workflow/src/devcontainer_bridge.rs b/lib/crates/fabro-workflow/src/devcontainer_bridge.rs index 37e2cf255..5d9cc092d 100644 --- a/lib/crates/fabro-workflow/src/devcontainer_bridge.rs +++ b/lib/crates/fabro-workflow/src/devcontainer_bridge.rs @@ -184,7 +184,7 @@ async fn run_single_lifecycle_command( .exec_command(command, timeout_ms, None, None, Some(child_token.clone())) .await .map_err(|e| { - Error::engine_with_source(format!("Devcontainer {phase} command failed"), &e) + Error::engine_with_source(format!("Devcontainer {phase} command failed"), e) })?; if cancel_token.is_cancelled() { return Err(Error::Cancelled); diff --git a/lib/crates/fabro-workflow/src/error.rs b/lib/crates/fabro-workflow/src/error.rs index 187129b44..cd830eb05 100644 --- a/lib/crates/fabro-workflow/src/error.rs +++ b/lib/crates/fabro-workflow/src/error.rs @@ -2,9 +2,9 @@ use fabro_graphviz::Error as GraphvizError; use fabro_llm::{Error as LlmError, ProviderErrorKind}; pub use fabro_types::failure_signature::FailureSignature; pub use fabro_types::outcome::FailureCategory; -use fabro_util::error::{collect_causes, collect_chain, render_with_causes}; +use fabro_types::{FailureReason, RunFailure}; +use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes}; use fabro_validate::Diagnostic; -use serde::{Deserialize, Serialize}; use thiserror::Error as ThisError; use crate::outcome::{FailureDetail, Outcome, StageOutcome}; @@ -194,8 +194,7 @@ impl FailureSignatureExt for FailureSignature { } } -#[derive(ThisError, Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", content = "data", rename_all = "snake_case")] +#[derive(ThisError, Debug, Clone)] pub enum Error { #[error("Parse error: {0}")] Parse(String), @@ -210,16 +209,16 @@ pub enum Error { Engine { message: String, failure_class: FailureCategory, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - causes: Vec, + #[source] + source: Option, }, #[error("Handler error: {message}")] Handler { message: String, failure_class: FailureCategory, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - causes: Vec, + #[source] + source: Option, }, #[error("LLM error: {0}")] @@ -256,27 +255,28 @@ impl Error { Self::Handler { message, failure_class, - causes: Vec::new(), + source: None, } } pub fn handler_with_source( message: impl Into, - source: &(dyn std::error::Error + 'static), + source: impl Into, ) -> Self { let message = message.into(); - let causes = collect_chain(source); + let source = SharedError::new(source.into()); + let causes = collect_chain(&source); let rendered = render_with_causes(&message, &causes); let failure_class = classify_failure_reason(&rendered); Self::Handler { message, failure_class, - causes, + source: Some(source), } } - pub fn handler_with_anyhow(message: impl Into, source: &anyhow::Error) -> Self { - Self::handler_with_source(message, source.as_ref()) + pub fn handler_with_anyhow(message: impl Into, source: anyhow::Error) -> Self { + Self::handler_with_source(message, source) } /// Smart constructor for Engine errors. Classifies the failure reason @@ -287,33 +287,36 @@ impl Error { Self::Engine { message, failure_class, - causes: Vec::new(), + source: None, } } pub fn engine_with_source( message: impl Into, - source: &(dyn std::error::Error + 'static), + source: impl Into, ) -> Self { let message = message.into(); - let causes = collect_chain(source); + let source = SharedError::new(source.into()); + let causes = collect_chain(&source); let rendered = render_with_causes(&message, &causes); let failure_class = classify_failure_reason(&rendered); Self::Engine { message, failure_class, - causes, + source: Some(source), } } - pub fn engine_with_anyhow(message: impl Into, source: &anyhow::Error) -> Self { - Self::engine_with_source(message, source.as_ref()) + pub fn engine_with_anyhow(message: impl Into, source: anyhow::Error) -> Self { + Self::engine_with_source(message, source) } #[must_use] pub fn causes(&self) -> Vec { match self { - Self::Engine { causes, .. } | Self::Handler { causes, .. } => causes.clone(), + Self::Engine { source, .. } | Self::Handler { source, .. } => source + .as_ref() + .map_or_else(Vec::new, |source| collect_chain(source)), Self::Llm(err) => collect_causes(err), _ => Vec::new(), } @@ -396,6 +399,39 @@ impl Error { } } +#[must_use] +pub fn run_failure_from_error(error: &Error, reason: FailureReason) -> RunFailure { + let message = match error { + Error::Engine { message, .. } | Error::Handler { message, .. } => message.clone(), + _ => error.to_string(), + }; + RunFailure { + message, + causes: error.causes(), + reason, + category: error.failure_category(), + system_actor: None, + signature: error.failure_signature_hint().map(FailureSignature), + exec_output_tail: fabro_sandbox::default_redacted_output_tail(error), + } +} + +#[must_use] +pub fn run_failure_from_outcome_failure( + failure: &FailureDetail, + reason: FailureReason, +) -> RunFailure { + RunFailure { + message: failure.message.clone(), + causes: Vec::new(), + reason, + category: failure.category, + system_actor: failure.system_actor, + signature: failure.signature.clone().map(FailureSignature), + exec_output_tail: None, + } +} + impl From for Error { fn from(err: std::io::Error) -> Self { Self::Io(err.to_string()) @@ -431,13 +467,15 @@ impl From for Error { impl From for Error { fn from(err: fabro_checkpoint::MetadataError) -> Self { - let message = err.to_string(); match err { - fabro_checkpoint::MetadataError::Deserialize { + err @ fabro_checkpoint::MetadataError::Deserialize { entity: "checkpoint", .. - } => Self::Checkpoint(message), - _ => Self::engine(message), + } => Self::Checkpoint(err.to_string()), + err => { + let message = err.to_string(); + Self::engine_with_source(message, err) + } } } } @@ -520,7 +558,7 @@ mod tests { message: "Failed to pull Docker image buildpack-deps:noble", source: TestCause("connection refused"), }; - let err = Error::engine_with_source("Failed to initialize sandbox", &source); + let err = Error::engine_with_source("Failed to initialize sandbox", source); assert_eq!( err.to_string(), @@ -1707,14 +1745,10 @@ mod tests { } #[test] - fn handler_eager_classification_roundtrip() { + fn handler_eager_classification_survives_clone() { let err = Error::handler("connection refused"); - let json = serde_json::to_string(&err).unwrap(); - let deserialized: Error = serde_json::from_str(&json).unwrap(); - assert_eq!( - deserialized.failure_category(), - FailureCategory::TransientInfra - ); + let cloned = err.clone(); + assert_eq!(cloned.failure_category(), FailureCategory::TransientInfra); } #[test] @@ -1730,7 +1764,7 @@ mod tests { } #[test] - fn arc_error_serde_roundtrip_all_variants() { + fn error_clone_preserves_display_for_all_variants() { let errors: Vec = vec![ Error::Parse("bad".into()), Error::Validation("bad".into()), @@ -1755,10 +1789,8 @@ mod tests { Error::Io("io err".into()), Error::Cancelled, ]; - for err in &errors { - let json = serde_json::to_string(err).unwrap(); - let deserialized: Error = serde_json::from_str(&json).unwrap(); - assert_eq!(err.to_string(), deserialized.to_string()); + for err in errors { + assert_eq!(err.to_string(), err.clone().to_string()); } } @@ -1876,27 +1908,14 @@ mod tests { } #[test] - fn e2e_serde_stability_arc_error() { + fn e2e_run_failure_projection_uses_handler_error_shape() { let err = Error::handler("connection refused"); - let json = serde_json::to_string(&err).unwrap(); - let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + let failure = run_failure_from_error(&err, FailureReason::WorkflowError); - // Verify wire format - assert_eq!(v["type"], "handler"); - assert!( - v["data"]["message"] - .as_str() - .unwrap() - .contains("connection refused") - ); - assert_eq!(v["data"]["failure_class"], "transient_infra"); - - // Round-trip - let deserialized: Error = serde_json::from_str(&json).unwrap(); - assert_eq!( - deserialized.failure_category(), - FailureCategory::TransientInfra - ); + assert_eq!(failure.message, "connection refused"); + assert_eq!(failure.causes, Vec::::new()); + assert_eq!(failure.reason, FailureReason::WorkflowError); + assert_eq!(failure.category, FailureCategory::TransientInfra); } #[test] diff --git a/lib/crates/fabro-workflow/src/event/convert.rs b/lib/crates/fabro-workflow/src/event/convert.rs index da2af76fc..3ae8bbfaf 100644 --- a/lib/crates/fabro-workflow/src/event/convert.rs +++ b/lib/crates/fabro-workflow/src/event/convert.rs @@ -166,20 +166,19 @@ fn event_body_from_event(event: &Event) -> EventBody { billing: billing.clone(), }), Event::WorkflowRunFailed { - error, + failure, duration_ms, - reason, - git_commit_sha, + final_git_commit_sha, final_patch, diff_summary, + billing, } => EventBody::RunFailed(fabro_types::RunFailedProps { - error: error.to_string(), - causes: error.causes(), - duration_ms: *duration_ms, - reason: *reason, - git_commit_sha: git_commit_sha.clone(), - final_patch: final_patch.clone(), - diff_summary: *diff_summary, + failure: failure.clone(), + duration_ms: *duration_ms, + final_git_commit_sha: final_git_commit_sha.clone(), + final_patch: final_patch.clone(), + diff_summary: *diff_summary, + billing: billing.clone(), }), Event::RunNotice { level, @@ -1528,44 +1527,82 @@ mod tests { #[test] fn run_event_workflow_failure_uses_display_error() { - let stored = to_run_event(&fixtures::RUN_6, &Event::WorkflowRunFailed { - error: Error::handler("boom"), - duration_ms: 900, - reason: FailureReason::WorkflowError, - git_commit_sha: Some("abc123".to_string()), - final_patch: None, - diff_summary: None, - }); + let event = Event::workflow_run_failed_from_error( + &Error::handler("boom"), + 900, + FailureReason::WorkflowError, + Some("abc123".to_string()), + None, + None, + None, + ); + let stored = to_run_event(&fixtures::RUN_6, &event); assert_eq!(stored.event_name(), "run.failed"); let properties = stored.properties().unwrap(); - assert_eq!(properties["error"], "Handler error: boom"); + assert_eq!(properties["failure"]["message"], "boom"); assert_eq!(properties["duration_ms"], 900); } #[test] fn run_event_workflow_failure_serializes_causes() { let source = EventTestCause; - let stored = to_run_event(&fixtures::RUN_6, &Event::WorkflowRunFailed { - error: Error::engine_with_source("Failed to initialize sandbox", &source), - duration_ms: 900, - reason: FailureReason::WorkflowError, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }); + let event = Event::workflow_run_failed_from_error( + &Error::engine_with_source("Failed to initialize sandbox", source), + 900, + FailureReason::WorkflowError, + None, + None, + None, + None, + ); + let stored = to_run_event(&fixtures::RUN_6, &event); let properties = stored.properties().unwrap(); assert_eq!( - properties["error"], - "Engine error: Failed to initialize sandbox" + properties["failure"]["message"], + "Failed to initialize sandbox" ); assert_eq!( - properties["causes"], + properties["failure"]["causes"], serde_json::json!(["connection refused"]) ); } + #[test] + fn run_event_workflow_failure_projects_nested_failure_contract() { + let source = EventTestCause; + let event = Event::workflow_run_failed_from_error( + &Error::engine_with_source("Failed to initialize sandbox", source), + 900, + FailureReason::SandboxInitFailed, + Some("abc123".to_string()), + None, + None, + None, + ); + let stored = to_run_event(&fixtures::RUN_6, &event); + + assert_eq!(stored.event_name(), "run.failed"); + let properties = stored.properties().unwrap(); + assert_eq!( + properties["failure"]["message"], + "Failed to initialize sandbox" + ); + assert_eq!( + properties["failure"]["causes"], + serde_json::json!(["connection refused"]) + ); + assert_eq!(properties["failure"]["reason"], "sandbox_init_failed"); + assert_eq!(properties["failure"]["category"], "transient_infra"); + assert_eq!(properties["duration_ms"], 900); + assert_eq!(properties["final_git_commit_sha"], "abc123"); + assert!(properties.get("error").is_none()); + assert!(properties.get("causes").is_none()); + assert!(properties.get("reason").is_none()); + assert!(properties.get("git_commit_sha").is_none()); + } + #[test] fn stage_started_populates_parallel_ids_when_present() { let stored = to_run_event_at( diff --git a/lib/crates/fabro-workflow/src/event/events.rs b/lib/crates/fabro-workflow/src/event/events.rs index f2067c5f4..3de11ce1b 100644 --- a/lib/crates/fabro-workflow/src/event/events.rs +++ b/lib/crates/fabro-workflow/src/event/events.rs @@ -2,14 +2,14 @@ use std::collections::BTreeMap; use ::fabro_types::{ BilledTokenCounts, BlockedReason, CommandTermination, DiffSummary, FailureReason, - ForkSourceRef, GitContext, ParallelBranchId, Principal, PullRequestRecord, RunBlobId, RunId, - RunNoticeLevel, RunProvenance, SandboxProvider, StageId, SuccessReason, + ForkSourceRef, GitContext, ParallelBranchId, Principal, PullRequestRecord, RunBlobId, + RunFailure, RunId, RunNoticeLevel, RunProvenance, SandboxProvider, StageId, SuccessReason, run_event as fabro_types, }; use fabro_agent::{AgentEvent, SandboxEvent}; use serde::{Deserialize, Serialize}; -use crate::error::Error; +use crate::error::{Error, run_failure_from_error}; use crate::outcome::{BilledModelUsage, FailureDetail, Outcome}; /// Events emitted during workflow run execution for observability. @@ -133,15 +133,16 @@ pub enum Event { billing: Option, }, WorkflowRunFailed { - error: Error, - duration_ms: u64, - reason: FailureReason, + failure: RunFailure, + duration_ms: u64, #[serde(default, skip_serializing_if = "Option::is_none")] - git_commit_sha: Option, + final_git_commit_sha: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - final_patch: Option, + final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - diff_summary: Option, + diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + billing: Option, }, RunNotice { level: RunNoticeLevel, @@ -680,6 +681,26 @@ pub enum Event { } impl Event { + #[must_use] + pub fn workflow_run_failed_from_error( + error: &Error, + duration_ms: u64, + reason: FailureReason, + final_git_commit_sha: Option, + final_patch: Option, + diff_summary: Option, + billing: Option, + ) -> Self { + Self::WorkflowRunFailed { + failure: run_failure_from_error(error, reason), + duration_ms, + final_git_commit_sha, + final_patch, + diff_summary, + billing, + } + } + pub fn pull_request_created(record: &PullRequestRecord, draft: bool) -> Self { Self::PullRequestCreated { pr_url: record.html_url.clone(), @@ -781,11 +802,24 @@ impl Event { ); } Self::WorkflowRunFailed { - error, duration_ms, .. + failure, + duration_ms, + .. } => { + let tail = + fabro_types::ExecOutputTail::trace_summary(failure.exec_output_tail.as_ref()); error!( - error = %error, - causes = ?error.causes(), + message = %failure.message, + reason = %failure.reason, + category = %failure.category, + system_actor = ?failure.system_actor, + signature = ?failure.signature, + cause_count = failure.causes.len(), + exec_output_tail_present = tail.present, + exec_stdout_tail_bytes = tail.stdout_bytes, + exec_stderr_tail_bytes = tail.stderr_bytes, + exec_stdout_truncated = tail.stdout_truncated, + exec_stderr_truncated = tail.stderr_truncated, duration_ms, "Workflow run failed" ); diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index bd244a99f..6734ebe1c 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -46,7 +46,7 @@ pub fn ensure_clean(repo: &Path) -> Result<()> { let output = git_cmd(repo) .args(["status", "--porcelain"]) .output() - .map_err(|e| Error::engine_with_source("git status failed", &e))?; + .map_err(|e| Error::engine_with_source("git status failed", e))?; if !output.status.success() { return Err(git_error("not a git repository")); @@ -65,7 +65,7 @@ pub fn head_sha(repo: &Path) -> Result { let output = git_cmd(repo) .args(["rev-parse", "HEAD"]) .output() - .map_err(|e| Error::engine_with_source("git rev-parse failed", &e))?; + .map_err(|e| Error::engine_with_source("git rev-parse failed", e))?; if !output.status.success() { return Err(git_error("git rev-parse HEAD failed")); @@ -79,7 +79,7 @@ pub fn create_branch(repo: &Path, name: &str) -> Result<()> { let output = git_cmd(repo) .args(["branch", "--force", name, "HEAD"]) .output() - .map_err(|e| Error::engine_with_source("git branch failed", &e))?; + .map_err(|e| Error::engine_with_source("git branch failed", e))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -96,7 +96,7 @@ pub fn add_worktree(repo: &Path, path: &Path, branch: &str) -> Result<()> { .arg(path) .arg(branch) .output() - .map_err(|e| Error::engine_with_source("git worktree add failed", &e))?; + .map_err(|e| Error::engine_with_source("git worktree add failed", e))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -112,7 +112,7 @@ pub fn remove_worktree(repo: &Path, path: &Path) -> Result<()> { .args(["worktree", "remove", "--force"]) .arg(path) .output() - .map_err(|e| Error::engine_with_source("git worktree remove failed", &e))?; + .map_err(|e| Error::engine_with_source("git worktree remove failed", e))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -132,7 +132,7 @@ pub fn replace_worktree(repo: &Path, path: &Path, branch: &str) -> Result<()> { fn run_git_push(cmd: &mut Command) -> Result<()> { let output = cmd .output() - .map_err(|e| Error::engine_with_source("git push failed", &e))?; + .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}"))); diff --git a/lib/crates/fabro-workflow/src/handler/agent.rs b/lib/crates/fabro-workflow/src/handler/agent.rs index 376d45431..2f8b5abf1 100644 --- a/lib/crates/fabro-workflow/src/handler/agent.rs +++ b/lib/crates/fabro-workflow/src/handler/agent.rs @@ -286,7 +286,7 @@ impl Handler for AgentHandler { let run_id = context .run_id() .parse::() - .map_err(|err| Error::handler_with_source("invalid internal run_id", &err))?; + .map_err(|err| Error::handler_with_source("invalid internal run_id", err))?; let tool_hooks: Option> = services.run.hook_runner.as_ref().map(|hr| { Arc::new(fabro_hooks::WorkflowToolHookCallback { diff --git a/lib/crates/fabro-workflow/src/handler/command.rs b/lib/crates/fabro-workflow/src/handler/command.rs index 9916aaf82..c830bb8b6 100644 --- a/lib/crates/fabro-workflow/src/handler/command.rs +++ b/lib/crates/fabro-workflow/src/handler/command.rs @@ -105,7 +105,7 @@ impl Handler for CommandHandler { let env = services .env_for_stage() .await - .map_err(|err| Error::handler_with_anyhow("Failed to resolve stage env", &err))?; + .map_err(|err| Error::handler_with_anyhow("Failed to resolve stage env", err))?; let env_vars = if env.is_empty() { None } else { Some(&env) }; let cancel_token = services.run.cancel_token().child_token(); let stage_id = stage_scope.stage_id(); @@ -140,7 +140,7 @@ impl Handler for CommandHandler { Ok(streaming) => streaming, Err(err) => { recorder.discard().await?; - return Err(Error::handler_with_source("Failed to spawn script", &err)); + return Err(Error::handler_with_source("Failed to spawn script", err)); } }; let result = streaming.result; diff --git a/lib/crates/fabro-workflow/src/handler/llm/acp.rs b/lib/crates/fabro-workflow/src/handler/llm/acp.rs index 971a4c8ed..f3e27c4f1 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/acp.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/acp.rs @@ -240,7 +240,7 @@ fn acp_command_error_to_workflow(error: AcpCommandError) -> Error { Error::handler("only stdio ACP commands are supported") } AcpCommandError::Parse(source) => { - Error::handler_with_source("Failed to resolve ACP command", &source) + Error::handler_with_source("Failed to resolve ACP command", source) } } } @@ -258,8 +258,8 @@ fn acp_error_to_workflow(error: AcpError) -> Error { AcpError::StopReason { stop_reason, text } => { Error::handler(format!("ACP prompt stopped with {stop_reason}: {text}")) } - AcpError::Sandbox(source) => Error::handler_with_source("ACP turn failed", &source), - other => Error::handler_with_source("ACP turn failed", &other), + AcpError::Sandbox(source) => Error::handler_with_source("ACP turn failed", source), + other => Error::handler_with_source("ACP turn failed", other), } } diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index 5270490f7..722b43b1b 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -348,7 +348,7 @@ impl AgentApiBackend { ) -> Result { let client = Client::from_source(source) .await - .map_err(|e| Error::handler_with_source("Failed to create LLM client", &e))?; + .map_err(|e| Error::handler_with_source("Failed to create LLM client", e))?; let mut profile = build_profile(model, provider); @@ -485,7 +485,7 @@ impl CodergenBackend for AgentApiBackend { let client = Client::from_source(self.source.as_ref()) .await - .map_err(|e| Error::handler_with_source("Failed to create LLM client", &e))?; + .map_err(|e| Error::handler_with_source("Failed to create LLM client", e))?; let model = node.model().unwrap_or(&self.model); let provider = node diff --git a/lib/crates/fabro-workflow/src/handler/llm/cli.rs b/lib/crates/fabro-workflow/src/handler/llm/cli.rs index 129583c83..e4e60ea27 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/cli.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/cli.rs @@ -96,7 +96,7 @@ async fn verify_cli_available( ) .await .map_err(|e| { - Error::handler_with_source(format!("Failed to check {cli_name} availability"), &e) + Error::handler_with_source(format!("Failed to check {cli_name} availability"), e) })?; if availability_check.is_success() { @@ -400,7 +400,7 @@ impl CodergenBackend for AgentCliBackend { sandbox .write_file(&prompt_path, prompt) .await - .map_err(|e| Error::handler_with_source("Failed to write prompt file", &e))?; + .map_err(|e| Error::handler_with_source("Failed to write prompt file", e))?; // 3. Build CLI command let model = node.model().unwrap_or(&self.model); @@ -451,7 +451,7 @@ impl CodergenBackend for AgentCliBackend { sandbox .write_file(&env_path, &env_lines.join("\n")) .await - .map_err(|e| Error::handler_with_source("Failed to write env file", &e))?; + .map_err(|e| Error::handler_with_source("Failed to write env file", e))?; // Disable auto-stop so the sandbox stays alive during long CLI runs. if let Err(e) = sandbox.set_autostop_interval(0).await { @@ -522,10 +522,7 @@ impl CodergenBackend for AgentCliBackend { Ok(streaming) => streaming, Err(err) => { cleanup_temp_files().await; - return Err(Error::handler_with_source( - "Failed to run CLI command", - &err, - )); + return Err(Error::handler_with_source("Failed to run CLI command", err)); } }; let result = streaming.result; diff --git a/lib/crates/fabro-workflow/src/handler/llm/launch_env.rs b/lib/crates/fabro-workflow/src/handler/llm/launch_env.rs index 134dea313..6f43da2a3 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/launch_env.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/launch_env.rs @@ -38,7 +38,7 @@ pub(crate) async fn resolve_agent_launch_env( .map_err(|err| { Error::handler_with_source( format!("Failed to resolve {} credential", request.stage_label), - &err, + err, ) })?; let ResolvedCredential::Cli(cli_credential) = resolved else { @@ -58,7 +58,7 @@ pub(crate) async fn resolve_agent_launch_env( .map_err(|err| { Error::handler_with_source( format!("{} credential login failed", request.stage_label), - &err, + err, ) })?; if !login_result.is_success() { @@ -97,7 +97,7 @@ pub(crate) async fn resolve_agent_launch_env( let tool_env = provider.resolve().await.map_err(|err| { Error::handler_with_anyhow( format!("Failed to resolve {} agent env", request.stage_label), - &err, + err, ) })?; launch_env.extend(tool_env); diff --git a/lib/crates/fabro-workflow/src/handler/parallel.rs b/lib/crates/fabro-workflow/src/handler/parallel.rs index 083688526..f67a8d37d 100644 --- a/lib/crates/fabro-workflow/src/handler/parallel.rs +++ b/lib/crates/fabro-workflow/src/handler/parallel.rs @@ -167,7 +167,7 @@ impl Handler for ParallelHandler { let run_id = context .run_id() .parse::() - .map_err(|err| Error::handler_with_source("invalid internal run_id", &err))?; + .map_err(|err| Error::handler_with_source("invalid internal run_id", err))?; let mut hook_ctx = HookContext::new(HookEvent::ParallelStart, run_id, graph.name.clone()); set_hook_node(&mut hook_ctx, node); @@ -200,7 +200,7 @@ impl Handler for ParallelHandler { match result { Ok(sha) => Some(sha), Err(e) if e.to_string() == "sandbox git unavailable" => { - return Err(Error::handler_with_source("sandbox git unavailable", &e)); + return Err(Error::handler_with_source("sandbox git unavailable", e)); } Err(e) => { tracing::warn!( @@ -277,7 +277,7 @@ impl Handler for ParallelHandler { wt_sandbox .initialize() .await - .map_err(|e| Error::handler_with_source("worktree setup failed", &e))?; + .map_err(|e| Error::handler_with_source("worktree setup failed", e))?; branch_context.set(keys::INTERNAL_WORK_DIR, serde_json::json!(&wt_path_str)); @@ -330,7 +330,7 @@ impl Handler for ParallelHandler { let _permit = sem .acquire() .await - .map_err(|e| Error::handler_with_source("semaphore error", &e))?; + .map_err(|e| Error::handler_with_source("semaphore error", e))?; parent_run.emitter.emit_scoped( &Event::ParallelBranchStarted { @@ -572,7 +572,7 @@ impl Handler for ParallelHandler { let run_id = context .run_id() .parse::() - .map_err(|err| Error::handler_with_source("invalid internal run_id", &err))?; + .map_err(|err| Error::handler_with_source("invalid internal run_id", err))?; let mut hook_ctx = HookContext::new(HookEvent::ParallelComplete, run_id, graph.name.clone()); set_hook_node(&mut hook_ctx, node); diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index a2a95c422..d5a6c9af1 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -1210,7 +1210,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 10, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, diff --git a/lib/crates/fabro-workflow/src/operations/archive.rs b/lib/crates/fabro-workflow/src/operations/archive.rs index 56c3394b7..8a6044f16 100644 --- a/lib/crates/fabro-workflow/src/operations/archive.rs +++ b/lib/crates/fabro-workflow/src/operations/archive.rs @@ -183,16 +183,18 @@ mod tests { event::append_event(&run_store, run_id, &Event::RunRunning) .await .unwrap(); - event::append_event(&run_store, run_id, &Event::WorkflowRunFailed { - error: crate::error::Error::engine("boom"), - duration_ms: 10, - reason: FailureReason::WorkflowError, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }) - .await - .unwrap(); + let failure_event = Event::workflow_run_failed_from_error( + &crate::error::Error::engine("boom"), + 10, + FailureReason::WorkflowError, + None, + None, + None, + None, + ); + event::append_event(&run_store, run_id, &failure_event) + .await + .unwrap(); } async fn seed_running(store: &Database, run_id: &RunId) { diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index a4febb472..1962a24ac 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -151,7 +151,7 @@ pub async fn create( ) }) .await - .map_err(|err| Error::engine_with_source("workflow create task failed", &err))??; + .map_err(|err| Error::engine_with_source("workflow create task failed", err))??; let workflow_config = resolved .workflow_toml_path diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index dc9b823c3..bff57dd32 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -260,16 +260,16 @@ async fn persist_terminal_engine_failure( RunStatus::Failed { reason } => reason, _ => FailureReason::WorkflowError, }; - if let Err(err) = append_event_to_sink(event_sink, &run_id, &Event::WorkflowRunFailed { - error: error.clone(), - duration_ms: crate::millis_u64(duration), + let failure_event = Event::workflow_run_failed_from_error( + error, + crate::millis_u64(duration), reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }) - .await - { + None, + None, + None, + None, + ); + if let Err(err) = append_event_to_sink(event_sink, &run_id, &failure_event).await { tracing::warn!(error = %err, "Failed to append terminal engine failure event"); } } @@ -747,6 +747,13 @@ impl RunSession { } } } + event if matches!(&event.body, EventBody::RunFailed(_)) => { + if let EventBody::RunFailed(props) = &event.body { + if let Some(sha) = props.final_git_commit_sha.as_ref() { + *sha_clone.lock().unwrap() = Some(sha.clone()); + } + } + } event if matches!(&event.body, EventBody::GitCommit(_)) => { if let EventBody::GitCommit(props) = &event.body { *sha_clone.lock().unwrap() = Some(props.sha.clone()); @@ -891,15 +898,16 @@ impl Drop for DetachedRunBootstrapGuard { let event_sink = self.event_sink.clone(); if let Ok(handle) = Handle::try_current() { handle.spawn(async move { - let _ = append_event_to_sink(&event_sink, &run_id, &Event::WorkflowRunFailed { - error: Error::engine(reason.to_string()), - duration_ms: 0, + let failure_event = Event::workflow_run_failed_from_error( + &Error::engine(reason.to_string()), + 0, reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }) - .await; + None, + None, + None, + None, + ); + let _ = append_event_to_sink(&event_sink, &run_id, &failure_event).await; }); } } @@ -957,15 +965,16 @@ impl Drop for DetachedRunCompletionGuard { let run_id = self.run_id; if let Ok(handle) = Handle::try_current() { handle.spawn(async move { - let _ = append_event_to_sink(&event_sink, &run_id, &Event::WorkflowRunFailed { - error: Error::engine(message.to_string()), - duration_ms: 0, + let failure_event = Event::workflow_run_failed_from_error( + &Error::engine(message.to_string()), + 0, reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }) - .await; + None, + None, + None, + None, + ); + let _ = append_event_to_sink(&event_sink, &run_id, &failure_event).await; let _ = append_event_to_sink(&event_sink, &run_id, &Event::RunNotice { level: RunNoticeLevel::Error, code: code.to_string(), @@ -988,16 +997,9 @@ async fn persist_detached_failure( ) -> Result<(), Error> { let message = error.to_string(); - if let Err(err) = append_event_to_sink(event_sink, &run_id, &Event::WorkflowRunFailed { - error: error.clone(), - duration_ms: 0, - reason, - git_commit_sha: None, - final_patch: None, - diff_summary: None, - }) - .await - { + let failure_event = + Event::workflow_run_failed_from_error(error, 0, reason, None, None, None, None); + if let Err(err) = append_event_to_sink(event_sink, &run_id, &failure_event).await { tracing::warn!(error = %err, "Failed to append detached failure event"); } @@ -1498,7 +1500,7 @@ mod tests { timestamp: Utc::now(), status: StageOutcome::Succeeded, duration_ms: 1, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: vec![], billing: None, diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index 09dbbeb86..6ea3ff537 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -5,14 +5,14 @@ use std::time::Instant; use fabro_dump::RunDump; use fabro_hooks::{HookContext, HookEvent}; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; -use fabro_types::{BilledTokenCounts, DiffSummary, EventBody, RunProjection}; +use fabro_types::{BilledTokenCounts, DiffSummary, EventBody, RunFailure, RunProjection}; use fabro_util::error::collect_causes; use fabro_util::time::elapsed_ms; use super::types::{Concluded, Executed, FinalizeOptions}; -use crate::error::Error; +use crate::error::{Error, run_failure_from_error, run_failure_from_outcome_failure}; use crate::event::{Event, RunNoticeCode, RunNoticeLevel}; -use crate::outcome::{Outcome, OutcomeExt, StageOutcome}; +use crate::outcome::{Outcome, StageOutcome}; use crate::records::{Checkpoint, Conclusion, StageSummary}; use crate::run_metadata::MetadataSnapshot; use crate::run_options::RunOptions; @@ -24,11 +24,13 @@ use crate::{ProjectionBillingRollup, billing_rollup_from_projection}; pub fn classify_engine_result( engine_result: &Result, -) -> (StageOutcome, Option, RunStatus) { +) -> (StageOutcome, Option, RunStatus) { match engine_result { Ok(outcome) => { let status = outcome.status; - let failure_reason = outcome.failure_reason().map(String::from); + let failure = outcome.failure.as_ref().map(|failure| { + run_failure_from_outcome_failure(failure, FailureReason::WorkflowError) + }); let run_status = match status { StageOutcome::Succeeded | StageOutcome::Skipped => RunStatus::Succeeded { reason: SuccessReason::Completed, @@ -40,13 +42,16 @@ pub fn classify_engine_result( reason: FailureReason::WorkflowError, }, }; - (status, failure_reason, run_status) + (status, failure, run_status) } Err(Error::Cancelled) => ( StageOutcome::Failed { retry_requested: false, }, - Some("Cancelled".to_string()), + Some(run_failure_from_error( + &Error::Cancelled, + FailureReason::Cancelled, + )), RunStatus::Failed { reason: FailureReason::Cancelled, }, @@ -55,7 +60,7 @@ pub fn classify_engine_result( StageOutcome::Failed { retry_requested: false, }, - Some(err.display_with_causes()), + Some(run_failure_from_error(err, FailureReason::WorkflowError)), RunStatus::Failed { reason: FailureReason::WorkflowError, }, @@ -66,7 +71,7 @@ pub fn classify_engine_result( pub(crate) async fn build_conclusion_from_store( run_store: &RunStoreHandle, status: StageOutcome, - failure_reason: Option, + failure: Option, run_duration_ms: u64, final_git_commit_sha: Option, ) -> Conclusion { @@ -88,7 +93,7 @@ pub(crate) async fn build_conclusion_from_store( &projection_billing, &projection_order, status, - failure_reason, + failure, run_duration_ms, final_git_commit_sha, ) @@ -99,7 +104,7 @@ fn build_conclusion_from_parts( projection_billing: &ProjectionBillingRollup, projection_order: &HashMap, status: StageOutcome, - failure_reason: Option, + failure: Option, run_duration_ms: u64, final_git_commit_sha: Option, ) -> Conclusion { @@ -178,7 +183,7 @@ fn build_conclusion_from_parts( timestamp: chrono::Utc::now(), status, duration_ms: run_duration_ms, - failure_reason, + failure, final_git_commit_sha, stages, billing: projection_billing.billing_if_present(), @@ -445,17 +450,6 @@ pub(crate) fn build_terminal_event( diff_summary: Option, billing: Option, ) -> Event { - if matches!(outcome, Err(Error::Cancelled)) { - return Event::WorkflowRunFailed { - error: Error::Cancelled, - duration_ms, - reason: FailureReason::Cancelled, - git_commit_sha: final_git_commit_sha, - final_patch, - diff_summary, - }; - } - let outcome_status = outcome.as_ref().map_or( StageOutcome::Failed { retry_requested: false, @@ -483,21 +477,27 @@ pub(crate) fn build_terminal_event( }; } - let error = match outcome { - Err(err) => err.clone(), - Ok(o) => Error::engine( - o.failure - .as_ref() - .map_or_else(|| "run failed".to_string(), |f| f.message.clone()), - ), + let failure = match outcome { + Err(Error::Cancelled) => { + run_failure_from_error(&Error::Cancelled, FailureReason::Cancelled) + } + Err(err) => run_failure_from_error(err, FailureReason::WorkflowError), + Ok(outcome) => { + if let Some(failure) = outcome.failure.as_ref() { + run_failure_from_outcome_failure(failure, FailureReason::WorkflowError) + } else { + let fallback = Error::engine("run failed"); + run_failure_from_error(&fallback, FailureReason::WorkflowError) + } + } }; Event::WorkflowRunFailed { - error, + failure, duration_ms, - reason: FailureReason::WorkflowError, - git_commit_sha: final_git_commit_sha, + final_git_commit_sha, final_patch, diff_summary, + billing, } } @@ -1091,7 +1091,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 10, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, @@ -1154,7 +1154,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 10, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, @@ -1206,7 +1206,7 @@ mod tests { timestamp: chrono::Utc::now(), status: StageOutcome::Succeeded, duration_ms: 10, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index bd80072cd..dd83aaae6 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -93,12 +93,12 @@ fn build_sandbox_env( }; let https_url = fabro_github::ssh_url_to_https(origin_url); let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url) - .map_err(|err| Error::engine_with_anyhow("Failed to parse GitHub origin", &err))?; + .map_err(|err| Error::engine_with_anyhow("Failed to parse GitHub origin", err))?; let permissions = serde_json::to_value(permissions).map_err(|err| { - Error::engine_with_source("Failed to serialize GitHub permissions", &err) + Error::engine_with_source("Failed to serialize GitHub permissions", err) })?; let http = fabro_http::http_client() - .map_err(|err| Error::engine_with_source("Failed to build HTTP client", &err))?; + .map_err(|err| Error::engine_with_source("Failed to build HTTP client", err))?; let install_url = app.installation_url(&owner); let minter = AppIatMinter::new( app.clone(), @@ -242,7 +242,7 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> { let config = fabro_devcontainer::DevcontainerResolver::resolve(&devcontainer.resolve_dir) .await - .map_err(|e| Error::engine_with_source("Failed to resolve devcontainer", &e))?; + .map_err(|e| Error::engine_with_source("Failed to resolve devcontainer", e))?; let lifecycle_command_count = config.on_create_commands.len() + config.post_create_commands.len() @@ -279,7 +279,7 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> { .map_err(|e| { Error::engine_with_source( format!("Failed to execute devcontainer initializeCommand: {shell_command}"), - &e, + e, ) })?; @@ -410,7 +410,7 @@ pub async fn initialize( Some(Arc::clone(&sandbox_event_callback)), ) .await - .map_err(|err| Error::engine_with_anyhow("Failed to reconnect sandbox for resume", &err))?; + .map_err(|err| Error::engine_with_anyhow("Failed to reconnect sandbox for resume", err))?; sandbox_initialized = false; Arc::new(ReadBeforeWriteSandbox::new(Arc::from(sandbox))) } else { @@ -419,7 +419,7 @@ pub async fn initialize( .sandbox .build(Some(Arc::clone(&sandbox_event_callback))) .await - .map_err(|e| Error::engine_with_anyhow("Failed to build sandbox", &e))?, + .map_err(|e| Error::engine_with_anyhow("Failed to build sandbox", e))?, )) }; let cleanup_guard = (!attach_existing).then(|| { @@ -436,12 +436,12 @@ pub async fn initialize( sandbox .start() .await - .map_err(|e| Error::engine_with_source("Failed to start sandbox", &e))?; + .map_err(|e| Error::engine_with_source("Failed to start sandbox", e))?; } else { sandbox .initialize() .await - .map_err(|e| Error::engine_with_source("Failed to initialize sandbox", &e))?; + .map_err(|e| Error::engine_with_source("Failed to initialize sandbox", e))?; } let hook_ctx = HookContext::new( @@ -526,7 +526,7 @@ pub async fn initialize( sandbox_git .ensure_git_available(&*sandbox) .await - .map_err(|err| Error::engine_with_source("sandbox git unavailable", &err))?; + .map_err(|err| Error::engine_with_source("sandbox git unavailable", err))?; } match sandbox.setup_git(&intent).await { Ok(Some(info)) => { @@ -563,7 +563,7 @@ pub async fn initialize( } } Err(e) => { - return Err(Error::engine_with_source("Sandbox git setup failed", &e)); + return Err(Error::engine_with_source("Sandbox git setup failed", e)); } } } @@ -589,7 +589,7 @@ pub async fn initialize( Some(cancel_token.clone()), ) .await - .map_err(|e| Error::engine_with_source("Setup command failed", &e))?; + .map_err(|e| Error::engine_with_source("Setup command failed", e))?; if options.run_options.cancel_token.is_cancelled() { return Err(Error::Cancelled); } diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index bf504e728..1b53e0b89 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -845,7 +845,7 @@ mod tests { timestamp: Utc::now(), status: crate::outcome::StageOutcome::Succeeded, duration_ms: 150_000, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: vec![ StageSummary { diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 67ca26fd8..07bc6b80c 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -6997,7 +6997,7 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() { timestamp: Utc::now(), status: StageOutcome::Succeeded, duration_ms: 1, - failure_reason: None, + failure: None, final_git_commit_sha: None, stages: Vec::new(), billing: None, diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 714dd5cb1..b5ad4cda8 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -95,9 +95,11 @@ models/error-response-entry.ts models/error-response.ts models/event-envelope.ts models/event-seq.ts +models/exec-output-tail.ts models/execute-query-request.ts models/execute-query-response-rows-inner-inner.ts models/execute-query-response.ts +models/failure-category.ts models/failure-reason.ts models/features-namespace.ts models/file-checkpoint.ts @@ -251,6 +253,7 @@ models/run-diff.ts models/run-error.ts models/run-event.ts models/run-execution-settings.ts +models/run-failure.ts models/run-files-meta.ts models/run-git-settings.ts models/run-goal-file.ts diff --git a/lib/packages/fabro-api-client/src/api/models-api.ts b/lib/packages/fabro-api-client/src/api/models-api.ts index 8d28f601f..1f119b1d1 100644 --- a/lib/packages/fabro-api-client/src/api/models-api.ts +++ b/lib/packages/fabro-api-client/src/api/models-api.ts @@ -5,7 +5,7 @@ * HTTP API for managing Fabro workflow run executions. * * The version of the OpenAPI document: 0.1.0 - * + * * * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). * https://openapi-generator.tech diff --git a/lib/packages/fabro-api-client/src/models/conclusion.ts b/lib/packages/fabro-api-client/src/models/conclusion.ts index daf429e69..b25c1b32f 100644 --- a/lib/packages/fabro-api-client/src/models/conclusion.ts +++ b/lib/packages/fabro-api-client/src/models/conclusion.ts @@ -21,6 +21,9 @@ import type { BilledTokenCounts } from './billed-token-counts'; import type { RunDiff } from './run-diff'; // May contain unused imports in some cases // @ts-ignore +import type { RunFailure } from './run-failure'; +// May contain unused imports in some cases +// @ts-ignore import type { StageOutcome } from './stage-outcome'; // May contain unused imports in some cases // @ts-ignore @@ -33,7 +36,7 @@ export interface Conclusion { 'timestamp': string; 'status': StageOutcome; 'duration_ms': number; - 'failure_reason'?: string | null; + 'failure'?: RunFailure | null; 'final_git_commit_sha'?: string | null; 'stages': Array; 'billing'?: BilledTokenCounts | null; diff --git a/lib/packages/fabro-api-client/src/models/exec-output-tail.ts b/lib/packages/fabro-api-client/src/models/exec-output-tail.ts new file mode 100644 index 000000000..ea85dead1 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/exec-output-tail.ts @@ -0,0 +1,26 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Redacted tail of command stdout/stderr captured for diagnostics. + */ +export interface ExecOutputTail { + 'stdout'?: string | null; + 'stderr'?: string | null; + 'stdout_truncated'?: boolean; + 'stderr_truncated'?: boolean; +} + diff --git a/lib/packages/fabro-api-client/src/models/failure-category.ts b/lib/packages/fabro-api-client/src/models/failure-category.ts new file mode 100644 index 000000000..cc4c38929 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/failure-category.ts @@ -0,0 +1,33 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Product-level classification for grouping and retry policy. + */ + +export const FailureCategory = { + TRANSIENT_INFRA: 'transient_infra', + DETERMINISTIC: 'deterministic', + BUDGET_EXHAUSTED: 'budget_exhausted', + COMPILATION_LOOP: 'compilation_loop', + CANCELED: 'canceled', + STRUCTURAL: 'structural' +} as const; + +export type FailureCategory = typeof FailureCategory[keyof typeof FailureCategory]; + + + diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index fddfb31bc..27a1a8488 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -73,9 +73,11 @@ export * from './error-response'; export * from './error-response-entry'; export * from './event-envelope'; export * from './event-seq'; +export * from './exec-output-tail'; export * from './execute-query-request'; export * from './execute-query-response'; export * from './execute-query-response-rows-inner-inner'; +export * from './failure-category'; export * from './failure-reason'; export * from './features-namespace'; export * from './file-checkpoint'; @@ -229,6 +231,7 @@ export * from './run-diff'; export * from './run-error'; export * from './run-event'; export * from './run-execution-settings'; +export * from './run-failure'; export * from './run-files-meta'; export * from './run-git-settings'; export * from './run-goal'; diff --git a/lib/packages/fabro-api-client/src/models/model.ts b/lib/packages/fabro-api-client/src/models/model.ts index 37154fb75..4b03f415b 100644 --- a/lib/packages/fabro-api-client/src/models/model.ts +++ b/lib/packages/fabro-api-client/src/models/model.ts @@ -5,7 +5,7 @@ * HTTP API for managing Fabro workflow run executions. * * The version of the OpenAPI document: 0.1.0 - * + * * * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). * https://openapi-generator.tech @@ -67,9 +67,7 @@ export interface Model { */ 'default': boolean; /** - * Whether credential material is present for this model\'s provider on the server (vault entry or environment variable). Does NOT imply the credential is valid or that requests will succeed; call `POST /models/{id}/test` to verify usability. + * Whether credential material is present for this model\'s provider on the server (vault entry or environment variable). Does NOT imply the credential is valid or that requests will succeed; call `POST /models/{id}/test` to verify usability. */ 'configured': boolean; } - - diff --git a/lib/packages/fabro-api-client/src/models/run-failure.ts b/lib/packages/fabro-api-client/src/models/run-failure.ts new file mode 100644 index 000000000..dc70023d0 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/run-failure.ts @@ -0,0 +1,46 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +// May contain unused imports in some cases +// @ts-ignore +import type { ExecOutputTail } from './exec-output-tail'; +// May contain unused imports in some cases +// @ts-ignore +import type { FailureCategory } from './failure-category'; +// May contain unused imports in some cases +// @ts-ignore +import type { FailureReason } from './failure-reason'; +// May contain unused imports in some cases +// @ts-ignore +import type { SystemActorKind } from './system-actor-kind'; + +/** + * Rich terminal run failure diagnostics. + */ +export interface RunFailure { + 'message': string; + 'causes'?: Array; + 'reason': FailureReason; + 'category': FailureCategory; + 'system_actor'?: SystemActorKind | null; + /** + * Stable normalized signature for grouping related failures. + */ + 'signature'?: string | null; + 'exec_output_tail'?: ExecOutputTail | null; +} + + +