gabriel / muse public
Closed #83 Bug design
filed by gabriel human · 8 days ago

Architectural plan: unified-store snapshot-content-ID migration (fixes musehub#134's remaining 2 corrupted commits)

0 Anchors
Blast radius
Churn 30d
0 Proposals

Unified-store snapshot-content-ID migration: recompute-and-cascade repair for commits whose declared snapshot_id never matched their manifest

Status: architectural intention, not yet implemented

This is a load-bearing design document, not a quick patch. It exists to record why this tool needs to exist, what it must guarantee, and the order in which it must be built and verified — before a single line of the tool itself is written. Nothing in this document has been executed. Read-only investigation only, per explicit instruction, until this plan is reviewed.

Background

While investigating why aaronrene could not clone gabriel/musehub (musehub#84), traced and fixed a real write-path bug: musehub#134 (musehub/services/musehub_wire_push.py) — when a pushed snapshot's parent was already on the server from an earlier push and stored delta-only, the server silently substituted {} as the base instead of reconstructing it, permanently corrupting the child snapshot's manifest. That fix is written, tested (211 passed), and sitting on fix/wire-push-external-parent-manifest in musehub, unmerged.

Of the 9 snapshots on staging that failed client-side hash verification during clone, one (ff9c4d233...) was exactly this bug and was successfully repaired live via the wire_repair_snapshot endpoint — confirmed by re-cloning and watching the hash-mismatch count drop from 9 to 2.

The remaining 2 (a1ed2a9cf35b7..., 1a58293f4680d...) are not the musehub#134 bug. Investigation (documented in full in this session's transcript, summarized here) proved:

  • Both commits' own declared snapshot_id field has never matched their manifest, since the moment they were created on 2026-05-27 — a client-side defect baked into the commit at creation time, not a server storage bug.
  • Proof: independently reconstructing each snapshot two different ways (the server's own delta-chain walk, and manually applying the exact file-level diff to a verified-correct parent) produces the same hash both times — but that hash does not match the commit's declared snapshot_id. Two independent reconstructions agreeing with each other, and disagreeing with the declared ID, means the declared ID is the thing that's wrong, not the reconstruction.
  • wire_repair_snapshot cannot fix this: it requires the supplied manifest to hash-verify against the declared snapshot_id before accepting it, and by definition nothing ever will for these two, because the declared ID was never valid.
  • Fixing this requires assigning each affected commit a new, correct snapshot_id — which changes the commit's own commit_id (a hash over parent_ids + snapshot_id + message + committed_at + author + signer_public_key) — which cascades: every descendant commit's parent pointer must also be rewritten, transitively, all the way to the current branch tips. Because these are very early commits (2026-05-27, near the start of this repo's history), the cascade likely touches nearly the entire commit graph.
  • muse code migrate was investigated as a ready-made fix and ruled out: read muse/core/migrate.py in full. migrate_snapshot_ids() / migrate_commit_ids() only operate on the legacy pre-unified-object-store layout (.muse/snapshots/sha256/*.msgpack, .muse/commits/sha256/*.msgpack), which gabriel/musehub's local .muse/ no longer has (already migrated to .muse/objects/) — so those functions are a no-op on our data. The actual migrate() orchestrator's commit-rewrite step (_make_canonical_dict) reads snapshot_id straight off the existing record and passes it through unchanged into hash_commit(...) — it never calls hash_snapshot(), never reads a manifest, never validates a snapshot. It solves a different, already-resolved problem (v1→v2 commit ID formula, legacy directory layout). There is currently no tool, anywhere in the muse or musehub codebases, that detects or repairs a commit whose declared snapshot_id disagrees with its own manifest.

This document specs that tool.

Why this must be TDD and why it's load-bearing

This tool rewrites the identity of (likely) most of a real repository's commit history and re-signs the result. There is no acceptable "mostly works" outcome — a bug here either silently produces a second, different, harder-to-detect corruption, or destroys provenance (signatures that no longer verify, refs that point at nothing). Every phase below must have failing tests written first, proving the exact failure mode it closes, before any implementation. No phase begins until the previous phase's tests are green and reviewed.

This is also explicitly load-bearing: once run, its output (a corrected local history) becomes the one and only source of truth pushed to a recreated staging repo. There is no second chance to get the ID/signature math right after that push happens — errors discovered post-push require either another full rewrite-and-repush cycle, or living with the mistake.

Non-negotiable invariants

  1. Non-destructive locally. Every existing object (blob/snapshot/commit) under its old ID stays exactly where it is. New, corrected objects are written under their newly-computed IDs alongside the old ones. This mirrors the existing migrate_snapshot_ids/migrate_commit_ids philosophy exactly.
  2. Only ever run against scratch copies, never ~/ecosystem/muse or ~/ecosystem/musehub in place. Per explicit instruction: fresh copies at ~/dev/copies 3/muse 01 and ~/dev/copies 3/musehub 01 (note: musehub, not muse, for the second path — the working copies named in the original request both said "muse 01"; confirm the second is meant to be musehub 01 before any copy is made).
  3. Every snapshot that already hash-verifies is provably untouched. The tool must produce zero changes for 1427 of the 1429 snapshots on gabriel/musehub. This is itself a test, not an assumption.
  4. Every commit whose ID changes must be re-signed with the current signing identity — a stale signature over a superseded commit_id is a silent authenticity regression, not a cosmetic issue. (This is the one real gap found in reusing migrate_commit_ids() as-is — it recomputes commit_id but never touches signature/signer_public_key.)
  5. Refs, remote-tracking refs, and reflogs must resolve to the new IDs. Reuse _migrate_refs/_migrate_remote_refs/_migrate_reflogs as-is — they are already generic over any id_map: dict[str, str].
  6. Built as a standalone, explicitly-invoked tool, not folded into muse code migrate's default pass list. This is a one-time repair for one known corruption event on one repository. Wiring it into the general migration path risks it firing unexpectedly against unrelated repos in the future for a condition (declared snapshot_id ≠ manifest hash) that should never occur again once musehub#134's write-path fix is deployed.

Architecture

Three new pieces of code, each independently testable, composed in sequence:

Phase 1 — migrate_snapshot_content_ids() (detection + correction, snapshot layer)

New function, modeled directly on migrate_snapshot_ids() but sourced from the unified object store instead of the legacy layout:

def migrate_snapshot_content_ids(
    repo_root: pathlib.Path,
    dry_run: bool = False,
) -> SnapshotContentMigrateResult:
  • Walk .muse/objects/sha256/*/*, filter to files whose content starts with b"snapshot " (the typed-object header already used everywhere else in this codebase for disambiguating object kind).
  • Parse {manifest, directories} from each.
  • Recompute hash_snapshot(manifest, directories or None).
  • Where recomputed != declared: record old_id → new_id. Where they match: do nothing (no entry, no write) — this is the invariant-3 guarantee.
  • For entries needing correction, write the object (same JSON "snapshot <size>\0<json>" format used by migrate_snapshot_ids) under the new ID. Skip (record as skipped, per existing convention) if an object already exists at the new path.

TDD for this phase:

  • RED: a hand-built repo fixture with one snapshot whose manifest hash-verifies (must produce zero changes) and one whose declared ID is deliberately wrong (must appear in the id_map with the correct recomputed ID).
  • RED: confirm the function does not look at .muse/snapshots/ at all — a fixture with only a legacy-layout mismatch must produce zero changes (proves this is additive to, not a replacement for, the existing legacy function).
  • GREEN: both pass; a full run against a copy of gabriel/musehub's local .muse/ produces an id_map with exactly the 2 known snapshot IDs documented in musehub#134's investigation, and no others.

Phase 2 — cascading commit rewrite + re-sign (commit layer)

Extend (or wrap, TBD during implementation — see open question below) migrate_commit_ids()'s existing topological two-pass structure with the missing re-sign step, modeled on the cascade-resign logic already proven correct in _make_canonical_dict():

  • A commit must be re-signed when: its own snapshot_id changed (via Phase 1's id_map), OR either parent's commit_id changed (transitively, via this pass's own in-progress id_map — same pattern _make_canonical_dict already uses for parents_changed).
  • Re-signing requires a SigningIdentity — this pass is not safely runnable without one (unlike Phase 1, which is pure recomputation).
  • Every non-re-signed, non-ID-changed commit must be byte-identical to its current unified-store representation — this pass must not touch commits outside the blast radius.

TDD for this phase:

  • RED: a fixture chain A -> B(bad snapshot) -> C -> D (A clean, B's declared snapshot_id wrong, C and D otherwise normal). Before the fix: recomputing B's commit_id with the corrected snapshot_id, then re-deriving C and D's commit_ids from B's new ID, does not happen — C and D still point at the old B. After the fix: id_map contains all of B, C, D mapped to new IDs; A is absent from the id_map (untouched).
  • RED: every commit in the id_map has a signature that verifies against its new commit_id with the signing identity's public key; the old signature (if checked against the new commit_id) must fail — proving re-signing actually happened, not just ID recomputation.
  • RED: a commit outside the blast radius (sibling branch, unrelated history) is provably byte-identical before and after.
  • GREEN: full fixture chain resolves correctly; running twice is idempotent (second run produces an empty id_map).

Phase 3 — ref / reflog integration

No new code — reuse _migrate_refs, _migrate_remote_refs, _migrate_reflogs exactly as they exist today, feeding them the combined id_map from Phases 1+2 (same transitive-closure merge pattern already implemented in migrate()'s own combination of its legacy id_map with Phase 7's id_map — copy that exact pattern, don't reinvent it).

TDD for this phase:

  • RED: a branch head pointing at a rewritten commit resolves to the new ID after the pass; a branch head pointing at an untouched commit is byte-identical.
  • RED: reflog entries mentioning a rewritten commit_id are rewritten; reflog entries mentioning an untouched commit_id are not.

Phase 4 — full dry-run against scratch copies

Only after Phases 1–3 are green against synthetic fixtures:

  • Copy ~/ecosystem/musehub to the confirmed scratch path (see invariant 2 — path needs confirming before this phase, not assumed).
  • Run the new tool in --dry-run mode (zero writes, per the existing convention this whole module already follows).
  • Assert: id_map contains exactly the 2 known bad snapshot IDs plus every transitively-affected commit — no more, no fewer. Cross-check the affected commit count against an independent muse rev-list walk from the 2 known bad commits forward to every branch tip.
  • Only proceed to a real (non-dry-run) execution on the scratch copy after this assertion is manually reviewed and confirmed.

Phase 5 — verify the corrected scratch copy round-trips cleanly

  • Push the corrected scratch copy's full history to a local throwaway hub repo (the same technique already used successfully this session to validate musehub#134's fix before touching staging).
  • Clone that local hub repo fresh. Assert zero hash mismatches, 100% of commits land — matching the clean result already observed for gabriel/musehub's unmodified history when pushed to local in bulk.
  • This is the gate before staging is touched at all.

Phase 6 — staging cutover

Not scoped in detail here (separate decision point once Phase 5 is green): delete and recreate gabriel/musehub on staging, push the corrected history, deploy musehub#134's write-path fix so future incremental pushes can't reintroduce this class of bug, then re-run the original clone repro as the final acceptance check for unblocking aaronrene (musehub#84).

Open questions to resolve before implementation starts

  1. Confirm the scratch-copy paths — the original instruction named both as ~/dev/copies 3/muse 01; almost certainly the second should be ~/dev/copies 3/musehub 01 (mirroring the earlier muse 04/musehub 04 convention from the git-bridge-backup work), but this must be confirmed, not assumed, before any copying happens.
  2. Whether Phase 2 extends migrate_commit_ids() in place (adding the resign branch) or wraps it with a separate post-pass. Leaning toward extending in place, since the re-sign decision needs to be made per-commit during the same topological walk (a separate pass would need to redo the parent-changed detection). Decide during implementation, not here.
  3. Whether the 2 known bad commits' original (pre-migration) objects should ever be deleted from the corrected scratch copy, or left in place permanently as non-destructive per invariant 1 (leaning toward: leave them — muse gc/muse prune already exist for reachability-based cleanup if ever needed, and this tool should not take on garbage collection as a second responsibility).

Acceptance criteria

  • Phase 1 function exists, is unit-tested (RED-first), and produces exactly the 2 known-bad snapshot IDs in its id_map when run against a copy of gabriel/musehub's local .muse/, and zero changes otherwise.
  • Phase 2 cascading rewrite + re-sign is unit-tested (RED-first) against a synthetic fixture proving both the cascade and the re-signing.
  • Phase 3 ref/reflog integration is unit-tested (RED-first).
  • Phase 4 dry-run against the real scratch copy produces an id_map whose size is independently cross-checked against a muse rev-list walk.
  • Phase 5: corrected scratch copy pushes to a local throwaway hub and clones back with zero hash mismatches and 100% of commits landing.
  • Full regression suite for muse/core/migrate.py and its existing tests remains green throughout — this tool must not regress the existing (unrelated) legacy-layout migration behavior.
  • This document's "Open questions" section is fully resolved and struck through (not deleted — kept as a record of what was decided and why) before Phase 4 begins.

Out of scope

  • Phase 6 (staging cutover) itself — tracked as a follow-up decision once Phase 5 is green, not part of this implementation ticket.
  • Root-causing why the client produced a wrong snapshot_id at commit time on 2026-05-27 in the first place (a separate, historical muse-CLI-version archaeology question — interesting, but not required to fix the data).
  • Any change to muse code migrate's default behavior or its existing legacy-layout passes.
  • Repairing any other repository — this plan is scoped to the 2 known-bad commits on gabriel/musehub found during musehub#134's investigation.
Activity2
gabriel opened this issue 8 days ago
gabriel 8 days ago

Status update: fully implemented (Phases 1-3 + CLI, 17 tests, all green), but turned out NOT to be needed for musehub#134's remaining 2 snapshots — those were never actually corrupted (see musehub#134's closing comment; methodology error on my part, not real data corruption).

The tool itself is still correct, real, and tested for the genuine class of bug it targets (a commit whose declared snapshot_id truly disagrees with its manifest). Sitting on feat/snapshot-content-id-migration, unmerged, never exercised against real corrupted data since none was found. Leaving open for gabriel to decide: merge as defensive tooling for a bug class that hasn't recurred, or close as unnecessary for now.

gabriel 8 days ago

Merged into dev (fast-forward, no conflicts) — keeping this as tested, defensive tooling for the bug class it targets, even though it turned out not to be needed for musehub#134's specific corruption.

17/17 tests still passing post-merge. muse migrate (unrelated top-level namespace, deleted separately in muse#38) and muse code migrate are both unaffected -- muse code fix-snapshot-ids is a fully standalone addition.

Closing.

closed this issue 8 days ago