Clone corruption: push silently substitutes {} for an unresolvable external parent snapshot
Clone corruption: push silently substitutes {} for an unresolvable external parent snapshot
Background
aaronrene reported being blocked from starting musehub#84 — cloning gabriel/musehub
either produced "no commits" or a truncated working tree, matching the exact failure
mode already tracked in muse#35 (clone exits 0 but writes far fewer commits than the
remote has). Reproduced directly against staging.musehub.ai with real instrumentation.
Confirmed, traced to exact lines — not assumed
Reproducing muse clone https://staging.musehub.ai/gabriel/musehub today: 1185/1444
commits land; 9 snapshots fail client-side hash verification
(⚠️ apply_mpack: snapshot ... hash mismatch (reconstructed=...) — skipped), and every
commit downstream of those 9 (~260 commits, including everything pushed this session)
gets dropped as "parent not in mpack or local store".
Pulled one of the 9 failing snapshots (sha256:ff9c4d233...) directly from the real
staging RDS database (not the abandoned local musehub_postgres container on the EC2
box — that was a dead end, see below). Its manifest, as actually served by
/fetch/mpack, does not reproduce its own snapshot_id under any directories variant
(stored, derived, or none). The manifest is missing every file that should have been
inherited from its parent snapshot.
Root cause, in musehub/services/musehub_wire_push.py:
# lines 715-729
_external_parent_sids = {
_sd.get("parent_snapshot_id")
for _sd in _raw_snaps
if _sd.get("parent_snapshot_id") and _sd.get("parent_snapshot_id") not in _snap_ids_in_mpack
}
_parent_snap_manifests: dict[str, dict] = {}
if _external_parent_sids:
_psnap_rows = (await session.execute(
select(MusehubSnapshot.snapshot_id, MusehubSnapshot.manifest_blob)
.where(MusehubSnapshot.snapshot_id.in_(_external_parent_sids))
)).all()
for _psid, _pblob in _psnap_rows:
if _pblob:
_parent_snap_manifests[_psid] = dict(_msgpack.unpackb(_pblob, raw=False))
# line 764
_parent_base = _parent_snap_manifests.get(_parent_sid) or {}
When a pushed snapshot's parent is not included in this push's own payload (an
"external" parent — already on the server from an earlier push), the server looks it
up by manifest_blob alone. Only root/head snapshots of a given push batch ever get a
manifest_blob (see lines 786-798) — every other snapshot is stored delta-only
(manifest_blob=NULL, reconstructible only by walking delta_blob + parent_snapshot_id).
If the external parent is delta-only, or its row is missing outright, _pblob is falsy,
the snapshot_id is never added to _parent_snap_manifests, and line 764 silently falls
back to {}. The child's delta then gets applied on top of nothing — the new manifest
permanently and irrecoverably drops every inherited file, and is written to the DB in
that broken state. No later read-side fix can repair it; the wrong bytes are already
committed to manifest_blob.
_reconstruct_manifest (the delta-chain walker in musehub_wire_shared.py) is imported
into this file (line 51) but never called here — the exact tool needed to fix this
already exists elsewhere in the codebase and just isn't wired into the write path.
The corrupted snapshot's parent is a total phantom — even worse than delta-only
For the specific snapshot reproduced above, its parent_snapshot_id
(sha256:1fcf68ea...) has no row at all in musehub_snapshots, and no
musehub_commits row has that value as its own snapshot_id either — checked
directly against the real RDS instance. It isn't merely delta-only; it never existed
(or was later deleted, e.g. by a GC job, while still referenced). How this specific
phantom reference came to exist is not yet root-caused and may be a second, distinct
bug (or GC-vs-reachability bug) — out of scope for the fix below, which must hold
regardless: the server must never silently persist a manifest it knows is missing data,
whether the parent is delta-only or fully absent.
A regression test already exists for this exact bug — and is a false positive
tests/test_push_delta_only_parent_manifest.py::test_child_of_delta_only_parent_keeps_complete_manifest
sets up precisely this topology (push 1: A→B→C, push 2: branch off B, add D) and
currently passes. It doesn't actually exercise the vulnerable code path: muse push
computes have from the literal remote branch-tip commit IDs only (walk_commits's
prune=lambda cid: cid in have_set — no ancestor-closure expansion, confirmed in
muse/core/mpack.py). Since main's tip is C, not B, pushing feat (which forks at
B) does not prune at B — the walk continues past it, and A+B get resent as
part of push 2's own payload. That makes B an in-payload sibling, not an external
parent, so the buggy _parent_snap_manifests path is never reached. The test provides
false confidence; this is why the bug has persisted through multiple prior "fix" landings
(see symbol-log on _snap_row_to_wire_s3, May 29 – June 13, all read-side fixes for a
write-side bug).
Also ruled out this session
- Staging Alembic migration state: current (
0074, matchesdev) — checked against the real RDS instance directly, not the abandoned localmusehub_postgrescontainer. muse code migrate(client-side ID-format rewrite): doesn't apply — that command operates on a local.muse/working directory, not MuseHub's server-side Postgres rows.
Also found and cleaned up (unrelated but discovered during this investigation)
musehub_postgres + musehub_runner on the staging EC2 instance (i-07547cd20bee2dea5)
were a legacy docker-compose stack predating the RDS/ECR blue-green deploy, doing nothing
for the live site (musehub_runner was stuck in a DNS-resolution-failure loop). Backed up
via pg_dump (verified checksum, saved to gabriel's machine) and stopped — not deleted;
the Docker volume is untouched and the stop is fully reversible.
Goal
- Fix
wire_push_unpack_mpack's external-parent resolution: when a parent'smanifest_blobis NULL, fall back to delta-chain reconstruction (_reconstruct_manifest_validatedor equivalent) instead of{}. When reconstruction itself fails (phantom/unrecoverable parent), reject the push loudly rather than persist a manifest that will never hash-verify. - Replace or fix
test_push_delta_only_parent_manifest.pyso it actually forces the external-parent path (e.g. callwire_push_unpack_mpackdirectly with a hand-built payload, bypassingmuse push's have-negotiation) — a CLI-level integration test cannot reliably force this topology given howhaveis computed. - Decide whether to repair the already-corrupted rows on staging once the write path is fixed (separate follow-up — this ticket is the write-path fix itself).
Out of Scope
- Root-causing how the specific phantom parent (
1fcf68ea...) came to be referenced with no backing row — possibly a GC/reachability bug, tracked separately if it recurs after this fix lands. - Repairing already-corrupted snapshot rows on staging.
- musehub#131 (wire push has no repo-authorization check) — unrelated class of bug, found in an earlier session.
Fully resolved and verified — closing.
Correction to my earlier analysis: the "2 remaining unfixable" snapshots
were never actually corrupted. My investigation used directories_from_manifest()
to guess a directories list instead of reading the real one stored on the
object — that guess was wrong for these two, making them look like permanent
identity mismatches. Once I read the actual stored directories field, both
hash-verified cleanly. All 3 corrupted snapshots were fixed the simple way,
via wire_repair_snapshot — no history rewrite needed.
Also hit and fixed an operational snag: manually deleting a stale
musehub_fetch_mpack_cache row via direct SQL skips the job-enqueue side
effect wire_repair_snapshot normally handles, causing a temporary
"server busy" 503 loop. Fixed by re-submitting the repair (idempotent),
which re-triggered the rebuild correctly.
Final verification — fresh clone into a clean /tmp directory:
muse clone https://staging.musehub.ai/gabriel/musehub musehub --json
18.2s total, 1444/1444 commits, 0 hash mismatches, 999 blobs, clean working tree (1009 files). aaronrene confirmed unblocked on #84.
The write-path fix itself (fix/wire-push-external-parent-manifest) is
still real and still worth merging — it prevents this exact class of
corruption from recurring on future incremental pushes. Not yet merged.
Theory confirmed cheaply — no more staging round-trips needed. Tested and fixed on branch
fix/wire-push-external-parent-manifest.How the theory was tested: a CLI-level
muse pushintegration test can't reliably force the external-parent path —havenegotiation is a literal branch-tip match with no ancestor-closure expansion, so a real second push resends the whole chain back to the nearest tip, masking the bug (this is exactly whytest_push_delta_only_parent_manifest.pyalready existed and already passed without catching it). Instead, calledwire_push_unpack_mpackdirectly with a hand-built wire payload containing only the new commit — full control over what's "external," no staging access, no CLI subprocess, ~1.5s per test.Result: confirmed the theory on the first try (
external=1 loaded=0in the server's own instrumentation), then fixed it:manifest_bloblookup misses, reconstruct via_reconstruct_manifest_validated(delta-chain walk) instead of defaulting to{}.snapshot_idbefore writing — a genuinely unrecoverable parent (the phantom-reference case found on staging) now rejects the push loudly instead of silently persisting unverifiable data.TDD:
tests/test_wire_push_external_parent_reconstruction.py, 2 tests, red-first, both green. Full wire-push regression sweep: 211 passed (42 direct + 169 broader mpack/push/ intel suite).Not yet merged to
dev— staying on the feature branch per convention. Repairing the already-corrupted rows on staging is a separate follow-up (goal #3 in the issue body), not done yet.