gabriel / muse public
test_push_force_delta.py python
458 lines 17.8 KB
Raw
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor ⚠ breaking 125 days ago
1 """TDD — force push must use have anchors to send only new objects.
2
3 Root cause
4 ----------
5 When ``--force`` is passed, ``push.run`` clears all have anchors::
6
7 if force:
8 have = [] # ← WRONG: sends every object in the local store
9
10 Even on a force push the server already holds every object reachable from its
11 OTHER branch heads (e.g. ``main``). Clearing ``have`` on force causes the
12 client to walk the entire commit+object graph and re-upload every object the
13 server has already stored, regardless of how little actually changed.
14
15 Observable in production::
16
17 muse push local dev --force
18 [stream] loaded 6695 objects ... ← all objects, even ones on main
19
20 Fix
21 ---
22 Remove the ``if force: have = []`` special case. The non-force filter
23 already does the right thing::
24
25 have = [c for c in candidate_have
26 if c != local_head and _is_valid_commit_id(c) and commit_exists(root, c)]
27
28 The ``commit_exists`` guard already excludes commits that don't exist locally
29 (e.g. a diverged remote branch whose history the client never fetched), so the
30 force-push safety concern about "missing ancestor objects" is handled
31 automatically. No special casing needed.
32
33 Test plan
34 ---------
35 A1 Force push dev when main already holds the shared objects — only the new
36 object is sent, not the entire pool.
37 A2 Force push when the remote has NO other branches — all objects sent
38 (correct baseline: nothing to exclude).
39 A3 Force push when remote dev is at a diverged HEAD (not in local store) —
40 diverged HEAD is correctly excluded by commit_exists(), other branches
41 are still used as anchors.
42 B1 Normal branch → commit → merge → push workflow succeeds without force
43 when remote dev was at the correct ancestor.
44 """
45 from __future__ import annotations
46
47 import datetime
48 import json
49 import pathlib
50 import unittest.mock as mock
51 from collections.abc import Mapping
52 from unittest.mock import AsyncMock, MagicMock
53
54 import pytest
55
56 from muse._version import __version__
57 from muse.core.object_store import write_object
58 from muse.core.pack import PushResult, RemoteInfo, collect_object_ids
59 from muse.core.snapshot import compute_commit_id, compute_snapshot_id
60 from muse.core.store import (
61 CommitRecord,
62 SnapshotRecord,
63 write_commit,
64 write_snapshot,
65 )
66 from muse.core.types import blob_id, long_id
67 from muse.core.paths import ref_path, muse_dir
68 from tests.cli_test_helper import CliRunner
69
70 cli = None
71 runner = CliRunner()
72
73 _REPO_ID = "test-repo"
74 _REMOTE_URL = "https://hub.example.com/repos/test-repo"
75
76
77 # ---------------------------------------------------------------------------
78 # Helpers
79 # ---------------------------------------------------------------------------
80
81
82
83
84 def _bare_repo(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> pathlib.Path:
85 dot_muse = muse_dir(tmp_path)
86 for d in ("commits", "snapshots", "objects", "refs/heads", "remotes"):
87 (dot_muse / d).mkdir(parents=True, exist_ok=True)
88 (dot_muse / "HEAD").write_text("ref: refs/heads/main\n")
89 (dot_muse / "repo.json").write_text(
90 json.dumps({"repo_id": _REPO_ID, "schema_version": __version__, "domain": "code"})
91 )
92 (dot_muse / "config.toml").write_text(
93 f'[remotes.origin]\nurl = "{_REMOTE_URL}"\n'
94 )
95 monkeypatch.setenv("MUSE_REPO_ROOT", str(tmp_path))
96 monkeypatch.chdir(tmp_path)
97 return tmp_path
98
99
100 def _write_obj(root: pathlib.Path, content: bytes) -> str:
101 oid = blob_id(content)
102 write_object(root, oid, content)
103 return oid
104
105
106 def _commit_with_manifest(
107 root: pathlib.Path,
108 manifest: dict[str, str],
109 *,
110 branch: str = "main",
111 parent_id: str | None = None,
112 idx: int = 0,
113 ) -> CommitRecord:
114 snap_id = compute_snapshot_id(manifest)
115 write_snapshot(root, SnapshotRecord(snapshot_id=snap_id, manifest=manifest))
116 ts = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) + datetime.timedelta(hours=idx)
117 parent_ids = [parent_id] if parent_id else []
118 cid = compute_commit_id(
119 parent_ids=parent_ids,
120 snapshot_id=snap_id,
121 message=f"commit {branch} {idx}",
122 committed_at_iso=ts.isoformat(),
123 )
124 commit = CommitRecord(
125 repo_id=_REPO_ID,
126 commit_id=cid,
127 branch=branch,
128 snapshot_id=snap_id,
129 message=f"commit {branch} {idx}",
130 committed_at=ts,
131 parent_commit_id=parent_id,
132 )
133 write_commit(root, commit)
134 (ref_path(root, branch)).write_text(cid)
135 return commit
136
137
138 def _push_result(branch: str, head: str) -> PushResult:
139 return PushResult(ok=True, message="ok", branch_heads={branch: head})
140
141
142 def _transport_mock(branch_heads: Mapping[str, str], result_branch: str, result_head: str) -> MagicMock:
143 transport = mock.MagicMock()
144 transport.fetch_remote_info.return_value = RemoteInfo(
145 domain="code",
146 default_branch="main",
147 branch_heads=branch_heads,
148 )
149 transport.push_stream_coro = AsyncMock(
150 return_value=_push_result(result_branch, result_head)
151 )
152 return transport
153
154
155 def _objects_sent(transport: MagicMock) -> set[str]:
156 """Collect all object IDs sent across all push_stream_coro calls."""
157 ids: set[str] = set()
158 for call in transport.push_stream_coro.call_args_list:
159 for obj in call.kwargs.get("objects", []):
160 ids.add(obj["object_id"])
161 return ids
162
163
164 # ---------------------------------------------------------------------------
165 # A — Force push object delta
166 # ---------------------------------------------------------------------------
167
168
169 class TestForcePushDelta:
170 """Force push must not re-send objects already held by other remote branches."""
171
172 def test_a1_force_push_sends_only_new_object(
173 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
174 ) -> None:
175 """A1: Remote has 'main' with 20 shared objects. Force push 'dev' adds
176 1 new object. Only the new object should be sent across the wire.
177
178 FAILS before fix because ``have=[]`` on force push causes all 21
179 objects to be collected by collect_object_ids.
180 """
181 root = _bare_repo(tmp_path, monkeypatch)
182
183 # 20 shared objects — already "on the server" via main
184 shared_oids = []
185 for i in range(20):
186 oid = _write_obj(root, f"shared-object-{i:03d}".encode())
187 shared_oids.append(oid)
188
189 shared_manifest = {f"file{i:02d}.txt": oid for i, oid in enumerate(shared_oids)}
190 main_commit = _commit_with_manifest(root, shared_manifest, branch="main", idx=0)
191
192 # dev commit: same 20 shared objects + 1 new
193 new_oid = _write_obj(root, b"brand-new-dev-only-object")
194 dev_manifest = {**shared_manifest, "new.txt": new_oid}
195 dev_commit = _commit_with_manifest(root, dev_manifest, branch="dev", idx=1)
196
197 # Remote: main at main_commit (server holds 20 shared objects),
198 # dev at some old diverged HEAD that doesn't exist locally.
199 old_dev_head = long_id("d" * 64)
200 transport = _transport_mock(
201 branch_heads={"main": main_commit.commit_id, "dev": old_dev_head},
202 result_branch="dev",
203 result_head=dev_commit.commit_id,
204 )
205
206 with mock.patch("muse.cli.commands.push.make_transport", return_value=transport):
207 result = runner.invoke(
208 cli, ["push", "origin", "dev", "--force"], catch_exceptions=False
209 )
210
211 assert result.exit_code == 0, result.output
212
213 sent = _objects_sent(transport)
214 assert new_oid in sent, "New object must be sent"
215 re_sent = [oid for oid in shared_oids if oid in sent]
216 assert re_sent == [], (
217 f"Force push must not re-send objects already on remote main.\n"
218 f"Re-sent {len(re_sent)} of 20 shared objects: {[s[:12] for s in re_sent]}\n"
219 f"Bug: 'if force: have = []' clears all have anchors, "
220 "causing the full object graph to be walked."
221 )
222
223 def test_a2_force_push_no_other_branches_sends_all(
224 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
225 ) -> None:
226 """A2: Remote has no other branches. Force push must send all local
227 objects (nothing to exclude). Correct baseline — must pass before
228 and after fix."""
229 root = _bare_repo(tmp_path, monkeypatch)
230
231 oids = [_write_obj(root, f"obj-{i}".encode()) for i in range(5)]
232 manifest = {f"f{i}.txt": oid for i, oid in enumerate(oids)}
233 dev_commit = _commit_with_manifest(root, manifest, branch="dev", idx=0)
234
235 # Remote: dev at a diverged HEAD, NO other branches
236 old_dev_head = long_id("e" * 64)
237 transport = _transport_mock(
238 branch_heads={"dev": old_dev_head},
239 result_branch="dev",
240 result_head=dev_commit.commit_id,
241 )
242
243 with mock.patch("muse.cli.commands.push.make_transport", return_value=transport):
244 result = runner.invoke(
245 cli, ["push", "origin", "dev", "--force"], catch_exceptions=False
246 )
247
248 assert result.exit_code == 0, result.output
249 sent = _objects_sent(transport)
250 for oid in oids:
251 assert oid in sent, f"All objects must be sent when no other branches exist: {oid[:12]}"
252
253 def test_a3_diverged_dev_head_excluded_other_branches_used(
254 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
255 ) -> None:
256 """A3: Remote dev HEAD is diverged (not in local store). The diverged
257 HEAD must be excluded by commit_exists(); other branches (main) are
258 still used as have anchors.
259
260 This confirms the commit_exists() guard handles the safety concern that
261 originally motivated 'if force: have = []'.
262 """
263 root = _bare_repo(tmp_path, monkeypatch)
264
265 # Shared objects on main
266 shared_oids = [_write_obj(root, f"shared-{i}".encode()) for i in range(10)]
267 main_manifest = {f"s{i}.txt": oid for i, oid in enumerate(shared_oids)}
268 main_commit = _commit_with_manifest(root, main_manifest, branch="main", idx=0)
269
270 # 1 new object on dev
271 new_oid = _write_obj(root, b"only-on-dev")
272 dev_manifest = {**main_manifest, "dev.txt": new_oid}
273 dev_commit = _commit_with_manifest(root, dev_manifest, branch="dev", idx=1)
274
275 # Remote: main is known, dev is diverged (commit not in local store)
276 diverged_dev = long_id("f" * 64) # does NOT exist locally
277 transport = _transport_mock(
278 branch_heads={"main": main_commit.commit_id, "dev": diverged_dev},
279 result_branch="dev",
280 result_head=dev_commit.commit_id,
281 )
282
283 with mock.patch("muse.cli.commands.push.make_transport", return_value=transport):
284 result = runner.invoke(
285 cli, ["push", "origin", "dev", "--force"], catch_exceptions=False
286 )
287
288 assert result.exit_code == 0, result.output
289 sent = _objects_sent(transport)
290
291 # diverged_dev excluded → new object sent, shared objects NOT re-sent
292 assert new_oid in sent
293 assert all(oid not in sent for oid in shared_oids), (
294 "Shared objects on main must be excluded even when dev HEAD is diverged"
295 )
296
297
298 # ---------------------------------------------------------------------------
299 # B — Normal branch → commit → merge → push (no force required)
300 # ---------------------------------------------------------------------------
301
302
303 class TestBranchMergePushNoForce:
304 """The standard solo-dev workflow: branch off dev, work, merge back, push.
305 No force should be required when the remote is at a known ancestor.
306 """
307
308 def test_b1_push_after_branch_merge_succeeds_without_force(
309 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
310 ) -> None:
311 """B1: Initial state: remote dev = commit A. Branch feat off A, commit B.
312 Merge feat → dev (fast-forward: dev = B). Push dev → succeeds, no force.
313
314 Documents the expected behavior and confirms no regression in the
315 non-force code path.
316 """
317 root = _bare_repo(tmp_path, monkeypatch)
318
319 # Commit A — the base dev commit (already on remote)
320 oid_a = _write_obj(root, b"commit-a-content")
321 commit_a = _commit_with_manifest(
322 root, {"a.txt": oid_a}, branch="dev", idx=0
323 )
324
325 # Commit B — feat branch commit (parent = A)
326 oid_b = _write_obj(root, b"commit-b-new-content")
327 commit_b = _commit_with_manifest(
328 root, {"a.txt": oid_a, "b.txt": oid_b},
329 branch="dev", # fast-forward: dev now points here
330 parent_id=commit_a.commit_id,
331 idx=1,
332 )
333
334 # Remote: dev is at A (the pre-branch state)
335 transport = _transport_mock(
336 branch_heads={"dev": commit_a.commit_id},
337 result_branch="dev",
338 result_head=commit_b.commit_id,
339 )
340
341 # Push WITHOUT --force
342 with mock.patch("muse.cli.commands.push.make_transport", return_value=transport):
343 result = runner.invoke(
344 cli, ["push", "origin", "dev"], catch_exceptions=False
345 )
346
347 assert result.exit_code == 0, (
348 f"Push after branch→merge should succeed without --force.\n"
349 f"Output: {result.output}"
350 )
351 # Only commit B should be in the push bundle (A was the remote HEAD)
352 commits_sent = transport.push_stream_coro.call_args.kwargs["commits"]
353 assert len(commits_sent) == 1, (
354 f"Only the new commit should be sent, got {len(commits_sent)}"
355 )
356 assert commits_sent[0]["commit_id"] == commit_b.commit_id
357 # Only the new object (b.txt) should be sent
358 sent = _objects_sent(transport)
359 assert oid_b in sent
360 assert oid_a not in sent, "Shared object from base commit must not be re-sent"
361
362 def test_b2_push_requires_force_when_dev_genuinely_diverged(
363 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
364 ) -> None:
365 """B2: When local dev has genuinely diverged from remote (different history),
366 the server correctly rejects with 409. Force is legitimately required.
367
368 This documents that the 409 behavior is CORRECT — not a bug — when the
369 histories have actually diverged (e.g. a rewrite or rename changed the
370 local history and was never reconciled with the remote).
371 """
372 root = _bare_repo(tmp_path, monkeypatch)
373
374 oid = _write_obj(root, b"local-only-content")
375 local_commit = _commit_with_manifest(
376 root, {"local.txt": oid}, branch="dev", idx=0
377 )
378
379 # Remote dev HEAD is some totally different commit (diverged history)
380 diverged_remote_head = long_id("a" * 64) # not an ancestor of local_commit
381
382 transport = mock.MagicMock()
383 transport.fetch_remote_info.return_value = RemoteInfo(
384 domain="code",
385 default_branch="main",
386 branch_heads={"dev": diverged_remote_head},
387 )
388 # Server returns 409 for non-fast-forward push
389 from muse.core.transport import TransportError
390 transport.push_stream_coro = AsyncMock(
391 return_value=PushResult(
392 ok=False,
393 message="non-fast-forward push to 'dev' — use force=true to overwrite",
394 branch_heads={},
395 code=409,
396 )
397 )
398
399 with mock.patch("muse.cli.commands.push.make_transport", return_value=transport):
400 result = runner.invoke(cli, ["push", "origin", "dev"])
401
402 # Exit code 1 (push rejected) is expected
403 assert result.exit_code != 0, (
404 "Push to a genuinely diverged remote must fail — force is required"
405 )
406
407
408 # ---------------------------------------------------------------------------
409 # C — Unit: collect_object_ids with have from non-ancestor (cross-branch)
410 # ---------------------------------------------------------------------------
411
412
413 class TestCollectObjectIdsCrossBranch:
414 """collect_object_ids must exclude have-snapshot objects even when the
415 have commit is NOT an ancestor of the new commits (diverged histories)."""
416
417 def test_c1_have_objects_excluded_even_for_non_ancestor(
418 self, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
419 ) -> None:
420 """C1: have commit is on a completely separate branch (not ancestor).
421 Objects in its snapshot must still be excluded from the delta.
422
423 This confirms the object-level dedup in collect_object_ids works
424 independently of commit ancestry — which is the mechanism that makes
425 the force push fix safe.
426 """
427 root = _bare_repo(tmp_path, monkeypatch)
428
429 # Object on 'main' branch (not ancestor of 'dev')
430 main_obj = _write_obj(root, b"main-only-object")
431 main_manifest = {"main.txt": main_obj}
432 main_commit = _commit_with_manifest(root, main_manifest, branch="main", idx=0)
433
434 # dev branch: shares main_obj + adds new_obj (NOT a child of main_commit)
435 new_obj = _write_obj(root, b"dev-new-object")
436 dev_manifest = {"main.txt": main_obj, "dev.txt": new_obj}
437 dev_commit = _commit_with_manifest(root, dev_manifest, branch="dev", idx=0)
438
439 # Verify these commits are NOT in an ancestor relationship
440 # (both have no parent — they're independent roots)
441 from muse.core.store import read_commit
442 mc = read_commit(root, main_commit.commit_id)
443 dc = read_commit(root, dev_commit.commit_id)
444 assert mc.parent_commit_id is None
445 assert dc.parent_commit_id is None
446
447 # collect_object_ids with have=[main_commit]:
448 # even though main is not an ancestor of dev, its snapshot objects
449 # should still be subtracted
450 result = collect_object_ids(
451 root, [dev_commit.commit_id], have=[main_commit.commit_id]
452 )
453
454 assert new_obj in result, "New dev object must be included"
455 assert main_obj not in result, (
456 "Object already in have-snapshot (main) must be excluded "
457 "even when main is not an ancestor of dev"
458 )
File History 1 commit
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d feat(pack): delta-encode snapshots in MPackBundle wire format Sonnet 4.6 minor ⚠ 125 days ago