From b01e08a52daa91c4f3caf2b740cf284d4bd21a1f Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Apr 2026 08:57:50 -0400 Subject: [PATCH] refactor(workflow): drop Concluded.run_id and pushed_branch Both fields were derivable from run_options (run_options.run_id and run_options.git.as_ref().and_then(|g| g.run_branch.clone())), so they were a second place to keep in sync with the canonical source. Drop both from Concluded, populate Finalized's copies from run_options at the pull_request phase boundary. Add RunOptions::run_branch() helper so the "reach into optional git opts" pattern reads as a single call. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-workflow/src/pipeline/finalize.rs | 2 -- .../fabro-workflow/src/pipeline/pull_request.rs | 8 +++----- lib/crates/fabro-workflow/src/pipeline/types.rs | 12 +++++------- lib/crates/fabro-workflow/src/run_options.rs | 5 +++++ 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index 3ea0bf11f..470e53381 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -374,10 +374,8 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result Finalized { let Concluded { - run_id, outcome, conclusion, - pushed_branch, graph, run_options, services, @@ -526,7 +524,7 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) -> let diff = load_pull_request_diff(&services.run_store).await; if let (Some(base_branch), Some(run_branch), Some(creds), Some(origin)) = ( &run_options.base_branch, - pushed_branch.as_deref(), + run_options.run_branch(), &options.github_app, &options.origin_url, ) { @@ -584,10 +582,10 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) -> } Finalized { - run_id, + run_id: run_options.run_id, outcome, conclusion, - pushed_branch, + pushed_branch: run_options.run_branch().map(str::to_string), pr_url, } } diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 7bf08413e..defa987f9 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -297,13 +297,11 @@ pub struct Retroed { /// Output of the FINALIZE phase. #[non_exhaustive] pub struct Concluded { - pub run_id: RunId, - pub outcome: Result, - pub conclusion: Conclusion, - pub pushed_branch: Option, - pub graph: Graph, - pub run_options: RunOptions, - pub services: Arc, + pub outcome: Result, + pub conclusion: Conclusion, + pub graph: Graph, + pub run_options: RunOptions, + pub services: Arc, } /// Output of the PULL_REQUEST phase. diff --git a/lib/crates/fabro-workflow/src/run_options.rs b/lib/crates/fabro-workflow/src/run_options.rs index 5ee57fb05..bdab44985 100644 --- a/lib/crates/fabro-workflow/src/run_options.rs +++ b/lib/crates/fabro-workflow/src/run_options.rs @@ -57,6 +57,11 @@ impl RunOptions { pub fn artifact_globs(&self) -> Vec { self.settings.run.artifacts.include.clone() } + + /// Run branch name from git checkpoint options, if set. + pub fn run_branch(&self) -> Option<&str> { + self.git.as_ref().and_then(|g| g.run_branch.as_deref()) + } } /// Options for sandbox lifecycle management within the engine.