mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
Skip git checkpoint commit for start node
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) <noreply@anthropic.com>
This commit is contained in:
parent
77fd3134c3
commit
a3abbaf191
2 changed files with 97 additions and 6 deletions
|
|
@ -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<String> = 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::<WorkflowRunEvent>::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:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue