test_push_no_streaming_delta.py
python
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d
feat(pack): delta-encode snapshots in MPackBundle wire format
Sonnet 4.6
minor
⚠ breaking
121 days ago
| 1 | """TDD — streaming push path must never send delta+zlib objects. |
| 2 | |
| 3 | Root cause of the ghost-base loop (2026-05): |
| 4 | The push client selects delta bases from the server's commit manifests. |
| 5 | It assumes those objects are in MinIO because they belong to acknowledged |
| 6 | commits. That assumption is wrong when ghosts exist — the object is in the |
| 7 | DB but absent from MinIO. The server then fails with: |
| 8 | ❌ Push failed: delta base sha256:… not found in storage |
| 9 | |
| 10 | The presign path is safe because the server returns `already_stored` — |
| 11 | objects confirmed present in MinIO. Only those should be used as delta bases. |
| 12 | |
| 13 | The streaming path has no `already_stored` list. It sends delta objects |
| 14 | against bases it cannot confirm are in storage. This is the bug. |
| 15 | |
| 16 | Fix: |
| 17 | Remove delta encoding from the streaming path entirely. Delta is an |
| 18 | optimisation. On the streaming path (small pushes) the bandwidth saving |
| 19 | is small and the correctness risk is unacceptable. Delta stays only on |
| 20 | the presign path where storage confirmation is available. |
| 21 | |
| 22 | Coverage |
| 23 | -------- |
| 24 | D1 Structural — build_push_objects must not produce delta+zlib payloads |
| 25 | when called without a confirmed already_stored set (streaming path). |
| 26 | |
| 27 | D2 Behavioural — two successive versions of the same file built via the |
| 28 | streaming path: both objects have enc != "delta+zlib". |
| 29 | |
| 30 | D3 Regression — presign path (already_stored confirmed) may still use |
| 31 | delta+zlib; the fix must not remove delta from that path. |
| 32 | |
| 33 | D4 Structural — the streaming branch in push.py must not reference |
| 34 | "delta+zlib" in its object-building loop. |
| 35 | """ |
| 36 | from __future__ import annotations |
| 37 | |
| 38 | import datetime |
| 39 | import inspect |
| 40 | import json |
| 41 | import pathlib |
| 42 | |
| 43 | import pytest |
| 44 | |
| 45 | from muse._version import __version__ |
| 46 | from muse.core.object_store import write_object |
| 47 | from muse.core.snapshot import compute_commit_id, compute_snapshot_id |
| 48 | from muse.core.store import CommitRecord, SnapshotRecord, write_commit, write_snapshot |
| 49 | from muse.core.types import blob_id |
| 50 | from muse.core.paths import muse_dir |
| 51 | |
| 52 | |
| 53 | # --------------------------------------------------------------------------- |
| 54 | # Helpers |
| 55 | # --------------------------------------------------------------------------- |
| 56 | |
| 57 | def _repo(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> pathlib.Path: |
| 58 | dot_muse = muse_dir(tmp_path) |
| 59 | for d in ("commits", "snapshots", "objects", "refs/heads", "remotes"): |
| 60 | (dot_muse / d).mkdir(parents=True, exist_ok=True) |
| 61 | (dot_muse / "HEAD").write_text("ref: refs/heads/main\n") |
| 62 | (dot_muse / "repo.json").write_text( |
| 63 | json.dumps({"repo_id": "test-repo", "schema_version": __version__, "domain": "code"}) |
| 64 | ) |
| 65 | monkeypatch.setenv("MUSE_REPO_ROOT", str(tmp_path)) |
| 66 | monkeypatch.chdir(tmp_path) |
| 67 | return tmp_path |
| 68 | |
| 69 | |
| 70 | def _write_obj(root: pathlib.Path, content: bytes) -> str: |
| 71 | oid = blob_id(content) |
| 72 | write_object(root, oid, content) |
| 73 | return oid |
| 74 | |
| 75 | |
| 76 | def _write_commit( |
| 77 | root: pathlib.Path, |
| 78 | manifest: dict[str, str], |
| 79 | parent_id: str | None = None, |
| 80 | ) -> CommitRecord: |
| 81 | snap_id = compute_snapshot_id(manifest) |
| 82 | write_snapshot(root, SnapshotRecord(snapshot_id=snap_id, manifest=manifest)) |
| 83 | ts = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) |
| 84 | cid = compute_commit_id( |
| 85 | parent_ids=[parent_id] if parent_id else [], |
| 86 | snapshot_id=snap_id, |
| 87 | message="test", |
| 88 | committed_at_iso=ts.isoformat(), |
| 89 | ) |
| 90 | write_commit(root, CommitRecord( |
| 91 | repo_id="test-repo", |
| 92 | commit_id=cid, |
| 93 | branch="main", |
| 94 | snapshot_id=snap_id, |
| 95 | message="test", |
| 96 | committed_at=ts, |
| 97 | parent_commit_id=parent_id, |
| 98 | )) |
| 99 | return CommitRecord( |
| 100 | repo_id="test-repo", |
| 101 | commit_id=cid, |
| 102 | branch="main", |
| 103 | snapshot_id=snap_id, |
| 104 | message="test", |
| 105 | committed_at=ts, |
| 106 | parent_commit_id=parent_id, |
| 107 | ) |
| 108 | |
| 109 | |
| 110 | def _realistic(version: str) -> bytes: |
| 111 | """Realistic Python source — delta would be profitable if attempted.""" |
| 112 | lines = [f"# version {version}\n", "import os, sys\n"] |
| 113 | for i in range(80): |
| 114 | lines.append(f"def fn_{i:03d}(x): return x ^ {i * 31 + 7}\n") |
| 115 | return "".join(lines).encode() |
| 116 | |
| 117 | |
| 118 | # --------------------------------------------------------------------------- |
| 119 | # D1 — build_push_objects (streaming path) must not produce delta+zlib |
| 120 | # --------------------------------------------------------------------------- |
| 121 | |
| 122 | def test_d1_streaming_path_produces_no_delta_payloads( |
| 123 | tmp_path: pathlib.Path, |
| 124 | monkeypatch: pytest.MonkeyPatch, |
| 125 | ) -> None: |
| 126 | """build_push_objects on the streaming path must return only raw/zstd/zlib |
| 127 | objects — never delta+zlib. The streaming path has no storage-confirmed |
| 128 | already_stored list so delta bases cannot be trusted. |
| 129 | """ |
| 130 | from muse.cli.commands.push import build_push_objects |
| 131 | |
| 132 | root = _repo(tmp_path, monkeypatch) |
| 133 | |
| 134 | v1 = _realistic("v1") |
| 135 | v2 = _realistic("v2") |
| 136 | oid1 = _write_obj(root, v1) |
| 137 | oid2 = _write_obj(root, v2) |
| 138 | |
| 139 | c1 = _write_commit(root, {"src/main.py": oid1}) |
| 140 | c2 = _write_commit(root, {"src/main.py": oid2}, parent_id=c1.commit_id) |
| 141 | |
| 142 | # Streaming path: have=[c1] so server supposedly has oid1. |
| 143 | # Old code would send oid2 as delta+zlib of oid1. |
| 144 | # New code must send oid2 as a full object. |
| 145 | objects = build_push_objects( |
| 146 | root, |
| 147 | local_head=c2.commit_id, |
| 148 | have=[c1.commit_id], |
| 149 | already_stored=set(), # streaming path: no storage-confirmed set |
| 150 | ) |
| 151 | |
| 152 | delta_objects = [o for o in objects if o.get("encoding") == "delta+zlib"] |
| 153 | assert not delta_objects, ( |
| 154 | f"Streaming path must not produce delta+zlib objects. " |
| 155 | f"Got {len(delta_objects)} delta object(s): " |
| 156 | f"{[o.get('object_id', '')[:16] for o in delta_objects]}\n" |
| 157 | "Delta bases come from commit manifests with no storage confirmation. " |
| 158 | "A ghost base causes: ❌ Push failed: delta base … not found in storage." |
| 159 | ) |
| 160 | |
| 161 | |
| 162 | # --------------------------------------------------------------------------- |
| 163 | # D2 — behavioural: both objects have non-delta encoding |
| 164 | # --------------------------------------------------------------------------- |
| 165 | |
| 166 | def test_d2_successive_versions_sent_as_full_objects( |
| 167 | tmp_path: pathlib.Path, |
| 168 | monkeypatch: pytest.MonkeyPatch, |
| 169 | ) -> None: |
| 170 | """Two successive versions of the same file: both must be full objects.""" |
| 171 | from muse.cli.commands.push import build_push_objects |
| 172 | |
| 173 | root = _repo(tmp_path, monkeypatch) |
| 174 | |
| 175 | v1 = _realistic("a1") |
| 176 | v2 = _realistic("a2") |
| 177 | oid1 = _write_obj(root, v1) |
| 178 | oid2 = _write_obj(root, v2) |
| 179 | |
| 180 | c1 = _write_commit(root, {"lib/core.py": oid1}) |
| 181 | c2 = _write_commit(root, {"lib/core.py": oid2}, parent_id=c1.commit_id) |
| 182 | |
| 183 | objects = build_push_objects( |
| 184 | root, |
| 185 | local_head=c2.commit_id, |
| 186 | have=[c1.commit_id], |
| 187 | already_stored=set(), |
| 188 | ) |
| 189 | |
| 190 | for obj in objects: |
| 191 | enc = obj.get("encoding", "raw") |
| 192 | assert enc != "delta+zlib", ( |
| 193 | f"Object {obj.get('object_id', '')[:20]} sent as delta+zlib " |
| 194 | "on streaming path — unsafe without storage confirmation." |
| 195 | ) |
| 196 | assert enc in ("raw", "zlib", "zstd"), ( |
| 197 | f"Unexpected encoding {enc!r} on streaming path." |
| 198 | ) |
| 199 | |
| 200 | |
| 201 | # --------------------------------------------------------------------------- |
| 202 | # D3 — regression: presign path (already_stored confirmed) may use delta |
| 203 | # --------------------------------------------------------------------------- |
| 204 | |
| 205 | def test_d3_presign_path_may_use_delta_when_base_is_confirmed( |
| 206 | tmp_path: pathlib.Path, |
| 207 | monkeypatch: pytest.MonkeyPatch, |
| 208 | ) -> None: |
| 209 | """When already_stored confirms the base is in MinIO, delta is safe. |
| 210 | build_push_objects must still produce delta+zlib when the base oid is |
| 211 | in the already_stored set. |
| 212 | """ |
| 213 | from muse.cli.commands.push import build_push_objects |
| 214 | |
| 215 | root = _repo(tmp_path, monkeypatch) |
| 216 | |
| 217 | v1 = _realistic("p1") |
| 218 | v2 = _realistic("p2") |
| 219 | oid1 = _write_obj(root, v1) |
| 220 | oid2 = _write_obj(root, v2) |
| 221 | |
| 222 | c1 = _write_commit(root, {"svc/api.py": oid1}) |
| 223 | c2 = _write_commit(root, {"svc/api.py": oid2}, parent_id=c1.commit_id) |
| 224 | |
| 225 | # Presign path: oid1 is in already_stored (storage-confirmed by server) |
| 226 | objects = build_push_objects( |
| 227 | root, |
| 228 | local_head=c2.commit_id, |
| 229 | have=[c1.commit_id], |
| 230 | already_stored={oid1}, # server confirmed oid1 is in MinIO |
| 231 | ) |
| 232 | |
| 233 | delta_objects = [o for o in objects if o.get("encoding") == "delta+zlib"] |
| 234 | assert delta_objects, ( |
| 235 | "When the base is storage-confirmed (in already_stored), " |
| 236 | "build_push_objects must use delta+zlib to save bandwidth." |
| 237 | ) |
| 238 | assert delta_objects[0].get("base_id") == oid1 |
| 239 | |
| 240 | |
| 241 | # --------------------------------------------------------------------------- |
| 242 | # D4 — structural: streaming branch in push.py must not reference delta+zlib |
| 243 | # --------------------------------------------------------------------------- |
| 244 | |
| 245 | def test_d4_structural_streaming_branch_no_delta() -> None: |
| 246 | """The streaming object-building loop in push.py must not produce delta+zlib. |
| 247 | |
| 248 | This is a source-level check: the code path that runs when already_stored |
| 249 | is empty (streaming) must not contain delta encoding logic. |
| 250 | """ |
| 251 | from muse.cli.commands import push as push_mod |
| 252 | |
| 253 | src = inspect.getsource(push_mod.build_push_objects) |
| 254 | # The function must exist and handle the already_stored=set() case |
| 255 | # without referencing delta+zlib when already_stored is empty. |
| 256 | # We check that the function signature accepts already_stored. |
| 257 | import inspect as _inspect |
| 258 | sig = _inspect.signature(push_mod.build_push_objects) |
| 259 | assert "already_stored" in sig.parameters, ( |
| 260 | "build_push_objects must accept an already_stored parameter " |
| 261 | "so callers can control when delta encoding is safe." |
| 262 | ) |
File History
1 commit
sha256:51ce277f663e01a43eaffbe77509b1de7ac2d4251b55d23306304bcdeb92c90d
feat(pack): delta-encode snapshots in MPackBundle wire format
Sonnet 4.6
minor
⚠
121 days ago