a73x

d9a7efd6

Address review: migration resume, sync migration, DAG-derived revision refs

a73x   2026-08-09 19:28

Commit message
Address review: migration resume, sync migration, DAG-derived revision refs

sync() now migrates before it reconciles or pushes. It was the one entry
point that never did, which is backwards: sync is precisely the command
run on a repo nothing else has touched.

Interrupted migrations now resume. Parking the tip only made the
delete-then-create window survivable if something read the park back,
and nothing did - a kill in that window removed the patch from every
listing permanently. One sweep handles both halves of the window:
resume where the events ref is missing, clear the park where it is not.

revise claims the revision ref before appending the event, so a
write-once rejection records nothing instead of leaving a revision in
the DAG with no ref - the exact state this design exists to prevent.
The write-once rule now lives in one place, state::write_revision_ref,
and migration uses it rather than forcing.

Revision refs are no longer adopted from the remote at all. They are
derived from the reconciled, signed DAG, with the remote's numbering
taken only as a hint applied to commits the DAG lists. Adoption by ref
name let any peer with push access plant permanently reachable objects
that no CLI command could remove, since nothing enumerates a patch with
no events ref.

That also fixes concurrent revise, which turned out worse than the
review described: the loser's revision ref was not merely orphaned, its
push was rejected as a non-fast-forward, so their sync failed and would
have kept failing. Honouring the remote's published numbering makes the
push additive and the two sides converge.

Merge detection reads the base that was actually recorded, searching
back from the revision the head came from. Recomputing a merge-base
against the current tip returns the head itself once the patch is
merged, so an exact fast-forward - the commonest merge for a
single-commit patch, and the shape of every migrated pre-base revision -
looked like a base that never moved and stayed Open forever. A patch
with no recorded base anywhere is indistinguishable from one created on
the base branch, so it keeps the behaviour it has always had.
src/patch.rs
Old New
@@ -20,29 +20,6 @@ fn revision_base(repo: &Repository, base_ref: &str, commit: Oid) -> Result<Oid,
20 Ok(repo.merge_base(base_oid, commit).unwrap_or(base_oid)) 20 Ok(repo.merge_base(base_oid, commit).unwrap_or(base_oid))
21 } 21 }
22 22
23 /// Record revision `number`'s commit as a write-once ref beside the patch's
24 /// event DAG. This is what keeps the objects behind a revision reachable once
25 /// the branch that produced them has been rebased away.
26 fn write_revision_ref(
27 repo: &Repository,
28 events_ref: &str,
29 number: u32,
30 commit: Oid,
31 ) -> Result<(), Error> {
32 let name = state::patch_revision_ref(events_ref, number);
33 if let Ok(existing) = repo.refname_to_id(&name) {
34 if existing != commit {
35 return Err(Error::Cmd(format!(
36 "revision {} already points at {} — revisions are write-once",
37 number, existing
38 )));
39 }
40 return Ok(());
41 }
42 repo.reference(&name, commit, false, "record revision")?;
43 Ok(())
44 }
45
46 pub fn create( 23 pub fn create(
47 repo: &Repository, 24 repo: &Repository,
48 title: &str, 25 title: &str,
@@ -102,7 +79,7 @@ pub fn create(
102 let id = oid.to_string(); 79 let id = oid.to_string();
103 let events_ref = state::patch_events_ref(&id); 80 let events_ref = state::patch_events_ref(&id);
104 repo.reference(&events_ref, oid, false, "patch create")?; 81 repo.reference(&events_ref, oid, false, "patch create")?;
105 write_revision_ref(repo, &events_ref, 1, tip_oid)?; 82 state::write_revision_ref(repo, &events_ref, 1, tip_oid)?;
106 Ok(id) 83 Ok(id)
107 } 84 }
108 85
@@ -395,6 +372,14 @@ pub fn revise(
395 let base_oid = revision_base(repo, &patch.base_ref, tip_oid)?; 372 let base_oid = revision_base(repo, &patch.base_ref, tip_oid)?;
396 let number = patch.revisions.last().map(|r| r.number).unwrap_or(0) + 1; 373 let number = patch.revisions.last().map(|r| r.number).unwrap_or(0) + 1;
397 let author = get_author(repo)?; 374 let author = get_author(repo)?;
375
376 // Claim the ref before recording the event. An event cannot be taken back,
377 // so appending first would let a write-once rejection leave a revision in
378 // the DAG with no ref pointing at it — precisely the state this design
379 // exists to prevent. This way a rejection records nothing and the user can
380 // simply retry.
381 state::write_revision_ref(repo, &ref_name, number, tip_oid)?;
382
398 let event = Event { 383 let event = Event {
399 timestamp: chrono::Utc::now().to_rfc3339(), 384 timestamp: chrono::Utc::now().to_rfc3339(),
400 author, 385 author,
@@ -407,7 +392,6 @@ pub fn revise(
407 clock: 0, 392 clock: 0,
408 }; 393 };
409 dag::append_event(repo, &ref_name, &event, &sk)?; 394 dag::append_event(repo, &ref_name, &event, &sk)?;
410 write_revision_ref(repo, &ref_name, number, tip_oid)?;
411 Ok(()) 395 Ok(())
412 } 396 }
413 397
src/state.rs
Old New
@@ -484,12 +484,8 @@ impl PatchState {
484 /// commits at all — either a branch name or, in the oldest shape, a raw OID 484 /// commits at all — either a branch name or, in the oldest shape, a raw OID
485 /// stored where the branch name now lives. 485 /// stored where the branch name now lives.
486 pub fn resolve_head(&self, repo: &Repository) -> Result<Oid, crate::error::Error> { 486 pub fn resolve_head(&self, repo: &Repository) -> Result<Oid, crate::error::Error> {
487 for rev in self.revisions.iter().rev() { 487 if let Some(oid) = self.latest_usable_commit(repo) {
488 if let Ok(oid) = Oid::from_str(&rev.commit) { 488 return Ok(oid);
489 if repo.find_commit(oid).is_ok() {
490 return Ok(oid);
491 }
492 }
493 } 489 }
494 if let Ok(oid) = Oid::from_str(&self.branch) { 490 if let Ok(oid) = Oid::from_str(&self.branch) {
495 if repo.find_commit(oid).is_ok() { 491 if repo.find_commit(oid).is_ok() {
@@ -505,15 +501,39 @@ impl PatchState {
505 }) 501 })
506 } 502 }
507 503
508 /// Where the latest revision branched off the base branch. Revisions 504 /// Index of the newest revision whose commit was recorded and whose objects
509 /// written before `base` was stored fall back to recomputing the 505 /// are still present. Everything anchored to "where the patch stands" — the
510 /// merge-base against the base branch as it stands now, which is what 506 /// head and the base it stands on — is read from here, or the two describe
511 /// every patch used to do at display time. 507 /// different points in the patch's history.
512 fn latest_base(&self, repo: &Repository, base_tip: Oid, head: Oid) -> Option<Oid> { 508 fn latest_usable_index(&self, repo: &Repository) -> Option<usize> {
513 match self.revisions.last().and_then(|r| r.base.as_deref()) { 509 self.revisions.iter().rposition(|r| {
514 Some(base) => Oid::from_str(base).ok(), 510 Oid::from_str(&r.commit)
515 None => repo.merge_base(base_tip, head).ok(), 511 .map(|oid| repo.find_commit(oid).is_ok())
516 } 512 .unwrap_or(false)
513 })
514 }
515
516 fn latest_usable_commit(&self, repo: &Repository) -> Option<Oid> {
517 let rev = self.revisions.get(self.latest_usable_index(repo)?)?;
518 Oid::from_str(&rev.commit).ok()
519 }
520
521 /// The base the patch currently stands on, searching back from the revision
522 /// the head came from for the newest one that recorded a base.
523 ///
524 /// The search matters because `base` is only written by revisions recorded
525 /// after this change: a migrated patch typically has one on its `PatchCreate`
526 /// and none on the `PatchRevision` events after it. Reading only the newest
527 /// revision would call the base unknown for exactly those patches, and an
528 /// unknown base cannot be recovered by recomputing a merge-base against the
529 /// tip as it stands now — once the patch is merged that yields the head
530 /// itself, which looks like a base that never moved.
531 fn effective_base(&self, repo: &Repository) -> Option<Oid> {
532 let end = self.latest_usable_index(repo)?;
533 self.revisions[..=end]
534 .iter()
535 .rev()
536 .find_map(|r| r.base.as_deref().and_then(|b| Oid::from_str(b).ok()))
517 } 537 }
518 538
519 /// Compute staleness: how many commits the branch is ahead of base, 539 /// Compute staleness: how many commits the branch is ahead of base,
@@ -538,6 +558,23 @@ impl PatchState {
538 /// an earlier revision; and it no longer routes through 558 /// an earlier revision; and it no longer routes through
539 /// `refs/heads/<branch>`, which used to make detection no-op silently 559 /// `refs/heads/<branch>`, which used to make detection no-op silently
540 /// whenever the source branch had been deleted or was never pushed. 560 /// whenever the source branch had been deleted or was never pushed.
561 ///
562 /// `base_moved` exists only to spare the degenerate patch whose recorded
563 /// base is its own head — one created on the base branch itself. With a
564 /// base recorded, everything else is already decided by reachability.
565 ///
566 /// The base therefore has to be the one actually recorded, never one
567 /// recomputed against the tip as it stands now: in the exact fast-forward
568 /// case a recomputed merge-base is the head itself, so a merged patch looks
569 /// unmoved and stays Open forever. That is the commonest merge for a
570 /// single-commit patch. `effective_base` searches back for the newest
571 /// recorded base rather than reading only the newest revision, which is
572 /// what keeps migrated patches — base on the create, none on the revisions
573 /// after it — out of that trap.
574 ///
575 /// A patch with no recorded base anywhere predates the field entirely.
576 /// Nothing distinguishes it from the degenerate case, so it keeps the
577 /// behaviour it has always had rather than being guessed at.
541 fn check_auto_merge(&mut self, repo: &Repository) { 578 fn check_auto_merge(&mut self, repo: &Repository) {
542 if self.status != PatchStatus::Open { 579 if self.status != PatchStatus::Open {
543 return; 580 return;
@@ -549,10 +586,10 @@ impl PatchState {
549 let Ok(base_tip) = repo.refname_to_id(&base_ref) else { 586 let Ok(base_tip) = repo.refname_to_id(&base_ref) else {
550 return; 587 return;
551 }; 588 };
552 let Some(base_at_revision) = self.latest_base(repo, base_tip, patch_head) else { 589 let base_moved = match self.effective_base(repo) {
553 return; 590 Some(base) => base != base_tip,
591 None => false,
554 }; 592 };
555 let base_moved = base_at_revision != base_tip;
556 let reachable = base_tip == patch_head 593 let reachable = base_tip == patch_head
557 || repo 594 || repo
558 .graph_descendant_of(base_tip, patch_head) 595 .graph_descendant_of(base_tip, patch_head)
@@ -1083,6 +1120,62 @@ pub fn delete_patch_refs(repo: &Repository, id: &str) -> Result<(), crate::error
1083 Ok(()) 1120 Ok(())
1084 } 1121 }
1085 1122
1123 /// Where a migration parks a patch's tip while it swaps the old ref for the
1124 /// new subtree. Outside the patch namespace so it cannot collide with either
1125 /// layout, and under `local/` so it is never pushed.
1126 const MIGRATION_PARK_PREFIX: &str = "refs/collab/local/migrating/patches/";
1127
1128 /// Finish, or clean up after, any migration that was interrupted partway.
1129 ///
1130 /// A migration deletes the old ref before it can create the events ref — git
1131 /// will not let `<id>` be both a ref and a directory, so the two cannot
1132 /// overlap. A kill in that window leaves the patch reachable only from the
1133 /// parked ref, where nothing else would ever look for it again: gone from
1134 /// `patch list`, `patch show` and the TUI, permanently, with its objects alive
1135 /// but unfindable. This is the only thing that makes parking worth anything.
1136 ///
1137 /// The later window — killed after the events ref exists but before the park
1138 /// was dropped — leaves the park behind as litter. Both come out of one loop,
1139 /// and because every write below forces, resuming over a partially migrated
1140 /// patch is idempotent.
1141 fn resume_interrupted_migrations(repo: &Repository) {
1142 let Ok(refs) = repo.references_glob(&format!("{}*", MIGRATION_PARK_PREFIX)) else {
1143 return;
1144 };
1145 let parked: Vec<(String, String)> = refs
1146 .filter_map(|r| {
1147 let name = r.ok()?.name()?.to_string();
1148 let id = name.strip_prefix(MIGRATION_PARK_PREFIX)?.to_string();
1149 Some((name, id))
1150 })
1151 .collect();
1152
1153 for (park_ref, id) in parked {
1154 // The park carries no namespace, so look for the patch in both. If it
1155 // already has an events ref anywhere, the migration got far enough and
1156 // only the park needs clearing.
1157 let already_migrated = [PATCH_PREFIX, ARCHIVE_PATCH_PREFIX]
1158 .iter()
1159 .any(|p| repo.refname_to_id(&format!("{}{}/events", p, id)).is_ok());
1160
1161 if !already_migrated {
1162 // Resume from the parked tip. The active namespace is the right
1163 // home: a patch is only archived by an explicit close, and a close
1164 // moves the whole subtree, so nothing half-migrated belongs there.
1165 if let Err(e) = finish_migration(repo, PATCH_PREFIX, &id, &park_ref) {
1166 eprintln!(
1167 "warning: could not resume interrupted migration of patch {:.8}: {}",
1168 id, e
1169 );
1170 continue;
1171 }
1172 }
1173 if let Ok(mut r) = repo.find_reference(&park_ref) {
1174 let _ = r.delete();
1175 }
1176 }
1177 }
1178
1086 /// Bring patches written in the pre-revision-refs layout — a single ref at 1179 /// Bring patches written in the pre-revision-refs layout — a single ref at
1087 /// `refs/collab/patches/<id>` — up to the current one. Called from every entry 1180 /// `refs/collab/patches/<id>` — up to the current one. Called from every entry
1088 /// point that enumerates or resolves a patch, so an old repository migrates on 1181 /// point that enumerates or resolves a patch, so an old repository migrates on
@@ -1091,6 +1184,10 @@ pub fn delete_patch_refs(repo: &Repository, id: &str) -> Result<(), crate::error
1091 /// Failures are reported and skipped rather than propagated: one unmigratable 1184 /// Failures are reported and skipped rather than propagated: one unmigratable
1092 /// patch must not make the whole list unreadable. 1185 /// patch must not make the whole list unreadable.
1093 pub fn migrate_patch_layout(repo: &Repository) { 1186 pub fn migrate_patch_layout(repo: &Repository) {
1187 // Before anything else: a patch stranded by an earlier interrupted run is
1188 // invisible to the scan below, because its old ref is already gone.
1189 resume_interrupted_migrations(repo);
1190
1094 for prefix in [PATCH_PREFIX, ARCHIVE_PATCH_PREFIX] { 1191 for prefix in [PATCH_PREFIX, ARCHIVE_PATCH_PREFIX] {
1095 let Ok(refs) = repo.references_glob(&format!("{}*", prefix)) else { 1192 let Ok(refs) = repo.references_glob(&format!("{}*", prefix)) else {
1096 continue; 1193 continue;
@@ -1126,22 +1223,43 @@ fn migrate_one_patch(
1126 1223
1127 // Read the revision list off the old ref before touching anything, so a 1224 // Read the revision list off the old ref before touching anything, so a
1128 // patch whose DAG will not materialize is left exactly as it was found. 1225 // patch whose DAG will not materialize is left exactly as it was found.
1129 let state = PatchState::from_ref_uncached(repo, old_ref, id)?; 1226 PatchState::from_ref_uncached(repo, old_ref, id)?;
1130 1227
1131 // The old ref and the new subtree cannot coexist: git refuses to have 1228 // The old ref and the new subtree cannot coexist: git refuses to have
1132 // `<id>` be both a ref and a directory. Park the tip somewhere outside the 1229 // `<id>` be both a ref and a directory. Park the tip outside the patch
1133 // patch namespace so an interrupted migration is still recoverable, then 1230 // namespace first — `resume_interrupted_migrations` reads it back, which is
1134 // do the delete-and-recreate. 1231 // what makes the delete-then-create window survivable.
1135 let parked = format!("refs/collab/local/migrating/patches/{}", id); 1232 let parked = format!("{}{}", MIGRATION_PARK_PREFIX, id);
1136 repo.reference(&parked, oid, true, "migrate patch: park tip")?; 1233 repo.reference(&parked, oid, true, "migrate patch: park tip")?;
1137 repo.find_reference(old_ref)?.delete()?; 1234 repo.find_reference(old_ref)?.delete()?;
1138 1235
1236 finish_migration(repo, prefix, id, &parked)?;
1237
1238 repo.find_reference(&parked)?.delete()?;
1239 Ok(())
1240 }
1241
1242 /// Create the events ref from a parked tip and give every recoverable revision
1243 /// a ref. Idempotent, so it can resume over a partially migrated patch.
1244 fn finish_migration(
1245 repo: &Repository,
1246 prefix: &str,
1247 id: &str,
1248 parked: &str,
1249 ) -> Result<(), crate::error::Error> {
1250 let oid = repo.refname_to_id(parked)?;
1139 let events_ref = format!("{}{}/events", prefix, id); 1251 let events_ref = format!("{}{}/events", prefix, id);
1140 repo.reference(&events_ref, oid, true, "migrate patch: events")?; 1252 repo.reference(&events_ref, oid, true, "migrate patch: events")?;
1141 1253
1142 // Give every revision whose commit is still present a ref of its own. 1254 // Give every revision whose commit is still present a ref of its own.
1143 // Revisions whose objects were already lost to a force-push cannot be 1255 // Revisions whose objects were already lost to a force-push cannot be
1144 // recovered and keep the `""`-means-unknown convention. 1256 // recovered and keep the `""`-means-unknown convention.
1257 //
1258 // Write-once applies here as everywhere else: migration runs off a DAG no
1259 // reconcile has touched, so its numbering is settled and a revision ref
1260 // that already exists can only be one this migration wrote on an earlier,
1261 // interrupted attempt.
1262 let state = PatchState::from_ref_uncached(repo, &events_ref, id)?;
1145 for rev in &state.revisions { 1263 for rev in &state.revisions {
1146 let Ok(commit) = Oid::from_str(&rev.commit) else { 1264 let Ok(commit) = Oid::from_str(&rev.commit) else {
1147 continue; 1265 continue;
@@ -1149,11 +1267,107 @@ fn migrate_one_patch(
1149 if repo.find_commit(commit).is_err() { 1267 if repo.find_commit(commit).is_err() {
1150 continue; 1268 continue;
1151 } 1269 }
1152 let name = patch_revision_ref(&events_ref, rev.number); 1270 write_revision_ref(repo, &events_ref, rev.number, commit)?;
1153 repo.reference(&name, commit, true, "migrate patch: revision")?;
1154 } 1271 }
1272 Ok(())
1273 }
1155 1274
1156 repo.find_reference(&parked)?.delete()?; 1275 /// Point revision `number` at `commit`, refusing to move a revision that is
1276 /// already recorded elsewhere. This is the write-once rule, in one place: a
1277 /// revision is never rewritten, a changed patch adds the next one.
1278 pub fn write_revision_ref(
1279 repo: &Repository,
1280 events_ref: &str,
1281 number: u32,
1282 commit: Oid,
1283 ) -> Result<(), crate::error::Error> {
1284 let name = patch_revision_ref(events_ref, number);
1285 if let Ok(existing) = repo.refname_to_id(&name) {
1286 if existing != commit {
1287 return Err(crate::error::Error::Cmd(format!(
1288 "revision {} already points at {} — revisions are write-once",
1289 number, existing
1290 )));
1291 }
1292 return Ok(());
1293 }
1294 repo.reference(&name, commit, false, "record revision")?;
1295 Ok(())
1296 }
1297
1298 /// Numbering a remote already published for this patch, read from the fetched
1299 /// sync refs. Only a hint: it is applied to commits the signed event DAG lists
1300 /// and to nothing else.
1301 pub type RevisionNumbering = std::collections::HashMap<u32, Oid>;
1302
1303 /// Make the patch's revision refs a function of its signed event DAG.
1304 ///
1305 /// Two problems meet here. Revision numbers come from the DAG walk while ref
1306 /// names are chosen at write time, so a concurrent `revise` renumbers one side
1307 /// and can leave its commit referenced by nothing once the sync refs are swept
1308 /// — the force-push orphan this design exists to prevent, arriving through
1309 /// concurrency instead of rebase. And a number the remote has already published
1310 /// cannot be renegotiated: pushing a different commit at it is a non-fast-
1311 /// forward that no retry will ever clear.
1312 ///
1313 /// So: honour the remote's numbering for any commit the DAG also lists, give
1314 /// every remaining revision its own number where free and the lowest free one
1315 /// otherwise, and write that. Whoever pushes first settles the numbering and
1316 /// everyone else adapts to it, which converges. A ref may move as a result, but
1317 /// no commit loses its ref: reconciliation only ever adds revisions, so every
1318 /// previously referenced commit is still in the list and still gets a number.
1319 ///
1320 /// Nothing outside the DAG is ever referenced, which is also what keeps a peer
1321 /// from planting reachable objects by pushing a ref name we would otherwise
1322 /// adopt on trust.
1323 pub fn reconcile_revision_refs(
1324 repo: &Repository,
1325 events_ref: &str,
1326 id: &str,
1327 remote: &RevisionNumbering,
1328 ) -> Result<(), crate::error::Error> {
1329 let state = PatchState::from_ref_uncached(repo, events_ref, id)?;
1330
1331 // Revisions we can actually point a ref at, in DAG order.
1332 let recoverable: Vec<(u32, Oid)> = state
1333 .revisions
1334 .iter()
1335 .filter_map(|r| {
1336 let oid = Oid::from_str(&r.commit).ok()?;
1337 repo.find_commit(oid).ok()?;
1338 Some((r.number, oid))
1339 })
1340 .collect();
1341
1342 let mut assigned: std::collections::BTreeMap<u32, Oid> = std::collections::BTreeMap::new();
1343 let mut placed: std::collections::HashSet<Oid> = std::collections::HashSet::new();
1344
1345 for (number, oid) in remote {
1346 if recoverable.iter().any(|(_, o)| o == oid) && !assigned.contains_key(number) {
1347 assigned.insert(*number, *oid);
1348 placed.insert(*oid);
1349 }
1350 }
1351 for (number, oid) in &recoverable {
1352 if placed.contains(oid) {
1353 continue;
1354 }
1355 let slot = if assigned.contains_key(number) {
1356 (1u32..).find(|n| !assigned.contains_key(n)).unwrap_or(*number)
1357 } else {
1358 *number
1359 };
1360 assigned.insert(slot, *oid);
1361 placed.insert(*oid);
1362 }
1363
1364 for (number, oid) in assigned {
1365 let name = patch_revision_ref(events_ref, number);
1366 if repo.refname_to_id(&name).ok() == Some(oid) {
1367 continue;
1368 }
1369 repo.reference(&name, oid, true, "reconcile revision refs")?;
1370 }
1157 Ok(()) 1371 Ok(())
1158 } 1372 }
1159 1373
src/sync.rs
Old New
@@ -1,3 +1,4 @@
1 use std::collections::HashMap;
1 use std::fs; 2 use std::fs;
2 use std::path::{Path, PathBuf}; 3 use std::path::{Path, PathBuf};
3 use std::process::Command; 4 use std::process::Command;
@@ -9,6 +10,7 @@ use crate::dag;
9 use crate::error::Error; 10 use crate::error::Error;
10 use crate::identity::get_author; 11 use crate::identity::get_author;
11 use crate::signing; 12 use crate::signing;
13 use crate::state;
12 use crate::sync_lock::SyncLock; 14 use crate::sync_lock::SyncLock;
13 use crate::trust; 15 use crate::trust;
14 16
@@ -461,6 +463,16 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
461 // `repo` handle won't see them. `repo.path()` returns the `.git` 463 // `repo` handle won't see them. `repo.path()` returns the `.git`
462 // directory, which is the correct argument to `Repository::open`. 464 // directory, which is the correct argument to `Repository::open`.
463 let repo = Repository::open(repo.path())?; 465 let repo = Repository::open(repo.path())?;
466
467 // Migrate before reconciling or pushing. `sync` is precisely the command a
468 // contributor runs on a repo they have not otherwise touched, so it cannot
469 // rely on some earlier read having migrated: an incoming `<id>/events`
470 // would collide with a local bare `<id>` as a directory-vs-file lock
471 // conflict that aborts the whole patches reconcile, and the bare ref would
472 // be pushed outbound where a migrated remote must reject it for the same
473 // reason, with nothing to tell the user why.
474 state::migrate_patch_layout(&repo);
475
464 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; 476 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
465 reconcile_refs(&repo, "issues", &author, &sk)?; 477 reconcile_refs(&repo, "issues", &author, &sk)?;
466 reconcile_refs(&repo, "patches", &author, &sk)?; 478 reconcile_refs(&repo, "patches", &author, &sk)?;
@@ -616,9 +628,14 @@ fn push_refs(workdir: &Path, remote_name: &str, refs: &[String]) -> SyncResult {
616 enum SyncRef { 628 enum SyncRef {
617 /// An event DAG. Verified, then reconciled against the local ref. 629 /// An event DAG. Verified, then reconciled against the local ref.
618 Events { local_ref: String }, 630 Events { local_ref: String },
619 /// A patch revision: an ordinary source commit, not an event, so it 631 /// A patch revision. Never adopted on the strength of its name: it carries
620 /// carries no signature to verify and is never merged. Write-once. 632 /// no event signature, and a ref we adopt is reachable in every clone that
621 Revision { local_ref: String }, 633 /// syncs afterwards and removable through no CLI command, since nothing
634 /// enumerates a patch with no events ref. It contributes only the number
635 /// the remote published, which `state::reconcile_revision_refs` applies to
636 /// commits the signed DAG lists and to nothing else. The objects arrive
637 /// with the fetch either way.
638 Revision { number: u32 },
622 } 639 }
623 640
624 /// Classify a fetched sync ref by the part of its name after the kind prefix, 641 /// Classify a fetched sync ref by the part of its name after the kind prefix,
@@ -641,11 +658,14 @@ fn classify_sync_ref(kind: &str, rest: &str) -> Option<(String, SyncRef)> {
641 Some((id, suffix)) => (id, suffix), 658 Some((id, suffix)) => (id, suffix),
642 None => (rest, "events"), 659 None => (rest, "events"),
643 }; 660 };
644 let local_ref = format!("refs/collab/patches/{}/{}", id, suffix);
645 let classified = if suffix == "events" { 661 let classified = if suffix == "events" {
646 SyncRef::Events { local_ref } 662 SyncRef::Events {
647 } else if suffix.starts_with("r/") { 663 local_ref: format!("refs/collab/patches/{}/events", id),
648 SyncRef::Revision { local_ref } 664 }
665 } else if let Some(n) = suffix.strip_prefix("r/") {
666 SyncRef::Revision {
667 number: n.parse().ok()?,
668 }
649 } else { 669 } else {
650 return None; 670 return None;
651 }; 671 };
@@ -672,6 +692,22 @@ fn reconcile_refs(
672 .collect() 692 .collect()
673 }; 693 };
674 694
695 // The numbering each patch's revisions carry on the remote, keyed by patch
696 // id. Gathered before any events are reconciled so it describes what the
697 // remote actually published, and applied only to commits the signed DAG
698 // lists.
699 let mut remote_numbering: HashMap<String, state::RevisionNumbering> = HashMap::new();
700 for (remote_ref, id, classified) in &sync_refs {
701 if let SyncRef::Revision { number } = classified {
702 if let Ok(oid) = repo.refname_to_id(remote_ref) {
703 remote_numbering
704 .entry(id.clone())
705 .or_default()
706 .insert(*number, oid);
707 }
708 }
709 }
710
675 // Load trust policy once for all refs of this kind 711 // Load trust policy once for all refs of this kind
676 let trust_policy = trust::load_trust_policy(repo)?; 712 let trust_policy = trust::load_trust_policy(repo)?;
677 let mut warned_unconfigured = false; 713 let mut warned_unconfigured = false;
@@ -683,21 +719,9 @@ fn reconcile_refs(
683 continue; 719 continue;
684 } 720 }
685 721
686 // A revision ref points at the author's source commit. It carries no 722 // Revision refs are derived from the reconciled DAG below, never
687 // event signature, and it is write-once: an existing local revision is 723 // adopted from the remote.
688 // never overwritten by a remote one claiming the same number. 724 if matches!(classified, SyncRef::Revision { .. }) {
689 if let SyncRef::Revision { local_ref } = classified {
690 let oid = repo.refname_to_id(remote_ref)?;
691 match repo.refname_to_id(local_ref) {
692 Ok(existing) if existing != oid => eprintln!(
693 " Keeping local {} (remote claims {}); revisions are write-once",
694 local_ref, oid
695 ),
696 Ok(_) => {}
697 Err(_) => {
698 repo.reference(local_ref, oid, false, "sync: new revision from remote")?;
699 }
700 }
701 continue; 725 continue;
702 } 726 }
703 727
@@ -757,6 +781,22 @@ fn reconcile_refs(
757 repo.reference(local_ref, oid, false, "sync: new from remote")?; 781 repo.reference(local_ref, oid, false, "sync: new from remote")?;
758 println!(" New {} {:.8} from remote", kind, id); 782 println!(" New {} {:.8} from remote", kind, id);
759 } 783 }
784
785 // Bring the revision refs back into agreement with the DAG we just
786 // reconciled, while the fetched sync refs still hold the remote's
787 // objects — `cleanup_sync_refs` drops them shortly after this returns,
788 // and any revision commit without a ref of its own becomes gc-eligible
789 // at that point.
790 if kind == "patches" {
791 let empty = state::RevisionNumbering::new();
792 let numbering = remote_numbering.get(id).unwrap_or(&empty);
793 if let Err(e) = state::reconcile_revision_refs(repo, local_ref, id, numbering) {
794 eprintln!(
795 " Failed to reconcile revision refs for patch {:.8}: {}",
796 id, e
797 );
798 }
799 }
760 } 800 }
761 Ok(()) 801 Ok(())
762 } 802 }
tests/collab_test.rs
Old New
@@ -687,6 +687,16 @@ fn create_branch_patch(
687 }, 687 },
688 clock: 0, 688 clock: 0,
689 }; 689 };
690 // Load-bearing for test_resolve_head_branch_based and
691 // test_resolve_head_deleted_branch_error: they only test the legacy
692 // fallback because this commit cannot be resolved. Assert it rather than
693 // just documenting it, so an edit here cannot make both tests pass
694 // vacuously by giving them a real commit to find.
695 assert!(
696 repo.find_commit(git2::Oid::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap())
697 .is_err(),
698 "the placeholder commit must not be in the object database"
699 );
690 let oid = dag::create_root_event(repo, &event, &sk).unwrap(); 700 let oid = dag::create_root_event(repo, &event, &sk).unwrap();
691 let id = oid.to_string(); 701 let id = oid.to_string();
692 let patch_ref = git_collab::state::patch_events_ref(&id); 702 let patch_ref = git_collab::state::patch_events_ref(&id);
tests/revision_refs_test.rs
Old New
@@ -241,6 +241,40 @@ fn revise_reads_head_by_default_and_a_named_branch_on_request() {
241 assert_eq!(revisions[1]["commit"], r2); 241 assert_eq!(revisions[1]["commit"], r2);
242 } 242 }
243 243
244 #[test]
245 fn revise_records_nothing_when_the_revision_ref_is_taken() {
246 // The event must not outlive the ref write. A PatchRevision in the DAG with
247 // no r/<n> beside it is exactly the state this design exists to prevent, so
248 // a write-once rejection has to leave the patch untouched, not half-written.
249 let repo = TestRepo::new("Alice", "alice@example.com");
250 let (short, id, _r1) = patch_on_branch(&repo, "feat", "a.txt");
251
252 // Squat on r/2 with an unrelated commit.
253 let squatter = repo.git(&["rev-parse", "main"]).trim().to_string();
254 repo.git(&[
255 "update-ref",
256 &format!("refs/collab/patches/{}/r/2", id),
257 &squatter,
258 ]);
259
260 repo.commit_file("b.txt", "v2", "second commit");
261 let err = repo.run_err(&["patch", "revise", &short]);
262 assert!(err.contains("write-once"), "unexpected error: {}", err);
263
264 let json = show_json(&repo, &short);
265 assert_eq!(
266 json["revisions"].as_array().unwrap().len(),
267 1,
268 "a rejected revise must not leave a revision in the DAG: {}",
269 json
270 );
271 assert_eq!(
272 ref_target(&repo, &format!("refs/collab/patches/{}/r/2", id)).as_deref(),
273 Some(squatter.as_str()),
274 "and must not move the ref it collided with"
275 );
276 }
277
244 // =========================================================================== 278 // ===========================================================================
245 // Identity is declared, not derived from a branch name 279 // Identity is declared, not derived from a branch name
246 // =========================================================================== 280 // ===========================================================================
@@ -338,6 +372,218 @@ fn merge_detection_reads_the_latest_revisions_base_not_the_first() {
338 assert_eq!(show_json(&repo, &short)["status"], "merged"); 372 assert_eq!(show_json(&repo, &short)["status"], "merged");
339 } 373 }
340 374
375 fn tree_of(repo: &TestRepo, commit: &str) -> String {
376 let git_repo = git2::Repository::open(repo.dir.path()).unwrap();
377 let tree = git_repo
378 .find_commit(git2::Oid::from_str(commit).unwrap())
379 .unwrap()
380 .tree()
381 .unwrap()
382 .id()
383 .to_string();
384 tree
385 }
386
387 /// A patch in the shape migration produces: a `PatchCreate` carrying
388 /// `base_commit`, followed by a `PatchRevision` from before revisions recorded
389 /// a base of their own. Returns the full id.
390 fn patch_with_baseless_revision(
391 repo: &TestRepo,
392 r1: &str,
393 base: &str,
394 r2: &str,
395 title: &str,
396 ) -> String {
397 let git_repo = git2::Repository::open(repo.dir.path()).unwrap();
398 let root = write_raw_event(
399 &git_repo,
400 None,
401 json!({
402 "type": "patch.create",
403 "title": title,
404 "body": "",
405 "base_ref": "main",
406 "branch": "feat",
407 "commit": r1,
408 "tree": tree_of(repo, r1),
409 "base_commit": base,
410 }),
411 1,
412 );
413 let tip = write_raw_event(
414 &git_repo,
415 Some(root),
416 json!({
417 "type": "patch.revision",
418 "commit": r2,
419 "tree": tree_of(repo, r2),
420 }),
421 2,
422 );
423 let id = root.to_string();
424 git_repo
425 .reference(
426 &format!("refs/collab/patches/{}", id),
427 tip,
428 false,
429 "old layout",
430 )
431 .unwrap();
432 id
433 }
434
435 #[test]
436 fn a_migrated_patch_merged_by_exact_fast_forward_is_detected() {
437 // The latest revision predates `base`, which is the normal shape of a
438 // migrated patch. In the exact fast-forward case — main fast-forwarded onto
439 // the patch head, so base tip == head — recomputing a merge-base yields the
440 // head itself, so the base looks like it never moved and the merged patch
441 // stays Open forever. That is the commonest merge for a single-commit
442 // patch, so it has to come from the base that was actually recorded.
443 let repo = TestRepo::new("Alice", "alice@example.com");
444 let base = repo.git(&["rev-parse", "main"]).trim().to_string();
445 repo.git(&["checkout", "-b", "feat"]);
446 let r1 = repo.commit_file("a.txt", "v1", "the patch");
447 let r2 = repo.commit_file("b.txt", "v2", "revision 2");
448 repo.git(&["checkout", "main"]);
449
450 let id = patch_with_baseless_revision(&repo, &r1, &base, &r2, "Migrated fast-forward");
451
452 repo.git(&["merge", "--ff-only", "feat"]);
453 assert_eq!(
454 repo.git(&["rev-parse", "main"]).trim(),
455 r2,
456 "precondition: base tip and patch head are the same commit"
457 );
458
459 assert_eq!(show_json(&repo, &id[..8])["status"], "merged");
460 }
461
462 #[test]
463 fn a_migrated_patch_that_was_not_merged_stays_open() {
464 let repo = TestRepo::new("Alice", "alice@example.com");
465 let base = repo.git(&["rev-parse", "main"]).trim().to_string();
466 repo.git(&["checkout", "-b", "feat"]);
467 let r1 = repo.commit_file("a.txt", "v1", "the patch");
468 let r2 = repo.commit_file("b.txt", "v2", "revision 2");
469 repo.git(&["checkout", "main"]);
470
471 let id = patch_with_baseless_revision(&repo, &r1, &base, &r2, "Migrated unmerged");
472
473 // main moves, but not onto the patch.
474 repo.commit_file("unrelated.txt", "x", "upstream work");
475
476 assert_eq!(show_json(&repo, &id[..8])["status"], "open");
477 }
478
479 #[test]
480 fn a_patch_created_on_the_base_branch_is_not_reported_merged() {
481 // The degenerate case `base_moved` exists for: the recorded base IS the
482 // patch's own head, so the head is trivially reachable from the base tip
483 // without anything having been merged. Guards against widening the
484 // unknown-base handling until it swallows this.
485 let repo = TestRepo::new("Alice", "alice@example.com");
486 let tip = repo.git(&["rev-parse", "main"]).trim().to_string();
487 repo.git(&["branch", "feat"]);
488
489 let git_repo = git2::Repository::open(repo.dir.path()).unwrap();
490 let root = write_raw_event(
491 &git_repo,
492 None,
493 json!({
494 "type": "patch.create",
495 "title": "Created on the base branch",
496 "body": "",
497 "base_ref": "main",
498 "branch": "feat",
499 "commit": tip,
500 "tree": tree_of(&repo, &tip),
501 "base_commit": tip,
502 }),
503 1,
504 );
505 let id = root.to_string();
506 git_repo
507 .reference(
508 &format!("refs/collab/patches/{}", id),
509 root,
510 false,
511 "old layout",
512 )
513 .unwrap();
514 drop(git_repo);
515
516 assert_eq!(show_json(&repo, &id[..8])["status"], "open");
517 }
518
519 #[test]
520 fn merge_detection_reads_the_base_of_the_revision_it_resolved_the_head_from() {
521 // `resolve_head` walks back past revisions whose objects are gone. The base
522 // has to come from the same revision, or the head is compared against a
523 // base it never stood on.
524 let repo = TestRepo::new("Alice", "alice@example.com");
525 repo.git(&["checkout", "-b", "feat"]);
526 let head = repo.commit_file("a.txt", "v1", "the patch");
527 repo.git(&["checkout", "main"]);
528 let base = repo.git(&["rev-parse", "main"]).trim().to_string();
529
530 let git_repo = git2::Repository::open(repo.dir.path()).unwrap();
531 let tree = git_repo
532 .find_commit(git2::Oid::from_str(&head).unwrap())
533 .unwrap()
534 .tree()
535 .unwrap()
536 .id()
537 .to_string();
538 let root = write_raw_event(
539 &git_repo,
540 None,
541 json!({
542 "type": "patch.create",
543 "title": "Head from r1, base must follow",
544 "body": "",
545 "base_ref": "main",
546 "branch": "feat",
547 "commit": head,
548 "tree": tree,
549 "base_commit": base,
550 }),
551 1,
552 );
553 // Revision 2 points at objects that are not in this repository, so
554 // `resolve_head` falls back to revision 1.
555 let tip = write_raw_event(
556 &git_repo,
557 Some(root),
558 json!({
559 "type": "patch.revision",
560 "commit": "2222222222222222222222222222222222222222",
561 "tree": tree,
562 "base": "3333333333333333333333333333333333333333",
563 }),
564 2,
565 );
566 let id = root.to_string();
567 git_repo
568 .reference(
569 &format!("refs/collab/patches/{}", id),
570 tip,
571 false,
572 "old layout",
573 )
574 .unwrap();
575 drop(git_repo);
576
577 repo.git(&["merge", "--ff-only", "feat"]);
578
579 let json = show_json(&repo, &id[..8]);
580 assert_eq!(
581 json["status"], "merged",
582 "head came from r1, so the base must come from r1 too: {}",
583 json
584 );
585 }
586
341 #[test] 587 #[test]
342 fn each_revision_records_the_base_it_was_written_against() { 588 fn each_revision_records_the_base_it_was_written_against() {
343 let repo = TestRepo::new("Alice", "alice@example.com"); 589 let repo = TestRepo::new("Alice", "alice@example.com");
@@ -473,6 +719,92 @@ fn demote_to_old_layout(repo: &TestRepo, id: &str) {
473 repo.git(&["update-ref", &format!("refs/collab/patches/{}", id), &tip]); 719 repo.git(&["update-ref", &format!("refs/collab/patches/{}", id), &tip]);
474 } 720 }
475 721
722 /// Reproduce the state a migration killed midway leaves behind: the tip parked
723 /// outside the patch namespace, the old ref already deleted, and no events ref
724 /// yet. Nothing enumerates the patch in this state, so only a resume can find
725 /// it again.
726 fn interrupt_migration_after_delete(repo: &TestRepo, id: &str) {
727 let events = format!("refs/collab/patches/{}/events", id);
728 let tip = ref_target(repo, &events).expect("events ref");
729 for name in patch_refs(repo) {
730 if name.starts_with(&format!("refs/collab/patches/{}/", id)) {
731 repo.git(&["update-ref", "-d", &name]);
732 }
733 }
734 repo.git(&[
735 "update-ref",
736 &format!("refs/collab/local/migrating/patches/{}", id),
737 &tip,
738 ]);
739 }
740
741 #[test]
742 fn migration_resumes_after_an_interrupted_run() {
743 // A kill between deleting the old ref and creating the events ref used to
744 // lose the patch from every listing permanently — the objects survived but
745 // nothing could find them. The parked ref only makes that recoverable if
746 // something reads it back.
747 let repo = TestRepo::new("Alice", "alice@example.com");
748 let (short, id, r1) = patch_on_branch(&repo, "feat", "a.txt");
749 repo.commit_file("b.txt", "v2", "second commit");
750 repo.run_ok(&["patch", "revise", &short]);
751 let r2 = repo.git(&["rev-parse", "HEAD"]).trim().to_string();
752
753 interrupt_migration_after_delete(&repo, &id);
754 assert!(
755 patch_refs(&repo)
756 .iter()
757 .all(|r| !r.starts_with(&format!("refs/collab/patches/{}", id))),
758 "precondition: the patch is unreachable from the patch namespace"
759 );
760
761 let out = repo.run_ok(&["patch", "list"]);
762 assert!(out.contains(&short), "the patch must come back: {}", out);
763
764 let refs = patch_refs(&repo);
765 assert!(
766 refs.contains(&format!("refs/collab/patches/{}/events", id)),
767 "{:?}",
768 refs
769 );
770 assert_eq!(
771 ref_target(&repo, &format!("refs/collab/patches/{}/r/1", id)).as_deref(),
772 Some(r1.as_str())
773 );
774 assert_eq!(
775 ref_target(&repo, &format!("refs/collab/patches/{}/r/2", id)).as_deref(),
776 Some(r2.as_str())
777 );
778 assert!(
779 ref_target(&repo, &format!("refs/collab/local/migrating/patches/{}", id)).is_none(),
780 "the park must be cleared once the patch is whole"
781 );
782 }
783
784 #[test]
785 fn migration_clears_a_park_left_by_a_late_interruption() {
786 // The other half of the same window: killed after the events ref was
787 // created but before the park was deleted. The patch is fine; the park is
788 // litter that would otherwise never be collected.
789 let repo = TestRepo::new("Alice", "alice@example.com");
790 let (short, id, _r1) = patch_on_branch(&repo, "feat", "a.txt");
791 let tip = ref_target(&repo, &format!("refs/collab/patches/{}/events", id)).unwrap();
792 repo.git(&[
793 "update-ref",
794 &format!("refs/collab/local/migrating/patches/{}", id),
795 &tip,
796 ]);
797
798 repo.run_ok(&["patch", "list"]);
799
800 assert!(
801 ref_target(&repo, &format!("refs/collab/local/migrating/patches/{}", id)).is_none(),
802 "a park whose patch already has an events ref must be swept"
803 );
804 let json = show_json(&repo, &short);
805 assert_eq!(json["revisions"].as_array().unwrap().len(), 1);
806 }
807
476 #[test] 808 #[test]
477 fn an_old_layout_patch_is_migrated_on_first_use() { 809 fn an_old_layout_patch_is_migrated_on_first_use() {
478 let repo = TestRepo::new("Alice", "alice@example.com"); 810 let repo = TestRepo::new("Alice", "alice@example.com");
tests/sync_test.rs
Old New
@@ -1887,3 +1887,256 @@ fn patch_revisions_reach_a_second_clone_without_pushing_any_branch() {
1887 ); 1887 );
1888 assert_eq!(patch.resolve_head(&bob_repo).unwrap(), feat); 1888 assert_eq!(patch.resolve_head(&bob_repo).unwrap(), feat);
1889 } 1889 }
1890
1891 /// Commit `content` on top of `parent`, on `branch`, in `repo`.
1892 fn commit_on_branch(
1893 repo: &Repository,
1894 branch: &str,
1895 parent: git2::Oid,
1896 file: &str,
1897 content: &[u8],
1898 ) -> git2::Oid {
1899 let parent_commit = repo.find_commit(parent).unwrap();
1900 let sig = git2::Signature::now("Author", "author@example.com").unwrap();
1901 let blob = repo.blob(content).unwrap();
1902 let mut tb = repo
1903 .treebuilder(Some(&parent_commit.tree().unwrap()))
1904 .unwrap();
1905 tb.insert(file, blob, 0o100644).unwrap();
1906 let tree_oid = tb.write().unwrap();
1907 let tree = repo.find_tree(tree_oid).unwrap();
1908 repo.commit(
1909 Some(&format!("refs/heads/{}", branch)),
1910 &sig,
1911 &sig,
1912 file,
1913 &tree,
1914 &[&parent_commit],
1915 )
1916 .unwrap()
1917 }
1918
1919 #[test]
1920 fn sync_migrates_an_old_layout_repo_before_reconciling() {
1921 // sync is precisely the command a contributor runs on a repo they have not
1922 // otherwise touched, so it cannot rely on some earlier read having
1923 // migrated. Left unmigrated, an incoming <id>/events collides with the
1924 // local bare <id> as a directory-vs-file lock conflict that aborts the
1925 // whole patches reconcile, and the bare ref is pushed outbound where a
1926 // migrated remote must reject it for the same reason.
1927 let cluster = TestCluster::new();
1928 let alice_repo = cluster.alice_repo();
1929
1930 let base = make_commit_with_message(&alice_repo, "base");
1931 let feat = commit_on_branch(&alice_repo, "feat", base, "feature.txt", b"work");
1932 cluster.run_collab_ok(
1933 cluster.alice_dir.path(),
1934 &["patch", "create", "-t", "Old layout", "-B", "feat"],
1935 );
1936
1937 // Demote to the pre-revision-refs layout, as a repo last written by an
1938 // older git-collab would be.
1939 let id = {
1940 let patches = state::list_patches(&alice_repo).unwrap();
1941 patches[0].id.clone()
1942 };
1943 let alice_repo = Repository::open(cluster.alice_dir.path()).unwrap();
1944 let events = format!("refs/collab/patches/{}/events", id);
1945 let tip = alice_repo.refname_to_id(&events).unwrap();
1946 for suffix in ["events", "r/1"] {
1947 alice_repo
1948 .find_reference(&format!("refs/collab/patches/{}/{}", id, suffix))
1949 .unwrap()
1950 .delete()
1951 .unwrap();
1952 }
1953 alice_repo
1954 .reference(&format!("refs/collab/patches/{}", id), tip, false, "demote")
1955 .unwrap();
1956
1957 // sync must migrate before it reconciles or pushes.
1958 sync::sync(&alice_repo, "origin").unwrap();
1959
1960 let alice_repo = Repository::open(cluster.alice_dir.path()).unwrap();
1961 assert!(
1962 alice_repo.refname_to_id(&events).is_ok(),
1963 "sync must migrate the local layout"
1964 );
1965 assert_eq!(
1966 alice_repo
1967 .refname_to_id(&format!("refs/collab/patches/{}/r/1", id))
1968 .ok(),
1969 Some(feat)
1970 );
1971
1972 // And the migrated layout is what reached the remote.
1973 let bob_repo = cluster.bob_repo();
1974 sync::sync(&bob_repo, "origin").unwrap();
1975 let bob_repo = Repository::open(cluster.bob_dir.path()).unwrap();
1976 assert_eq!(
1977 bob_repo
1978 .refname_to_id(&format!("refs/collab/patches/{}/r/1", id))
1979 .ok(),
1980 Some(feat)
1981 );
1982 }
1983
1984 #[test]
1985 fn sync_does_not_adopt_a_revision_ref_the_signed_dag_does_not_vouch_for() {
1986 // Revision refs are reachable objects in every clone that syncs. Adopting
1987 // them on the strength of the ref name alone lets anyone with push access
1988 // plant permanent objects — and because nothing enumerates a patch with no
1989 // events ref, `patch delete` cannot resolve them either, so they are also
1990 // unremovable through the CLI.
1991 let cluster = TestCluster::new();
1992 let alice_repo = cluster.alice_repo();
1993
1994 let base = make_commit_with_message(&alice_repo, "base");
1995 let feat = commit_on_branch(&alice_repo, "feat", base, "feature.txt", b"work");
1996 cluster.run_collab_ok(
1997 cluster.alice_dir.path(),
1998 &["patch", "create", "-t", "Real patch", "-B", "feat"],
1999 );
2000 sync::sync(&alice_repo, "origin").unwrap();
2001 let id = {
2002 let repo = Repository::open(cluster.alice_dir.path()).unwrap();
2003 state::list_patches(&repo).unwrap()[0].id.clone()
2004 };
2005
2006 // Plant three shapes directly on the remote: a well-formed id with no
2007 // events ref, a non-decimal revision number, and a revision number on a
2008 // real patch whose commit the signed DAG never mentions.
2009 let planted = commit_on_branch(&alice_repo, "planted", base, "planted.txt", b"payload");
2010 let orphan_id = "0".repeat(40);
2011 Command::new("git")
2012 .args([
2013 "push",
2014 "origin",
2015 &format!("{}:refs/collab/patches/{}/r/1", planted, orphan_id),
2016 &format!("{}:refs/collab/patches/{}/r/notanumber", planted, id),
2017 &format!("{}:refs/collab/patches/{}/r/9", planted, id),
2018 ])
2019 .current_dir(cluster.alice_dir.path())
2020 .status()
2021 .unwrap();
2022
2023 let bob_repo = cluster.bob_repo();
2024 sync::sync(&bob_repo, "origin").unwrap();
2025 let bob_repo = Repository::open(cluster.bob_dir.path()).unwrap();
2026
2027 // The real patch still arrives intact.
2028 assert_eq!(
2029 bob_repo
2030 .refname_to_id(&format!("refs/collab/patches/{}/r/1", id))
2031 .ok(),
2032 Some(feat)
2033 );
2034
2035 for bad in [
2036 format!("refs/collab/patches/{}/r/1", orphan_id),
2037 format!("refs/collab/patches/{}/r/notanumber", id),
2038 format!("refs/collab/patches/{}/r/9", id),
2039 ] {
2040 assert!(
2041 bob_repo.refname_to_id(&bad).is_err(),
2042 "planted ref must not be adopted: {}",
2043 bad
2044 );
2045 }
2046 }
2047
2048 #[test]
2049 fn concurrent_revise_leaves_every_revision_reachable() {
2050 // Revision numbering is derived from the DAG walk while ref names are
2051 // derived at write time, so a concurrent revise can renumber the loser.
2052 // If nothing reconciles the refs against the DAG afterwards, that
2053 // revision's commit ends up referenced by nothing once the sync refs are
2054 // swept — the force-push orphan this design exists to prevent, arriving
2055 // through concurrency instead of rebase.
2056 let cluster = TestCluster::new();
2057 let alice_repo = cluster.alice_repo();
2058
2059 let base = make_commit_with_message(&alice_repo, "base");
2060 let r1 = commit_on_branch(&alice_repo, "feat", base, "feature.txt", b"v1");
2061 cluster.run_collab_ok(
2062 cluster.alice_dir.path(),
2063 &["patch", "create", "-t", "Concurrent", "-B", "feat"],
2064 );
2065 sync::sync(&alice_repo, "origin").unwrap();
2066
2067 let bob_repo = cluster.bob_repo();
2068 sync::sync(&bob_repo, "origin").unwrap();
2069 let bob_repo = Repository::open(cluster.bob_dir.path()).unwrap();
2070 // The cluster's clones have no local refs/heads/main; `revise` needs the
2071 // base branch to compute the revision's base.
2072 let origin_main = bob_repo
2073 .refname_to_id("refs/remotes/origin/main")
2074 .unwrap_or(base);
2075 bob_repo
2076 .reference("refs/heads/main", origin_main, true, "seed local main")
2077 .unwrap();
2078 let id = state::list_patches(&bob_repo).unwrap()[0].id.clone();
2079 let short = &id[..8];
2080
2081 // Both sides revise from the same starting point, without seeing each
2082 // other, and both go through the real command.
2083 let alice_rev = commit_on_branch(&alice_repo, "feat", r1, "alice.txt", b"alice");
2084 cluster.run_collab_ok(
2085 cluster.alice_dir.path(),
2086 &["patch", "revise", short, "-B", "feat"],
2087 );
2088 let bob_rev = commit_on_branch(&bob_repo, "feat", r1, "bob.txt", b"bob");
2089 cluster.run_collab_ok(
2090 cluster.bob_dir.path(),
2091 &["patch", "revise", short, "-B", "feat"],
2092 );
2093
2094 sync::sync(&Repository::open(cluster.alice_dir.path()).unwrap(), "origin").unwrap();
2095 sync::sync(&Repository::open(cluster.bob_dir.path()).unwrap(), "origin").unwrap();
2096 sync::sync(&Repository::open(cluster.alice_dir.path()).unwrap(), "origin").unwrap();
2097
2098 for (label, dir) in [
2099 ("alice", cluster.alice_dir.path()),
2100 ("bob", cluster.bob_dir.path()),
2101 ] {
2102 let repo = Repository::open(dir).unwrap();
2103 let patch = state::list_patches(&repo)
2104 .unwrap()
2105 .into_iter()
2106 .find(|p| p.id == id)
2107 .unwrap();
2108 assert_eq!(
2109 patch.revisions.len(),
2110 3,
2111 "{}: both revisions should have merged into the DAG",
2112 label
2113 );
2114
2115 // Every revision the DAG lists must be reachable from a ref in this
2116 // repo, or the sync-ref sweep has just made it gc-eligible.
2117 let referenced: Vec<git2::Oid> = repo
2118 .references_glob(&format!("refs/collab/patches/{}/r/*", id))
2119 .unwrap()
2120 .filter_map(|r| r.ok()?.target())
2121 .collect();
2122 for rev in &patch.revisions {
2123 let oid = git2::Oid::from_str(&rev.commit).unwrap();
2124 assert!(
2125 referenced.contains(&oid),
2126 "{}: revision {} ({}) is referenced by no ref; refs hold {:?}",
2127 label,
2128 rev.number,
2129 rev.commit,
2130 referenced
2131 );
2132 }
2133 for oid in [alice_rev, bob_rev] {
2134 assert!(
2135 repo.find_commit(oid).is_ok(),
2136 "{}: objects for {} are gone",
2137 label,
2138 oid
2139 );
2140 }
2141 }
2142 }