test_wire_push_external_parent_reconstruction.py
file-level
1
files
1
commits
0
hotspots
0
🧊 dead
0
💥 blast risk
| 1 | """TDD — musehub#134: wire_push_unpack_mpack must not persist an empty base when |
| 2 | a pushed snapshot's parent is an EXTERNAL delta-only (or missing) snapshot. |
| 3 | |
| 4 | Root cause, in ``musehub_wire_push.py``: |
| 5 | |
| 6 | _psnap_rows = (await session.execute( |
| 7 | select(MusehubSnapshot.snapshot_id, MusehubSnapshot.manifest_blob) |
| 8 | .where(MusehubSnapshot.snapshot_id.in_(_external_parent_sids)) |
| 9 | )).all() |
| 10 | for _psid, _pblob in _psnap_rows: |
| 11 | if _pblob: |
| 12 | _parent_snap_manifests[_psid] = dict(_msgpack.unpackb(_pblob, raw=False)) |
| 13 | ... |
| 14 | _parent_base = _parent_snap_manifests.get(_parent_sid) or {} |
| 15 | |
| 16 | Only checks ``manifest_blob`` directly. A delta-only external parent |
| 17 | (``manifest_blob=NULL``, reconstructible via ``delta_blob`` + ``parent_snapshot_id``) |
| 18 | is silently dropped, and the child's delta is applied on top of ``{}`` — a manifest |
| 19 | that is missing every inherited file and can never hash back to its own snapshot_id. |
| 20 | |
| 21 | This test calls ``wire_push_unpack_mpack`` DIRECTLY with a hand-built wire mpack that |
| 22 | contains ONLY the new commit (its parent snapshot is genuinely absent from the |
| 23 | payload) — this is the only reliable way to force the external-parent path. A |
| 24 | CLI-level ``muse push`` integration test (see ``test_push_delta_only_parent_manifest.py``) |
| 25 | cannot force it: ``muse push``'s ``have`` negotiation is a literal-branch-tip match |
| 26 | with no ancestor-closure expansion (``muse/core/mpack.py::walk_commits`` — |
| 27 | ``prune=lambda cid: cid in have_set``), so a real second push naturally resends the |
| 28 | whole chain back to the nearest branch tip, making the "external" parent an in-payload |
| 29 | sibling instead — which is exactly why that existing test passes without exercising |
| 30 | this bug. |
| 31 | |
| 32 | RED before the fix (D's stored manifest drops f1/f2, inherited from delta-only B). |
| 33 | GREEN after (parent reconstructed from its delta chain before applying D's delta). |
| 34 | """ |
| 35 | from __future__ import annotations |
| 36 | |
| 37 | import datetime |
| 38 | from unittest.mock import AsyncMock, MagicMock, patch |
| 39 | |
| 40 | import msgpack |
| 41 | import pytest |
| 42 | from sqlalchemy.ext.asyncio import AsyncSession |
| 43 | |
| 44 | from muse.core.ids import hash_snapshot |
| 45 | from muse.core.mpack import build_wire_mpack |
| 46 | from muse.core.types import blob_id |
| 47 | from musehub.core.genesis import compute_identity_id |
| 48 | from musehub.db.musehub_repo_models import ( |
| 49 | MusehubCommit, |
| 50 | MusehubCommitGraph, |
| 51 | MusehubSnapshot, |
| 52 | MusehubSnapshotRef, |
| 53 | ) |
| 54 | from musehub.services.musehub_repository import create_repo |
| 55 | from musehub.services.musehub_wire_push import wire_push_unpack_mpack |
| 56 | |
| 57 | _OWNER = "gabriel" |
| 58 | _IDENTITY_ID = compute_identity_id(b"gabriel") |
| 59 | |
| 60 | |
| 61 | def _cid(seed: str) -> str: |
| 62 | return blob_id(f"ext-parent-commit-{seed}".encode()) |
| 63 | |
| 64 | |
| 65 | def _now() -> datetime.datetime: |
| 66 | return datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) |
| 67 | |
| 68 | |
| 69 | def _raw_commit(cid: str, parent: str | None, snap_id: str, *, branch: str = "feat") -> dict: |
| 70 | return { |
| 71 | "commit_id": cid, |
| 72 | "branch": branch, |
| 73 | "message": f"commit {cid[:12]}", |
| 74 | "author": _OWNER, |
| 75 | "committed_at": _now().isoformat(), |
| 76 | "parent_commit_id": parent, |
| 77 | "parent2_commit_id": None, |
| 78 | "snapshot_id": snap_id, |
| 79 | "agent_id": "", |
| 80 | "model_id": "", |
| 81 | "toolchain_id": "", |
| 82 | "sem_ver_bump": "none", |
| 83 | "breaking_changes": [], |
| 84 | "signature": "", |
| 85 | "signer_key_id": "", |
| 86 | "signer_public_key": "", |
| 87 | "prompt_hash": "", |
| 88 | } |
| 89 | |
| 90 | |
| 91 | def _mock_backend(mpack_bytes: bytes) -> MagicMock: |
| 92 | backend = MagicMock() |
| 93 | backend.get_mpack = AsyncMock(return_value=mpack_bytes) |
| 94 | backend.put = AsyncMock(return_value=None) |
| 95 | backend.put_mpack = AsyncMock(return_value=None) |
| 96 | backend.quarantine_mpack = AsyncMock(return_value=None) |
| 97 | backend.presign_get = AsyncMock(return_value="") |
| 98 | backend.presign_mpack_get = AsyncMock(return_value="") |
| 99 | return backend |
| 100 | |
| 101 | |
| 102 | @pytest.mark.asyncio |
| 103 | async def test_child_of_external_delta_only_parent_reconstructs_full_manifest( |
| 104 | db_session: AsyncSession, |
| 105 | ) -> None: |
| 106 | """RED: D's parent B is delta-only and NOT included in D's push payload. |
| 107 | |
| 108 | Seeded directly (simulating an earlier, separate push that already landed |
| 109 | A as root/full-manifest and B as a middle/delta-only snapshot — exactly |
| 110 | what a real ``muse push`` of A->B->C produces): |
| 111 | |
| 112 | A: root, full manifest {f1.txt} |
| 113 | B: delta-only, manifest_blob=NULL, delta_blob={+f2.txt}, parent=A |
| 114 | |
| 115 | Then push ONLY commit D (parent=B, delta={+f4.txt}) — B is genuinely |
| 116 | external: absent from this push's own commits/snapshots list entirely. |
| 117 | """ |
| 118 | repo = await create_repo( |
| 119 | db_session, |
| 120 | name="ext-parent-repro", |
| 121 | owner=_OWNER, |
| 122 | owner_user_id=_IDENTITY_ID, |
| 123 | visibility="public", |
| 124 | initialize=False, |
| 125 | ) |
| 126 | await db_session.commit() |
| 127 | |
| 128 | oid1 = blob_id(b"content-f1") |
| 129 | oid2 = blob_id(b"content-f2") |
| 130 | oid4 = blob_id(b"content-f4") |
| 131 | |
| 132 | manifest_a = {"f1.txt": oid1} |
| 133 | manifest_b = {"f1.txt": oid1, "f2.txt": oid2} |
| 134 | snap_a = hash_snapshot(manifest_a) |
| 135 | snap_b = hash_snapshot(manifest_b) |
| 136 | |
| 137 | # Seed A: root, full manifest (exactly what a real push stores for a batch root). |
| 138 | db_session.add(MusehubSnapshot( |
| 139 | snapshot_id=snap_a, |
| 140 | directories=[], |
| 141 | manifest_blob=msgpack.packb(manifest_a, use_bin_type=True), |
| 142 | entry_count=len(manifest_a), |
| 143 | created_at=_now(), |
| 144 | parent_snapshot_id=None, |
| 145 | delta_blob=None, |
| 146 | )) |
| 147 | # Seed B: delta-only middle snapshot — manifest_blob=NULL, exactly what a real |
| 148 | # push stores for a non-root/non-head snapshot. |
| 149 | db_session.add(MusehubSnapshot( |
| 150 | snapshot_id=snap_b, |
| 151 | directories=[], |
| 152 | manifest_blob=None, |
| 153 | entry_count=len(manifest_b), |
| 154 | created_at=_now(), |
| 155 | parent_snapshot_id=snap_a, |
| 156 | delta_blob=msgpack.packb({"add": {"f2.txt": oid2}}, use_bin_type=True), |
| 157 | )) |
| 158 | db_session.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=snap_a)) |
| 159 | db_session.add(MusehubSnapshotRef(repo_id=repo.repo_id, snapshot_id=snap_b)) |
| 160 | |
| 161 | # Seed A/B's musehub_commits rows (authoritative DAG) + A's commit_graph row |
| 162 | # (anchor, generation 0) — exactly what an earlier real push of A->B already |
| 163 | # landed on the server before this test's push of D. |
| 164 | a_commit = _cid("a") |
| 165 | b_commit = _cid("b") |
| 166 | db_session.add(MusehubCommit( |
| 167 | commit_id=a_commit, branch="feat", message="A", author=_OWNER, |
| 168 | timestamp=_now(), parent_ids=[], snapshot_id=snap_a, |
| 169 | )) |
| 170 | db_session.add(MusehubCommit( |
| 171 | commit_id=b_commit, branch="feat", message="B", author=_OWNER, |
| 172 | timestamp=_now(), parent_ids=[a_commit], snapshot_id=snap_b, |
| 173 | )) |
| 174 | db_session.add(MusehubCommitGraph( |
| 175 | commit_id=a_commit, parent_ids=[], generation=0, snapshot_id=snap_a, created_at=_now(), |
| 176 | )) |
| 177 | await db_session.commit() |
| 178 | |
| 179 | d_commit = _cid("d") |
| 180 | manifest_d = {"f1.txt": oid1, "f2.txt": oid2, "f4.txt": oid4} |
| 181 | snap_d = hash_snapshot(manifest_d) |
| 182 | |
| 183 | # The push payload for D ONLY — B is genuinely absent, exactly as a real |
| 184 | # incremental push would send once `have` correctly recognizes B is already |
| 185 | # on the remote (see module docstring for why muse push's CLI negotiation |
| 186 | # can't reliably force this in an integration test). |
| 187 | mpack_bytes = build_wire_mpack({ |
| 188 | "commits": [_raw_commit(d_commit, b_commit, snap_d)], |
| 189 | "snapshots": [{ |
| 190 | "snapshot_id": snap_d, |
| 191 | "parent_snapshot_id": snap_b, |
| 192 | "delta_upsert": {"f4.txt": oid4}, |
| 193 | "delta_remove": [], |
| 194 | "directories": [], |
| 195 | }], |
| 196 | "blobs": [{"object_id": oid4, "content": b"content-f4"}], |
| 197 | "tags": [], |
| 198 | }) |
| 199 | mpack_key = blob_id(mpack_bytes) |
| 200 | backend = _mock_backend(mpack_bytes) |
| 201 | |
| 202 | with patch("musehub.services.musehub_wire.get_backend", return_value=backend), \ |
| 203 | patch("musehub.services.musehub_wire_push.get_backend", return_value=backend), \ |
| 204 | patch("musehub.storage.backends.get_backend", return_value=backend): |
| 205 | await wire_push_unpack_mpack( |
| 206 | db_session, |
| 207 | repo.repo_id, |
| 208 | mpack_key, |
| 209 | pusher_id=_OWNER, |
| 210 | branch="feat", |
| 211 | head_commit_id=d_commit, |
| 212 | commits_count=1, |
| 213 | blobs_count=1, |
| 214 | force=True, |
| 215 | ) |
| 216 | await db_session.flush() |
| 217 | |
| 218 | stored = await db_session.get(MusehubSnapshot, snap_d) |
| 219 | assert stored is not None, "D's snapshot row was not written at all" |
| 220 | assert stored.manifest_blob is not None, "D is the head of its own push batch — must get a full manifest_blob" |
| 221 | |
| 222 | stored_manifest = dict(msgpack.unpackb(stored.manifest_blob, raw=False)) |
| 223 | must_have = {"f1.txt", "f2.txt", "f4.txt"} |
| 224 | assert must_have <= set(stored_manifest), ( |
| 225 | f"D's stored manifest dropped files inherited from the external delta-only " |
| 226 | f"parent B (base resolved to empty instead of being reconstructed).\n" |
| 227 | f" stored = {sorted(stored_manifest)}\n" |
| 228 | f" must_have = {sorted(must_have)}\n" |
| 229 | f" missing = {sorted(must_have - set(stored_manifest))}" |
| 230 | ) |
| 231 | assert hash_snapshot(stored_manifest, list(stored.directories or [])) == snap_d, ( |
| 232 | f"stored manifest does not reproduce snapshot_id {snap_d[:18]} — " |
| 233 | f"the exact corruption reproduced live on staging.musehub.ai/gabriel/musehub" |
| 234 | ) |
| 235 | |
| 236 | |
| 237 | @pytest.mark.asyncio |
| 238 | async def test_push_rejected_when_external_parent_is_a_total_phantom( |
| 239 | db_session: AsyncSession, |
| 240 | ) -> None: |
| 241 | """RED: D's parent snapshot has NO row at all — not delta-only, genuinely absent |
| 242 | (the exact state found on staging: a snapshot_id referenced as a parent with no |
| 243 | backing musehub_snapshots row and no musehub_commits row either). Reconstruction |
| 244 | cannot recover any content. The push must be REJECTED, not silently persisted |
| 245 | with a manifest that will never hash-verify again. |
| 246 | """ |
| 247 | repo = await create_repo( |
| 248 | db_session, |
| 249 | name="phantom-parent-repro", |
| 250 | owner=_OWNER, |
| 251 | owner_user_id=_IDENTITY_ID, |
| 252 | visibility="public", |
| 253 | initialize=False, |
| 254 | ) |
| 255 | await db_session.commit() |
| 256 | |
| 257 | oid4 = blob_id(b"content-f4-phantom") |
| 258 | phantom_snap_id = hash_snapshot({"never.txt": blob_id(b"never-existed")}) |
| 259 | manifest_d = {"never.txt": blob_id(b"never-existed"), "f4.txt": oid4} |
| 260 | snap_d = hash_snapshot(manifest_d) |
| 261 | |
| 262 | d_commit = _cid("phantom-d") |
| 263 | mpack_bytes = build_wire_mpack({ |
| 264 | "commits": [_raw_commit(d_commit, _cid("phantom-parent-commit"), snap_d)], |
| 265 | "snapshots": [{ |
| 266 | "snapshot_id": snap_d, |
| 267 | "parent_snapshot_id": phantom_snap_id, |
| 268 | "delta_upsert": {"f4.txt": oid4}, |
| 269 | "delta_remove": [], |
| 270 | "directories": [], |
| 271 | }], |
| 272 | "blobs": [{"object_id": oid4, "content": b"content-f4-phantom"}], |
| 273 | "tags": [], |
| 274 | }) |
| 275 | mpack_key = blob_id(mpack_bytes) |
| 276 | backend = _mock_backend(mpack_bytes) |
| 277 | |
| 278 | with patch("musehub.services.musehub_wire.get_backend", return_value=backend), \ |
| 279 | patch("musehub.services.musehub_wire_push.get_backend", return_value=backend), \ |
| 280 | patch("musehub.storage.backends.get_backend", return_value=backend): |
| 281 | with pytest.raises(ValueError, match="cannot be reconstructed"): |
| 282 | await wire_push_unpack_mpack( |
| 283 | db_session, |
| 284 | repo.repo_id, |
| 285 | mpack_key, |
| 286 | pusher_id=_OWNER, |
| 287 | branch="feat", |
| 288 | head_commit_id=d_commit, |
| 289 | commits_count=1, |
| 290 | blobs_count=1, |
| 291 | force=True, |
| 292 | ) |
| 293 | |
| 294 | stored = await db_session.get(MusehubSnapshot, snap_d) |
| 295 | assert stored is None, "a snapshot that can never be verified must never be persisted" |