diff --git a/src/daemon.rs b/src/daemon.rs index 0b960ffcd1..980d44b9dd 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -2668,6 +2668,8 @@ struct PendingRootSlot { type CommitFileTimestampSnapshotHandle = tokio::task::JoinHandle>; type CommitFileTimestampSnapshotHandles = HashMap; +type ReflogStartOffsets = HashMap; +type AsyncReflogStartOffsetsByRoot = HashMap; const COMMIT_FILE_TIMESTAMP_SNAPSHOT_WAIT: Duration = Duration::from_millis(500); const SESSION_EVENT_RECOVERY_PREFLIGHT_WAIT: Duration = Duration::from_secs(2); @@ -2721,7 +2723,6 @@ struct TraceIngressState { root_families: HashMap, root_argv: HashMap>, root_started_at_ns: HashMap, - root_reflog_start_offsets: HashMap>, root_mutating: HashMap, root_target_repo_only: HashMap, root_last_activity_ns: HashMap, @@ -2783,6 +2784,10 @@ pub struct ActorDaemonCoordinator { processed_trace_ingest_seq: AtomicUsize, trace_ingest_progress_notify: Notify, trace_ingress_state: Mutex, + /// Command-start reflog snapshots owned by the serialized async ingest + /// worker. Keeping them here lets split trace2 metadata converge without + /// putting repository reads back on the listener's critical path. + async_reflog_start_offsets_by_root: Mutex, shutting_down: AtomicBool, shutdown_action: AtomicU8, shutdown_notify: Notify, @@ -2885,6 +2890,7 @@ impl ActorDaemonCoordinator { processed_trace_ingest_seq: AtomicUsize::new(0), trace_ingest_progress_notify: Notify::new(), trace_ingress_state: Mutex::new(TraceIngressState::default()), + async_reflog_start_offsets_by_root: Mutex::new(HashMap::new()), shutting_down: AtomicBool::new(false), shutdown_action: AtomicU8::new(DaemonExitAction::Stop.as_u8()), shutdown_notify: Notify::new(), @@ -3663,7 +3669,6 @@ impl ActorDaemonCoordinator { .get(root_sid) .copied() .unwrap_or(false) - || ingress.root_reflog_start_offsets.contains_key(root_sid) } fn clear_trace_ingress_root_locked(ingress: &mut TraceIngressState, root_sid: &str) { @@ -3671,7 +3676,6 @@ impl ActorDaemonCoordinator { ingress.root_families.remove(root_sid); ingress.root_argv.remove(root_sid); ingress.root_started_at_ns.remove(root_sid); - ingress.root_reflog_start_offsets.remove(root_sid); ingress.root_mutating.remove(root_sid); ingress.root_target_repo_only.remove(root_sid); ingress.root_last_activity_ns.remove(root_sid); @@ -3682,6 +3686,7 @@ impl ActorDaemonCoordinator { fn record_trace_connection_close(&self, roots: &[String]) -> Result, GitAiError> { let mut close_marker_candidates = Vec::new(); + let mut roots_cleared_without_marker = Vec::new(); let mut ingress = self .trace_ingress_state .lock() @@ -3696,6 +3701,7 @@ impl ActorDaemonCoordinator { } if !Self::trace_root_needs_close_marker(&ingress, root_sid) { Self::clear_trace_ingress_root_locked(&mut ingress, root_sid); + roots_cleared_without_marker.push(root_sid); continue; } if ingress.root_close_markers_enqueued.contains(root_sid) { @@ -3704,6 +3710,26 @@ impl ActorDaemonCoordinator { ingress.root_close_markers_enqueued.insert(root_sid.clone()); close_marker_candidates.push(root_sid.clone()); } + drop(ingress); + let mut roots_safe_to_clear = Vec::new(); + if !roots_cleared_without_marker.is_empty() { + let queued = self.queued_trace_payloads_by_root.lock().map_err(|_| { + GitAiError::Generic("queued trace payloads by root lock poisoned".to_string()) + })?; + for root_sid in roots_cleared_without_marker { + if queued.get(root_sid).copied().unwrap_or(0) > 0 { + close_marker_candidates.push(root_sid.clone()); + } else { + roots_safe_to_clear.push(root_sid); + } + } + } + if !roots_safe_to_clear.is_empty() { + let mut offsets = self.async_reflog_start_offsets_by_root()?; + for root_sid in roots_safe_to_clear { + offsets.remove(root_sid); + } + } self.trace_ingest_progress_notify.notify_waiters(); Ok(close_marker_candidates) } @@ -3794,6 +3820,7 @@ impl ActorDaemonCoordinator { GitAiError::Generic("queued trace payloads by root lock poisoned".to_string()) })?; queued.remove(root_sid); + self.async_reflog_start_offsets_by_root()?.remove(root_sid); self.trace_ingest_progress_notify.notify_waiters(); Ok(()) } @@ -4515,6 +4542,7 @@ impl ActorDaemonCoordinator { ingress .root_mutating .entry(root.clone()) + .and_modify(|mutating| *mutating |= command_mutates_refs) .or_insert(command_mutates_refs); let target_repo_only = trace_command_uses_target_repo_context_only(Some(primary)); ingress @@ -4524,26 +4552,12 @@ impl ActorDaemonCoordinator { } let terminal = is_terminal_root_trace_event(&event, &sid, &root); - if command_mutates_refs - && !terminal - && !ingress.root_reflog_start_offsets.contains_key(&root) - && let Some(worktree) = worktree_hint - .clone() - .or_else(|| ingress.root_worktrees.get(&root).cloned()) - { - let offsets = - crate::daemon::ref_cursor::capture_reflog_start_offsets_for_worktree(&worktree); - ingress - .root_reflog_start_offsets - .insert(root.clone(), offsets); - } let read_only_root = event_is_read_only || ingress.root_definitely_read_only.contains(&root); let inherited = ( ingress.root_argv.get(&root).cloned(), ingress.root_started_at_ns.get(&root).copied(), - ingress.root_reflog_start_offsets.get(&root).cloned(), ingress.root_worktrees.get(&root).cloned(), ); if terminal { @@ -4551,7 +4565,6 @@ impl ActorDaemonCoordinator { ingress.root_families.remove(&root); ingress.root_argv.remove(&root); ingress.root_started_at_ns.remove(&root); - ingress.root_reflog_start_offsets.remove(&root); ingress.root_mutating.remove(&root); ingress.root_target_repo_only.remove(&root); ingress.root_last_activity_ns.remove(&root); @@ -4575,18 +4588,10 @@ impl ActorDaemonCoordinator { json!(started_at_ns), ); } - if object.get(TRACE_ROOT_REFLOG_START_OFFSETS_FIELD).is_none() - && let Some(offsets) = inherited.2 - { - object.insert( - TRACE_ROOT_REFLOG_START_OFFSETS_FIELD.to_string(), - json!(offsets), - ); - } if object.get(TRACE_ROOT_WORKTREE_FIELD).is_none() && object.get("worktree").is_none() && object.get("repo_working_dir").is_none() - && let Some(worktree) = inherited.3 + && let Some(worktree) = inherited.2 { object.insert( TRACE_ROOT_WORKTREE_FIELD.to_string(), @@ -6537,10 +6542,99 @@ impl ActorDaemonCoordinator { Ok(()) } + fn async_reflog_start_offsets_by_root( + &self, + ) -> Result, GitAiError> { + self.async_reflog_start_offsets_by_root.lock().map_err(|_| { + GitAiError::Generic("async reflog start offsets by root lock poisoned".to_string()) + }) + } + + async fn attach_reflog_start_offsets_for_async_ingest( + &self, + payload: &mut Value, + ) -> Result<(), GitAiError> { + let event = payload + .get("event") + .and_then(Value::as_str) + .unwrap_or_default(); + let sid = payload + .get("sid") + .and_then(Value::as_str) + .unwrap_or_default(); + let root_sid = trace_root_sid(sid); + if sid.is_empty() + || sid != root_sid + || event == TRACE_CONNECTION_CLOSED_EVENT + || (event == "def_repo" + && crate::daemon::trace_normalizer::def_repo_is_secondary(payload)) + { + return Ok(()); + } + + if let Some(offsets) = payload + .get(TRACE_ROOT_REFLOG_START_OFFSETS_FIELD) + .and_then(Value::as_object) + .and_then(|offsets| serde_json::from_value(Value::Object(offsets.clone())).ok()) + { + self.async_reflog_start_offsets_by_root()? + .entry(root_sid.to_string()) + .or_insert(offsets); + return Ok(()); + } + + if let Some(offsets) = self + .async_reflog_start_offsets_by_root()? + .get(root_sid) + .cloned() + { + if let Some(object) = payload.as_object_mut() { + object.insert( + TRACE_ROOT_REFLOG_START_OFFSETS_FIELD.to_string(), + json!(offsets), + ); + } + return Ok(()); + } + + if is_terminal_root_trace_event(event, sid, root_sid) { + return Ok(()); + } + + let argv = trace_payload_effective_argv(payload); + let primary = + trace_payload_primary_command(payload).or_else(|| trace_argv_primary_command(&argv)); + if !trace_invocation_may_mutate_refs(primary.as_deref(), &argv) { + return Ok(()); + } + let Some(worktree) = trace_payload_worktree_hint(payload) else { + return Ok(()); + }; + + let offsets = tokio::task::spawn_blocking(move || { + crate::daemon::ref_cursor::capture_reflog_start_offsets_for_worktree(&worktree) + }) + .await + .map_err(|error| { + GitAiError::Generic(format!("reflog start capture worker failed: {error}")) + })?; + self.async_reflog_start_offsets_by_root()? + .insert(root_sid.to_string(), offsets.clone()); + if let Some(object) = payload.as_object_mut() { + object.insert( + TRACE_ROOT_REFLOG_START_OFFSETS_FIELD.to_string(), + json!(offsets), + ); + } + Ok(()) + } + async fn apply_trace_payload_to_state( &self, - payload: Value, + mut payload: Value, ) -> Result { + self.attach_reflog_start_offsets_for_async_ingest(&mut payload) + .await?; let payload_root_sid = Self::trace_payload_root_sid(&payload); let event = payload .get("event") @@ -9961,6 +10055,9 @@ mod tests { "git init failed: {}", String::from_utf8_lossy(&init.stderr) ); + let head_log = repo.join(".git/logs/HEAD"); + std::fs::create_dir_all(head_log.parent().unwrap()).unwrap(); + std::fs::write(&head_log, b"pre-command HEAD entry\n").unwrap(); let sid = "20260411T120000.000000-Psid-split-metadata"; let mut def_repo = serde_json::json!({ @@ -9990,6 +10087,23 @@ mod tests { "time_ns": 2u64, }); assert!(coord.prepare_trace_payload_for_ingest(&mut start)); + coord + .attach_reflog_start_offsets_for_async_ingest(&mut start) + .await + .unwrap(); + let head_key = format!( + "worktree:{}:HEAD", + repo.join(".git").canonicalize().unwrap().to_string_lossy() + ); + assert_eq!( + start + .get(TRACE_ROOT_REFLOG_START_OFFSETS_FIELD) + .and_then(Value::as_object) + .and_then(|offsets| offsets.get(&head_key)) + .and_then(Value::as_u64), + Some(std::fs::metadata(&head_log).unwrap().len()), + "async ingestion must capture the baseline after split repo/argv metadata becomes complete" + ); coord .apply_trace_payload_to_state(start) .await @@ -10006,7 +10120,7 @@ mod tests { } #[tokio::test] - async fn mutating_trace_payload_captures_repo_reflog_start_offsets() { + async fn mutating_trace_payload_defers_repo_reflog_capture_to_async_ingest() { let coord = ActorDaemonCoordinator::new(); let temp = tempfile::tempdir().unwrap(); let repo = temp.path().join("repo"); @@ -10025,18 +10139,26 @@ mod tests { std::fs::write(&stash_log, old_reflog).unwrap(); std::fs::write(&branch_log, old_branch_reflog).unwrap(); let mut payload = serde_json::json!({ - "event": "start", + "event": "def_repo", "sid": "20260411T120000.000000-Psid-reflog", "argv": ["git", "reset", "--hard", "HEAD~1"], "worktree": repo, }); assert!(coord.prepare_trace_payload_for_ingest(&mut payload)); + assert!( + payload.get(TRACE_ROOT_REFLOG_START_OFFSETS_FIELD).is_none(), + "listener-side trace preparation must not read or attach repository state" + ); + coord + .attach_reflog_start_offsets_for_async_ingest(&mut payload) + .await + .unwrap(); let offsets = payload .get(TRACE_ROOT_REFLOG_START_OFFSETS_FIELD) .and_then(Value::as_object) - .expect("mutating trace payload should include reflog start offsets"); + .expect("async trace ingestion should attach reflog start offsets"); let head_key = format!( "worktree:{}:HEAD", git_dir.canonicalize().unwrap().to_string_lossy() @@ -10168,6 +10290,16 @@ mod tests { let sid = "20260411T120000.000000-Psid-close"; coord.trace_root_connection_opened(sid).unwrap(); + let mut early_child = serde_json::json!({ + "event": "cmd_name", + "sid": format!("{sid}/20260411T120000.000001-Pchild"), + "name": "rev-list", + "time_ns": 0u64, + }); + assert!( + !coord.prepare_trace_payload_for_ingest(&mut early_child), + "read-only child metadata should stay off the ingest queue" + ); let mut start = serde_json::json!({ "event": "start", "sid": sid, @@ -10176,6 +10308,17 @@ mod tests { "time_ns": 1u64, }); assert!(coord.prepare_trace_payload_for_ingest(&mut start)); + assert_eq!( + coord + .trace_ingress_state + .lock() + .unwrap() + .root_mutating + .get(sid) + .copied(), + Some(true), + "a mutating root start must upgrade an earlier read-only child classification" + ); coord.enqueue_trace_payload(start).unwrap(); finalize_trace_connection_roots(coord.clone(), [sid.to_string()].into_iter().collect()) @@ -10190,6 +10333,14 @@ mod tests { .contains_key(sid), "closing the trace stream without root atexit must not leave the family sequencer wedged" ); + assert!( + !coord + .async_reflog_start_offsets_by_root + .lock() + .unwrap() + .contains_key(sid), + "closing the trace stream must discard the async reflog baseline" + ); coord.request_shutdown(); } @@ -10198,6 +10349,11 @@ mod tests { let coord = ActorDaemonCoordinator::new(); let sid = "20260411T120000.000000-Psid-readonly-close"; coord.trace_root_connection_opened(sid).unwrap(); + coord + .async_reflog_start_offsets_by_root + .lock() + .unwrap() + .insert(sid.to_string(), HashMap::new()); let mut start = make_start_payload(&["git", "status", "--short"]); start["sid"] = serde_json::json!(sid); assert!(!coord.prepare_trace_payload_for_ingest(&mut start)); @@ -10214,6 +10370,46 @@ mod tests { assert!(!ingress.root_argv.contains_key(sid)); assert!(!ingress.root_definitely_read_only.contains(sid)); assert!(!ingress.root_open_connections.contains_key(sid)); + drop(ingress); + assert!( + !coord + .async_reflog_start_offsets_by_root + .lock() + .unwrap() + .contains_key(sid), + "plain-close teardown must discard an async reflog baseline" + ); + } + + #[tokio::test] + async fn trace_connection_close_orders_async_cleanup_after_queued_payloads() { + let coord = ActorDaemonCoordinator::new(); + let sid = "20260411T120000.000000-Psid-queued-close"; + coord.trace_root_connection_opened(sid).unwrap(); + coord + .async_reflog_start_offsets_by_root + .lock() + .unwrap() + .insert(sid.to_string(), HashMap::new()); + coord + .queued_trace_payloads_by_root + .lock() + .unwrap() + .insert(sid.to_string(), 1); + + let close_marker_roots = coord + .record_trace_connection_close(&[sid.to_string()]) + .unwrap(); + + assert_eq!(close_marker_roots, vec![sid.to_string()]); + assert!( + coord + .async_reflog_start_offsets_by_root + .lock() + .unwrap() + .contains_key(sid), + "queued payloads must retain their baseline until the ordered close marker runs" + ); } #[tokio::test] diff --git a/src/daemon/analyzers/mod.rs b/src/daemon/analyzers/mod.rs index 774517cc14..cee5d624d0 100644 --- a/src/daemon/analyzers/mod.rs +++ b/src/daemon/analyzers/mod.rs @@ -1,5 +1,6 @@ use crate::daemon::domain::{AnalysisResult, NormalizedCommand}; use crate::error::GitAiError; +use crate::git::cli_parser::parse_git_cli_args; use std::collections::HashMap; use std::path::Path; use std::sync::Arc; @@ -109,3 +110,32 @@ pub(crate) fn normalized_args(argv: &[String]) -> Vec { argv.to_vec() } } + +pub(crate) fn checkout_is_path_checkout(cmd: &NormalizedCommand) -> bool { + let args = if cmd.invoked_args.is_empty() { + parse_git_cli_args(&normalized_args(&cmd.raw_argv)).command_args + } else { + cmd.invoked_args.clone() + }; + let mut positionals = 0usize; + let mut skip_option_value = false; + for arg in &args { + if skip_option_value { + skip_option_value = false; + continue; + } + match arg.as_str() { + "--" | "-p" | "--patch" | "--ours" | "--theirs" => return true, + "-b" | "-B" | "--orphan" | "--conflict" => skip_option_value = true, + value if value.starts_with("--pathspec") => return true, + value if value.starts_with('-') => {} + _ => { + positionals += 1; + if positionals > 1 { + return true; + } + } + } + } + false +} diff --git a/src/daemon/analyzers/workspace.rs b/src/daemon/analyzers/workspace.rs index 3b62d4ede8..282b7abde7 100644 --- a/src/daemon/analyzers/workspace.rs +++ b/src/daemon/analyzers/workspace.rs @@ -1,4 +1,6 @@ -use crate::daemon::analyzers::{AnalysisView, CommandAnalyzer, command_args, normalized_args}; +use crate::daemon::analyzers::{ + AnalysisView, CommandAnalyzer, checkout_is_path_checkout, command_args, normalized_args, +}; use crate::daemon::domain::{ AnalysisResult, CommandClass, Confidence, NormalizedCommand, SemanticEvent, StashOpKind, }; @@ -14,7 +16,6 @@ impl CommandAnalyzer for WorkspaceAnalyzer { state: AnalysisView<'_>, ) -> Result { let name = cmd.primary_command.as_deref().unwrap_or_default(); - let args = command_args(cmd); let mut events = Vec::new(); match name { @@ -26,7 +27,7 @@ impl CommandAnalyzer for WorkspaceAnalyzer { }); } "checkout" => { - if is_path_checkout(&args) { + if checkout_is_path_checkout(cmd) { events.push(SemanticEvent::CheckoutPaths); } else if let Some(change) = cmd.ref_changes.first() { events.push(SemanticEvent::RefUpdated { @@ -87,13 +88,6 @@ fn infer_stash_kind(args: &[String]) -> StashOpKind { } } -fn is_path_checkout(args: &[String]) -> bool { - args.iter().any(|arg| arg == "--") - || args - .iter() - .any(|arg| arg.starts_with("--pathspec") || arg == "--ours" || arg == "--theirs") -} - fn current_head_for_workspace_command( cmd: &NormalizedCommand, refs: &std::collections::HashMap, diff --git a/src/daemon/family_actor.rs b/src/daemon/family_actor.rs index 5de00d2339..3215202019 100644 --- a/src/daemon/family_actor.rs +++ b/src/daemon/family_actor.rs @@ -110,17 +110,10 @@ pub fn spawn_family_actor(family_key: FamilyKey) -> FamilyActorHandle { match msg { FamilyMsg::Apply(cmd, respond_to) => { let mut cmd = *cmd; - let result = ref_cursor.enrich_command(&mut cmd, &state).and_then( - |command_start_refs| { - reducer::reduce_family_command_with_ref_snapshot( - &mut state, - cmd, - &analyzers, - &command_start_refs, - ) + let result = ref_cursor.enrich_command(&mut cmd, &state).and_then(|()| { + reducer::reduce_family_command(&mut state, cmd, &analyzers) .map(|(applied, _)| applied) - }, - ); + }); let _ = respond_to.send(result); } FamilyMsg::ApplyCheckpoint(respond_to) => { diff --git a/src/daemon/reducer.rs b/src/daemon/reducer.rs index b9246ae7e6..5e33233602 100644 --- a/src/daemon/reducer.rs +++ b/src/daemon/reducer.rs @@ -1,4 +1,4 @@ -use crate::daemon::analyzers::{AnalysisView, AnalyzerRegistry}; +use crate::daemon::analyzers::{AnalysisView, AnalyzerRegistry, checkout_is_path_checkout}; use crate::daemon::domain::{ AnalysisResult, AppliedCommand, FamilyState, GlobalState, NormalizedCommand, WorktreeState, }; @@ -9,44 +9,9 @@ pub fn reduce_family_command( state: &mut FamilyState, cmd: NormalizedCommand, analyzers: &AnalyzerRegistry, -) -> Result<(AppliedCommand, AnalysisResult), GitAiError> { - reduce_family_command_with_ref_snapshot( - state, - cmd, - analyzers, - &std::collections::HashMap::new(), - ) -} - -pub fn reduce_family_command_with_ref_snapshot( - state: &mut FamilyState, - cmd: NormalizedCommand, - analyzers: &AnalyzerRegistry, - command_start_refs: &std::collections::HashMap, ) -> Result<(AppliedCommand, AnalysisResult), GitAiError> { // Analyze against pre-command state so history/ref analyzers can infer old->new correctly. - let refs_for_analysis; - let analysis_refs = if command_start_refs.is_empty() { - &state.refs - } else { - refs_for_analysis = state - .refs - .iter() - .map(|(reference, oid)| (reference.clone(), oid.clone())) - .chain( - command_start_refs - .iter() - .map(|(reference, oid)| (reference.clone(), oid.clone())), - ) - .collect(); - &refs_for_analysis - }; - let analysis = analyzers.analyze( - &cmd, - AnalysisView { - refs: analysis_refs, - }, - )?; + let analysis = analyzers.analyze(&cmd, AnalysisView { refs: &state.refs })?; apply_ref_changes(state, &cmd); apply_worktree_state(state, &cmd); @@ -105,24 +70,21 @@ fn apply_worktree_state(state: &mut FamilyState, cmd: &NormalizedCommand) { .ref_changes .iter() .rfind(|change| change.reference == "HEAD"); - - let (head, branch, detached) = if let Some(head_change) = head_change { - // DEFERRED (code-review #12): `detached` is inferred as "no unique - // branch ref moved with HEAD". When a checkout/switch to an EXISTING - // branch produces an ambiguous ref-change pairing (e.g. multiple - // refs/heads/* share the same old->new as HEAD, so - // unique_branch_for_head_change returns None), the worktree is - // misclassified as detached. Harmless for attribution today (the head - // OID is still correct); a precise fix would consult the actual - // post-command symbolic-ref/branch name rather than inferring from - // ref-change pairing. - let branch = unique_branch_for_head_change(cmd, head_change); + let orphan_branch = (cmd.exit_code == 0) + .then(|| checkout_orphan_branch_target(cmd)) + .flatten(); + let command_branch = checkout_or_switch_branch_target(cmd, state); + + let (head, branch, detached) = if let Some(branch) = orphan_branch { + (None, Some(branch), false) + } else if let Some(head_change) = head_change { + let branch = command_branch.or_else(|| unique_branch_for_head_change(cmd, head_change)); ( Some(head_change.new.clone()), branch.clone(), branch.is_none(), ) - } else if let Some(branch) = checkout_or_switch_branch_target(cmd) { + } else if let Some(branch) = command_branch { ( previous.and_then(|worktree| worktree.head.clone()), Some(branch), @@ -136,6 +98,13 @@ fn apply_worktree_state(state: &mut FamilyState, cmd: &NormalizedCommand) { ) }; + if cmd.exit_code == 0 + && let Some(created_branch) = checkout_or_switch_created_branch_target(cmd) + && let Some(head) = head.as_ref() + { + state.refs.insert(created_branch, head.clone()); + } + state.worktrees.insert( key, WorktreeState { @@ -167,21 +136,52 @@ fn unique_branch_for_head_change( Some(first) } -fn checkout_or_switch_branch_target(cmd: &NormalizedCommand) -> Option { +fn checkout_or_switch_branch_target( + cmd: &NormalizedCommand, + state: &FamilyState, +) -> Option { + if cmd.exit_code != 0 + && !cmd + .ref_changes + .iter() + .any(|change| change.reference == "HEAD") + { + return None; + } let command = cmd.primary_command.as_deref()?; let args = command_args(cmd); match command { - "checkout" => checkout_created_branch_target(&args), + "checkout" if !checkout_is_path_checkout(cmd) => checkout_branch_target(&args, state), "switch" => switch_branch_target(&args), _ => None, } - .map(|branch| { - if branch.starts_with("refs/") { - branch - } else { - format!("refs/heads/{branch}") - } - }) + .map(qualify_branch_ref) +} + +fn checkout_or_switch_created_branch_target(cmd: &NormalizedCommand) -> Option { + let command = cmd.primary_command.as_deref()?; + let args = command_args(cmd); + let branch = match command { + "checkout" => checkout_created_branch_target(&args), + "switch" => switch_created_branch_target(&args), + _ => None, + }?; + Some(qualify_branch_ref(branch)) +} + +fn checkout_orphan_branch_target(cmd: &NormalizedCommand) -> Option { + if cmd.primary_command.as_deref() != Some("checkout") { + return None; + } + checkout_orphan_target(&command_args(cmd)).map(qualify_branch_ref) +} + +fn qualify_branch_ref(branch: String) -> String { + if branch.starts_with("refs/") { + branch + } else { + format!("refs/heads/{branch}") + } } fn command_args(cmd: &NormalizedCommand) -> Vec { @@ -202,6 +202,33 @@ fn command_args(cmd: &NormalizedCommand) -> Vec { .collect() } +fn checkout_branch_target(args: &[String], state: &FamilyState) -> Option { + if let Some(created) = checkout_created_branch_target(args) { + return Some(created); + } + let mut idx = usize::from(args.first().is_some_and(|arg| arg == "checkout")); + let mut candidate = None; + while idx < args.len() { + match args[idx].as_str() { + "--detach" | "-d" | "--" => return None, + "--conflict" => idx += 2, + value if value.starts_with("--conflict=") => idx += 1, + value if value.starts_with('-') => idx += 1, + value => { + candidate = Some(value.to_string()); + break; + } + } + } + let candidate = candidate?; + let reference = if candidate.starts_with("refs/heads/") { + candidate + } else { + format!("refs/heads/{candidate}") + }; + state.refs.contains_key(&reference).then_some(reference) +} + fn checkout_created_branch_target(args: &[String]) -> Option { let mut idx = usize::from(args.first().is_some_and(|arg| arg == "checkout")); while idx < args.len() { @@ -220,7 +247,37 @@ fn checkout_created_branch_target(args: &[String]) -> Option { None } +fn checkout_orphan_target(args: &[String]) -> Option { + let mut idx = usize::from(args.first().is_some_and(|arg| arg == "checkout")); + while idx < args.len() { + match args[idx].as_str() { + "--orphan" => return args.get(idx + 1).cloned(), + value if value.starts_with("--orphan=") => { + return Some(value["--orphan=".len()..].to_string()); + } + "--" => return None, + _ => idx += 1, + } + } + None +} + fn switch_branch_target(args: &[String]) -> Option { + if let Some(created) = switch_created_branch_target(args) { + return Some(created); + } + let mut idx = usize::from(args.first().is_some_and(|arg| arg == "switch")); + while idx < args.len() { + match args[idx].as_str() { + "--detach" | "-d" | "--" => return None, + value if !value.starts_with('-') => return Some(value.to_string()), + _ => idx += 1, + } + } + None +} + +fn switch_created_branch_target(args: &[String]) -> Option { let mut idx = usize::from(args.first().is_some_and(|arg| arg == "switch")); while idx < args.len() { match args[idx].as_str() { @@ -237,8 +294,7 @@ fn switch_branch_target(args: &[String]) -> Option { value if value.starts_with("-C") && value.len() > 2 => { return Some(value[2..].to_string()); } - "--detach" | "-d" | "--" => return None, - value if !value.starts_with('-') => return Some(value.to_string()), + "--" => return None, _ => idx += 1, } } @@ -456,6 +512,212 @@ mod tests { assert_eq!(worktree.head.as_deref(), Some("aaa")); assert_eq!(worktree.branch.as_deref(), Some("refs/heads/feature")); assert!(!worktree.detached); + assert_eq!( + state.refs.get("refs/heads/feature").map(String::as_str), + Some("aaa") + ); + } + + #[test] + fn reducer_updates_branch_for_checkout_existing_branch_with_head_move() { + let mut state = family_state(); + state + .refs + .insert("refs/heads/main".to_string(), "bbb".to_string()); + state.worktrees.insert( + PathBuf::from("/tmp/repo"), + WorktreeState { + head: Some("aaa".to_string()), + branch: Some("refs/heads/feature".to_string()), + detached: false, + last_updated_ns: 1, + }, + ); + let registry = AnalyzerRegistry::new(); + let mut cmd = normalized(); + cmd.raw_argv = vec![ + "git".to_string(), + "checkout".to_string(), + "main".to_string(), + ]; + cmd.primary_command = Some("checkout".to_string()); + cmd.invoked_command = Some("checkout".to_string()); + cmd.invoked_args = vec!["main".to_string()]; + cmd.ref_changes = vec![RefChange { + reference: "HEAD".to_string(), + old: "aaa".to_string(), + new: "bbb".to_string(), + }]; + + let (_applied, _analysis) = reduce_family_command(&mut state, cmd, ®istry).unwrap(); + let worktree = state.worktrees.get(&PathBuf::from("/tmp/repo")).unwrap(); + + assert_eq!(worktree.head.as_deref(), Some("bbb")); + assert_eq!(worktree.branch.as_deref(), Some("refs/heads/main")); + assert!(!worktree.detached); + } + + #[test] + fn reducer_tracks_nonzero_merge_checkout_when_head_move_was_observed() { + let mut state = family_state(); + state + .refs + .insert("refs/heads/main".to_string(), "bbb".to_string()); + state.worktrees.insert( + PathBuf::from("/tmp/repo"), + WorktreeState { + head: Some("aaa".to_string()), + branch: Some("refs/heads/feature".to_string()), + detached: false, + last_updated_ns: 1, + }, + ); + let registry = AnalyzerRegistry::new(); + let mut cmd = normalized(); + cmd.raw_argv = vec![ + "git".to_string(), + "checkout".to_string(), + "--merge".to_string(), + "main".to_string(), + ]; + cmd.primary_command = Some("checkout".to_string()); + cmd.invoked_command = Some("checkout".to_string()); + cmd.invoked_args = vec!["--merge".to_string(), "main".to_string()]; + cmd.exit_code = 1; + cmd.ref_changes = vec![RefChange { + reference: "HEAD".to_string(), + old: "aaa".to_string(), + new: "bbb".to_string(), + }]; + + let (_applied, _analysis) = reduce_family_command(&mut state, cmd, ®istry).unwrap(); + let worktree = state.worktrees.get(&PathBuf::from("/tmp/repo")).unwrap(); + + assert_eq!(worktree.head.as_deref(), Some("bbb")); + assert_eq!(worktree.branch.as_deref(), Some("refs/heads/main")); + assert!(!worktree.detached); + } + + #[test] + fn reducer_finds_checkout_after_raw_git_global_options() { + let mut state = family_state(); + state + .refs + .insert("refs/heads/main".to_string(), "bbb".to_string()); + state.worktrees.insert( + PathBuf::from("/tmp/repo"), + WorktreeState { + head: Some("aaa".to_string()), + branch: Some("refs/heads/feature".to_string()), + detached: false, + last_updated_ns: 1, + }, + ); + let registry = AnalyzerRegistry::new(); + let mut cmd = normalized(); + cmd.raw_argv = vec![ + "git".to_string(), + "-C".to_string(), + "/tmp/repo".to_string(), + "checkout".to_string(), + "main".to_string(), + ]; + cmd.primary_command = Some("checkout".to_string()); + cmd.invoked_command = Some("checkout".to_string()); + cmd.invoked_args.clear(); + cmd.ref_changes = vec![RefChange { + reference: "HEAD".to_string(), + old: "aaa".to_string(), + new: "bbb".to_string(), + }]; + + let (_applied, _analysis) = reduce_family_command(&mut state, cmd, ®istry).unwrap(); + let worktree = state.worktrees.get(&PathBuf::from("/tmp/repo")).unwrap(); + + assert_eq!(worktree.branch.as_deref(), Some("refs/heads/main")); + assert!(!worktree.detached); + } + + #[test] + fn reducer_preserves_branch_for_checkout_path_forms_and_failed_checkout() { + for (args, exit_code) in [ + (vec!["main", "--", "src/lib.rs"], 0), + (vec!["main", "--", "checkout"], 0), + (vec!["main", "src/lib.rs"], 0), + (vec!["-p", "main"], 0), + (vec!["main"], 1), + ] { + let mut state = family_state(); + state + .refs + .insert("refs/heads/main".to_string(), "bbb".to_string()); + state.worktrees.insert( + PathBuf::from("/tmp/repo"), + WorktreeState { + head: Some("aaa".to_string()), + branch: Some("refs/heads/feature".to_string()), + detached: false, + last_updated_ns: 1, + }, + ); + let registry = AnalyzerRegistry::new(); + let mut cmd = normalized(); + cmd.raw_argv = std::iter::once("git") + .chain(std::iter::once("checkout")) + .chain(args.iter().copied()) + .map(str::to_string) + .collect(); + cmd.primary_command = Some("checkout".to_string()); + cmd.invoked_command = Some("checkout".to_string()); + cmd.invoked_args = args.iter().map(|arg| arg.to_string()).collect(); + cmd.exit_code = exit_code; + cmd.ref_changes.clear(); + + let (_applied, _analysis) = reduce_family_command(&mut state, cmd, ®istry).unwrap(); + let worktree = state.worktrees.get(&PathBuf::from("/tmp/repo")).unwrap(); + + assert_eq!( + worktree.branch.as_deref(), + Some("refs/heads/feature"), + "checkout args {args:?} exit_code={exit_code} changed the tracked branch" + ); + assert_eq!(worktree.head.as_deref(), Some("aaa")); + assert!(!worktree.detached); + } + } + + #[test] + fn reducer_tracks_checkout_orphan_as_unborn_without_inventing_a_tip() { + let mut state = family_state(); + state.worktrees.insert( + PathBuf::from("/tmp/repo"), + WorktreeState { + head: Some("aaa".to_string()), + branch: Some("refs/heads/main".to_string()), + detached: false, + last_updated_ns: 1, + }, + ); + let registry = AnalyzerRegistry::new(); + let mut cmd = normalized(); + cmd.raw_argv = vec![ + "git".to_string(), + "checkout".to_string(), + "--orphan".to_string(), + "empty".to_string(), + ]; + cmd.primary_command = Some("checkout".to_string()); + cmd.invoked_command = Some("checkout".to_string()); + cmd.invoked_args = vec!["--orphan".to_string(), "empty".to_string()]; + cmd.ref_changes.clear(); + + let (_applied, _analysis) = reduce_family_command(&mut state, cmd, ®istry).unwrap(); + let worktree = state.worktrees.get(&PathBuf::from("/tmp/repo")).unwrap(); + + assert_eq!(worktree.head, None); + assert_eq!(worktree.branch.as_deref(), Some("refs/heads/empty")); + assert!(!worktree.detached); + assert!(!state.refs.contains_key("refs/heads/empty")); } #[test] diff --git a/src/daemon/ref_cursor.rs b/src/daemon/ref_cursor.rs index d5508c25fd..712dea453d 100644 --- a/src/daemon/ref_cursor.rs +++ b/src/daemon/ref_cursor.rs @@ -1,4 +1,4 @@ -use crate::daemon::analyzers::{command_args, normalized_args}; +use crate::daemon::analyzers::{checkout_is_path_checkout, command_args, normalized_args}; use crate::daemon::domain::{Confidence, FamilyKey, FamilyState, NormalizedCommand, RefChange}; use crate::error::GitAiError; use crate::git::cli_parser::{ @@ -117,6 +117,9 @@ enum ColdSeedMatchSpec { RebaseSpan { expected: ExpectedTransition, }, + UniqueStashPush { + expected_message: Option, + }, } impl RefCursor { @@ -137,21 +140,25 @@ impl RefCursor { &mut self, cmd: &mut NormalizedCommand, state: &FamilyState, - ) -> Result, GitAiError> { + ) -> Result<(), GitAiError> { cmd.ref_changes.clear(); self.initialize_from_command_reflog_start_offsets(cmd)?; - let command_start_refs = - refs_at_reflog_start_offsets(&self.family, &cmd.reflog_start_offsets)?; - + // Reflog offsets are sampled asynchronously and may describe repository + // state after this command -- or even after later commands. They are safe + // only as cursor-selection hints. Analyzer ref state comes from the + // serialized family actor, while ref-moving commands contribute exact + // old/new transitions recovered by this cursor. A cold ref-neutral command + // without enough in-order evidence therefore fails closed instead of + // guessing from mutable repository state. if cmd.exit_code != 0 && !command_can_move_refs_on_nonzero(cmd.primary_command.as_deref()) { - return Ok(command_start_refs); + return Ok(()); } let Some(primary) = cmd.primary_command.as_deref() else { - return Ok(command_start_refs); + return Ok(()); }; if !command_uses_ref_cursor(primary) { - return Ok(command_start_refs); + return Ok(()); } match primary { @@ -204,7 +211,7 @@ impl RefCursor { if !cmd.ref_changes.is_empty() { cmd.confidence = Confidence::High; } - Ok(command_start_refs) + Ok(()) } fn initialize_from_command_reflog_start_offsets( @@ -335,6 +342,35 @@ impl RefCursor { ColdSeedMatchSpec::RebaseSpan { expected } => { self.clamp_seed_to_rebase_span_entry(key, &path, offset, expected) } + ColdSeedMatchSpec::UniqueStashPush { expected_message } => { + let Some(reference) = key + .strip_prefix("common:") + .filter(|reference| *reference == "refs/stash") + else { + return Ok(offset); + }; + let entries = read_reflog_entries(key.to_string(), &path, reference, None)?; + let log_end = entries + .last() + .map(|entry| entry.end_offset) + .unwrap_or(offset); + let mut candidates = entries.iter().filter(|entry| { + expected_message + .as_deref() + .is_none_or(|message| stash_reflog_message_matches(&entry.message, message)) + }); + let Some(candidate) = candidates.next() else { + return Ok(offset); + }; + if candidates.next().is_some() { + // A late asynchronous hint cannot distinguish this push from + // prior or later same-shaped stash history. Baseline all + // currently ambiguous history so this command fails closed. + Ok(log_end.max(offset)) + } else { + Ok(candidate.start_offset.min(offset)) + } + } } } @@ -445,6 +481,13 @@ impl RefCursor { limit, }) } + "stash" => { + let stash_args = stash_command_args(&args); + let kind = stash_args.first().map(String::as_str).unwrap_or("push"); + matches!(kind, "push" | "save").then(|| ColdSeedMatchSpec::UniqueStashPush { + expected_message: stash_push_message_from_args(stash_args, kind), + }) + } _ => None, } } @@ -510,7 +553,7 @@ impl RefCursor { return Ok(()); }; - self.consume_head_entry_for_command(cmd, entry) + self.consume_head_entry_for_command(cmd, state, entry) } fn find_commit_head_entry( @@ -1262,6 +1305,7 @@ impl RefCursor { fn consume_head_entry_for_command( &mut self, cmd: &mut NormalizedCommand, + state: &FamilyState, entry: CursorEntry, ) -> Result<(), GitAiError> { crate::wltrace::wltrace( @@ -1284,6 +1328,7 @@ impl RefCursor { let new = entry.new.clone(); let mut changes = vec![entry_to_ref_change(&entry)]; self.consume_common_refs_matching_transition(&old, &new, &mut changes)?; + append_checked_out_branch_change(cmd, state, &old, &new, &mut changes); dedup_ref_changes(&mut changes); cmd.ref_changes = changes; Ok(()) @@ -1292,7 +1337,7 @@ impl RefCursor { fn consume_head_transition_for_command( &mut self, cmd: &mut NormalizedCommand, - _state: &FamilyState, + state: &FamilyState, message_prefixes: &[&str], expected: ExpectedTransition, ) -> Result<(), GitAiError> { @@ -1302,7 +1347,7 @@ impl RefCursor { return Ok(()); }; - self.consume_head_entry_for_command(cmd, entry) + self.consume_head_entry_for_command(cmd, state, entry) } fn consume_head_span_for_command_limited( @@ -2343,47 +2388,6 @@ pub(crate) fn capture_reflog_start_offsets_for_worktree(worktree: &Path) -> Hash offsets } -pub(crate) fn refs_at_reflog_start_offsets( - family: &FamilyKey, - offsets: &HashMap, -) -> Result, GitAiError> { - let common_dir = PathBuf::from(&family.0); - let mut refs = HashMap::new(); - - for (key, offset) in offsets { - if *offset == 0 { - continue; - } - let Some((reference, path)) = reflog_reference_and_path_for_key(&common_dir, key) else { - continue; - }; - let Some(record) = read_reflog_record_ending_at(&path, *offset)? else { - continue; - }; - if valid_non_zero_oid(&record.new) { - refs.insert(reference, record.new); - } - } - - Ok(refs) -} - -fn reflog_reference_and_path_for_key(common_dir: &Path, key: &str) -> Option<(String, PathBuf)> { - if let Some(reference) = key.strip_prefix("common:") { - return Some(( - reference.to_string(), - common_dir.join("logs").join(reference), - )); - } - let git_dir = key - .strip_prefix("worktree:") - .and_then(|value| value.strip_suffix(":HEAD"))?; - Some(( - "HEAD".to_string(), - PathBuf::from(git_dir).join("logs").join("HEAD"), - )) -} - impl From<&CursorEntry> for ReflogAnchor { fn from(entry: &CursorEntry) -> Self { Self { @@ -2426,6 +2430,41 @@ fn current_worktree_branch_ref<'a>( .and_then(|worktree| worktree.branch.as_deref()) } +fn append_checked_out_branch_change( + cmd: &NormalizedCommand, + state: &FamilyState, + old: &str, + new: &str, + changes: &mut Vec, +) { + if !command_updates_checked_out_branch(cmd.primary_command.as_deref()) { + return; + } + let Some(reference) = current_worktree_branch_ref(cmd, state) else { + return; + }; + if changes.iter().any(|change| { + change.reference == reference + || (change.reference.starts_with("refs/heads/") + && change.old == old + && change.new == new) + }) { + return; + } + changes.push(RefChange { + reference: reference.to_string(), + old: old.to_string(), + new: new.to_string(), + }); +} + +fn command_updates_checked_out_branch(primary: Option<&str>) -> bool { + matches!( + primary, + Some("commit" | "revert" | "reset" | "merge" | "cherry-pick" | "rebase" | "pull") + ) +} + impl ExpectedTransition { fn with_reflog_messages(mut self, messages: HashSet) -> Self { self.messages = messages; @@ -3619,14 +3658,6 @@ fn working_log_base_oids(worktree: &Path) -> HashSet { out } -fn checkout_is_path_checkout(cmd: &NormalizedCommand) -> bool { - let args = command_args(cmd); - args.iter().any(|arg| arg == "--") - || args - .iter() - .any(|arg| arg.starts_with("--pathspec") || arg == "--ours" || arg == "--theirs") -} - fn stash_command_args(args: &[String]) -> &[String] { if args.first().is_some_and(|arg| arg == "stash") { &args[1..] @@ -4809,6 +4840,48 @@ mod tests { ); } + #[test] + fn recovered_branch_transition_overrides_stale_worktree_branch_state() { + let temp = tempfile::tempdir().unwrap(); + let worktree = temp.path().join("repo"); + fs::create_dir_all(&worktree).unwrap(); + let family = FamilyKey::new(temp.path().to_string_lossy().to_string()); + let stale_branch = "refs/heads/main"; + let actual_branch = "refs/heads/feature"; + let mut state = family_state(&family); + state.worktrees.insert( + worktree.canonicalize().unwrap(), + WorktreeState { + head: Some(A.to_string()), + branch: Some(stale_branch.to_string()), + detached: false, + last_updated_ns: 0, + }, + ); + let cmd = command_with_worktree(&family, Some(worktree), &["commit", "-m", "next"]); + let mut changes = vec![ + RefChange { + reference: "HEAD".to_string(), + old: A.to_string(), + new: B.to_string(), + }, + RefChange { + reference: actual_branch.to_string(), + old: A.to_string(), + new: B.to_string(), + }, + ]; + + append_checked_out_branch_change(&cmd, &state, A, B, &mut changes); + + assert_eq!(changes.len(), 2); + assert!( + changes + .iter() + .all(|change| change.reference != stale_branch) + ); + } + #[test] fn direct_branch_update_ref_does_not_attach_head_when_state_names_different_branch() { let temp = tempfile::tempdir().unwrap(); @@ -6074,7 +6147,7 @@ mod tests { } #[test] - fn cold_stash_push_uses_command_reflog_boundary_without_message() { + fn cold_stash_push_without_message_fails_closed_when_async_boundary_is_ambiguous() { let temp = tempfile::tempdir().unwrap(); let old_line = format!("{A} {B} Test User 0 +0000\tWIP on main\n"); let old_history_len = old_line.len() as u64; @@ -6091,6 +6164,26 @@ mod tests { cursor.enrich_command(&mut cmd, &state).unwrap(); + assert!(cmd.ref_changes.is_empty()); + assert!(cursor.stash_stack.is_empty()); + } + + #[test] + fn cold_stash_push_clamps_a_late_async_boundary_to_unique_entry() { + let temp = tempfile::tempdir().unwrap(); + let current_line = format!("{B} {C} Test User 0 +0000\tWIP on main\n"); + let path = temp.path().join("logs/refs/stash"); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, ¤t_line).unwrap(); + let family = FamilyKey::new(temp.path().to_string_lossy().to_string()); + let state = family_state(&family); + let mut cursor = RefCursor::new(family.clone()); + let mut cmd = command(&family, &["stash", "push", "--", "a.txt"]); + cmd.reflog_start_offsets + .insert(common_key("refs/stash"), current_line.len() as u64); + + cursor.enrich_command(&mut cmd, &state).unwrap(); + assert_eq!( cmd.ref_changes, vec![RefChange { @@ -6102,6 +6195,59 @@ mod tests { assert_eq!(cursor.stash_stack, vec![C.to_string()]); } + #[test] + fn cold_stash_push_fails_closed_when_late_boundary_has_ambiguous_entries() { + let temp = tempfile::tempdir().unwrap(); + let old_line = format!("{A} {B} Test User 0 +0000\tWIP on main\n"); + let current_line = format!("{B} {C} Test User 0 +0000\tWIP on main\n"); + let path = temp.path().join("logs/refs/stash"); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, format!("{old_line}{current_line}")).unwrap(); + let family = FamilyKey::new(temp.path().to_string_lossy().to_string()); + let state = family_state(&family); + let mut cursor = RefCursor::new(family.clone()); + let mut cmd = command(&family, &["stash", "push"]); + cmd.reflog_start_offsets.insert( + common_key("refs/stash"), + (old_line.len() + current_line.len()) as u64, + ); + + cursor.enrich_command(&mut cmd, &state).unwrap(); + + assert!(cmd.ref_changes.is_empty()); + assert!(cursor.stash_stack.is_empty()); + } + + #[test] + fn cold_stash_push_does_not_move_unrelated_ref_boundaries() { + let temp = tempfile::tempdir().unwrap(); + let worktree = temp.path().join("repo"); + let git_dir = create_git_dir(&worktree); + let old_line = format!("{A} {B} Test User 0 +0000\tcommit: old\n"); + let later_line = format!("{B} {C} Test User 0 +0000\tcommit: later\n"); + let true_boundary = old_line.len() as u64; + for path in [ + git_dir.join("logs/HEAD"), + git_dir.join("logs/refs/heads/main"), + ] { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, format!("{old_line}{later_line}")).unwrap(); + } + let family = FamilyKey::new(git_dir.to_string_lossy().to_string()); + let cursor = RefCursor::new(family.clone()); + let cmd = command_with_worktree(&family, Some(worktree), &["stash", "push"]); + + for key in [head_key(&git_dir), common_key("refs/heads/main")] { + assert_eq!( + cursor + .clamp_seed_to_own_entry(&key, true_boundary, &cmd) + .unwrap(), + true_boundary, + "stash matching must not advance or rewind unrelated ref {key}" + ); + } + } + #[test] fn cold_stash_save_uses_message_to_skip_raw_stash_history() { let temp = tempfile::tempdir().unwrap(); diff --git a/tests/commit_tree_update_ref.rs b/tests/commit_tree_update_ref.rs index e7d6f77bb5..0ed1e908db 100644 --- a/tests/commit_tree_update_ref.rs +++ b/tests/commit_tree_update_ref.rs @@ -731,6 +731,34 @@ fn test_trace_listener_bootstrap_captures_commit_ref_transition_before_worker_sp assert_note_has_ai_for_file(&repo, &committed.commit_sha, "bootstrap-race.txt"); } +#[cfg(not(windows))] +#[test] +fn test_late_async_ref_snapshot_does_not_move_squash_merge_onto_past_command() { + let repo = TestRepo::new_with_daemon_env(&[( + "GIT_AI_TEST_TRACE_LISTENER_WORKER_SPAWN_DELAY_MS", + "500", + )]); + let mut file = repo.filename("squash.txt"); + fs::write(repo.path().join("squash.txt"), "base\n").unwrap(); + repo.stage_all_and_commit("base").unwrap(); + file.assert_committed_lines(lines!["base".unattributed_human()]); + let default_branch = repo.current_branch(); + + repo.git(&["checkout", "-b", "feature"]).unwrap(); + repo.git_ai(&["checkpoint", "human", "squash.txt"]).unwrap(); + fs::write(repo.path().join("squash.txt"), "base\nfeature ai\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "squash.txt"]) + .unwrap(); + repo.stage_all_and_commit("feature ai").unwrap(); + file.assert_committed_lines(lines!["base".unattributed_human(), "feature ai".ai()]); + + repo.git(&["checkout", &default_branch]).unwrap(); + repo.git(&["merge", "--squash", "feature"]).unwrap(); + repo.commit("squash feature").unwrap(); + + file.assert_committed_lines(lines!["base".unattributed_human(), "feature ai".ai()]); +} + #[test] #[ignore = "stock trace2 does not record merge --squash source oid after SQUASH_MSG is gone"] fn test_delayed_squash_merge_trace_replay_preserves_source_attribution() { diff --git a/tests/integration/cold_trace2_repo.rs b/tests/integration/cold_trace2_repo.rs index 6fd239cb1c..5b556bf8b7 100644 --- a/tests/integration/cold_trace2_repo.rs +++ b/tests/integration/cold_trace2_repo.rs @@ -586,7 +586,7 @@ fn test_cold_repo_first_traced_squash_merge_is_processed() { } #[test] -fn test_cold_daemon_first_traced_squash_merge_preserves_source_ai_authorship() { +fn test_cold_daemon_first_traced_symbolic_squash_merge_fails_closed() { let mut repo = TestRepo::new_dedicated_daemon(); let mut file = repo.filename("document.txt"); @@ -612,6 +612,10 @@ fn test_cold_daemon_first_traced_squash_merge_preserves_source_ai_authorship() { .unwrap(); repo.restart_dedicated_daemon_for_test(); + // A restarted daemon has no ordered ref state, and stock trace2 does not + // record either the symbolic squash source's OID or the pre-command HEAD. + // An asynchronously sampled ref snapshot may already include later commands, + // so it is not valid evidence. Fail closed instead of guessing attribution. repo.git(&["merge", "--squash", "feature"]).unwrap(); repo.stage_all_and_commit("squashed feature").unwrap(); @@ -620,8 +624,8 @@ fn test_cold_daemon_first_traced_squash_merge_preserves_source_ai_authorship() { "// Master update at top".human(), "section 1".human(), "section 2".human(), - "section 3".ai(), - "// AI feature addition at end".ai() + "section 3".human(), + "// AI feature addition at end".human() ]); } @@ -709,7 +713,7 @@ crate::reuse_tests_in_worktree!( test_cold_repo_mid_merge_commit_preserves_ai_conflict_resolution, test_cold_repo_first_traced_cherry_pick_is_processed, test_cold_repo_first_traced_squash_merge_is_processed, - test_cold_daemon_first_traced_squash_merge_preserves_source_ai_authorship, + test_cold_daemon_first_traced_symbolic_squash_merge_fails_closed, test_cold_repo_first_traced_merge_is_processed, test_cold_repo_first_traced_stash_pop_is_processed, test_cold_repo_traced_stash_after_raw_stash_history_preserves_current_ai_attribution, diff --git a/tests/integration/stash_attribution.rs b/tests/integration/stash_attribution.rs index 53fb002a60..bcd0aa4eec 100644 --- a/tests/integration/stash_attribution.rs +++ b/tests/integration/stash_attribution.rs @@ -1291,11 +1291,19 @@ fn test_stash_apply_shift_uses_final_commit_tree_after_later_edit() { /// entire stash attribution restore -- silently dropping the AI note. #[test] fn test_stash_apply_shift_survives_case_colliding_target_tree() { - let repo = TestRepo::new(); + let repo = TestRepo::new_with_daemon_env(&[( + "GIT_AI_TEST_TRACE_LISTENER_WORKER_SPAWN_DELAY_MS", + "500", + )]); let file_path = repo.path().join("example.txt"); + let mut file = repo.filename("example.txt"); fs::write(&file_path, "root\nanchor\n").unwrap(); repo.stage_all_and_commit("initial").unwrap(); + file.assert_committed_lines(crate::lines![ + "root".unattributed_human(), + "anchor".unattributed_human(), + ]); // Stash an AI change against the current base. fs::write(&file_path, "root\nAI stashed\nanchor\n").unwrap(); @@ -1311,6 +1319,10 @@ fn test_stash_apply_shift_survives_case_colliding_target_tree() { let mut readme = repo.filename("README.md"); readme.set_contents(vec!["# Test Repo".to_string()]); repo.stage_all_and_commit("add README").unwrap(); + file.assert_committed_lines(crate::lines![ + "root".unattributed_human(), + "anchor".unattributed_human(), + ]); let readme_blob = repo .git_og(&["rev-parse", "HEAD:README.md"]) @@ -1324,8 +1336,15 @@ fn test_stash_apply_shift_survives_case_colliding_target_tree() { &format!("100644,{readme_blob},readme.md"), ]) .unwrap(); - repo.git_og(&["commit", "-m", "add case-colliding readme.md"]) + // Route the ref-moving plumbing commit through trace2. `git_og` deliberately + // bypasses the test daemon; using it here made the old test depend on a later + // mutable-ref sample discovering an operation Git AI never observed. + repo.git(&["commit", "-m", "add case-colliding readme.md"]) .unwrap(); + file.assert_committed_lines(crate::lines![ + "root".unattributed_human(), + "anchor".unattributed_human(), + ]); // Apply the stash onto the new HEAD and commit. repo.git(&["stash", "apply"]) @@ -1334,7 +1353,6 @@ fn test_stash_apply_shift_survives_case_colliding_target_tree() { let commit = repo.commit("apply stash onto case-colliding tree").unwrap(); // The AI attribution must survive despite the case-colliding target tree. - let mut file = repo.filename("example.txt"); file.assert_committed_lines(crate::lines![ "root".unattributed_human(), "AI stashed".ai(), @@ -1623,7 +1641,10 @@ fn test_partial_stash_trims_unstashed_initial_metadata() { /// for the stashed paths and leave unstashed attribution live. #[test] fn test_stash_push_pathspec_excludes_unstashed_file_from_stash_log() { - let repo = TestRepo::new(); + let repo = TestRepo::new_with_daemon_env(&[( + "GIT_AI_TEST_TRACE_LISTENER_WORKER_SPAWN_DELAY_MS", + "500", + )]); let mut readme = repo.filename("README.md"); readme.set_contents(vec!["# Test Repo".to_string()]); repo.stage_all_and_commit("initial commit").unwrap();