diff --git a/lib/apps/fabro-cli/src/commands/run/attach.rs b/lib/apps/fabro-cli/src/commands/run/attach.rs index 15473dcf0..fe428ecd5 100644 --- a/lib/apps/fabro-cli/src/commands/run/attach.rs +++ b/lib/apps/fabro-cli/src/commands/run/attach.rs @@ -218,7 +218,7 @@ async fn attach_petri_run_with_client( printer: Printer, ) -> Result { let is_tty = std::io::stderr().is_terminal(); - let mut progress_ui = run_progress::ProgressUI::new(is_tty, opts.verbose); + let mut progress_ui = run_progress::ProgressUI::new(is_tty, opts.verbose, &state.spec); let ctrl_c_signal = ctrl_c(); tokio::pin!(ctrl_c_signal); diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs index 201e0ed5b..24cfa271f 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs @@ -27,7 +27,6 @@ pub(super) enum ProgressEvent { }, WorkflowStarted { worktree_dir: Option, - base_branch: Option, base_sha: Option, }, StageStarted { diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs index d57f59b70..ee02a3e6f 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs @@ -3,7 +3,7 @@ reason = "sync CLI run-progress renderer: writes to std::io::stderr directly" )] -use fabro_types::{RunNoticeCode, RunStreamItem}; +use fabro_types::{RunNoticeCode, RunSpec, RunStreamItem, RunTarget}; mod event; mod info_display; @@ -26,10 +26,11 @@ pub(crate) struct ProgressUI { info: InfoDisplay, saw_metadata_snapshot_failure: bool, petri: PetriProgressState, + source_branch: Option, } impl ProgressUI { - pub(crate) fn new(is_tty: bool, verbose: bool) -> Self { + pub(crate) fn new(is_tty: bool, verbose: bool, spec: &RunSpec) -> Self { let renderer = if is_tty { ProgressRenderer::new_tty() } else { @@ -38,7 +39,7 @@ impl ProgressUI { console::colors_enabled_stderr(), ) }; - Self::with_renderer(renderer, verbose) + Self::with_renderer(renderer, verbose).with_source_spec(spec) } fn with_renderer(renderer: ProgressRenderer, verbose: bool) -> Self { @@ -48,9 +49,25 @@ impl ProgressUI { info: InfoDisplay::new(verbose), saw_metadata_snapshot_failure: false, petri: PetriProgressState::default(), + source_branch: None, } } + fn with_source_spec(mut self, spec: &RunSpec) -> Self { + // A fork starts at a checkpoint, and a tag need not name the input + // branch's history. Keep those bases commit-only instead of pairing + // the inherited working branch with an unrelated revision. + if spec.fork_source_ref.is_none() + && !matches!(&spec.target, Some(RunTarget::Git(target)) if target.tag.is_some()) + { + self.source_branch = spec + .base_branch() + .filter(|branch| !branch.trim().is_empty()) + .map(str::to_owned); + } + self + } + pub(crate) fn hide_bars(&self) { self.renderer.hide(); } @@ -82,14 +99,13 @@ impl ProgressUI { } ProgressEvent::WorkflowStarted { worktree_dir, - base_branch, base_sha, } => { if let Some(worktree_dir) = worktree_dir { InfoDisplay::show_worktree(renderer, std::path::Path::new(&worktree_dir)); } if let Some(base_sha) = base_sha { - InfoDisplay::show_base_info(renderer, base_branch.as_deref(), &base_sha); + InfoDisplay::show_base_info(renderer, self.source_branch.as_deref(), &base_sha); } } ProgressEvent::StageStarted { diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/petri.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/petri.rs index e1f6dbfab..1ae1a5510 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/petri.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/petri.rs @@ -221,10 +221,6 @@ fn platform_progress_event(record: &Value) -> Option { }), "run.branch" => Some(ProgressEvent::WorkflowStarted { worktree_dir: None, - base_branch: record - .get("run_branch") - .and_then(Value::as_str) - .map(str::to_string), base_sha: record .get("base_sha") .and_then(Value::as_str) diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/tests.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/tests.rs index 3a33ef351..38068927e 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/tests.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/tests.rs @@ -6,7 +6,9 @@ use std::io::{self, Write}; use std::sync::{Arc, Mutex}; -use fabro_types::{RunStreamItemKind, fixtures}; +use fabro_types::{ + DirtyStatus, ForkSourceRef, GitContext, GitRunTarget, RunStreamItemKind, fixtures, test_support, +}; use fabro_util::terminal::Styles; use serde_json::{Value, json}; @@ -203,3 +205,82 @@ fn skipped_stage_is_dim_in_both_renderers_and_visible_in_plain_replay_without_a_ " ⊘ work skipped 0ms\n" ); } + +fn branch_record(base_sha: Option<&str>) -> RunStreamItem { + RunStreamItem { + run_id: fixtures::RUN_1, + stream_seq: 2, + kind: RunStreamItemKind::Platform, + id: "2".to_string(), + recorded_at: 1_789_706_579_000, + item: json!({"record": { + "kind": "run.branch", + "run_branch": format!("fabro/run/{}", fixtures::RUN_1), + "base_sha": base_sha + }}), + } +} + +fn source_spec() -> RunSpec { + RunSpec { + git: Some(GitContext { + origin_url: String::new(), + branch: "main".to_string(), + // The branch-point record, not this creation-time observation, + // supplies the SHA displayed next to the source branch. + sha: Some("1111111111111111111111111111111111111111".to_string()), + dirty: DirtyStatus::Clean, + }), + ..test_support::test_run_spec() + } +} + +fn render_base(spec: &RunSpec, base_sha: Option<&str>, verbose: bool) -> String { + let output = Output::default(); + let renderer = ProgressRenderer::new_plain(Box::new(output.clone()), false); + let mut ui = ProgressUI::with_renderer(renderer, verbose).with_source_spec(spec); + let record = branch_record(base_sha); + let raw = petri_stream::raw_line(&record).unwrap(); + ui.handle_stream_item(&record); + ui.finish(); + assert_eq!(petri_stream::raw_line(&record).unwrap(), raw); + output.text() +} + +#[test] +fn base_header_uses_saved_source_branch_and_recorded_base_commit() { + let spec = source_spec(); + for verbose in [false, true] { + insta::allow_duplicates! { + insta::assert_snapshot!(render_base(&spec, Some("abcdef0123456789"), verbose), @" Base: main (abcdef012345)"); + } + assert_eq!(render_base(&spec, None, verbose), ""); + } +} + +#[test] +fn base_header_uses_only_the_commit_when_source_branch_is_unknown_or_ambiguous() { + let mut absent = source_spec(); + absent.git = None; + let mut empty = source_spec(); + empty.git.as_mut().unwrap().branch = " ".to_string(); + let mut fork = source_spec(); + fork.fork_source_ref = Some(ForkSourceRef { + source_run_id: fixtures::RUN_2, + checkpoint_sha: "abcdef0123456789".to_string(), + }); + let mut tag = source_spec(); + tag.target = Some(RunTarget::Git(GitRunTarget { + repo: "acme/widgets".to_string(), + branch: "main".to_string(), + tag: Some("v1".to_string()), + sha: None, + })); + for spec in [absent, empty, fork, tag] { + for verbose in [false, true] { + insta::allow_duplicates! { + insta::assert_snapshot!(render_base(&spec, Some("abcdef0123456789"), verbose), @" Base: abcdef012345"); + } + } + } +} diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index a30df85d8..1f6125c4f 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -2905,3 +2905,51 @@ fn attach_json_errors_without_prompting_for_human_input() { }); wait_for_status(&run.run_dir, &["succeeded"]); } + +#[test] +fn attach_base_header_preserves_the_saved_source_branch() { + let context = test_context!(); + let workflow = super::support::write_workflow(&context.temp_dir, "base-header", "BaseHeader"); + let workspace = context.temp_dir.join("source"); + context.write_temp("source/README.md", "source commit\n"); + let source_sha = super::support::init_remote_fixture(&workspace, "main"); + let run = context + .run_cmd() + .args([ + "--auto-approve", + "--detach", + "--environment", + "local", + "--target-from", + ]) + .arg(&workspace) + .arg(&workflow) + .timeout(SHARED_DAEMON_TIMEOUT) + .assert() + .success(); + let run_id = created_run_id(run.get_output()); + context + .command() + .args(["wait", &run_id]) + .timeout(SHARED_DAEMON_TIMEOUT) + .assert() + .success(); + + // Reattach must use the stored source, even if the caller changes branches. + super::support::run_git(&workspace, &["checkout", "-b", "later"]); + let output = context + .command() + .args(["attach", &run_id]) + .current_dir(&workspace) + .timeout(SHARED_DAEMON_TIMEOUT) + .assert() + .success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + let base_lines: Vec<_> = stderr + .lines() + .map(str::trim) + .filter(|line| line.starts_with("Base:")) + .collect(); + // Do not apply the shared Base filter: it would hide a wrong branch name. + assert_eq!(base_lines, [format!("Base: main ({})", &source_sha[..12])]); +} diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index f0514e84f..7dc21fa27 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -74,6 +74,7 @@ static INSTA_FILTERS: &[(&str, &str)] = &[ "Duration: [DURATION]", ), (r"Base: [^\n]+ \([0-9a-f]{7,40}\)", "Base: [BASE]"), + (r"Base: [0-9a-f]{7,40}\b", "Base: [BASE]"), (r"(Branch: [^\n]+ from )[0-9a-f]{7,40}", "${1}[SHA]"), // The sandbox driver's events: per-process event source ids, operation // ids, sub-second durations, and a local sandbox's path-derived id.