From 07d3890bb890826780c968447a89e07507ba6217 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 29 Apr 2026 19:43:30 -0400 Subject: [PATCH] refactor(workflow): share metadata snapshot helpers Centralize compatibility notice detection and elapsed-time/error-cause helpers used by metadata snapshot event rendering and emission. --- lib/crates/fabro-cli/src/commands/run/logs.rs | 8 +----- .../src/commands/run/run_progress/mod.rs | 8 +----- lib/crates/fabro-types/src/run_event/infra.rs | 14 ++++++++++ lib/crates/fabro-util/src/lib.rs | 1 + lib/crates/fabro-util/src/time.rs | 6 +++++ .../fabro-workflow/src/lifecycle/git.rs | 26 ++++--------------- .../fabro-workflow/src/pipeline/finalize.rs | 24 +++-------------- 7 files changed, 32 insertions(+), 55 deletions(-) create mode 100644 lib/crates/fabro-util/src/time.rs diff --git a/lib/crates/fabro-cli/src/commands/run/logs.rs b/lib/crates/fabro-cli/src/commands/run/logs.rs index 372cfc109..b5cde4456 100644 --- a/lib/crates/fabro-cli/src/commands/run/logs.rs +++ b/lib/crates/fabro-cli/src/commands/run/logs.rs @@ -14,6 +14,7 @@ use std::time::Duration; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_redact::redact_jsonl_line; +use fabro_types::run_event::is_metadata_snapshot_compat_notice_code; use fabro_util::json::normalize_json_value; use fabro_util::terminal::Styles; use tokio::time; @@ -762,13 +763,6 @@ fn is_metadata_snapshot_compat_notice(envelope: &serde_json::Value) -> bool { prop_str_field(envelope, "code").is_some_and(is_metadata_snapshot_compat_notice_code) } -fn is_metadata_snapshot_compat_notice_code(code: &str) -> bool { - matches!( - code, - "checkpoint_metadata_write_failed" | "checkpoint_metadata_push_failed" - ) -} - fn str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str> { value.get(key)?.as_str() } diff --git a/lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs b/lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs index 19bee74ff..43e47bf21 100644 --- a/lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs +++ b/lib/crates/fabro-cli/src/commands/run/run_progress/mod.rs @@ -4,6 +4,7 @@ )] use fabro_types::RunEvent; +use fabro_types::run_event::is_metadata_snapshot_compat_notice_code; mod event; mod info_display; @@ -442,13 +443,6 @@ impl ProgressUI { } } -fn is_metadata_snapshot_compat_notice_code(code: &str) -> bool { - matches!( - code, - "checkpoint_metadata_write_failed" | "checkpoint_metadata_push_failed" - ) -} - #[cfg(test)] mod tests { #![allow( diff --git a/lib/crates/fabro-types/src/run_event/infra.rs b/lib/crates/fabro-types/src/run_event/infra.rs index 1771366e4..3dc726b77 100644 --- a/lib/crates/fabro-types/src/run_event/infra.rs +++ b/lib/crates/fabro-types/src/run_event/infra.rs @@ -1,5 +1,19 @@ use serde::{Deserialize, Serialize}; +/// Legacy `run.notice` codes paired with the new `metadata.snapshot.failed` +/// event for backward compatibility. Display layers suppress these so the +/// typed event renders without a duplicate raw warning. +pub const NOTICE_CODE_CHECKPOINT_METADATA_WRITE_FAILED: &str = "checkpoint_metadata_write_failed"; +pub const NOTICE_CODE_CHECKPOINT_METADATA_PUSH_FAILED: &str = "checkpoint_metadata_push_failed"; + +#[must_use] +pub fn is_metadata_snapshot_compat_notice_code(code: &str) -> bool { + matches!( + code, + NOTICE_CODE_CHECKPOINT_METADATA_WRITE_FAILED | NOTICE_CODE_CHECKPOINT_METADATA_PUSH_FAILED + ) +} + #[derive( Debug, Clone, diff --git a/lib/crates/fabro-util/src/lib.rs b/lib/crates/fabro-util/src/lib.rs index 303cfa4a4..5993f2e39 100644 --- a/lib/crates/fabro-util/src/lib.rs +++ b/lib/crates/fabro-util/src/lib.rs @@ -13,6 +13,7 @@ pub mod run_log; pub mod session_secret; pub mod terminal; pub mod text; +pub mod time; pub mod version; pub mod warnings; diff --git a/lib/crates/fabro-util/src/time.rs b/lib/crates/fabro-util/src/time.rs new file mode 100644 index 000000000..fbfeaad39 --- /dev/null +++ b/lib/crates/fabro-util/src/time.rs @@ -0,0 +1,6 @@ +use std::time::Instant; + +#[must_use] +pub fn elapsed_ms(started: Instant) -> u64 { + u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX) +} diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index 1eb7b1208..d9aea0ea0 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -9,6 +9,8 @@ use fabro_core::outcome::NodeResult; use fabro_core::state::ExecutionState; use fabro_types::RunId; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; +use fabro_util::error::collect_causes; +use fabro_util::time::elapsed_ms; use crate::artifact; use crate::event::{Emitter, Event, RunNoticeLevel, StageScope}; @@ -110,7 +112,7 @@ impl RunLifecycle for GitLifecycle { started, MetadataSnapshotFailureKind::LoadState, message.clone(), - anyhow_causes(&err), + collect_causes(err.as_ref()), None, None, None, @@ -179,7 +181,7 @@ impl RunLifecycle for GitLifecycle { started, MetadataSnapshotFailureKind::LoadState, message.clone(), - anyhow_causes(&err), + collect_causes(err.as_ref()), None, None, None, @@ -355,7 +357,7 @@ impl GitLifecycle { started, MetadataSnapshotFailureKind::Write, message.clone(), - error_causes(&err), + collect_causes(&err), None, None, None, @@ -455,24 +457,6 @@ impl GitLifecycle { } } -fn elapsed_ms(started: Instant) -> u64 { - u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX) -} - -fn anyhow_causes(error: &anyhow::Error) -> Vec { - error.chain().skip(1).map(ToString::to_string).collect() -} - -fn error_causes(error: &(dyn std::error::Error + 'static)) -> Vec { - let mut causes = Vec::new(); - let mut source = error.source(); - while let Some(cause) = source { - causes.push(cause.to_string()); - source = cause.source(); - } - causes -} - #[cfg(test)] mod tests { use std::collections::{BTreeMap, HashMap}; diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index 611f152b3..251ef28d0 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -3,6 +3,8 @@ use std::time::Instant; use fabro_hooks::{HookContext, HookEvent}; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; use fabro_types::{BilledTokenCounts, EventBody}; +use fabro_util::error::collect_causes; +use fabro_util::time::elapsed_ms; use super::types::{Concluded, FinalizeOptions, Retroed}; use crate::error::Error; @@ -171,7 +173,7 @@ pub async fn write_finalize_commit( started, MetadataSnapshotFailureKind::LoadState, message.clone(), - anyhow_causes(&err), + collect_causes(err.as_ref()), None, None, None, @@ -221,7 +223,7 @@ pub async fn write_finalize_commit( started, MetadataSnapshotFailureKind::Write, message.clone(), - error_causes(&err), + collect_causes(&err), None, None, None, @@ -294,24 +296,6 @@ fn emit_metadata_warning(services: &RunServices, code: &str, message: String) { } } -fn elapsed_ms(started: Instant) -> u64 { - u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX) -} - -fn anyhow_causes(error: &anyhow::Error) -> Vec { - error.chain().skip(1).map(ToString::to_string).collect() -} - -fn error_causes(error: &(dyn std::error::Error + 'static)) -> Vec { - let mut causes = Vec::new(); - let mut source = error.source(); - while let Some(cause) = source { - causes.push(cause.to_string()); - source = cause.source(); - } - causes -} - /// Failed and cancelled runs use a shorter diff timeout so a corrupted /// workspace can't stall downstream consumers waiting on the terminal event. async fn compute_final_patch(