Fix audit regressions from pipeline migration

- Gate pr_config on dry_run_mode to prevent PR creation during dry runs
- Restore em dash (—) separator in retro output
- Print "Retro unavailable" when retro is enabled but returns None
- Fix pre-existing clippy warnings (derivable_impls, needless_borrow)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-25 09:47:37 -04:00
parent 34a72b83b1
commit 5cedd970d0
No known key found for this signature in database
7 changed files with 32 additions and 24 deletions

View file

@ -60,7 +60,7 @@ pub fn run(args: &GraphArgs, styles: &Styles) -> anyhow::Result<()> {
let validated = fabro_workflows::operations::create_from_file(&dot_path)?;
let diagnostics = validated.diagnostics();
print_diagnostics(&diagnostics, styles);
print_diagnostics(diagnostics, styles);
if diagnostics.iter().any(|d| d.severity == Severity::Error) {
bail!("Validation failed");

View file

@ -1242,7 +1242,11 @@ async fn run_resumed(
&run_defaults,
);
let run_start = Instant::now();
let pr_config = settings.pull_request().cloned();
let pr_config = if dry_run_mode {
None
} else {
settings.pull_request().cloned()
};
let started = start(
validated,
StartOptions {
@ -1299,6 +1303,9 @@ async fn run_resumed(
Ok(started) => {
if let Some(ref retro) = started.retro {
print_retro_result(retro, started.retro_duration, &run_dir, styles);
} else if !args.no_retro && project_config::is_retro_enabled() {
eprintln!("\n{}", styles.bold.apply_to("=== Retro ==="));
eprintln!("{}", styles.dim.apply_to("Retro unavailable"));
}
let finalized = started.finalized;
print_run_conclusion(

View file

@ -1733,7 +1733,11 @@ async fn run_command_impl(
status_guard.defuse();
let run_start = Instant::now();
let pr_config = config.pull_request().cloned();
let pr_config = if dry_run_mode {
None
} else {
config.pull_request().cloned()
};
let started = start(
validated,
StartOptions {
@ -1784,6 +1788,9 @@ async fn run_command_impl(
Ok(started) => {
if let Some(ref retro) = started.retro {
print_retro_result(retro, started.retro_duration, &run_dir, styles);
} else if !no_retro_flag && project_config::is_retro_enabled() {
eprintln!("\n{}", styles.bold.apply_to("=== Retro ==="));
eprintln!("{}", styles.dim.apply_to("Retro unavailable"));
}
let finalized = started.finalized;
print_run_conclusion(
@ -1974,7 +1981,7 @@ pub(crate) fn print_retro_result(
.map(|s| s.to_string())
.unwrap_or_else(|| "unknown".to_string());
let outcome_str = retro.outcome.as_deref().unwrap_or("No outcome recorded");
let line1_content = format!("Retro: {smoothness_str} - {outcome_str}");
let line1_content = format!("Retro: {smoothness_str} \u{2014} {outcome_str}");
let term_width = console::Term::stderr().size().1 as usize;
let pad1 = term_width.saturating_sub(line1_content.len() + retro_dur.len());
eprintln!(
@ -1982,7 +1989,7 @@ pub(crate) fn print_retro_result(
styles.bold.apply_to("Retro:"),
styles
.dim
.apply_to(format!("{smoothness_str} - {outcome_str}")),
.apply_to(format!("{smoothness_str} \u{2014} {outcome_str}")),
"",
styles.dim.apply_to(&retro_dur),
);

View file

@ -32,7 +32,7 @@ pub fn run(args: &ValidateArgs, styles: &Styles) -> anyhow::Result<()> {
styles.dim.apply_to(relative_path(&dot_path)),
);
print_diagnostics(&diagnostics, styles);
print_diagnostics(diagnostics, styles);
if diagnostics.iter().any(|d| d.severity == Severity::Error) {
bail!("Validation failed");

View file

@ -6,20 +6,12 @@ use crate::error::FabroError;
use crate::pipeline::{self, TransformOptions, Validated};
use crate::transform::Transform;
#[derive(Default)]
pub struct CreateOptions {
pub base_dir: Option<PathBuf>,
pub custom_transforms: Vec<Box<dyn Transform>>,
}
impl Default for CreateOptions {
fn default() -> Self {
Self {
base_dir: None,
custom_transforms: Vec::new(),
}
}
}
/// Parse, transform, and validate a DOT source string.
///
/// Returns `Validated` even when validation produced errors. Call

View file

@ -6,7 +6,7 @@ mod start;
pub use create::{create, create_from_file, create_from_graph, CreateOptions};
pub use fork::fork;
pub use rewind::{
build_timeline, find_run_id_by_prefix, load_parallel_map, parse_target, resolve_target,
rewind, TimelineEntry,
build_timeline, find_run_id_by_prefix, load_parallel_map, parse_target, resolve_target, rewind,
TimelineEntry,
};
pub use start::{start, StartFinalizeConfig, StartOptions, StartRetroConfig, Started};

View file

@ -329,18 +329,20 @@ mod tests {
_run_dir: &Path,
services: &crate::handler::EngineServices,
) -> Result<Outcome, FabroError> {
services.emitter.emit(&WorkflowRunEvent::CheckpointCompleted {
node_id: node.id.clone(),
status: "success".to_string(),
git_commit_sha: Some("sha-test".to_string()),
});
services
.emitter
.emit(&WorkflowRunEvent::CheckpointCompleted {
node_id: node.id.clone(),
status: "success".to_string(),
git_commit_sha: Some("sha-test".to_string()),
});
Ok(Outcome::success())
}
}
fn validated_workflow(dot: &str) -> Validated {
let validated = crate::operations::create(dot, crate::operations::CreateOptions::default())
.unwrap();
let validated =
crate::operations::create(dot, crate::operations::CreateOptions::default()).unwrap();
validated.raise_on_errors().unwrap();
validated
}