From a3abbaf1917942bd77440d6c4054fa687e39926d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 8 Mar 2026 03:02:57 -0400 Subject: [PATCH] Skip git checkpoint commit for start node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The start node is a no-op (StartHandler returns success immediately), so its git checkpoint commit is always empty. Skipping it reduces noise in git history without losing any data — the checkpoint JSON is still saved to disk, and the next node's diff falls back to base_sha correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/arc-workflows/src/engine.rs | 87 +++++++++++++++++++++++ crates/arc-workflows/tests/integration.rs | 16 +++-- 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/crates/arc-workflows/src/engine.rs b/crates/arc-workflows/src/engine.rs index 05068b478..b781467d7 100644 --- a/crates/arc-workflows/src/engine.rs +++ b/crates/arc-workflows/src/engine.rs @@ -1893,6 +1893,12 @@ impl WorkflowRunEngine { } // Step 6b: Write shadow branch first, then run branch commit with trailer + // Skip git checkpoint for the start node — it's a no-op, so the commit is always empty. + let is_start_node = graph + .find_start_node() + .map(|n| n.id == node.id) + .unwrap_or(false); + if !is_start_node { if let Some(ref mode) = config.git_checkpoint { // Shadow commit (best-effort): extract repo path from either variant let shadow_sha: Option = if config.meta_branch.is_some() { @@ -2025,6 +2031,7 @@ impl WorkflowRunEngine { context.append_log("git checkpoint commit failed".to_string()); } } + } // Step 7: Follow selected edge (or direct jump) if let Some(target) = jump_target { @@ -4838,4 +4845,84 @@ mod tests { "expected failure signature in context, got: {sig_str}" ); } + + #[tokio::test] + async fn git_checkpoint_skipped_for_start_node() { + // Set up a real git repo so GitCheckpointMode::Host works + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(repo) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["-c", "user.name=Test", "-c", "user.email=test@test.com", "commit", "--allow-empty", "-m", "initial"]) + .current_dir(repo) + .output() + .unwrap(); + let base_sha = String::from_utf8( + std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap() + .stdout, + ) + .unwrap() + .trim() + .to_string(); + + let logs_dir = tempfile::tempdir().unwrap(); + + // Build start -> work -> exit graph so work node produces a git checkpoint + let mut g = simple_graph(); + let work = Node::new("work"); + g.nodes.insert("work".to_string(), work); + g.edges.clear(); + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let events = std::sync::Arc::new(std::sync::Mutex::new(Vec::::new())); + let events_clone = events.clone(); + let mut emitter = EventEmitter::new(); + emitter.on_event(move |event| { + events_clone.lock().unwrap().push(event.clone()); + }); + + let engine = WorkflowRunEngine::new(make_registry(), Arc::new(emitter), local_env()); + let config = RunConfig { + logs_root: logs_dir.path().to_path_buf(), + cancel_token: None, + dry_run: false, + run_id: "git-cp-test".into(), + git_checkpoint: Some(GitCheckpointMode::Host(repo.to_path_buf())), + base_sha: Some(base_sha), + run_branch: None, + meta_branch: None, + labels: HashMap::new(), + checkpoint_exclude_globs: Vec::new(), + github_app: None, + git_author: crate::git::GitAuthor::default(), + }; + engine.run(&g, &config).await.unwrap(); + + let collected = events.lock().unwrap(); + let git_checkpoint_node_ids: Vec<&str> = collected + .iter() + .filter_map(|e| match e { + WorkflowRunEvent::GitCheckpoint { node_id, .. } => Some(node_id.as_str()), + _ => None, + }) + .collect(); + + assert!( + !git_checkpoint_node_ids.contains(&"start"), + "start node should not have a git checkpoint, but found: {git_checkpoint_node_ids:?}" + ); + assert!( + git_checkpoint_node_ids.contains(&"work"), + "work node should have a git checkpoint, but found: {git_checkpoint_node_ids:?}" + ); + } } diff --git a/crates/arc-workflows/tests/integration.rs b/crates/arc-workflows/tests/integration.rs index fa695f825..f7deaca77 100644 --- a/crates/arc-workflows/tests/integration.rs +++ b/crates/arc-workflows/tests/integration.rs @@ -10180,12 +10180,16 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { } }) .collect(); - // start, work, exit = 3 nodes, each gets a checkpoint commit + // work node gets a checkpoint commit (start is skipped, exit is terminal) assert!( - git_events.len() >= 2, - "expected at least 2 GitCheckpoint events, got {}", + git_events.len() >= 1, + "expected at least 1 GitCheckpoint event, got {}", git_events.len() ); + assert!( + !git_events.iter().any(|(id, _)| id == "start"), + "start node should not have a git checkpoint" + ); // Each SHA should be a valid 40-char hex string assert!( git_events @@ -10194,15 +10198,15 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { "all SHAs should be 40-char hex, got: {git_events:?}" ); - // 7. Assert diff.patch was written for the "start" node (where hello.txt is committed) + // 7. diff.patch is NOT written for the start node (git checkpoint skipped) let start_diff = logs_dir .path() .join("nodes") .join("start") .join("diff.patch"); assert!( - start_diff.exists(), - "diff.patch should exist for start node (hello.txt committed there)" + !start_diff.exists(), + "diff.patch should not exist for start node (git checkpoint skipped)" ); // 8. Verify checkpoint.json has git_commit_sha