test_mwp7_sibling_negotiation.py
python
sha256:c0eba5ad689cec79f4a3fcdc4f5da78556cb4b8cb7b330f944634356c10379ed
chore: pivot to nightly channel — bump version to 0.2.0.dev…
Sonnet 5
patch
13 days ago
| 1 | """TDD — Phase 1: Ground truth for #55 sibling-ref push negotiation. |
| 2 | |
| 3 | Issue: https://staging.musehub.ai/gabriel/muse/issues/55 |
| 4 | Tracker: https://staging.musehub.ai/gabriel/muse/issues/62 |
| 5 | |
| 6 | Scenario under test (the #55 shape): |
| 7 | remote has: dev → D (a linear chain root..D, full manifests on remote) |
| 8 | local has: dev → D AND main → M, where M = merge commit with parent D |
| 9 | push main → remote_branch_heads = {"dev": D} (main absent on remote) |
| 10 | expected: commits_sent == 1 (just M), objects_sent ≈ 0 (only M's net-new blobs) |
| 11 | |
| 12 | Test IDs |
| 13 | -------- |
| 14 | GT_01 helper builders (_chain, _merge_commit) |
| 15 | GT_02 commit-boundary truth — walk_commits stops at sibling tip D |
| 16 | GT_03 blob-dedup truth — all_blob_ids contains only M's net-new blob(s) |
| 17 | GT_04 run()-level live path truth — branch_have from all remote heads; commits_sent==1 |
| 18 | GT_05 diverged / force edge — main present at old diverged tip M_old on remote |
| 19 | |
| 20 | GROUND TRUTH VERDICT: CORRECT — Phase 2 Mode A applies. |
| 21 | |
| 22 | All GT_02..GT_05 tests pass (9/9 green, 0.53 s, 2026-06-29). |
| 23 | |
| 24 | GT_02 PASS — walk_commits(root, [M], have=[D])["commits"] == 1 ✓ |
| 25 | GT_03 PASS — all_blob_ids contains only M's net-new blob; D's manifest excluded ✓ |
| 26 | GT_04 PASS — run() builds branch_have from all remote heads; D included; commits_sent==1 ✓ |
| 27 | GT_05 PASS — diverged M_old case: branch_have=[D, M_old]; commits_sent==1 ✓ |
| 28 | |
| 29 | Conclusion: the live push path (push.py:819-822) correctly uses ALL remote branch |
| 30 | heads as the BFS boundary. The RC-7 claim is empirically verified. #55 is |
| 31 | closed at the logic level — staging evidence (Phase 3) is still required to |
| 32 | formally close the issue. Phase 2 Mode A: promote GT tests to regression |
| 33 | assertions; run adjacent push corpus (LF_01..LF_03). |
| 34 | """ |
| 35 | from __future__ import annotations |
| 36 | |
| 37 | import argparse |
| 38 | import datetime |
| 39 | import io |
| 40 | import json |
| 41 | import pathlib |
| 42 | from contextlib import redirect_stdout |
| 43 | from typing import Any |
| 44 | from unittest.mock import MagicMock, patch |
| 45 | |
| 46 | import pytest |
| 47 | |
| 48 | from muse.core.commits import read_commit, write_commit, CommitRecord |
| 49 | from muse.core.mpack import walk_commits, collect_blob_ids |
| 50 | from muse.core.object_store import write_object |
| 51 | from muse.core.paths import init_repo_dirs as init_repo |
| 52 | from muse.core.refs import write_branch_ref |
| 53 | from muse.core.snapshot import compute_commit_id, compute_snapshot_id |
| 54 | from muse.core.snapshots import read_snapshot, write_snapshot, SnapshotRecord |
| 55 | from muse.core.types import Manifest, blob_id |
| 56 | |
| 57 | pytestmark = pytest.mark.wire |
| 58 | |
| 59 | _TS = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) |
| 60 | |
| 61 | |
| 62 | # --------------------------------------------------------------------------- |
| 63 | # GT_01 — Helper builders |
| 64 | # |
| 65 | # Reuses patterns from: |
| 66 | # tests/test_push_branch_have.py::_commit / _build_linear_chain |
| 67 | # tests/test_push_have_filter.py::_make_commit |
| 68 | # --------------------------------------------------------------------------- |
| 69 | |
| 70 | def _obj(root: pathlib.Path, content: bytes) -> str: |
| 71 | """Write a blob to the object store and return its ID.""" |
| 72 | oid = blob_id(content) |
| 73 | write_object(root, oid, content) |
| 74 | return oid |
| 75 | |
| 76 | |
| 77 | def _snap(root: pathlib.Path, manifest: Manifest) -> str: |
| 78 | """Write a snapshot for *manifest* and return its ID.""" |
| 79 | sid = compute_snapshot_id(manifest) |
| 80 | write_snapshot(root, SnapshotRecord(snapshot_id=sid, manifest=manifest)) |
| 81 | return sid |
| 82 | |
| 83 | |
| 84 | def _chain(root: pathlib.Path, n: int) -> list[str]: |
| 85 | """Build a linear commit chain of *n* commits; return IDs oldest first. |
| 86 | |
| 87 | Each commit adds exactly one unique file (file0.txt … fileN-1.txt) so |
| 88 | every commit contributes one new blob and the full chain has n blobs. |
| 89 | """ |
| 90 | commits: list[str] = [] |
| 91 | manifest: dict[str, str] = {} |
| 92 | prev: str | None = None |
| 93 | for i in range(n): |
| 94 | content = f"chain-file-{i}".encode() |
| 95 | oid = _obj(root, content) |
| 96 | manifest = {**manifest, f"file{i}.txt": oid} |
| 97 | sid = _snap(root, manifest) |
| 98 | cid = compute_commit_id( |
| 99 | parent_ids=[prev] if prev else [], |
| 100 | snapshot_id=sid, |
| 101 | message=f"commit {i}", |
| 102 | committed_at_iso=_TS.isoformat(), |
| 103 | author="test", |
| 104 | ) |
| 105 | write_commit(root, CommitRecord( |
| 106 | commit_id=cid, |
| 107 | branch="dev", |
| 108 | snapshot_id=sid, |
| 109 | message=f"commit {i}", |
| 110 | committed_at=_TS, |
| 111 | parent_commit_id=prev, |
| 112 | author="test", |
| 113 | )) |
| 114 | commits.append(cid) |
| 115 | prev = cid |
| 116 | return commits |
| 117 | |
| 118 | |
| 119 | def _merge_commit( |
| 120 | root: pathlib.Path, |
| 121 | parent_id: str, |
| 122 | content: bytes = b"merge-file-content", |
| 123 | branch: str = "main", |
| 124 | ) -> str: |
| 125 | """Create a commit whose manifest = parent's manifest + 1 new file. |
| 126 | |
| 127 | Models the merge-commit shape from #55: main advances by exactly one |
| 128 | commit on top of dev's tip, adding one new blob. Everything else in the |
| 129 | manifest is shared with the parent, so the blob dedup in walk_commits |
| 130 | will correctly exclude all pre-existing blobs when have=[parent]. |
| 131 | """ |
| 132 | parent_commit = read_commit(root, parent_id) |
| 133 | assert parent_commit is not None, f"parent commit {parent_id[:16]} not found" |
| 134 | parent_snap = read_snapshot(root, parent_commit.snapshot_id) |
| 135 | assert parent_snap is not None, f"parent snapshot not found for {parent_id[:16]}" |
| 136 | |
| 137 | new_oid = _obj(root, content) |
| 138 | manifest: dict[str, str] = {**parent_snap.manifest, "merge-file.txt": new_oid} |
| 139 | sid = _snap(root, manifest) |
| 140 | |
| 141 | cid = compute_commit_id( |
| 142 | parent_ids=[parent_id], |
| 143 | snapshot_id=sid, |
| 144 | message=f"merge: {branch} from dev", |
| 145 | committed_at_iso=_TS.isoformat(), |
| 146 | author="test", |
| 147 | ) |
| 148 | write_commit(root, CommitRecord( |
| 149 | commit_id=cid, |
| 150 | branch=branch, |
| 151 | snapshot_id=sid, |
| 152 | message=f"merge: {branch} from dev", |
| 153 | committed_at=_TS, |
| 154 | parent_commit_id=parent_id, |
| 155 | author="test", |
| 156 | )) |
| 157 | return cid |
| 158 | |
| 159 | |
| 160 | # --------------------------------------------------------------------------- |
| 161 | # GT_02 — Commit-boundary truth |
| 162 | # --------------------------------------------------------------------------- |
| 163 | |
| 164 | class TestGT02CommitBoundary: |
| 165 | """walk_commits(root, [M], have=[D]) yields exactly one commit: M.""" |
| 166 | |
| 167 | def test_GT02_walk_stops_at_sibling_tip(self, tmp_path: pathlib.Path) -> None: |
| 168 | """Assert len(walk_commits(root, [M], have=[D])["commits"]) == 1. |
| 169 | |
| 170 | Proves the BFS commit walk stops the moment it reaches the sibling |
| 171 | branch tip D, leaving only the merge commit M in commits_to_send. |
| 172 | This is the core invariant for the #55 scenario: the remote already |
| 173 | holds every commit reachable from D (via the dev push), so the push |
| 174 | of main should send only M. |
| 175 | |
| 176 | Mirrors test_push_branch_have.py::test_BH2 but framed for #55's |
| 177 | merge-commit shape rather than a linear second-branch push. |
| 178 | """ |
| 179 | init_repo(tmp_path) |
| 180 | commits = _chain(tmp_path, 4) # root, C1, C2, D |
| 181 | D = commits[-1] |
| 182 | M = _merge_commit(tmp_path, D) |
| 183 | |
| 184 | result = walk_commits(tmp_path, [M], have=[D]) |
| 185 | sent_ids = [c.commit_id for c in result["commits"]] |
| 186 | |
| 187 | assert len(sent_ids) == 1, ( |
| 188 | f"Expected commits_sent=1 (only M); got {len(sent_ids)}: " |
| 189 | f"{[cid[:16] for cid in sent_ids]}" |
| 190 | ) |
| 191 | assert M in sent_ids, "M (the merge commit) must be the one commit sent" |
| 192 | assert D not in sent_ids, ( |
| 193 | "D must NOT be sent — it is already on the remote under dev" |
| 194 | ) |
| 195 | |
| 196 | def test_GT02_empty_have_sends_full_chain(self, tmp_path: pathlib.Path) -> None: |
| 197 | """Baseline: have=[] causes walk to return all commits (no pruning).""" |
| 198 | init_repo(tmp_path) |
| 199 | commits = _chain(tmp_path, 4) |
| 200 | D = commits[-1] |
| 201 | M = _merge_commit(tmp_path, D) |
| 202 | |
| 203 | result = walk_commits(tmp_path, [M], have=[]) |
| 204 | # M + the 4 chain commits |
| 205 | assert len(result["commits"]) == 5, ( |
| 206 | f"With have=[], all 5 commits should be walked; got {len(result['commits'])}" |
| 207 | ) |
| 208 | |
| 209 | |
| 210 | # --------------------------------------------------------------------------- |
| 211 | # GT_03 — Blob-dedup truth |
| 212 | # --------------------------------------------------------------------------- |
| 213 | |
| 214 | class TestGT03BlobDedup: |
| 215 | """walk_commits blob subtraction correctly excludes all blobs from D's manifest.""" |
| 216 | |
| 217 | def test_GT03_only_merge_blob_in_all_blob_ids(self, tmp_path: pathlib.Path) -> None: |
| 218 | """all_blob_ids contains only M's net-new blob; none of D's manifest blobs. |
| 219 | |
| 220 | Directly exercises have_blobs subtraction at mpack.py:330-362. |
| 221 | The flat-manifest model means D's snapshot lists every file in the |
| 222 | entire repo at that point. Subtracting D's have_blobs from |
| 223 | manifest_blobs leaves only the one blob M adds — merge-file.txt. |
| 224 | """ |
| 225 | init_repo(tmp_path) |
| 226 | commits = _chain(tmp_path, 4) |
| 227 | D = commits[-1] |
| 228 | merge_content = b"unique-merge-file-content-for-GT03" |
| 229 | M = _merge_commit(tmp_path, D, content=merge_content) |
| 230 | |
| 231 | result = walk_commits(tmp_path, [M], have=[D]) |
| 232 | all_blob_ids: list[str] = result["all_blob_ids"] |
| 233 | |
| 234 | # M's new blob must be present. |
| 235 | expected_oid = blob_id(merge_content) |
| 236 | assert expected_oid in all_blob_ids, ( |
| 237 | f"M's new blob ({expected_oid[:16]}) must appear in all_blob_ids" |
| 238 | ) |
| 239 | |
| 240 | # No blob from D's manifest should appear — they are all in have_blobs. |
| 241 | d_commit = read_commit(tmp_path, D) |
| 242 | assert d_commit is not None |
| 243 | d_snap = read_snapshot(tmp_path, d_commit.snapshot_id) |
| 244 | assert d_snap is not None |
| 245 | d_blobs = set(d_snap.manifest.values()) |
| 246 | |
| 247 | stale_blobs = d_blobs & set(all_blob_ids) |
| 248 | assert not stale_blobs, ( |
| 249 | "Blobs from D's manifest must NOT appear in all_blob_ids — they are " |
| 250 | "in have_blobs and must be excluded from the push delta. " |
| 251 | f"Leaking: {[b[:16] for b in stale_blobs]}" |
| 252 | ) |
| 253 | |
| 254 | def test_GT03_have_blobs_covers_entire_chain(self, tmp_path: pathlib.Path) -> None: |
| 255 | """have_blobs includes every blob reachable from D's snapshot manifest. |
| 256 | |
| 257 | Because Muse snapshots are flat full-repo manifests, D's snapshot |
| 258 | manifest lists every file from root through D. Subtracting it from |
| 259 | manifest_blobs leaves only M's additions. |
| 260 | """ |
| 261 | init_repo(tmp_path) |
| 262 | commits = _chain(tmp_path, 4) |
| 263 | D = commits[-1] |
| 264 | M = _merge_commit(tmp_path, D) |
| 265 | |
| 266 | result = walk_commits(tmp_path, [M], have=[D]) |
| 267 | have_blobs: set[str] = result["have_blobs"] |
| 268 | |
| 269 | d_commit = read_commit(tmp_path, D) |
| 270 | assert d_commit is not None |
| 271 | d_snap = read_snapshot(tmp_path, d_commit.snapshot_id) |
| 272 | assert d_snap is not None |
| 273 | |
| 274 | for path, oid in d_snap.manifest.items(): |
| 275 | assert oid in have_blobs, ( |
| 276 | f"Blob {oid[:16]} for path {path!r} (from D's manifest) " |
| 277 | f"must be in have_blobs — the flat-manifest model guarantees this" |
| 278 | ) |
| 279 | |
| 280 | def test_GT03_only_one_new_blob_in_merge_case(self, tmp_path: pathlib.Path) -> None: |
| 281 | """all_blob_ids has exactly 1 entry: M's single net-new blob.""" |
| 282 | init_repo(tmp_path) |
| 283 | commits = _chain(tmp_path, 4) |
| 284 | D = commits[-1] |
| 285 | M = _merge_commit(tmp_path, D, content=b"just-one-new-file") |
| 286 | |
| 287 | result = walk_commits(tmp_path, [M], have=[D]) |
| 288 | assert len(result["all_blob_ids"]) == 1, ( |
| 289 | f"Exactly 1 new blob (merge-file.txt); got {len(result['all_blob_ids'])}" |
| 290 | ) |
| 291 | |
| 292 | |
| 293 | # --------------------------------------------------------------------------- |
| 294 | # GT_04 — run()-level live path truth |
| 295 | # --------------------------------------------------------------------------- |
| 296 | |
| 297 | def _make_push_ns( |
| 298 | remote: str = "staging", |
| 299 | branch: str = "main", |
| 300 | force: bool = False, |
| 301 | dry_run: bool = False, |
| 302 | json_out: bool = False, |
| 303 | ) -> argparse.Namespace: |
| 304 | """Build an argparse.Namespace for push.run() with sensible defaults.""" |
| 305 | return argparse.Namespace( |
| 306 | remote=remote, |
| 307 | branch_pos=branch, |
| 308 | branch_flag=None, |
| 309 | force=force, |
| 310 | force_with_lease=False, |
| 311 | delete_branch=False, |
| 312 | dry_run=dry_run, |
| 313 | json_out=json_out, |
| 314 | set_upstream_flag=False, |
| 315 | workers=4, |
| 316 | hub=None, |
| 317 | ) |
| 318 | |
| 319 | |
| 320 | def _write_tracking_ref(root: pathlib.Path, remote: str, branch: str, commit_id: str) -> None: |
| 321 | """Write a local tracking ref to simulate a previous push of remote/branch. |
| 322 | |
| 323 | The file at .muse/remotes/<remote>/<branch> is what _all_known_have_anchors |
| 324 | and get_remote_head read. Writing it simulates "this branch was pushed |
| 325 | to this remote at this commit" without doing a real push. |
| 326 | """ |
| 327 | ref_file = root / ".muse" / "remotes" / remote / branch |
| 328 | ref_file.parent.mkdir(parents=True, exist_ok=True) |
| 329 | ref_file.write_text(commit_id) |
| 330 | |
| 331 | |
| 332 | def _run_dry_push( |
| 333 | tmp_path: pathlib.Path, |
| 334 | local_head: str, |
| 335 | remote: str = "staging", |
| 336 | ) -> dict[str, Any]: |
| 337 | """Invoke push.run() in dry-run JSON mode; return parsed JSON from stdout. |
| 338 | |
| 339 | Does NOT mock _push_mpack (dry-run never reaches it). |
| 340 | Tracking refs are read from disk — write them with _write_tracking_ref first. |
| 341 | """ |
| 342 | out = io.StringIO() |
| 343 | ns = _make_push_ns(remote=remote, dry_run=True, json_out=True) |
| 344 | with ( |
| 345 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 346 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 347 | patch("muse.cli.commands.push.get_head_commit_id", return_value=local_head), |
| 348 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 349 | redirect_stdout(out), |
| 350 | ): |
| 351 | from muse.cli.commands.push import run |
| 352 | run(ns) |
| 353 | return json.loads(out.getvalue()) |
| 354 | |
| 355 | |
| 356 | def _spy_push_mpack( |
| 357 | captured: list[dict[str, Any]], |
| 358 | ) -> Any: |
| 359 | """Return a spy function for _push_mpack that captures branch_have, commits_sent, objects_sent.""" |
| 360 | def _spy( |
| 361 | transport: Any, |
| 362 | url: str, |
| 363 | signing: Any, |
| 364 | root: pathlib.Path, |
| 365 | local_head: str, |
| 366 | have: list[str], |
| 367 | branch: str, |
| 368 | force: bool, |
| 369 | branch_have: list[str] | None = None, |
| 370 | ) -> None: |
| 371 | # Reproduce the exact calculation _push_mpack performs, without network I/O. |
| 372 | walk = walk_commits(root, [local_head], have=branch_have or []) |
| 373 | captured.append({ |
| 374 | "branch_have": list(branch_have or []), |
| 375 | "have": list(have), |
| 376 | "commits_sent": len(walk["commits"]), |
| 377 | "objects_sent": len(walk["all_blob_ids"]), |
| 378 | }) |
| 379 | raise SystemExit(0) # abort before any network I/O |
| 380 | return _spy |
| 381 | |
| 382 | |
| 383 | class TestGT04LivePathTruth: |
| 384 | """push.py::run() builds branch_have from ALL remote heads; commits_sent==1 for #55 shape.""" |
| 385 | |
| 386 | def test_GT04_run_uses_sibling_dev_as_commit_boundary( |
| 387 | self, tmp_path: pathlib.Path |
| 388 | ) -> None: |
| 389 | """In-process proxy for the staging #55 scenario. |
| 390 | |
| 391 | Transport returns branch_heads={"dev": D} — main is absent on the remote. |
| 392 | Spy on _push_mpack to capture (branch_have, commits_sent) that run() |
| 393 | computes. Asserts: |
| 394 | - branch_have contains D (dev's remote tip used as BFS boundary) |
| 395 | - commits_sent == 1 (only M — the merge commit — is new) |
| 396 | |
| 397 | If commits_sent > 1 here, the live path (#55 claim) is NOT correct and |
| 398 | Phase 2 Mode B must fix push.py:819-822. |
| 399 | """ |
| 400 | init_repo(tmp_path) |
| 401 | commits = _chain(tmp_path, 4) |
| 402 | D = commits[-1] |
| 403 | M = _merge_commit(tmp_path, D) |
| 404 | write_branch_ref(tmp_path, "dev", D) |
| 405 | write_branch_ref(tmp_path, "main", M) |
| 406 | |
| 407 | captured: list[dict[str, Any]] = [] |
| 408 | mock_transport = MagicMock() |
| 409 | mock_transport.fetch_remote_info.return_value = { |
| 410 | "branch_heads": {"dev": D}, |
| 411 | "domain": "code", |
| 412 | "default_branch": "dev", |
| 413 | } |
| 414 | |
| 415 | with ( |
| 416 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 417 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 418 | patch("muse.cli.commands.push.get_head_commit_id", return_value=M), |
| 419 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 420 | patch("muse.cli.commands.push.get_signing_identity", return_value=None), |
| 421 | patch("muse.cli.commands.push.make_transport", return_value=mock_transport), |
| 422 | patch("muse.cli.commands.push._push_mpack", _spy_push_mpack(captured)), |
| 423 | ): |
| 424 | from muse.cli.commands.push import run |
| 425 | try: |
| 426 | run(_make_push_ns()) |
| 427 | except SystemExit: |
| 428 | pass |
| 429 | |
| 430 | assert captured, ( |
| 431 | "_push_mpack spy was never called — run() aborted before reaching _push_mpack. " |
| 432 | "Check mock setup or whether there is a new early-exit path." |
| 433 | ) |
| 434 | c = captured[0] |
| 435 | |
| 436 | # branch_have must contain D (dev's remote tip). |
| 437 | assert D in c["branch_have"], ( |
| 438 | f"branch_have must include D (dev's remote tip) so the BFS stops there. " |
| 439 | f"Got: {[h[:16] for h in c['branch_have']]}. " |
| 440 | f"If D is absent, run() is NOT using all remote heads as the walk boundary." |
| 441 | ) |
| 442 | |
| 443 | # commits_sent must be 1 — only M. |
| 444 | assert c["commits_sent"] == 1, ( |
| 445 | f"Expected commits_sent=1 (only M, the merge commit). " |
| 446 | f"Got {c['commits_sent']}. " |
| 447 | f"If > 1, the #55 bug is present in the live path: run() is walking back " |
| 448 | f"into commits already on the remote via dev, re-sending their full history." |
| 449 | ) |
| 450 | |
| 451 | def test_GT04_branch_have_excludes_local_head( |
| 452 | self, tmp_path: pathlib.Path |
| 453 | ) -> None: |
| 454 | """branch_have must not contain local_head (M), only remote heads. |
| 455 | |
| 456 | Including local_head would cause walk_commits to prune immediately and |
| 457 | report commits_sent=0, giving a false 'up_to_date' result. |
| 458 | """ |
| 459 | init_repo(tmp_path) |
| 460 | commits = _chain(tmp_path, 3) |
| 461 | D = commits[-1] |
| 462 | M = _merge_commit(tmp_path, D) |
| 463 | write_branch_ref(tmp_path, "dev", D) |
| 464 | write_branch_ref(tmp_path, "main", M) |
| 465 | |
| 466 | captured: list[dict[str, Any]] = [] |
| 467 | mock_transport = MagicMock() |
| 468 | mock_transport.fetch_remote_info.return_value = { |
| 469 | "branch_heads": {"dev": D}, |
| 470 | "domain": "code", |
| 471 | "default_branch": "dev", |
| 472 | } |
| 473 | |
| 474 | with ( |
| 475 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 476 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 477 | patch("muse.cli.commands.push.get_head_commit_id", return_value=M), |
| 478 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 479 | patch("muse.cli.commands.push.get_signing_identity", return_value=None), |
| 480 | patch("muse.cli.commands.push.make_transport", return_value=mock_transport), |
| 481 | patch("muse.cli.commands.push._push_mpack", _spy_push_mpack(captured)), |
| 482 | ): |
| 483 | from muse.cli.commands.push import run |
| 484 | try: |
| 485 | run(_make_push_ns()) |
| 486 | except SystemExit: |
| 487 | pass |
| 488 | |
| 489 | assert captured |
| 490 | assert M not in captured[0]["branch_have"], ( |
| 491 | "M (local_head) must not appear in branch_have — " |
| 492 | "including it would prune the walk at M itself and report commits_sent=0" |
| 493 | ) |
| 494 | |
| 495 | |
| 496 | # --------------------------------------------------------------------------- |
| 497 | # GT_05 — Diverged / force edge |
| 498 | # --------------------------------------------------------------------------- |
| 499 | |
| 500 | class TestGT05DivergedEdge: |
| 501 | """Remote has main at M_old (an older, diverged tip not on M's ancestry). |
| 502 | |
| 503 | This is the most likely place a residual #55 bug hides: the up-to-date |
| 504 | short-circuit (push.py:789) and the remote_head / have split (push.py:749-757). |
| 505 | |
| 506 | With branch_have=[D, M_old] and M's parent = D: |
| 507 | - walk_commits(root, [M], have=[D, M_old]) prunes at D → commits_sent=1 |
| 508 | - If the code wrongly uses branch_have=[M_old] only: |
| 509 | walk does not stop at D → walks back through D, C, B → over-counts |
| 510 | """ |
| 511 | |
| 512 | def test_GT05_diverged_remote_main_sends_only_merge_commit( |
| 513 | self, tmp_path: pathlib.Path |
| 514 | ) -> None: |
| 515 | """Push main when remote/main is at a diverged old tip M_old. |
| 516 | |
| 517 | branch_have must include BOTH D (dev's tip) AND M_old (main's old tip) |
| 518 | so the BFS stops at D, yielding commits_sent=1. |
| 519 | """ |
| 520 | init_repo(tmp_path) |
| 521 | # Chain: commits[0]=root, commits[1]=A, commits[2]=B, commits[3]=C, commits[4]=D |
| 522 | commits = _chain(tmp_path, 5) |
| 523 | D = commits[-1] |
| 524 | B = commits[2] |
| 525 | |
| 526 | # M = merge commit on top of D (the correct local main) |
| 527 | M = _merge_commit(tmp_path, D, content=b"new-merge-content-GT05") |
| 528 | # M_old = an old merge commit branching from B (diverged from D) |
| 529 | M_old = _merge_commit(tmp_path, B, content=b"old-merge-content-GT05", branch="main") |
| 530 | |
| 531 | write_branch_ref(tmp_path, "dev", D) |
| 532 | write_branch_ref(tmp_path, "main", M) |
| 533 | |
| 534 | captured: list[dict[str, Any]] = [] |
| 535 | mock_transport = MagicMock() |
| 536 | # Remote knows both branches: dev at D, main at M_old (the old diverged tip). |
| 537 | mock_transport.fetch_remote_info.return_value = { |
| 538 | "branch_heads": {"dev": D, "main": M_old}, |
| 539 | "domain": "code", |
| 540 | "default_branch": "dev", |
| 541 | } |
| 542 | |
| 543 | with ( |
| 544 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 545 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 546 | patch("muse.cli.commands.push.get_head_commit_id", return_value=M), |
| 547 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 548 | patch("muse.cli.commands.push.get_signing_identity", return_value=None), |
| 549 | patch("muse.cli.commands.push.make_transport", return_value=mock_transport), |
| 550 | patch("muse.cli.commands.push._push_mpack", _spy_push_mpack(captured)), |
| 551 | ): |
| 552 | from muse.cli.commands.push import run |
| 553 | # force=True: M and M_old are diverged; without force the push would be rejected |
| 554 | try: |
| 555 | run(_make_push_ns(force=True)) |
| 556 | except SystemExit: |
| 557 | pass |
| 558 | |
| 559 | assert captured, ( |
| 560 | "_push_mpack spy was never called. " |
| 561 | "Check whether force=True triggers an unexpected early-exit path." |
| 562 | ) |
| 563 | c = captured[0] |
| 564 | |
| 565 | # Both remote heads must be in branch_have. |
| 566 | assert D in c["branch_have"], ( |
| 567 | f"D (dev's remote tip) must be in branch_have. " |
| 568 | f"Got: {[h[:16] for h in c['branch_have']]}" |
| 569 | ) |
| 570 | assert M_old in c["branch_have"], ( |
| 571 | f"M_old (main's old remote tip) must be in branch_have. " |
| 572 | f"Got: {[h[:16] for h in c['branch_have']]}" |
| 573 | ) |
| 574 | |
| 575 | # With branch_have=[D, M_old] and M's parent = D: |
| 576 | # walk_commits prunes at D → only M is sent. |
| 577 | assert c["commits_sent"] == 1, ( |
| 578 | f"Expected commits_sent=1: M's parent D is in branch_have so walk stops there. " |
| 579 | f"Got {c['commits_sent']}. " |
| 580 | f"If > 1, there is a residual #55 bug in the diverged-tip handling path: " |
| 581 | f"branch_have is not including all remote heads as the BFS boundary." |
| 582 | ) |
| 583 | |
| 584 | def test_GT05_main_absent_on_remote_also_sends_one( |
| 585 | self, tmp_path: pathlib.Path |
| 586 | ) -> None: |
| 587 | """Control: main absent on remote (original #55 shape) → commits_sent=1. |
| 588 | |
| 589 | Repeats GT_04 as a cross-check under GT_05 naming to make the |
| 590 | GT_05 diverged vs GT_04 absent comparison explicit. |
| 591 | """ |
| 592 | init_repo(tmp_path) |
| 593 | commits = _chain(tmp_path, 4) |
| 594 | D = commits[-1] |
| 595 | M = _merge_commit(tmp_path, D, content=b"GT05-control-content") |
| 596 | write_branch_ref(tmp_path, "dev", D) |
| 597 | write_branch_ref(tmp_path, "main", M) |
| 598 | |
| 599 | captured: list[dict[str, Any]] = [] |
| 600 | mock_transport = MagicMock() |
| 601 | mock_transport.fetch_remote_info.return_value = { |
| 602 | "branch_heads": {"dev": D}, # main absent |
| 603 | "domain": "code", |
| 604 | "default_branch": "dev", |
| 605 | } |
| 606 | |
| 607 | with ( |
| 608 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 609 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 610 | patch("muse.cli.commands.push.get_head_commit_id", return_value=M), |
| 611 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 612 | patch("muse.cli.commands.push.get_signing_identity", return_value=None), |
| 613 | patch("muse.cli.commands.push.make_transport", return_value=mock_transport), |
| 614 | patch("muse.cli.commands.push._push_mpack", _spy_push_mpack(captured)), |
| 615 | ): |
| 616 | from muse.cli.commands.push import run |
| 617 | try: |
| 618 | run(_make_push_ns()) |
| 619 | except SystemExit: |
| 620 | pass |
| 621 | |
| 622 | assert captured |
| 623 | assert c["commits_sent"] == 1 if (c := captured[0]) else True, ( |
| 624 | f"commits_sent should be 1 (only M); got {captured[0]['commits_sent']}" |
| 625 | ) |
| 626 | |
| 627 | |
| 628 | # --------------------------------------------------------------------------- |
| 629 | # Helpers for Phase 2 (LF_01, LF_02) |
| 630 | # --------------------------------------------------------------------------- |
| 631 | |
| 632 | def _merge_commit_pure( |
| 633 | root: pathlib.Path, |
| 634 | parent_id: str, |
| 635 | branch: str = "main", |
| 636 | ) -> str: |
| 637 | """Create a merge commit with the SAME manifest as the parent — zero new blobs. |
| 638 | |
| 639 | This models the common case where merging dev into main when main was |
| 640 | directly behind dev produces a commit whose snapshot is identical to dev's |
| 641 | tip. objects_sent for this commit should be 0 (every blob is already on |
| 642 | the remote via dev). |
| 643 | """ |
| 644 | parent_commit = read_commit(root, parent_id) |
| 645 | assert parent_commit is not None, f"parent commit {parent_id[:16]} not found" |
| 646 | # Reuse the parent's snapshot_id — same manifest, same objects. |
| 647 | sid = parent_commit.snapshot_id |
| 648 | |
| 649 | cid = compute_commit_id( |
| 650 | parent_ids=[parent_id], |
| 651 | snapshot_id=sid, |
| 652 | message=f"merge: {branch} from dev (pure — no new files)", |
| 653 | committed_at_iso=_TS.isoformat(), |
| 654 | author="test", |
| 655 | ) |
| 656 | write_commit(root, CommitRecord( |
| 657 | commit_id=cid, |
| 658 | branch=branch, |
| 659 | snapshot_id=sid, |
| 660 | message=f"merge: {branch} from dev (pure — no new files)", |
| 661 | committed_at=_TS, |
| 662 | parent_commit_id=parent_id, |
| 663 | author="test", |
| 664 | )) |
| 665 | return cid |
| 666 | |
| 667 | |
| 668 | def _run_push_spy( |
| 669 | tmp_path: pathlib.Path, |
| 670 | local_head: str, |
| 671 | remote_branch_heads: dict[str, str], |
| 672 | force: bool = False, |
| 673 | ) -> dict[str, Any]: |
| 674 | """Invoke push.run() with a spy on _push_mpack; return the captured result dict.""" |
| 675 | captured: list[dict[str, Any]] = [] |
| 676 | mock_transport = MagicMock() |
| 677 | mock_transport.fetch_remote_info.return_value = { |
| 678 | "branch_heads": remote_branch_heads, |
| 679 | "domain": "code", |
| 680 | "default_branch": next(iter(remote_branch_heads), "dev"), |
| 681 | } |
| 682 | with ( |
| 683 | patch("muse.cli.commands.push.require_repo", return_value=tmp_path), |
| 684 | patch("muse.cli.commands.push.read_current_branch", return_value="main"), |
| 685 | patch("muse.cli.commands.push.get_head_commit_id", return_value=local_head), |
| 686 | patch("muse.cli.commands.push.get_remote", return_value="https://staging.musehub.ai/repo"), |
| 687 | patch("muse.cli.commands.push.get_signing_identity", return_value=None), |
| 688 | patch("muse.cli.commands.push.make_transport", return_value=mock_transport), |
| 689 | patch("muse.cli.commands.push._push_mpack", _spy_push_mpack(captured)), |
| 690 | ): |
| 691 | from muse.cli.commands.push import run |
| 692 | try: |
| 693 | run(_make_push_ns(force=force)) |
| 694 | except SystemExit: |
| 695 | pass |
| 696 | assert captured, "spy was never called — check mock setup" |
| 697 | return captured[0] |
| 698 | |
| 699 | |
| 700 | # --------------------------------------------------------------------------- |
| 701 | # LF_01 — Permanent regression assertion for #55 |
| 702 | # --------------------------------------------------------------------------- |
| 703 | |
| 704 | class TestLF01RegressionAssertion: |
| 705 | """Permanent contract: pushing main after dev sends ONLY the merge commit. |
| 706 | |
| 707 | This test was promoted from GT_04 and is the named regression anchor for |
| 708 | muse#55 (sibling-ref push negotiation). Its presence in the wire suite |
| 709 | ensures the fix can never silently regress. |
| 710 | |
| 711 | Before the fix: commits_sent ≈ 1375, objects_sent ≈ 379 MB of redundant |
| 712 | blobs (the entire chain already on the remote via dev). |
| 713 | After verification: commits_sent == 1, objects_sent == 1 (one new blob). |
| 714 | """ |
| 715 | |
| 716 | def test_LF_push_main_after_dev_sends_only_merge_commit( |
| 717 | self, tmp_path: pathlib.Path |
| 718 | ) -> None: |
| 719 | """Regression guard for muse#55 — the 1375-commit over-send is closed. |
| 720 | |
| 721 | Scenario: dev has been pushed (D and all ancestors are on the remote). |
| 722 | Local main = D + one merge commit M. push main → remote only needs M. |
| 723 | |
| 724 | Assert commits_sent == 1. If this assertion ever fails, the sibling-ref |
| 725 | push negotiation has regressed: run() is no longer using all remote |
| 726 | branch heads as the BFS boundary (push.py:819-822). |
| 727 | |
| 728 | Historical data point from issue #55: |
| 729 | BEFORE fix: commits_sent ≈ 1375, objects ≈ 379 MB (full re-send) |
| 730 | AFTER fix: commits_sent == 1, objects == 1 new blob |
| 731 | """ |
| 732 | init_repo(tmp_path) |
| 733 | commits = _chain(tmp_path, 4) |
| 734 | D = commits[-1] |
| 735 | M = _merge_commit(tmp_path, D, content=b"LF01-merge-content") |
| 736 | write_branch_ref(tmp_path, "dev", D) |
| 737 | write_branch_ref(tmp_path, "main", M) |
| 738 | |
| 739 | c = _run_push_spy(tmp_path, M, remote_branch_heads={"dev": D}) |
| 740 | |
| 741 | assert c["commits_sent"] == 1, ( |
| 742 | f"REGRESSION: commits_sent={c['commits_sent']} (expected 1). " |
| 743 | f"muse#55 has regressed — push.py:819-822 is no longer using " |
| 744 | f"all remote_branch_heads.values() as the commit-walk boundary." |
| 745 | ) |
| 746 | assert D in c["branch_have"], ( |
| 747 | f"REGRESSION: D (dev's remote tip) not in branch_have. " |
| 748 | f"Got: {[h[:16] for h in c['branch_have']]}" |
| 749 | ) |
| 750 | |
| 751 | def test_LF_large_chain_sends_only_new_commits( |
| 752 | self, tmp_path: pathlib.Path |
| 753 | ) -> None: |
| 754 | """Scale variant: 17-commit dev chain (mirrors the #55 incident report). |
| 755 | |
| 756 | The original incident had dev at 17 commits / 46 blobs. Verify that |
| 757 | pushing a main that is dev + 1 merge commit still sends only 1 commit |
| 758 | regardless of the chain length. |
| 759 | """ |
| 760 | init_repo(tmp_path) |
| 761 | commits = _chain(tmp_path, 17) |
| 762 | D = commits[-1] |
| 763 | M = _merge_commit(tmp_path, D, content=b"LF01-17-commit-merge") |
| 764 | write_branch_ref(tmp_path, "dev", D) |
| 765 | write_branch_ref(tmp_path, "main", M) |
| 766 | |
| 767 | c = _run_push_spy(tmp_path, M, remote_branch_heads={"dev": D}) |
| 768 | |
| 769 | assert c["commits_sent"] == 1, ( |
| 770 | f"Expected commits_sent=1 for 17-commit chain; got {c['commits_sent']}. " |
| 771 | f"The #55 regression is back." |
| 772 | ) |
| 773 | # objects_sent == 1: just the merge blob (merge-file.txt is new) |
| 774 | assert c["objects_sent"] == 1, ( |
| 775 | f"Expected objects_sent=1 (one new blob from M); got {c['objects_sent']}." |
| 776 | ) |
| 777 | |
| 778 | |
| 779 | # --------------------------------------------------------------------------- |
| 780 | # LF_02 — objects_sent parametrized: 0 (pure merge) and K (merge with new blobs) |
| 781 | # --------------------------------------------------------------------------- |
| 782 | |
| 783 | class TestLF02ObjectsSent: |
| 784 | """objects_sent reflects only blobs genuinely absent from the remote. |
| 785 | |
| 786 | Parametrized over two cases: |
| 787 | n_new_blobs=0 (pure merge — M's manifest == D's manifest) |
| 788 | → objects_sent == 0; every blob already on remote via dev |
| 789 | n_new_blobs=1 (one new file added in the merge commit) |
| 790 | → objects_sent == 1; one net-new blob not reachable from D |
| 791 | """ |
| 792 | |
| 793 | @pytest.mark.parametrize("n_new_blobs,expected_objects", [ |
| 794 | pytest.param(0, 0, id="pure_merge_zero_new_blobs"), |
| 795 | pytest.param(1, 1, id="merge_with_one_new_blob"), |
| 796 | ]) |
| 797 | def test_LF_no_net_new_blobs_when_sibling_has_all_objects( |
| 798 | self, |
| 799 | tmp_path: pathlib.Path, |
| 800 | n_new_blobs: int, |
| 801 | expected_objects: int, |
| 802 | ) -> None: |
| 803 | """objects_sent == expected_objects for the given merge shape. |
| 804 | |
| 805 | pure_merge_zero_new_blobs (n_new_blobs=0): |
| 806 | M re-uses D's snapshot verbatim — no new files, no new blobs. |
| 807 | All blobs are in have_blobs (via D) → objects_sent == 0. |
| 808 | This is the ideal merge: when dev already contains everything |
| 809 | and main fast-forwards by recording the merge, the push of main |
| 810 | after dev is purely metadata (one commit, zero objects). |
| 811 | |
| 812 | merge_with_one_new_blob (n_new_blobs=1): |
| 813 | M adds one file not in D's manifest → objects_sent == 1. |
| 814 | Verifies we are not over-excluding (under-sending): the one |
| 815 | genuinely new blob must still be transmitted. |
| 816 | """ |
| 817 | init_repo(tmp_path) |
| 818 | commits = _chain(tmp_path, 4) |
| 819 | D = commits[-1] |
| 820 | |
| 821 | if n_new_blobs == 0: |
| 822 | M = _merge_commit_pure(tmp_path, D) |
| 823 | else: |
| 824 | M = _merge_commit(tmp_path, D, content=b"LF02-one-new-blob") |
| 825 | |
| 826 | write_branch_ref(tmp_path, "dev", D) |
| 827 | write_branch_ref(tmp_path, "main", M) |
| 828 | |
| 829 | c = _run_push_spy(tmp_path, M, remote_branch_heads={"dev": D}) |
| 830 | |
| 831 | assert c["commits_sent"] == 1, ( |
| 832 | f"commits_sent must always be 1 (only M) for both parametrize cases; " |
| 833 | f"got {c['commits_sent']}" |
| 834 | ) |
| 835 | assert c["objects_sent"] == expected_objects, ( |
| 836 | f"objects_sent={c['objects_sent']} (expected {expected_objects}) " |
| 837 | f"for n_new_blobs={n_new_blobs}. " |
| 838 | + ( |
| 839 | "pure merge: M re-uses D's snapshot so all blobs are in have_blobs — " |
| 840 | "objects_sent must be 0." |
| 841 | if expected_objects == 0 else |
| 842 | "merge with 1 new blob: that blob must NOT be excluded — " |
| 843 | "objects_sent must be 1." |
| 844 | ) |
| 845 | ) |
| 846 | |
| 847 | |
| 848 | # --------------------------------------------------------------------------- |
| 849 | # DR_01 — Red repro of #32: dry-run over-counts commits when sibling exists |
| 850 | # --------------------------------------------------------------------------- |
| 851 | |
| 852 | class TestDR01DryRunCommitCount: |
| 853 | """DR_01: dry-run commits_sent must equal live commits_sent (==1) for #55 shape. |
| 854 | |
| 855 | Bug (#32): push.py:668-672 builds dry_branch_have from only the TARGET branch's |
| 856 | cached tip (get_remote_head("staging", "main", root)). When main has not yet |
| 857 | been pushed, this is None → dry_branch_have=[] → walk from M touches ALL commits |
| 858 | → over-counts. |
| 859 | |
| 860 | Fix: use have = _all_known_have_anchors(root) for the commit walk too (the same |
| 861 | set already used for object dedup at push.py:664-667). |
| 862 | |
| 863 | Before fix: commits_sent == len(chain) + 1 (over-count, RED) |
| 864 | After fix: commits_sent == 1 (correct, GREEN) |
| 865 | """ |
| 866 | |
| 867 | def test_DR01_dry_run_overcounts_when_main_not_yet_pushed( |
| 868 | self, tmp_path: pathlib.Path |
| 869 | ) -> None: |
| 870 | """Repro of #32: dry_branch_have=[] when remote/main tracking ref is absent. |
| 871 | |
| 872 | Scenario: dev has been pushed (tracking ref staging/dev = D exists). |
| 873 | Local main = D + merge commit M. Main has NOT been pushed yet. |
| 874 | |
| 875 | Current code: get_remote_head("staging", "main") → None → dry_branch_have=[] |
| 876 | → walk_commits(root, [M], have=[]) walks ALL commits → over-counts. |
| 877 | Fixed code: have = _all_known_have_anchors(root) = [D] |
| 878 | → walk_commits(root, [M], have=[D]) → commits_sent=1. |
| 879 | |
| 880 | MUST be RED against current push.py (push.py:668-672 not yet fixed). |
| 881 | """ |
| 882 | init_repo(tmp_path) |
| 883 | commits = _chain(tmp_path, 4) |
| 884 | D = commits[-1] |
| 885 | M = _merge_commit(tmp_path, D, content=b"DR01-merge-content") |
| 886 | write_branch_ref(tmp_path, "dev", D) |
| 887 | write_branch_ref(tmp_path, "main", M) |
| 888 | # Simulate dev having been pushed: tracking ref staging/dev = D |
| 889 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 890 | # staging/main does NOT exist (main not yet pushed) |
| 891 | |
| 892 | result = _run_dry_push(tmp_path, M) |
| 893 | |
| 894 | assert result["commits_sent"] == 1, ( |
| 895 | f"DR_01 REPRO (#32): dry-run commits_sent={result['commits_sent']} (expected 1). " |
| 896 | f"Before fix: dry_branch_have=[] causes walk to return all " |
| 897 | f"{result['commits_sent']} commits. " |
| 898 | f"After fix (push.py:663-674): _all_known_have_anchors returns [D], " |
| 899 | f"walk_commits(root, [M], have=[D]) yields commits_sent=1." |
| 900 | ) |
| 901 | |
| 902 | def test_DR01_large_chain_mirrors_incident_report( |
| 903 | self, tmp_path: pathlib.Path |
| 904 | ) -> None: |
| 905 | """17-commit chain variant — mirrors the literal #32/#55 incident report. |
| 906 | |
| 907 | The #32 report observed commits_sent: 1271 for a push --dry-run that |
| 908 | the live push completed in commits_sent: 0. This test uses the same |
| 909 | scale (17 commits, same as the #55 shape) to confirm the fix works at |
| 910 | the incident's order of magnitude. |
| 911 | |
| 912 | Before fix: commits_sent == 18 (all 17 chain + merge commit) |
| 913 | After fix: commits_sent == 1 |
| 914 | """ |
| 915 | init_repo(tmp_path) |
| 916 | commits = _chain(tmp_path, 17) |
| 917 | D = commits[-1] |
| 918 | M = _merge_commit(tmp_path, D, content=b"DR01-17-commit-merge") |
| 919 | write_branch_ref(tmp_path, "dev", D) |
| 920 | write_branch_ref(tmp_path, "main", M) |
| 921 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 922 | |
| 923 | result = _run_dry_push(tmp_path, M) |
| 924 | |
| 925 | assert result["commits_sent"] == 1, ( |
| 926 | f"DR_01 large-chain: dry-run commits_sent={result['commits_sent']} (expected 1). " |
| 927 | f"Before fix: walk counts all 18 commits (17 dev + merge). " |
| 928 | f"After fix: walk stops at D (tracking ref staging/dev), yielding commits_sent=1." |
| 929 | ) |
| 930 | |
| 931 | |
| 932 | # --------------------------------------------------------------------------- |
| 933 | # DR_02 — Object dimension: dry-run objects_sent already correct (document) |
| 934 | # --------------------------------------------------------------------------- |
| 935 | |
| 936 | class TestDR02ObjectsSent: |
| 937 | """DR_02: dry-run objects_sent already uses all tracking refs — document and guard. |
| 938 | |
| 939 | The dry-run block at push.py:664-667 correctly builds `have` from |
| 940 | _all_known_have_anchors(root) for collect_blob_ids. Only the commit walk |
| 941 | (dry_branch_have) was broken. DR_02 documents this and provides a permanent |
| 942 | guard that objects parity is not accidentally regressed. |
| 943 | |
| 944 | After the DR_01 fix, both commit and object counts use the same `have` set. |
| 945 | """ |
| 946 | |
| 947 | def test_DR02_objects_sent_matches_live_merge_with_one_blob( |
| 948 | self, tmp_path: pathlib.Path |
| 949 | ) -> None: |
| 950 | """Dry-run objects_sent==1 matches live push when merge adds one new blob. |
| 951 | |
| 952 | Already correct before the fix (the `have` variable already uses |
| 953 | _all_known_have_anchors). This test is a permanent guard — it must |
| 954 | remain green after DR_03 and must never silently regress to over- or |
| 955 | under-counting objects. |
| 956 | """ |
| 957 | init_repo(tmp_path) |
| 958 | commits = _chain(tmp_path, 4) |
| 959 | D = commits[-1] |
| 960 | M = _merge_commit(tmp_path, D, content=b"DR02-one-new-blob") |
| 961 | write_branch_ref(tmp_path, "dev", D) |
| 962 | write_branch_ref(tmp_path, "main", M) |
| 963 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 964 | |
| 965 | result = _run_dry_push(tmp_path, M) |
| 966 | |
| 967 | assert result["objects_sent"] == 1, ( |
| 968 | f"DR_02: dry-run objects_sent={result['objects_sent']} (expected 1). " |
| 969 | f"The object dimension already uses _all_known_have_anchors (push.py:664-667) " |
| 970 | f"so exactly the 1 new blob from the merge commit should be counted. " |
| 971 | f"If this fails, collect_blob_ids dedup has regressed." |
| 972 | ) |
| 973 | |
| 974 | def test_DR02_objects_sent_zero_for_pure_merge( |
| 975 | self, tmp_path: pathlib.Path |
| 976 | ) -> None: |
| 977 | """Pure merge commit (same manifest as parent) → dry-run objects_sent==0. |
| 978 | |
| 979 | The merge commit re-uses D's snapshot_id — all blobs already in have_blobs. |
| 980 | After the fix, commits_sent==1 and objects_sent==0 for this case. |
| 981 | """ |
| 982 | init_repo(tmp_path) |
| 983 | commits = _chain(tmp_path, 4) |
| 984 | D = commits[-1] |
| 985 | M = _merge_commit_pure(tmp_path, D) |
| 986 | write_branch_ref(tmp_path, "dev", D) |
| 987 | write_branch_ref(tmp_path, "main", M) |
| 988 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 989 | |
| 990 | result = _run_dry_push(tmp_path, M) |
| 991 | |
| 992 | assert result["objects_sent"] == 0, ( |
| 993 | f"DR_02 pure merge: dry-run objects_sent={result['objects_sent']} (expected 0). " |
| 994 | f"M re-uses D's snapshot — every blob is in have_blobs. " |
| 995 | f"objects_sent must be 0 (no new blobs to send)." |
| 996 | ) |
| 997 | |
| 998 | |
| 999 | # --------------------------------------------------------------------------- |
| 1000 | # RG_01 — End-to-end #32 scenario: local main == remote dev → commits_sent=0 |
| 1001 | # --------------------------------------------------------------------------- |
| 1002 | |
| 1003 | class TestRG01UpToDate: |
| 1004 | """RG_01: the literal #32 report — 1271 → 0. |
| 1005 | |
| 1006 | In the #32 incident, the live push sent commits_sent=0 (up-to-date: remote dev |
| 1007 | already held every commit local main had). The dry-run reported 1271, because |
| 1008 | dry_branch_have ignored sibling tracking refs. |
| 1009 | |
| 1010 | After the Phase 4 fix (use _all_known_have_anchors for the commit walk), we still |
| 1011 | need to include local_head itself in the have anchor when a sibling tracking ref |
| 1012 | equals local_head. The Phase 4 fix filtered it out with `c != local_head`, which |
| 1013 | incorrectly yielded commits_sent=len(chain) instead of 0 for this exact scenario. |
| 1014 | |
| 1015 | Fix: remove the `c != local_head` guard — walk_commits prunes local_head on entry |
| 1016 | when it is already in have_set, cleanly reporting commits_sent=0. This does NOT |
| 1017 | break the DR_01 scenario because there local_head=M != D, so D remains in have. |
| 1018 | """ |
| 1019 | |
| 1020 | def test_RG01_dry_run_zero_when_local_main_equals_remote_dev( |
| 1021 | self, tmp_path: pathlib.Path |
| 1022 | ) -> None: |
| 1023 | """Push --dry-run main when main == dev tip that was already pushed. |
| 1024 | |
| 1025 | Scenario: dev was pushed at D. Local main has been fast-forwarded to D |
| 1026 | (no new commits on top). Push --dry-run main should report commits_sent=0 |
| 1027 | and objects_sent=0 — identical to the live push's up-to-date result. |
| 1028 | |
| 1029 | Historical data point from issue #32: |
| 1030 | BEFORE fix: commits_sent ≈ 1271 (walks full history ignoring have) |
| 1031 | AFTER fix: commits_sent == 0 (local_head D is in have_set; pruned immediately) |
| 1032 | """ |
| 1033 | init_repo(tmp_path) |
| 1034 | commits = _chain(tmp_path, 4) |
| 1035 | D = commits[-1] |
| 1036 | # local main is at D — same commit the remote already holds via dev |
| 1037 | write_branch_ref(tmp_path, "dev", D) |
| 1038 | write_branch_ref(tmp_path, "main", D) |
| 1039 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 1040 | |
| 1041 | result = _run_dry_push(tmp_path, D) |
| 1042 | |
| 1043 | assert result["commits_sent"] == 0, ( |
| 1044 | f"RG_01 (#32): dry-run commits_sent={result['commits_sent']} (expected 0). " |
| 1045 | f"local main == remote dev (both at D); the remote already holds every commit. " |
| 1046 | f"After the full fix (c != local_head removed): walk_commits([D], have=[D]) " |
| 1047 | f"prunes D immediately → commits_sent=0. " |
| 1048 | f"If > 0, the `c != local_head` filter is still excluding D from have." |
| 1049 | ) |
| 1050 | assert result["objects_sent"] == 0, ( |
| 1051 | f"RG_01: objects_sent={result['objects_sent']} (expected 0). " |
| 1052 | f"All blobs already on remote via dev." |
| 1053 | ) |
| 1054 | |
| 1055 | def test_RG01_dry_run_zero_when_main_tracking_ref_equals_local_head( |
| 1056 | self, tmp_path: pathlib.Path |
| 1057 | ) -> None: |
| 1058 | """Re-push dry-run: staging/main already points at local_head → 0 commits. |
| 1059 | |
| 1060 | Second variant: main was previously pushed to staging (staging/main = M). |
| 1061 | Local main is still at M (no new commits). Dry-run should report 0, |
| 1062 | matching the live up-to-date short-circuit. |
| 1063 | """ |
| 1064 | init_repo(tmp_path) |
| 1065 | commits = _chain(tmp_path, 3) |
| 1066 | D = commits[-1] |
| 1067 | M = _merge_commit(tmp_path, D, content=b"RG01-repush-variant") |
| 1068 | write_branch_ref(tmp_path, "dev", D) |
| 1069 | write_branch_ref(tmp_path, "main", M) |
| 1070 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 1071 | # staging/main also points at M (it was pushed in a prior session) |
| 1072 | _write_tracking_ref(tmp_path, "staging", "main", M) |
| 1073 | |
| 1074 | result = _run_dry_push(tmp_path, M) |
| 1075 | |
| 1076 | assert result["commits_sent"] == 0, ( |
| 1077 | f"RG_01 re-push variant: dry-run commits_sent={result['commits_sent']} " |
| 1078 | f"(expected 0). staging/main already points at M (local_head). " |
| 1079 | f"walk_commits([M], have=[M, D]) prunes M immediately → 0." |
| 1080 | ) |
| 1081 | |
| 1082 | |
| 1083 | # --------------------------------------------------------------------------- |
| 1084 | # RG_02 — Non-zero control: genuinely new commits → correct count |
| 1085 | # --------------------------------------------------------------------------- |
| 1086 | |
| 1087 | class TestRG02NonZeroControl: |
| 1088 | """RG_02: non-zero control — new commits produce the correct count. |
| 1089 | |
| 1090 | Verifies that the fix (removing c != local_head) does NOT cause under-sending: |
| 1091 | when there ARE genuinely new commits not reachable from any tracking ref, they |
| 1092 | are all counted correctly. Maps to #32's VL_02. |
| 1093 | """ |
| 1094 | |
| 1095 | def test_RG02_two_new_commits_counted_correctly( |
| 1096 | self, tmp_path: pathlib.Path |
| 1097 | ) -> None: |
| 1098 | """Two commits beyond D → commits_sent=2. |
| 1099 | |
| 1100 | Topology: |
| 1101 | chain(4): root→C1→C2→D |
| 1102 | M = merge commit on top of D (1 new file) |
| 1103 | N = one more commit on top of M (1 new file) |
| 1104 | staging/dev = D |
| 1105 | |
| 1106 | push --dry-run main (at N) should report commits_sent=2 (M and N), |
| 1107 | objects_sent=2 (one blob per new commit). Proves we are not under-counting |
| 1108 | because of the has-it-all-already shortcut. |
| 1109 | """ |
| 1110 | init_repo(tmp_path) |
| 1111 | commits = _chain(tmp_path, 4) |
| 1112 | D = commits[-1] |
| 1113 | M = _merge_commit(tmp_path, D, content=b"RG02-blob-1") |
| 1114 | N = _merge_commit(tmp_path, M, content=b"RG02-blob-2", branch="main") |
| 1115 | write_branch_ref(tmp_path, "dev", D) |
| 1116 | write_branch_ref(tmp_path, "main", N) |
| 1117 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 1118 | |
| 1119 | result = _run_dry_push(tmp_path, N) |
| 1120 | |
| 1121 | assert result["commits_sent"] == 2, ( |
| 1122 | f"RG_02: dry-run commits_sent={result['commits_sent']} (expected 2). " |
| 1123 | f"N and M are both new (not in any tracking ref); " |
| 1124 | f"walk stops at D (staging/dev)." |
| 1125 | ) |
| 1126 | assert result["objects_sent"] == 2, ( |
| 1127 | f"RG_02: objects_sent={result['objects_sent']} (expected 2). " |
| 1128 | f"Two new blobs (one per new commit); D's blobs are in have_blobs." |
| 1129 | ) |
| 1130 | |
| 1131 | def test_RG02_single_new_commit_matches_DR01_result( |
| 1132 | self, tmp_path: pathlib.Path |
| 1133 | ) -> None: |
| 1134 | """Single new merge commit → commits_sent=1 (matches DR_01). |
| 1135 | |
| 1136 | Cross-check that RG_02's result is consistent with DR_01's: one new |
| 1137 | commit above a pushed sibling always yields commits_sent=1. |
| 1138 | """ |
| 1139 | init_repo(tmp_path) |
| 1140 | commits = _chain(tmp_path, 4) |
| 1141 | D = commits[-1] |
| 1142 | M = _merge_commit(tmp_path, D, content=b"RG02-single-new") |
| 1143 | write_branch_ref(tmp_path, "dev", D) |
| 1144 | write_branch_ref(tmp_path, "main", M) |
| 1145 | _write_tracking_ref(tmp_path, "staging", "dev", D) |
| 1146 | |
| 1147 | result = _run_dry_push(tmp_path, M) |
| 1148 | |
| 1149 | assert result["commits_sent"] == 1 |
| 1150 | assert result["objects_sent"] == 1 |
File History
2 commits
sha256:c287f599c5429903a139eadf3c5db5d930520e57cb0c3c575d9570e953c3b2d6
chore: bump version to 0.2.0.dev2 — nightly.2
Sonnet 4.6
patch
12 days ago
sha256:8de4334a98c945aace420969d389ad678aa926d4ab4e886b2ac4c4241cb3bf2b
revert: keep pyproject.toml in canonical PEP 440 form
Sonnet 4.6
patch
15 days ago