fix: sha256:a5f8a020b53ddab68f231cf27489fe865525f85ee924133034262d39b332076e · gabriel/muse

fix(clone,pull): never claim success when a snapshot hash mismatch silently drops commits (muse#35)

rc11 report: a fresh `muse clone` of a repo with a corrupted snapshot delta exited 0, printed "Cloned into ... 0 commit(s)", and left the working tree empty -- then a follow-up `muse pull` reported "Already up to date" and never recovered. User-reported as still unresolved; confirmed it still reproduces on 0.2.1rc12 before touching anything.

Root cause chain, all three links now fixed:

1. muse/core/mpack.py — apply_mpack's snapshot-writing loop: when _apply_snapshot_deltas's delta-reconstruction fails (hash mismatch, invalid object IDs, unresolvable parent chain), the snapshot ID was simply absent from `resolved_manifests` and the main loop did a bare `continue` -- never recording it in `skipped_snapshot_ids`, which was only ever populated by the SEPARATE "references a failed blob object" check. The returned ApplyResult.skipped_snapshots stayed empty even though a snapshot (and every commit depending on it) had just been silently dropped -- no record anywhere that anything had gone wrong.

2. muse/cli/commands/clone.py — the branch-ref-writing loop had a comment literally saying "Only advance refs for branches whose tip commit landed cleanly," but never implemented that check: write_branch_ref and set_remote_head were called unconditionally for every branch the server advertised, regardless of whether apply_mpack actually wrote that branch's tip commit. This is THE bug: the local branch ref ends up pointing at a commit that doesn't exist in the store (0 commits, muse log shows nothing) while `muse status`/`muse pull` believe everything is fine. Also fixed total_skipped/exit_code to account for skipped_snapshots and commits the server sent but which were never written (previously only blob-level failures counted, so a 0-commit clone with 0 corrupted blobs still reported "cloned" success).

3. muse/cli/commands/pull.py — same unconditional-advance bug for the remote-tracking ref: set_remote_head ran whenever `failed_blobs` was empty, with no check for skipped_snapshots or whether the commit was actually written. THIS is what made the corruption permanent: once the tracking ref wrongly equals the remote's head, the next pull's Phase 1 short-circuit ("already_known == remote_commit_id" -> skip fetch entirely) means the skipped commit/snapshot can never be re-requested -- exactly the ticket's "follow-up pull does not repair the skipped state" symptom. Both clone.py and pull.py's new guards defensively treat a malformed/legacy-format commit ID as "not written" rather than letting validate_object_id's ValueError propagate and abort the whole command.

9 new/updated tests: tests/test_apply_mpack_partial_failure.py gains PF-5 (hash-mismatch skip is recorded in skipped_snapshots) and PF-5b (a retry with the correct delta for the same snapshot_id succeeds -- proving the skip is recoverable, satisfying the ticket's "follow-up pull can recover" criterion at the apply_mpack layer). tests/test_cmd_clone_hardening.py gains TestSnapshotHashMismatchGuard (3 tests: ref withheld + PARTIAL exit when the commit is missing, JSON schema gains skipped_snapshots/ commits_missing fields, happy-path regression guard). tests/test_cmd_pull_hardening.py gains TestTrackingRefNotAdvancedOnSkippedSnapshot (2 tests). Two pre-existing tests updated: test_clone_partial.py's skipped_blobs assertion widened from an exact count to >=1 now that cascading snapshot/commit skips are counted too (the old exact-1 assertion described the bug's own undercounting), and test_cmd_clone_hardening.py's test_branch_refs_written_for_every_remote_branch (which mocked apply_mpack without ever writing real commits) now also mocks read_commit to simulate the happy path explicitly, since its old assertion was testing the unconditional-write bug itself.

Confirmed via genuine RED: reverted all three fixes in turn, re-ran the new tests -- the clone.py revert reproduced the ticket's exact bug (branch ref written pointing at the missing commit ID) -- then restored and confirmed GREEN. 579-test regression sweep across every mpack/clone/pull-relevant suite passes with zero regressions. Live-verified the normal (non-corrupted) clone/push/pull path end-to-end against a real local MuseHub server: 2 commits pushed, cloned cleanly, working tree restored, log/status correct.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

+10

symbols added

symbols modified

files changed

1252

files in snapshot

dead code introduced

Semantic Changes 18 symbols

~ muse/cli/commands/clone.py .py 2 symbols modified

~ _CloneJson

~ run

~ muse/cli/commands/pull.py .py 1 symbol modified

~ run

~ muse/core/mpack.py .py 1 symbol modified

~ apply_mpack

~ tests/test_apply_mpack_partial_failure.py .py 2 symbols added

+ test_pf5_snapshot_hash_mismatch_recorded_in_skipped_snapshots function function test_pf5_snapshot_hash_mismatch_recorded_in_skipped_snapshots L300–367

+ test_pf5b_recovery_after_resending_correct_delta function function test_pf5b_recovery_after_resending_correct_delta L370–427

~ tests/test_clone_partial.py .py 2 symbols modified

~ TestClonePartial

~ test_json_envelope_includes_skipped_objects_count

~ tests/test_cmd_clone_hardening.py .py 5 symbols added, 2 symbols modified

+ TestSnapshotHashMismatchGuard class class TestSnapshotHashMismatchGuard L893–996

+ test_branch_ref_not_written_when_commit_missing_from_store method method test_branch_ref_not_written_when_commit_missing_from_store L894–946

+ test_happy_path_still_reports_cloned_and_writes_ref method method test_happy_path_still_reports_cloned_and_writes_ref L989–996

+ test_json_reports_partial_status_not_cloned method method test_json_reports_partial_status_not_cloned L948–987

+ ApplyResult import import ApplyResult L890–890

~ TestRunIntegration

~ test_branch_refs_written_for_every_remote_branch

~ tests/test_cmd_pull_hardening.py .py 3 symbols added

+ TestTrackingRefNotAdvancedOnSkippedSnapshot class class TestTrackingRefNotAdvancedOnSkippedSnapshot L746–810

+ test_set_remote_head_called_when_commit_actually_written method method test_set_remote_head_called_when_commit_actually_written L769–810

+ test_set_remote_head_not_called_when_commit_missing method method test_set_remote_head_not_called_when_commit_missing L747–767

Files Changed

1252 in snapshot

~ muse/cli/commands/clone.py .py

~ muse/cli/commands/pull.py .py

~ muse/core/mpack.py .py

~ tests/test_apply_mpack_partial_failure.py .py

~ tests/test_clone_partial.py .py

~ tests/test_cmd_clone_hardening.py .py

~ tests/test_cmd_pull_hardening.py .py

← Older Oldest on fix/35-clone-pull-snapshot-hash-mismatch

All commits

Newer → Latest on fix/35-clone-pull-snapshot-hash-mismatch

0 comments

No comments yet. Be the first to start the discussion.

To add a comment, use the Muse CLI: muse hub commit comment sha256:a5f8a020b53ddab68f231cf27489fe865525f85ee924133034262d39b332076e --body "your comment"