a73x

1b680f01

Surface the recorded merge commit, and scan without a remote

a73x   2026-08-10 16:32

Commit message
Surface the recorded merge commit, and scan without a remote

`Action::PatchMerge` has carried the commit that landed the patch since
the merge-recording design, and the fold threw it away: `PatchMerge { .. }`
set `status = Merged` and nothing else, so no view could name it. For a
squash or a rebase-merge that commit is the only route from the patch back
to the code — the patch's own commits are ancestors of nothing on the base
branch — so losing it left `merged` as a bare flag.

Fold it into `PatchState::merge_commit` under the *same* `(clock, oid)`
guard the status already uses. Concurrent recordings naming different
commits therefore converge the way the status does: every clone folds the
same event set, `(clock, oid)` totally orders it, so every clone picks the
same commit. A `PatchClose` that wins over a merge clears it, since
closing is the documented way to correct a merge recorded in error and a
commit left behind would name a merge the status denies.

Surfaced in `patch show` ("Merged in: <oid>"), in `patch show --json` and
`patch list --json` (both serialize `PatchState`), in `patch log`, and on
the web patch page as a link to the diff — which is what the commit was
recorded for. Only ever the recorded commit; nothing infers one. A patch
in this repo had its head dragged onto an unrelated commit that merely
happened to be reachable and was called merged for that reason, so a
derived commit here would be worse than none. Reachability keeps feeding
the `merged?` hint and deliberately carries no commit.

`CACHE_FORMAT_VERSION` is bumped to 6. `#[serde(default)]` makes a v5
entry deserialize cleanly with `merge_commit: None`, indistinguishable
from a patch with no merge, so only the version can force the refold.

Also fixes 93e080b8: local-only repos never ran either scan.

`sync_all` returned at the no-remotes gate, so `commit_link::scan_and_link`
and `merge_scan::scan_and_record_merges` were never reached. A repo with no
remote never auto-recorded a merge and never linked a commit to an issue,
however many trailers its history carried. Both scans are purely local —
they walk local refs and append local events — so the gate was the only
thing stopping them. It contradicts the premise that collaboration lives
in the repository, and it is the state everyone is in *before* adding a
remote.

The two scans move into `run_local_scans`, shared verbatim between the
remote path and a new `sync_local_only`. With no remotes `sync` now takes
the same advisory lock, migrates the patch layout, runs both scans, reports
what they did, and still points at `init`:

    No remotes with collab refspecs configured — scanning local history only.
    Linked 1 commit(s) to issues.
    Recorded 1 merged patch(es).
    Local scan complete. Add a remote and run `git-collab init` to share these events.

Auto-sync still returns early with no remotes, so only an explicit `sync`
scans — nothing appends events behind an unrelated command.

Reading still writes nothing. `patch show`, `patch list`, `patch log` and
their `--json` forms are covered on a merged patch by
`displaying_a_merged_patch_writes_no_event`, which asserts both the event
count and the events ref are unmoved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
src/cache.rs
Old New
@@ -27,7 +27,12 @@ fn sanitize_ref_name(ref_name: &str) -> String {
27 /// reachability against a base branch — a verdict no event backs and which 27 /// reachability against a base branch — a verdict no event backs and which
28 /// nothing recomputes any more, so it would be served as recorded fact 28 /// nothing recomputes any more, so it would be served as recorded fact
29 /// forever. The shape is unchanged, so only the version can reject it. 29 /// forever. The shape is unchanged, so only the version can reject it.
30 const CACHE_FORMAT_VERSION: u32 = 5; 30 /// v6: `PatchState` gained `merge_commit`. Same trap as v4's `labels` — a v5
31 /// entry is missing the key and `#[serde(default)]` deserializes it to `None`
32 /// indistinguishably from a patch that really has no recorded merge, so a
33 /// merged patch cached before this field existed would read as merged with no
34 /// commit forever. Only the version can force the refold.
35 const CACHE_FORMAT_VERSION: u32 = 6;
31 36
32 /// Cache entry stored on disk: the tip OID at cache time + serialized state. 37 /// Cache entry stored on disk: the tip OID at cache time + serialized state.
33 #[derive(serde::Serialize, serde::Deserialize)] 38 #[derive(serde::Serialize, serde::Deserialize)]
src/lib.rs
Old New
@@ -429,6 +429,13 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
429 println!("Branch: {}", p.branch); 429 println!("Branch: {}", p.branch);
430 } 430 }
431 } 431 }
432 // The commit that landed the patch, printed next to the head it
433 // replaced. For a squash there is no path from `Head` to the
434 // base branch at all, so this line is the only thing that says
435 // where the code went.
436 if let Some(ref merged_in) = p.merge_commit {
437 println!("Merged in: {:.8}", merged_in);
438 }
432 println!("Created: {}", p.created_at); 439 println!("Created: {}", p.created_at);
433 if !p.labels.is_empty() { 440 if !p.labels.is_empty() {
434 println!("Labels: {}", p.labels.join(", ")); 441 println!("Labels: {}", p.labels.join(", "));
src/patch.rs
Old New
@@ -594,7 +594,7 @@ pub fn patch_log_to_writer(
594 ) -> Result<(), Error> { 594 ) -> Result<(), Error> {
595 if patch.revisions.is_empty() { 595 if patch.revisions.is_empty() {
596 writeln!(writer, "No revisions recorded.")?; 596 writeln!(writer, "No revisions recorded.")?;
597 return Ok(()); 597 return write_merge_line(patch, writer);
598 } 598 }
599 599
600 for (i, rev) in patch.revisions.iter().enumerate() { 600 for (i, rev) in patch.revisions.iter().enumerate() {
@@ -649,6 +649,23 @@ pub fn patch_log_to_writer(
649 rev.number, short_oid, rev.timestamp, label, file_summary, body_display 649 rev.number, short_oid, rev.timestamp, label, file_summary, body_display
650 )?; 650 )?;
651 } 651 }
652 write_merge_line(patch, writer)
653 }
654
655 /// The last line of `patch log`: the commit that landed the patch, below the
656 /// revisions it landed.
657 ///
658 /// `patch log` reads as the patch's life story and used to stop one step short
659 /// of the end. The commit is the only thing that connects the last revision to
660 /// the base branch when the merge was a squash or a rebase, since neither
661 /// leaves the revision's own commits reachable from anything.
662 ///
663 /// Only ever the *recorded* commit — nothing here consults reachability, and
664 /// nothing here writes.
665 fn write_merge_line(patch: &PatchState, writer: &mut dyn std::io::Write) -> Result<(), Error> {
666 if let Some(ref merged_in) = patch.merge_commit {
667 writeln!(writer, "merged in {:.8}", merged_in)?;
668 }
652 Ok(()) 669 Ok(())
653 } 670 }
654 671
src/server/http/repo/patches.rs
Old New
@@ -105,6 +105,14 @@ pub struct PatchDetailView {
105 pub title: String, 105 pub title: String,
106 pub body: String, 106 pub body: String,
107 pub status: String, 107 pub status: String,
108 /// The commit that landed the patch, recorded by `Action::PatchMerge`.
109 ///
110 /// The link this page can offer is the reason the commit is in the event
111 /// at all: for a squash or a rebase-merge the revisions listed below are
112 /// ancestors of nothing on the base branch, so their `/diff/` links lead
113 /// away from where the code actually went. Recorded only — never inferred
114 /// from reachability, which points at whatever happens to be reachable.
115 pub merge_commit: Option<String>,
108 pub author: String, 116 pub author: String,
109 pub labels: String, 117 pub labels: String,
110 pub branch: String, 118 pub branch: String,
@@ -220,6 +228,7 @@ pub async fn patch_detail(
220 title: ps.title, 228 title: ps.title,
221 body: ps.body, 229 body: ps.body,
222 status: ps.status.as_str().to_string(), 230 status: ps.status.as_str().to_string(),
231 merge_commit: ps.merge_commit,
223 author: ps.author.name, 232 author: ps.author.name,
224 labels: ps.labels.join(", "), 233 labels: ps.labels.join(", "),
225 branch: ps.branch, 234 branch: ps.branch,
src/server/http/templates/patch_detail.html
Old New
@@ -9,6 +9,9 @@
9 &nbsp; by <strong>{{ patch.author }}</strong> 9 &nbsp; by <strong>{{ patch.author }}</strong>
10 &nbsp; <span class="mono" style="color: #666;">{{ patch.branch }} → {{ patch.base_ref }}</span> 10 &nbsp; <span class="mono" style="color: #666;">{{ patch.branch }} → {{ patch.base_ref }}</span>
11 </p> 11 </p>
12 {% if let Some(merge_commit) = patch.merge_commit %}
13 <p>Merged in <a class="mono" href="/{{ repo_name }}/diff/{{ merge_commit }}">{{ merge_commit }}</a></p>
14 {% endif %}
12 {% if !patch.labels.is_empty() %} 15 {% if !patch.labels.is_empty() %}
13 <p style="color: #666;">Labels: {{ patch.labels }}</p> 16 <p style="color: #666;">Labels: {{ patch.labels }}</p>
14 {% endif %} 17 {% endif %}
src/state.rs
Old New
@@ -231,6 +231,34 @@ pub struct PatchState {
231 pub title: String, 231 pub title: String,
232 pub body: String, 232 pub body: String,
233 pub status: PatchStatus, 233 pub status: PatchStatus,
234 /// The commit on the base branch that landed the patch, as recorded by the
235 /// winning `Action::PatchMerge`. `None` for any patch whose current status
236 /// was not decided by a merge.
237 ///
238 /// This is the whole reason the commit is carried in the event. For a
239 /// squash or a rebase-merge there is no other route from the patch back to
240 /// the code — the patch's own commits are ancestors of nothing on the base
241 /// branch — so this is the only durable evidence of *how* the patch landed,
242 /// and it is what a UI links to.
243 ///
244 /// Always a *recorded* commit, never a derived one. Reachability guesses
245 /// wrong in ways that are worse than silence: a patch in this repo had its
246 /// head dragged onto a commit about unrelated work that merely happened to
247 /// be reachable, and was called merged for that reason. The hint that
248 /// reachability still feeds (`looks_merged`) deliberately carries no commit.
249 ///
250 /// Written by the fold under the same `(clock, oid)` guard as `status`, so
251 /// two clones recording different commits converge on one — see
252 /// `from_ref_uncached`.
253 ///
254 /// `#[serde(default)]` is what lets a `PatchState` serialized before this
255 /// field existed deserialize at all; it is *not* what keeps the on-disk
256 /// fold cache correct. A stale entry missing this key deserializes just as
257 /// cleanly whether or not it predates a `patch.merge` folded into that ref,
258 /// so `cache::CACHE_FORMAT_VERSION` is bumped alongside this field to force
259 /// a refold instead of quietly serving a merge-commit-less hit.
260 #[serde(default)]
261 pub merge_commit: Option<String>,
234 pub base_ref: String, 262 pub base_ref: String,
235 pub fixes: Option<String>, 263 pub fixes: Option<String>,
236 /// The branch the patch was created from, recorded for provenance only. 264 /// The branch the patch was created from, recorded for provenance only.
@@ -699,6 +727,7 @@ impl PatchState {
699 title, 727 title,
700 body, 728 body,
701 status: PatchStatus::Open, 729 status: PatchStatus::Open,
730 merge_commit: None,
702 base_ref, 731 base_ref,
703 fixes, 732 fixes,
704 branch, 733 branch,
@@ -843,15 +872,31 @@ impl PatchState {
843 let key = (event.clock, oid.to_string()); 872 let key = (event.clock, oid.to_string());
844 if status_key.as_ref().is_none_or(|k| key >= *k) { 873 if status_key.as_ref().is_none_or(|k| key >= *k) {
845 s.status = PatchStatus::Closed; 874 s.status = PatchStatus::Closed;
875 // A close that wins over a merge un-merges the
876 // patch — the documented way to correct a
877 // `PatchMerge` recorded in error. Leaving the
878 // commit behind would leave `patch show` naming a
879 // merge the status denies.
880 s.merge_commit = None;
846 status_key = Some(key); 881 status_key = Some(key);
847 } 882 }
848 } 883 }
849 } 884 }
850 Action::PatchMerge { .. } => { 885 Action::PatchMerge { commit } => {
851 if let Some(ref mut s) = state { 886 if let Some(ref mut s) = state {
852 let key = (event.clock, oid.to_string()); 887 let key = (event.clock, oid.to_string());
853 if status_key.as_ref().is_none_or(|k| key >= *k) { 888 if status_key.as_ref().is_none_or(|k| key >= *k) {
854 s.status = PatchStatus::Merged; 889 s.status = PatchStatus::Merged;
890 // Under the *same* guard as the status, so the two
891 // can never disagree and concurrent merges naming
892 // different commits converge: every clone folds the
893 // same event set and `(clock, oid)` is a total
894 // order over it, so every clone picks the same one.
895 //
896 // Empty for a `patch.merge` written before the
897 // event carried a commit; those stay `None` rather
898 // than surfacing an empty string as a commit id.
899 s.merge_commit = (!commit.is_empty()).then(|| commit.clone());
855 status_key = Some(key); 900 status_key = Some(key);
856 } 901 }
857 } 902 }
src/sync.rs
Old New
@@ -524,8 +524,7 @@ pub fn sync_all(repo: &Repository) -> Result<(), Error> {
524 let remotes = collab_remotes(repo)?; 524 let remotes = collab_remotes(repo)?;
525 525
526 if remotes.is_empty() { 526 if remotes.is_empty() {
527 println!("No remotes with collab refspecs configured. Run `git-collab init` first."); 527 return sync_local_only(repo);
528 return Ok(());
529 } 528 }
530 529
531 let multiple = remotes.len() > 1; 530 let multiple = remotes.len() > 1;
@@ -578,6 +577,75 @@ pub fn sync_all(repo: &Repository) -> Result<(), Error> {
578 } 577 }
579 } 578 }
580 579
580 /// The half of `sync` that needs no remote: scan local history for `Issue:`
581 /// and `Patch:` trailers and append the events they name.
582 ///
583 /// Both scans walk local refs and append local events — nothing about either
584 /// reaches the network. They used to live inside the per-remote path, behind
585 /// the no-remotes gate, so a repo with no remote never auto-recorded a merge
586 /// and never linked a commit to an issue however many trailers its history
587 /// carried. That contradicts the premise that collaboration lives in the
588 /// repository, and it is the configuration everyone is in *before* adding a
589 /// remote, so it was also the first impression the features made.
590 ///
591 /// **Never breaks sync.** Both scans absorb their own per-commit and per-item
592 /// errors; a returned `Err` from either means only "skip that scan this time",
593 /// so neither can take down the other or the sync around them.
594 fn run_local_scans(
595 repo: &Repository,
596 author: &crate::event::Author,
597 sk: &ed25519_dalek::SigningKey,
598 ) {
599 // Scan local branches for Issue: trailers and emit link events.
600 match crate::commit_link::scan_and_link(repo, author, sk) {
601 Ok(n) if n > 0 => println!("Linked {} commit(s) to issues.", n),
602 Ok(_) => {}
603 Err(e) => eprintln!("warning: commit link scan failed: {}", e),
604 }
605
606 // Scan each open patch's own base branch for Patch: trailers and record
607 // the merges they name.
608 match crate::merge_scan::scan_and_record_merges(repo, author, sk) {
609 Ok(n) if n > 0 => println!("Recorded {} merged patch(es).", n),
610 Ok(_) => {}
611 Err(e) => eprintln!("warning: merge scan failed: {}", e),
612 }
613 }
614
615 /// `sync` in a repo where no remote carries collab refspecs.
616 ///
617 /// There is nothing to fetch and nowhere to push, but that is only half of what
618 /// `sync` does. The trailer scans are the other half and they apply unchanged,
619 /// so run them and report what they did rather than printing an instruction and
620 /// stopping. The instruction still goes out — nothing recorded here is being
621 /// shared with anyone — but as a closing note, not as a refusal.
622 fn sync_local_only(repo: &Repository) -> Result<(), Error> {
623 // The scans append events, so this path takes the same advisory lock the
624 // remote path does. Two syncs racing to record the same merge is exactly
625 // the case the lock exists for, and having no remote does not change it.
626 let _lock = SyncLock::acquire(repo)?;
627
628 println!("No remotes with collab refspecs configured — scanning local history only.");
629
630 let author = get_author(repo)?;
631 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
632
633 // Same reason the remote path migrates: a bare `<id>` patch ref predating
634 // the `<id>/events` layout is invisible to everything that folds patches,
635 // so a merge scan over it would silently find no patches to record against.
636 state::migrate_patch_layout(repo);
637
638 run_local_scans(repo, &author, &sk);
639
640 println!("Local scan complete. Add a remote and run `git-collab init` to share these events.");
641
642 // The same closing hint the remote path prints, and on the same terms: a
643 // suggestion drawn from reachability, which cannot see a squash — so its
644 // silence proves nothing, and it writes nothing.
645 crate::merge_scan::print_merge_hints(repo);
646 Ok(())
647 }
648
581 /// Sync with a specific remote: fetch, reconcile, push. 649 /// Sync with a specific remote: fetch, reconcile, push.
582 pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> { 650 pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
583 // Acquire advisory lock — held until _lock is dropped (RAII) 651 // Acquire advisory lock — held until _lock is dropped (RAII)
@@ -637,24 +705,12 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
637 reconcile_refs(&repo, "issues", &author, &sk)?; 705 reconcile_refs(&repo, "issues", &author, &sk)?;
638 reconcile_refs(&repo, "patches", &author, &sk)?; 706 reconcile_refs(&repo, "patches", &author, &sk)?;
639 707
640 // Step 2.5: Scan local branches for Issue: trailers and emit link events. 708 // Step 2.5/2.6: the local trailer scans. Shared verbatim with the
641 // Never breaks sync — scan_and_link absorbs per-commit/per-issue errors. 709 // no-remote path — they are the half of sync that needs no remote, which
642 match crate::commit_link::scan_and_link(&repo, &author, &sk) { 710 // is why they live in their own function rather than here. They run before
643 Ok(n) if n > 0 => println!("Linked {} commit(s) to issues.", n), 711 // the push so anything they record travels in the same sync rather than
644 Ok(_) => {} 712 // waiting for the next one.
645 Err(e) => eprintln!("warning: commit link scan failed: {}", e), 713 run_local_scans(&repo, &author, &sk);
646 }
647
648 // Step 2.6: Scan each open patch's own base branch for Patch: trailers and
649 // record the merges they name. Same contract as the link scan above: it
650 // absorbs its own per-commit and per-patch errors and can never break sync.
651 // This runs before the push so anything it records travels in the same
652 // sync rather than waiting for the next one.
653 match crate::merge_scan::scan_and_record_merges(&repo, &author, &sk) {
654 Ok(n) if n > 0 => println!("Recorded {} merged patch(es).", n),
655 Ok(_) => {}
656 Err(e) => eprintln!("warning: merge scan failed: {}", e),
657 }
658 714
659 // Step 3: Push collab refs individually 715 // Step 3: Push collab refs individually
660 println!("Pushing to '{}'...", remote_name); 716 println!("Pushing to '{}'...", remote_name);
src/tui/mod.rs
Old New
@@ -77,6 +77,7 @@ mod tests {
77 title: title.into(), 77 title: title.into(),
78 body: String::new(), 78 body: String::new(),
79 status, 79 status,
80 merge_commit: None,
80 base_ref: "main".into(), 81 base_ref: "main".into(),
81 fixes: None, 82 fixes: None,
82 branch: format!("feature/{}", id), 83 branch: format!("feature/{}", id),
@@ -274,6 +275,7 @@ mod tests {
274 } else { 275 } else {
275 PatchStatus::Closed 276 PatchStatus::Closed
276 }, 277 },
278 merge_commit: None,
277 base_ref: "main".to_string(), 279 base_ref: "main".to_string(),
278 fixes: None, 280 fixes: None,
279 branch: format!("feature/p{:07x}", i), 281 branch: format!("feature/p{:07x}", i),
@@ -1031,6 +1033,7 @@ mod tests {
1031 title: "Fix the thing".into(), 1033 title: "Fix the thing".into(),
1032 body: "Detailed description".into(), 1034 body: "Detailed description".into(),
1033 status: PatchStatus::Open, 1035 status: PatchStatus::Open,
1036 merge_commit: None,
1034 base_ref: "main".into(), 1037 base_ref: "main".into(),
1035 fixes: Some("i1".into()), 1038 fixes: Some("i1".into()),
1036 branch: "feature/fix-thing".into(), 1039 branch: "feature/fix-thing".into(),
tests/merge_recording_test.rs
Old New
@@ -949,3 +949,246 @@ fn json_reports_the_merge_hint_beside_the_status() {
949 assert_eq!(json["status"], "merged"); 949 assert_eq!(json["status"], "merged");
950 assert!(json.get("looks_merged").is_none(), "{}", json); 950 assert!(json.get("looks_merged").is_none(), "{}", json);
951 } 951 }
952
953 // ---------------------------------------------------------------------------
954 // The recorded merge commit has to reach the views
955 // ---------------------------------------------------------------------------
956
957 #[test]
958 fn the_recorded_merge_commit_reaches_show_json_and_patch_log() {
959 // The commit is the reason it is in the event at all: it is what a UI links
960 // to, and for a squash it is the only route from the patch back to the code
961 // — the patch's own commits are ancestors of nothing on the base branch.
962 // Folding it to a bare `status = merged` threw that away.
963 let repo = TestRepo::new("Alice", "alice@example.com");
964 repo.git(&["checkout", "-b", "feat"]);
965 repo.commit_file("a.txt", "x", "the patch");
966 let out = repo.run_ok(&["patch", "create", "-t", "Feature", "-B", "feat"]);
967 let short = out.trim().strip_prefix("Created patch ").unwrap().to_string();
968 repo.git(&["checkout", "main"]);
969 squash_merge_keeping_message(&repo, "feat");
970 let landed = repo.git(&["rev-parse", "main"]).trim().to_string();
971
972 // Before the merge is recorded there is no commit to report, and the field
973 // must not read as an assertion that one exists.
974 assert!(
975 show_json(&repo, &short)["merge_commit"].is_null(),
976 "an unmerged patch has no merge commit"
977 );
978
979 repo.run_ok(&["patch", "merge", &short]);
980
981 assert_eq!(
982 show_json(&repo, &short)["merge_commit"],
983 landed,
984 "the recorded commit must survive the fold into `patch show --json`"
985 );
986
987 let list: Value =
988 serde_json::from_str(&repo.run_ok(&["patch", "list", "--json", "--all"])).unwrap();
989 assert_eq!(
990 list[0]["merge_commit"], landed,
991 "list and show render the same entries; the commit has to be in both"
992 );
993
994 let show = repo.run_ok(&["patch", "show", &short]);
995 assert!(
996 show.contains(&landed[..8]),
997 "`patch show` must name the commit that landed the patch: {}",
998 show
999 );
1000
1001 let log = repo.run_ok(&["patch", "log", &short]);
1002 assert!(
1003 log.contains(&landed[..8]),
1004 "`patch log` must name the commit that landed the patch: {}",
1005 log
1006 );
1007 }
1008
1009 #[test]
1010 fn displaying_a_merged_patch_writes_no_event() {
1011 // The governing constraint, on the path that now renders the merge commit.
1012 // Surfacing a recorded fact must stay a pure read.
1013 let repo = TestRepo::new("Alice", "alice@example.com");
1014 repo.git(&["checkout", "-b", "feat"]);
1015 repo.commit_file("a.txt", "x", "the patch");
1016 let out = repo.run_ok(&["patch", "create", "-t", "Feature", "-B", "feat"]);
1017 let short = out.trim().strip_prefix("Created patch ").unwrap().to_string();
1018 repo.git(&["checkout", "main"]);
1019 repo.git(&["merge", "--ff-only", "feat"]);
1020 repo.run_ok(&["patch", "merge", &short]);
1021
1022 let events_ref = patch_events_ref(&repo, &short);
1023 let before = count_events(&repo, &events_ref);
1024 let tip_before = repo.git(&["rev-parse", &events_ref]).trim().to_string();
1025
1026 repo.run_ok(&["patch", "list", "--all"]);
1027 repo.run_ok(&["patch", "show", &short]);
1028 repo.run_ok(&["patch", "show", &short, "--json"]);
1029 repo.run_ok(&["patch", "list", "--json", "--all"]);
1030 repo.run_ok(&["patch", "log", &short]);
1031
1032 assert_eq!(count_events(&repo, &events_ref), before);
1033 assert_eq!(
1034 repo.git(&["rev-parse", &events_ref]).trim(),
1035 tip_before,
1036 "reading a merged patch must not move its ref"
1037 );
1038 }
1039
1040 #[test]
1041 fn two_clones_recording_different_merge_commits_agree_on_one() {
1042 // Concurrent recordings name different commits — one person points at the
1043 // squash commit, another at a later one. `(clock, oid)` decides, exactly as
1044 // it does for the status, so both clones fold the same event set to the
1045 // same answer. A merged patch whose commit differed per clone would be
1046 // worse than none: it is what a UI links to.
1047 let (alice, bare) = repo_with_origin();
1048 alice.git(&["checkout", "-b", "feat"]);
1049 alice.commit_file("a.txt", "x", "the patch");
1050 let out = alice.run_ok(&["patch", "create", "-t", "Feature", "-B", "feat"]);
1051 let short = out.trim().strip_prefix("Created patch ").unwrap().to_string();
1052 alice.git(&["checkout", "main"]);
1053 alice.git(&["merge", "--ff-only", "feat"]);
1054 let first = alice.git(&["rev-parse", "main"]).trim().to_string();
1055 // A second commit on main, so the two clones have two real commits to
1056 // disagree about.
1057 let second = alice.commit_file("b.txt", "y", "later work");
1058 alice.git(&["push", "origin", "main"]);
1059 alice.run_ok(&["sync"]);
1060
1061 let bob_root = TempDir::new().unwrap();
1062 let bob = bob_root.path().join("clone");
1063 git_in(
1064 &alice,
1065 bob_root.path(),
1066 &["clone", bare.path().to_str().unwrap(), "clone"],
1067 );
1068 git_in(&alice, &bob, &["config", "user.name", "Bob"]);
1069 git_in(&alice, &bob, &["config", "user.email", "bob@example.com"]);
1070 git_in(&alice, &bob, &["config", "collab.autoSync", "false"]);
1071 collab_in(&alice, &bob, &["init"]);
1072 collab_in(&alice, &bob, &["sync"]);
1073
1074 assert_ne!(first, second, "the two commits must actually differ");
1075
1076 // Neither has seen the other's event.
1077 alice.run_ok(&["patch", "merge", &short, "--commit", &first]);
1078 collab_in(
1079 &alice,
1080 &bob,
1081 &["patch", "merge", &short, "--commit", &second],
1082 );
1083
1084 alice.run_ok(&["sync"]);
1085 collab_in(&alice, &bob, &["sync"]);
1086 alice.run_ok(&["sync"]);
1087
1088 let alice_commit = show_json(&alice, &short)["merge_commit"].clone();
1089 let bob_json: Value = serde_json::from_str(&collab_in(
1090 &alice,
1091 &bob,
1092 &["patch", "show", &short, "--json"],
1093 ))
1094 .unwrap();
1095
1096 assert!(
1097 alice_commit == Value::String(first.clone()) || alice_commit == Value::String(second),
1098 "the winner must be one of the two recorded commits, got {}",
1099 alice_commit
1100 );
1101 assert_eq!(
1102 alice_commit, bob_json["merge_commit"],
1103 "both clones must agree on which commit landed the patch"
1104 );
1105 }
1106
1107 // ---------------------------------------------------------------------------
1108 // The local scans do not need a remote
1109 // ---------------------------------------------------------------------------
1110
1111 #[test]
1112 fn a_repo_with_no_remote_records_a_merge_and_links_a_commit() {
1113 // Both scans are purely local: they walk local refs and append local
1114 // events. Nothing about either needs a remote. A single clone with no
1115 // remote is the most basic case of the project's premise that
1116 // collaboration lives in the repository — and it is the state everyone is
1117 // in *before* adding a remote, so this is also the first impression.
1118 let repo = TestRepo::new("Alice", "alice@example.com");
1119 // Deliberately no `git remote add` and no `git-collab init`.
1120
1121 let issue = repo.issue_open("Broken thing");
1122 let short = patch_with_trailer(&repo, "feat", "a.txt");
1123 squash_merge_keeping_message(&repo, "feat");
1124 let linked = repo.commit_file("b.txt", "y", &format!("unrelated fix\n\nIssue: {}", issue));
1125
1126 let out = repo.run_ok(&["sync"]);
1127
1128 assert_eq!(
1129 show_json(&repo, &short)["status"],
1130 "merged",
1131 "a repo with no remote must still record a merge from a Patch: trailer.\nsync said: {}",
1132 out
1133 );
1134
1135 let linked_commits = issue_json(&repo, &issue)["linked_commits"]
1136 .as_array()
1137 .expect("issue JSON should carry linked_commits")
1138 .iter()
1139 .map(|c| c["commit"].as_str().unwrap_or_default().to_string())
1140 .collect::<Vec<_>>();
1141 assert!(
1142 linked_commits.contains(&linked),
1143 "a repo with no remote must still link a commit from an Issue: trailer, \
1144 got {:?}.\nsync said: {}",
1145 linked_commits,
1146 out
1147 );
1148 }
1149
1150 #[test]
1151 fn sync_with_no_remote_reports_what_it_did_instead_of_only_an_instruction() {
1152 // Returning at the no-remotes gate printed a bare instruction and skipped
1153 // the local half of sync's job entirely. It should still do that half and
1154 // say so — while still pointing at `init`, since nothing is being shared.
1155 let repo = TestRepo::new("Alice", "alice@example.com");
1156 let short = patch_with_trailer(&repo, "feat", "a.txt");
1157 squash_merge_keeping_message(&repo, "feat");
1158
1159 let out = repo.run_ok(&["sync"]);
1160
1161 assert!(
1162 out.contains("Recorded 1 merged patch(es)."),
1163 "sync must report the merge it recorded: {}",
1164 out
1165 );
1166 assert!(
1167 out.contains("git-collab init"),
1168 "and still say how to get a remote: {}",
1169 out
1170 );
1171 assert_eq!(show_json(&repo, &short)["status"], "merged");
1172 }
1173
1174 #[test]
1175 fn a_second_sync_with_no_remote_records_nothing_further() {
1176 // The no-remote path appends events, so it has to be as idempotent as the
1177 // remote one: a trailer stays in history forever.
1178 let repo = TestRepo::new("Alice", "alice@example.com");
1179 let short = patch_with_trailer(&repo, "feat", "a.txt");
1180 squash_merge_keeping_message(&repo, "feat");
1181
1182 repo.run_ok(&["sync"]);
1183 let events_ref = patch_events_ref(&repo, &short);
1184 let after_first = count_events(&repo, &events_ref);
1185
1186 repo.run_ok(&["sync"]);
1187 repo.run_ok(&["sync"]);
1188
1189 assert_eq!(
1190 count_events(&repo, &events_ref),
1191 after_first,
1192 "repeated syncs with no remote must append nothing further"
1193 );
1194 }