Hydrate notes at transport boundaries before rewrites

src/daemon.rs

@@ -1330,28 +1330,92 @@ fn transcript_sweep_triggers_for_events(
     triggers
 }
 
-fn apply_pull_notes_sync_side_effect(
+fn incoming_transport_revision_oids(
+    command: &crate::daemon::domain::NormalizedCommand,
+    remote: &str,
+) -> Vec<String> {
+    let remote_tracking_prefix = format!("refs/remotes/{remote}/");
+    let is_fetch = command.primary_command.as_deref() == Some("fetch");
+    let mut seen = HashSet::new();
+
+    let mut revisions: Vec<String> = command
+        .ref_changes
+        .iter()
+        .filter(|change| {
+            is_valid_oid(&change.new)
+                && !is_zero_oid(&change.new)
+                && change.old != change.new
+                && !change.reference.starts_with("refs/notes/")
+                && change.reference.starts_with(&remote_tracking_prefix)
+        })
+        .filter_map(|change| {
+            seen.insert(change.new.clone())
+                .then_some(change.new.clone())
+        })
+        .collect();
+
+    // A pull from a URL or path may update no remote-tracking ref. Its
+    // trace2-derived HEAD transitions are still immutable candidates, and
+    // batching all of them preserves the one-rev-list/one-API-request bound.
+    if revisions.is_empty() && !is_fetch {
+        revisions.extend(
+            command
+                .ref_changes
+                .iter()
+                .filter(|change| {
+                    change.reference == "HEAD"
+                        && is_valid_oid(&change.new)
+                        && !is_zero_oid(&change.new)
+                        && change.old != change.new
+                })
+                .filter_map(|change| {
+                    seen.insert(change.new.clone())
+                        .then_some(change.new.clone())
+                }),
+        );
+    }
+
+    revisions
+}
+
+fn apply_transport_notes_sync_side_effect(
     worktree: &str,
-    command: Option<&str>,
-    args: &[String],
+    command: &crate::daemon::domain::NormalizedCommand,
 ) -> Result<(), GitAiError> {
     use crate::config::NotesBackendKind;
+    use crate::git::cli_parser::is_dry_run;
+
+    let parsed = parsed_invocation_for_normalized_command(command);
+    if is_dry_run(&parsed.command_args) {
+        return Ok(());
+    }
 
     let repo = find_repository_in_path(worktree)?;
-    let parsed = parsed_invocation_for_side_effect(command, args);
     let remote = fetch_remote_from_args(&repo, &parsed)?;
     let notes_backend = crate::config::Config::fresh().notes_backend_kind();
+    let primary = command.primary_command.as_deref().unwrap_or("fetch");
 
-    tracing::info!(
-        command = command.unwrap_or("pull"),
-        remote = %remote,
-        backend = %notes_backend,
-        worktree = %worktree,
-        "handling pull notes sync"
-    );
+    if primary == "pull" {
+        tracing::info!(
+            command = primary,
+            remote = %remote,
+            backend = %notes_backend,
+            worktree = %worktree,
+            "handling pull notes sync"
+        );
+    } else {
+        tracing::info!(
+            command = primary,
+            remote = %remote,
+            backend = %notes_backend,
+            worktree = %worktree,
+            "handling fetch notes sync"
+        );
+    }
 
     if notes_backend == NotesBackendKind::Http {
-        return crate::git::notes_api::warm_cache_for_remote(&repo, &remote);
+        let revisions = incoming_transport_revision_oids(command, &remote);
+        return crate::git::notes_api::warm_cache_for_revisions(&repo, &revisions);
     }
 
     fetch_authorship_notes(&repo, &remote)?;
@@ -5662,6 +5726,28 @@ impl ActorDaemonCoordinator {
                 "side-effect trace"
             );
         }
+        let should_sync_transport_notes = cmd.exit_code == 0
+            && events.iter().any(|event| {
+                matches!(
+                    event,
+                    crate::daemon::domain::SemanticEvent::FetchCompleted { .. }
+                        | crate::daemon::domain::SemanticEvent::PullCompleted { .. }
+                )
+            });
+        let mut transport_notes_sync_error = None;
+        if should_sync_transport_notes
+            && let Some(worktree) = cmd.worktree.as_ref()
+            && let Err(error) =
+                apply_transport_notes_sync_side_effect(&worktree.to_string_lossy(), cmd)
+        {
+            tracing::debug!(
+                %error,
+                command = cmd.primary_command.as_deref().unwrap_or("unknown"),
+                "transport notes sync failed; deferring error until rewrite side effects finish"
+            );
+            transport_notes_sync_error = Some(error);
+        }
+
         // Non-FF rewrite detection: fires for commands that rewrite history via ref moves.
         // Skip for: checkout/switch/branch (no rewriting), cherry-pick (handled separately),
         // and plain commit/amend (CommitCreated/CommitAmended events handle those).
@@ -5884,13 +5970,6 @@ impl ActorDaemonCoordinator {
                     crate::daemon::domain::SemanticEvent::CloneCompleted { .. } => {
                         apply_clone_notes_sync_side_effect(&worktree)?;
                     }
-                    crate::daemon::domain::SemanticEvent::PullCompleted { .. } => {
-                        apply_pull_notes_sync_side_effect(
-                            &worktree,
-                            cmd.invoked_command.as_deref(),
-                            &cmd.invoked_args,
-                        )?;
-                    }
                     crate::daemon::domain::SemanticEvent::PushCompleted { .. } => {
                         apply_push_side_effect(
                             &worktree,
@@ -6374,6 +6453,10 @@ impl ActorDaemonCoordinator {
             self.trigger_transcript_sweep(trigger);
         }
 
+        if let Some(error) = transport_notes_sync_error {
+            return Err(error);
+        }
+
         Ok(())
     }
 

src/git/notes_api.rs

@@ -406,17 +406,38 @@ pub fn warm_cache_for_remote(repo: &Repository, remote: &str) -> Result<(), GitA
         }
     };
 
+    warm_cache_for_revisions(repo, &[rev_target])
+}
+
+/// Pre-warm the local notes cache for recent commits reachable from immutable
+/// revision OIDs captured by trace2.
+///
+/// All revisions are traversed by one bounded `rev-list` invocation and all
+/// cache misses are fetched in batches, so work does not scale in git process
+/// spawns with the number of updated refs or commits.
+pub fn warm_cache_for_revisions(repo: &Repository, revisions: &[String]) -> Result<(), GitAiError> {
+    use crate::git::repository::exec_git_with_stdin_writer;
+
+    if revisions.is_empty() {
+        return Ok(());
+    }
+
     let rev_list_args: Vec<String> = repo
         .global_args_for_exec()
         .into_iter()
         .chain([
             "rev-list".to_string(),
             "--max-count=500".to_string(),
-            rev_target,
+            "--stdin".to_string(),
         ])
         .collect();
-
-    let output = exec_git(&rev_list_args)?;
+    let output = exec_git_with_stdin_writer(&rev_list_args, |writer| {
+        for revision in revisions {
+            writer.write_all(revision.as_bytes())?;
+            writer.write_all(b"\n")?;
+        }
+        Ok(())
+    })?;
     let stdout = String::from_utf8_lossy(&output.stdout);
     let all_shas: Vec<String> = stdout
         .lines()
@@ -425,7 +446,7 @@ pub fn warm_cache_for_remote(repo: &Repository, remote: &str) -> Result<(), GitA
         .collect();
 
     if all_shas.is_empty() {
-        tracing::debug!("warm_cache_for_remote: no commits in HEAD history; skipping");
+        tracing::debug!("warm_cache_for_revisions: no reachable commits; skipping");
         return Ok(());
     }
 
@@ -438,18 +459,17 @@ pub fn warm_cache_for_remote(repo: &Repository, remote: &str) -> Result<(), GitA
         .collect();
 
     if uncached.is_empty() {
-        tracing::debug!("warm_cache_for_remote: all commits already cached; skipping");
+        tracing::debug!("warm_cache_for_revisions: all commits already cached; skipping");
         return Ok(());
     }
 
     tracing::info!(
-        remote = %remote,
         backend = %"http",
         uncached_commits = uncached.len(),
         "fetching authorship notes"
     );
     tracing::debug!(
-        "warm_cache_for_remote: fetching notes for {} uncached commits",
+        "warm_cache_for_revisions: fetching notes for {} uncached commits",
         uncached.len()
     );
 

tests/integration/cherry_pick.rs

@@ -9,6 +9,12 @@ use std::collections::HashMap;
 use std::fs;
 use std::path::PathBuf;
 
+const TRACE2_DISABLED_ENV: [(&str, &str); 3] = [
+    ("GIT_TRACE2", "0"),
+    ("GIT_TRACE2_EVENT", "0"),
+    ("GIT_TRACE2_PERF", "0"),
+];
+
 /// Test cherry-picking a single AI-authored commit
 #[test]
 fn test_single_commit_cherry_pick() {
@@ -813,7 +819,10 @@ fn test_cherry_pick_from_remote_without_prefetched_notes() {
         .unwrap();
     // Fetch only the branch objects, explicitly excluding notes.
     target_repo
-        .git(&["fetch", "source", "refs/heads/*:refs/remotes/source/*"])
+        .git_og_with_env(
+            &["fetch", "source", "refs/heads/*:refs/remotes/source/*"],
+            &TRACE2_DISABLED_ENV,
+        )
         .unwrap();
 
     // Confirm notes are absent (the fix relies on detecting this absence).
@@ -830,9 +839,8 @@ fn test_cherry_pick_from_remote_without_prefetched_notes() {
 }
 
 #[test]
-#[ignore = "temporarily restored by the stacked transport-aware notes sync follow-up"]
-fn test_cherry_pick_preserves_authoritative_remote_target_note() {
-    let (repo, upstream) = TestRepo::new_with_remote();
+fn test_cherry_pick_merges_existing_authoritative_target_note() {
+    let repo = TestRepo::new();
     let file_path = repo.path().join("file.txt");
 
     fs::write(&file_path, "base\n").unwrap();
@@ -883,12 +891,6 @@ fn test_cherry_pick_preserves_authoritative_remote_target_note() {
     write_note(&git_ai_repo, &source_commit.commit_sha, &source_note)
         .expect("write stale source note");
 
-    repo.git_og(&["push", "origin", "feature"]).unwrap();
-    repo.git_og(&["push", "origin", "refs/notes/ai:refs/notes/ai"])
-        .unwrap();
-    let source_notes_ref = repo.git_og(&["rev-parse", "refs/notes/ai"]).unwrap();
-    let source_notes_ref = source_notes_ref.trim();
-
     repo.git(&["checkout", &main_branch]).unwrap();
     let deterministic_date = "2030-01-03T00:00:00Z";
     repo.git_og_with_env(
@@ -918,28 +920,18 @@ fn test_cherry_pick_preserves_authoritative_remote_target_note() {
         .serialize_to_string()
         .expect("serialize authoritative target note");
     write_note(&git_ai_repo, target_commit, &target_note).expect("write authoritative target note");
-    repo.git_og(&["push", "--force", "origin", "refs/notes/ai:refs/notes/ai"])
-        .unwrap();
 
     repo.git_og(&["reset", "--hard", &base_commit.commit_sha])
         .unwrap();
-    repo.git_og(&["update-ref", "refs/notes/ai", source_notes_ref])
-        .unwrap();
-    repo.git_og(&["update-ref", "-d", "refs/notes/ai-remote/origin"])
-        .unwrap();
 
     assert!(
         repo.read_authorship_note(&source_commit.commit_sha)
             .is_some(),
         "precondition: stale source note should already exist locally"
     );
     assert!(
-        upstream.read_authorship_note(target_commit).is_some(),
-        "precondition: authoritative target note should exist remotely"
-    );
-    assert!(
-        repo.read_authorship_note(target_commit).is_none(),
-        "precondition: target note should not exist locally"
+        repo.read_authorship_note(target_commit).is_some(),
+        "precondition: authoritative target note should already exist locally"
     );
 
     repo.git_with_env(
@@ -1420,7 +1412,10 @@ fn test_cherry_pick_from_remote_continues_when_notes_import_fails() {
         ])
         .unwrap();
     target_repo
-        .git(&["fetch", "source", "refs/heads/*:refs/remotes/source/*"])
+        .git_og_with_env(
+            &["fetch", "source", "refs/heads/*:refs/remotes/source/*"],
+            &TRACE2_DISABLED_ENV,
+        )
         .unwrap();
     let _ = target_repo.git(&["update-ref", "-d", "refs/notes/ai"]);
     let _ = target_repo.git(&["update-ref", "-d", "refs/notes/ai-remote/source"]);
@@ -1484,15 +1479,11 @@ crate::reuse_tests_in_worktree!(
     test_cherry_pick_bad_args_dont_corrupt_subsequent_attribution,
     test_cherry_pick_skip_preserves_subsequent_attribution,
     test_cherry_pick_from_remote_without_prefetched_notes,
+    test_cherry_pick_merges_existing_authoritative_target_note,
     test_local_cherry_pick_does_not_fetch_notes_for_fresh_destination,
     test_cherry_pick_from_remote_continues_when_notes_import_fails,
     test_cherry_pick_no_commit_defers_to_final_commit_tree,
     test_cherry_pick_skip_failed_next_conflict_advances_pending_remote_tracking_source,
     test_cherry_pick_skip_then_continue_applies_remaining_commits,
     test_cherry_pick_skip_failed_next_conflict_does_not_double_skip_refcursor_sources,
 );
-
-crate::reuse_tests_in_worktree_with_attrs!(
-    (#[ignore = "temporarily restored by the stacked transport-aware notes sync follow-up"])
-    test_cherry_pick_preserves_authoritative_remote_target_note,
-);