From 6cc5512eadf1f6365c81684832ca79b3e5a243fe Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 14 Mar 2026 15:42:03 -0400 Subject: [PATCH] Add resolve_run() that accepts run ID prefix or workflow name Subcommands like cp, diff, preview, ssh, and pr previously only accepted run ID prefixes. The new resolve_run() tries run ID prefix first, then falls back to workflow name (most recent run), making these commands more ergonomic. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-workflows/src/cli/cp.rs | 4 +- lib/crates/fabro-workflows/src/cli/diff.rs | 4 +- lib/crates/fabro-workflows/src/cli/pr.rs | 6 +- lib/crates/fabro-workflows/src/cli/preview.rs | 4 +- lib/crates/fabro-workflows/src/cli/runs.rs | 198 ++++++++++++++++++ lib/crates/fabro-workflows/src/cli/ssh.rs | 4 +- 6 files changed, 209 insertions(+), 11 deletions(-) diff --git a/lib/crates/fabro-workflows/src/cli/cp.rs b/lib/crates/fabro-workflows/src/cli/cp.rs index beaae52ac..17a1461b0 100644 --- a/lib/crates/fabro-workflows/src/cli/cp.rs +++ b/lib/crates/fabro-workflows/src/cli/cp.rs @@ -4,7 +4,7 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::{debug, info}; -use crate::cli::runs::{default_runs_base, find_run_by_prefix}; +use crate::cli::runs::{default_runs_base, resolve_run}; use crate::sandbox_record::SandboxRecord; #[derive(Args)] @@ -180,7 +180,7 @@ async fn load_sandbox( base: &Path, run_prefix: &str, ) -> Result> { - let run_dir = find_run_by_prefix(base, run_prefix)?; + let run_dir = resolve_run(base, run_prefix)?.path; let sandbox_json = run_dir.join("sandbox.json"); debug!(path = %sandbox_json.display(), "Loading sandbox record"); let record = SandboxRecord::load(&sandbox_json).context( diff --git a/lib/crates/fabro-workflows/src/cli/diff.rs b/lib/crates/fabro-workflows/src/cli/diff.rs index 1446bb76a..ef8ce13eb 100644 --- a/lib/crates/fabro-workflows/src/cli/diff.rs +++ b/lib/crates/fabro-workflows/src/cli/diff.rs @@ -5,7 +5,7 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::{debug, info}; -use crate::cli::runs::{default_runs_base, find_run_by_prefix}; +use crate::cli::runs::{default_runs_base, resolve_run}; use crate::engine::GIT_REMOTE; use crate::manifest::Manifest; use crate::sandbox_record::SandboxRecord; @@ -28,7 +28,7 @@ pub struct DiffArgs { pub async fn diff_command(args: DiffArgs) -> Result<()> { info!(run_id = %args.run, "Showing diff"); let base = default_runs_base(); - let run_dir = find_run_by_prefix(&base, &args.run)?; + let run_dir = resolve_run(&base, &args.run)?.path; let patch = resolve_diff(&run_dir, &args).await?; diff --git a/lib/crates/fabro-workflows/src/cli/pr.rs b/lib/crates/fabro-workflows/src/cli/pr.rs index 3b89a82ce..4be38dd05 100644 --- a/lib/crates/fabro-workflows/src/cli/pr.rs +++ b/lib/crates/fabro-workflows/src/cli/pr.rs @@ -4,7 +4,7 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::info; -use crate::cli::runs::{default_runs_base, find_run_by_prefix, scan_runs}; +use crate::cli::runs::{default_runs_base, resolve_run, scan_runs}; use crate::conclusion::Conclusion; use crate::manifest::Manifest; use crate::outcome::StageStatus; @@ -48,7 +48,7 @@ pub struct PrCloseArgs { } fn load_pr_record(base: &Path, run_id: &str) -> Result<(PullRequestRecord, PathBuf)> { - let run_dir = find_run_by_prefix(base, run_id)?; + let run_dir = resolve_run(base, run_id)?.path; let pr_path = run_dir.join("pull_request.json"); let content = std::fs::read_to_string(&pr_path).with_context(|| { format!( @@ -325,7 +325,7 @@ async fn pr_create_from( args: PrCreateArgs, github_app: Option, ) -> Result<()> { - let run_dir = find_run_by_prefix(base, &args.run_id)?; + let run_dir = resolve_run(base, &args.run_id)?.path; let manifest = Manifest::load(&run_dir.join("manifest.json")).context("Failed to load manifest.json")?; diff --git a/lib/crates/fabro-workflows/src/cli/preview.rs b/lib/crates/fabro-workflows/src/cli/preview.rs index 3cb6ba4ed..bbfb76e69 100644 --- a/lib/crates/fabro-workflows/src/cli/preview.rs +++ b/lib/crates/fabro-workflows/src/cli/preview.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::info; -use crate::cli::runs::{default_runs_base, find_run_by_prefix}; +use crate::cli::runs::{default_runs_base, resolve_run}; use crate::sandbox_record::SandboxRecord; #[derive(Args)] @@ -52,7 +52,7 @@ fn format_signed_output(url: &str) -> String { pub async fn preview_command(args: PreviewArgs) -> Result<()> { let base = default_runs_base(); - let run_dir = find_run_by_prefix(&base, &args.run)?; + let run_dir = resolve_run(&base, &args.run)?.path; let sandbox_json = run_dir.join("sandbox.json"); let record = SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?", diff --git a/lib/crates/fabro-workflows/src/cli/runs.rs b/lib/crates/fabro-workflows/src/cli/runs.rs index b29868264..552dc45fd 100644 --- a/lib/crates/fabro-workflows/src/cli/runs.rs +++ b/lib/crates/fabro-workflows/src/cli/runs.rs @@ -275,6 +275,55 @@ pub fn find_run_by_prefix(base: &Path, prefix: &str) -> Result { } } +/// Resolve a user-supplied identifier to a `RunInfo`. +/// +/// Resolution order: +/// 1. Run ID prefix match (like `find_run_by_prefix`) +/// 2. Workflow name substring match, returning the most recent run +/// +/// Errors if no match is found, or if a run ID prefix is ambiguous. +pub fn resolve_run(base: &Path, identifier: &str) -> Result { + let runs = scan_runs(base).context("Failed to scan runs")?; + + // Step 1: try run ID prefix match + let id_matches: Vec<_> = runs + .iter() + .filter(|r| r.run_id.starts_with(identifier)) + .collect(); + + match id_matches.len() { + 1 => { + debug!(identifier, matched = %id_matches[0].run_id, "Resolved run by ID prefix"); + return Ok(id_matches[0].clone()); + } + n if n > 1 => { + let ids: Vec<&str> = id_matches.iter().map(|r| r.run_id.as_str()).collect(); + bail!( + "Ambiguous prefix '{identifier}': {n} runs match: {}", + ids.join(", ") + ) + } + _ => {} + } + + // Step 2: try workflow name substring match, return most recent (runs are sorted newest-first) + let wf_match = runs + .iter() + .filter(|r| !r.is_orphan) + .find(|r| r.workflow_name.contains(identifier)); + + match wf_match { + Some(run) => { + debug!(identifier, matched = %run.run_id, workflow = %run.workflow_name, "Resolved run by workflow name"); + Ok(run.clone()) + } + None => { + warn!(identifier, "No matching run found"); + bail!("No run found matching '{identifier}' (tried run ID prefix and workflow name)") + } + } +} + pub fn list_command(args: &RunsListArgs) -> Result<()> { let base = default_runs_base(); let runs = scan_runs(&base)?; @@ -1198,6 +1247,155 @@ mod tests { ); } + // === resolve_run tests === + + #[test] + fn resolve_run_by_run_id_prefix() { + let dir = tempfile::tempdir().unwrap(); + make_run_dir( + dir.path(), + "20260101-ABC123", + Some(serde_json::json!({ + "run_id": "abc123-full-id", + "workflow_name": "deploy", + "goal": "", + "start_time": "2026-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + + let info = resolve_run(dir.path(), "abc123").unwrap(); + assert_eq!(info.run_id, "abc123-full-id"); + } + + #[test] + fn resolve_run_falls_back_to_workflow_name() { + let dir = tempfile::tempdir().unwrap(); + make_run_dir( + dir.path(), + "20260101-AAA111", + Some(serde_json::json!({ + "run_id": "aaa111-old", + "workflow_name": "deploy", + "goal": "", + "start_time": "2026-01-01T11:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + make_run_dir( + dir.path(), + "20260102-BBB222", + Some(serde_json::json!({ + "run_id": "bbb222-new", + "workflow_name": "deploy", + "goal": "", + "start_time": "2026-01-02T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + + // "deploy" doesn't match any run ID prefix, so falls back to workflow name + let info = resolve_run(dir.path(), "deploy").unwrap(); + // Should return the most recent one + assert_eq!(info.run_id, "bbb222-new"); + } + + #[test] + fn resolve_run_id_prefix_takes_priority_over_workflow_name() { + let dir = tempfile::tempdir().unwrap(); + // run_id starts with "deploy" AND workflow is "deploy" + make_run_dir( + dir.path(), + "20260101-DEPLOY", + Some(serde_json::json!({ + "run_id": "deploy-run-1", + "workflow_name": "other-workflow", + "goal": "", + "start_time": "2026-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + make_run_dir( + dir.path(), + "20260102-ZZZ999", + Some(serde_json::json!({ + "run_id": "zzz999-newer", + "workflow_name": "deploy", + "goal": "", + "start_time": "2026-01-02T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + + // "deploy" matches run_id prefix of first run — should prefer that over workflow name match + let info = resolve_run(dir.path(), "deploy").unwrap(); + assert_eq!(info.run_id, "deploy-run-1"); + } + + #[test] + fn resolve_run_errors_on_no_match() { + let dir = tempfile::tempdir().unwrap(); + let result = resolve_run(dir.path(), "nonexistent"); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("No run found"), "got: {msg}"); + } + + #[test] + fn resolve_run_errors_on_ambiguous_prefix() { + let dir = tempfile::tempdir().unwrap(); + make_run_dir( + dir.path(), + "20260101-D1", + Some(serde_json::json!({ + "run_id": "abc-111", + "workflow_name": "test", + "goal": "", + "start_time": "2026-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + make_run_dir( + dir.path(), + "20260101-D2", + Some(serde_json::json!({ + "run_id": "abc-222", + "workflow_name": "test", + "goal": "", + "start_time": "2026-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 + })), + None, + false, + ); + + let result = resolve_run(dir.path(), "abc"); + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("Ambiguous"), + "Should mention ambiguity" + ); + } + // === Step 1: end_time tests === #[test] diff --git a/lib/crates/fabro-workflows/src/cli/ssh.rs b/lib/crates/fabro-workflows/src/cli/ssh.rs index 655228807..5776b5cad 100644 --- a/lib/crates/fabro-workflows/src/cli/ssh.rs +++ b/lib/crates/fabro-workflows/src/cli/ssh.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Context, Result}; use clap::Args; use tracing::info; -use crate::cli::runs::{default_runs_base, find_run_by_prefix}; +use crate::cli::runs::{default_runs_base, resolve_run}; use crate::sandbox_record::SandboxRecord; #[derive(Args)] @@ -33,7 +33,7 @@ fn format_output(ssh_command: &str) -> String { pub async fn ssh_command(args: SshArgs) -> Result<()> { let base = default_runs_base(); - let run_dir = find_run_by_prefix(&base, &args.run)?; + let run_dir = resolve_run(&base, &args.run)?.path; let sandbox_json = run_dir.join("sandbox.json"); let record = SandboxRecord::load(&sandbox_json).context( "Failed to load sandbox.json — was this run started with a recent version of arc?",