Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 64 additions & 20 deletions src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2669,22 +2669,33 @@ fn strict_cherry_pick_mappings_from_command(
context, new_head
)));
}
let mut source_commits = pending_source_commits;
if source_commits.is_empty() {
source_commits = cherry_pick_source_commits_from_command(cmd);
}
if source_commits.is_empty() {
return Err(GitAiError::Generic(format!(
"{} missing cherry-pick source commits",
context
)));
}
let worktree = cmd.worktree.as_deref().ok_or_else(|| {
GitAiError::Generic(format!(
"{} missing worktree for cherry-pick mapping new={}",
context, new_head
))
})?;
// Resolve source commits: prefer pending (cached from start event), fall
// back to parsing command args. Either path may contain short SHAs or
// symbolic refs, so resolve them to full OIDs via git rev-parse. This
// runs in the async side-effect path, not the daemon critical path.
let mut source_refs = pending_source_commits;
if source_refs.is_empty() {
source_refs = cherry_pick_source_refs_from_command(cmd);
}
if source_refs.is_empty() {
return Err(GitAiError::Generic(format!(
"{} missing cherry-pick source commits",
context
)));
}
let source_commits = resolve_cherry_pick_source_refs(&source_refs, worktree, context)?;
if source_commits.is_empty() {
return Err(GitAiError::Generic(format!(
"{} cherry-pick source refs resolved to no valid commits",
context
)));
}
let (original_head, new_commits) = resolve_linear_head_commit_chain_for_worktree(
worktree,
new_head,
Expand All @@ -2703,13 +2714,12 @@ fn strict_cherry_pick_mappings_from_command(
Ok((original_head, source_commits, new_commits))
}

fn append_unique_oid(target: &mut Vec<String>, value: &str) {
if is_valid_oid(value) && !is_zero_oid(value) && !target.iter().any(|seen| seen == value) {
target.push(value.to_string());
}
}

fn cherry_pick_source_commits_from_command(
/// Collect positional arguments from a cherry-pick command as potential commit
/// references. Unlike the full-OID-only `is_valid_oid` check, this accepts short SHA prefixes and
/// symbolic refs (e.g. branch names) that git would resolve on the command line.
/// Resolution to full OIDs happens later in `resolve_cherry_pick_source_refs`
/// which runs in the async side-effect path.
fn cherry_pick_source_refs_from_command(
cmd: &crate::daemon::domain::NormalizedCommand,
) -> Vec<String> {
let mut out = Vec::new();
Expand All @@ -2733,11 +2743,45 @@ fn cherry_pick_source_commits_from_command(
if arg.starts_with('-') {
continue;
}
append_unique_oid(&mut out, arg);
if !arg.is_empty() && !out.iter().any(|seen: &String| seen == arg) {
out.push(arg.to_string());
}
}
out
}

/// Resolve cherry-pick source refs (which may be short SHAs, branch names, or
/// full OIDs) to full commit OIDs. This calls `git rev-parse` and MUST only be
/// invoked from the async side-effect path, never the daemon critical path.
fn resolve_cherry_pick_source_refs(
source_refs: &[String],
worktree: &Path,
context: &str,
) -> Result<Vec<String>, GitAiError> {
let mut resolved = Vec::new();
let repo = find_repository_in_path(worktree.to_string_lossy().as_ref())?;
for src in source_refs {
if is_valid_oid(src) && !is_zero_oid(src) {
resolved.push(src.clone());
} else {
let obj = repo.revparse_single(src).map_err(|err| {
GitAiError::Generic(format!(
"{} failed to resolve cherry-pick source ref '{}': {}",
context, src, err
))
})?;
let oid = obj
.peel_to_commit()
.map(|c| c.id())
.unwrap_or_else(|_| obj.id());
if is_valid_oid(&oid) && !is_zero_oid(&oid) {
resolved.push(oid);
}
}
}
Ok(resolved)
}

fn rebase_is_control_mode(cmd: &crate::daemon::domain::NormalizedCommand) -> bool {
summarize_rebase_args(&cmd.invoked_args).is_control_mode
}
Expand Down Expand Up @@ -5972,8 +6016,8 @@ impl ActorDaemonCoordinator {
if cmd.invoked_args.iter().any(|arg| arg == "--abort") {
self.clear_pending_cherry_pick_sources_for_worktree(worktree)?;
} else if cmd.exit_code != 0 {
let source_commits = cherry_pick_source_commits_from_command(cmd);
self.set_pending_cherry_pick_sources_for_worktree(worktree, source_commits)?;
let source_refs = cherry_pick_source_refs_from_command(cmd);
self.set_pending_cherry_pick_sources_for_worktree(worktree, source_refs)?;
}
}
return Ok(());
Expand Down
236 changes: 236 additions & 0 deletions tests/daemon_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,242 @@ fn daemon_pure_trace_socket_cherry_pick_continue_emits_complete_event() {
);
}

#[test]
#[serial]
fn daemon_pure_trace_socket_rebase_with_short_sha_emits_complete_event() {
let repo = TestRepo::new_with_mode(GitTestMode::Wrapper);
let _daemon = DaemonGuard::start(&repo);
let trace_socket = daemon_trace_socket_path(&repo);
let env = git_trace_env(&trace_socket);
let env_refs = [(env[0].0, env[0].1.as_str()), (env[1].0, env[1].1.as_str())];
let default_branch = repo.current_branch();
let completion_baseline = repo.daemon_total_completion_count();
let mut expected_top_level_completions = 0u64;

// Create base commit on default branch
fs::write(repo.path().join("rebase-short.txt"), "base\n").expect("failed to write base");
traced_git_with_env(
&repo,
&["add", "rebase-short.txt"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("add should succeed");
traced_git_with_env(
&repo,
&["commit", "-m", "base"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("base commit should succeed");

// Create feature branch with a commit
traced_git_with_env(
&repo,
&["checkout", "-b", "feature-rebase-short"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("feature branch checkout should succeed");
fs::write(repo.path().join("feature-only.txt"), "feature content\n")
.expect("failed to write feature file");
traced_git_with_env(
&repo,
&["add", "feature-only.txt"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("feature add should succeed");
traced_git_with_env(
&repo,
&["commit", "-m", "feature change"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("feature commit should succeed");

// Go back to default branch and add a non-conflicting commit
traced_git_with_env(
&repo,
&["checkout", default_branch.as_str()],
&env_refs,
&mut expected_top_level_completions,
)
.expect("checkout default should succeed");
fs::write(repo.path().join("main-only.txt"), "main content\n")
.expect("failed to write main file");
traced_git_with_env(
&repo,
&["add", "main-only.txt"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("main add should succeed");
traced_git_with_env(
&repo,
&["commit", "-m", "main advance"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("main commit should succeed");

// Get the short SHA of the latest main commit
let main_full_sha = repo
.git(&["rev-parse", "HEAD"])
.expect("HEAD rev-parse should succeed")
.trim()
.to_string();
let main_short_sha = &main_full_sha[..7];

// Switch to feature branch and rebase onto main using SHORT SHA
traced_git_with_env(
&repo,
&["checkout", "feature-rebase-short"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("checkout feature should succeed");
traced_git_with_env(
&repo,
&["rebase", main_short_sha],
&env_refs,
&mut expected_top_level_completions,
)
.expect("rebase with short SHA should succeed");

wait_for_expected_top_level_completions(
&repo,
completion_baseline,
expected_top_level_completions,
);

let rewrite_log_path = git_common_dir(&repo).join("ai").join("rewrite_log");
let rewrite_log = fs::read_to_string(&rewrite_log_path)
.expect("rewrite log should exist after rebase with short SHA");
assert!(
rewrite_log
.lines()
.any(|line| line.contains("\"rebase_complete\"")),
"daemon should emit rebase_complete even when rebase uses a short SHA, rewrite_log: {}",
rewrite_log
);
}

#[test]
#[serial]
fn daemon_pure_trace_socket_cherry_pick_with_short_sha_emits_complete_event() {
let repo = TestRepo::new_with_mode(GitTestMode::Wrapper);
let _daemon = DaemonGuard::start(&repo);
let trace_socket = daemon_trace_socket_path(&repo);
let env = git_trace_env(&trace_socket);
let env_refs = [(env[0].0, env[0].1.as_str()), (env[1].0, env[1].1.as_str())];
let default_branch = repo.current_branch();
let completion_baseline = repo.daemon_total_completion_count();
let mut expected_top_level_completions = 0u64;

// Create base commit
fs::write(repo.path().join("short-sha-test.txt"), "base\n").expect("failed to write base");
traced_git_with_env(
&repo,
&["add", "short-sha-test.txt"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("add should succeed");
traced_git_with_env(
&repo,
&["commit", "-m", "base"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("base commit should succeed");

// Create topic branch with a commit
traced_git_with_env(
&repo,
&["checkout", "-b", "topic-short-sha"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("topic branch checkout should succeed");
fs::write(repo.path().join("short-sha-test.txt"), "topic content\n")
.expect("failed to write topic change");
traced_git_with_env(
&repo,
&["add", "short-sha-test.txt"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("topic add should succeed");
traced_git_with_env(
&repo,
&["commit", "-m", "topic change"],
&env_refs,
&mut expected_top_level_completions,
)
.expect("topic commit should succeed");

// Get the full SHA and derive a short (7-char) prefix
let topic_full_sha = repo
.git(&["rev-parse", "topic-short-sha"])
.expect("topic rev-parse should succeed")
.trim()
.to_string();
let topic_short_sha = &topic_full_sha[..7];

// Switch back to default branch
traced_git_with_env(
&repo,
&["checkout", default_branch.as_str()],
&env_refs,
&mut expected_top_level_completions,
)
.expect("checkout default branch should succeed");

// Cherry-pick using the SHORT SHA -- this is the key part of the test
traced_git_with_env(
&repo,
&["cherry-pick", topic_short_sha],
&env_refs,
&mut expected_top_level_completions,
)
.expect("cherry-pick with short SHA should succeed");

wait_for_expected_top_level_completions(
&repo,
completion_baseline,
expected_top_level_completions,
);

let rewrite_log_path = git_common_dir(&repo).join("ai").join("rewrite_log");
let rewrite_log = fs::read_to_string(&rewrite_log_path)
.expect("rewrite log should exist after cherry-pick with short SHA");
assert!(
rewrite_log
.lines()
.any(|line| line.contains("\"cherry_pick_complete\"")),
"daemon should emit cherry_pick_complete even when cherry-pick uses a short SHA, rewrite_log: {}",
rewrite_log
);

// Verify the source commits in the event contain the FULL SHA, not the short one
for line in rewrite_log.lines() {
if line.contains("\"cherry_pick_complete\"") {
assert!(
line.contains(&topic_full_sha),
"cherry_pick_complete event should contain full resolved SHA {}, got: {}",
topic_full_sha,
line
);
assert!(
!line.contains(&format!("\"{}\"", topic_short_sha))
|| line.contains(&topic_full_sha),
"cherry_pick_complete should not contain unresolved short SHA"
);
}
}
}

#[test]
#[serial]
fn daemon_pure_trace_socket_switch_tracks_success_and_conflict_failure() {
Expand Down
Loading