gabriel / muse public
mwp-mvp-master.md markdown
320 lines 18.6 KB
Raw
sha256:ac29b8ba9021514a03ab2782d92bf67671f0efa5b3b70d46f7598c5d4e923378 docs: record muse#74 Phase 4 live verification (SCR_13, SCR_14) Sonnet 5 13 days ago

Muse Wire Protocol — MVP hardening (master tracking issue)

Background

The Muse Wire Protocol (MWP) — clone, push, fetch, pull — is the single hardest, most time-consuming subsystem in the workspace. It is functionally equivalent to git smart-HTTP, rebuilt from scratch on an mpack packfile format with content-addressed objects, presigned R2/MinIO uploads, and an async server-side prebuild/index pipeline.

It mostly works. The blocker to declaring it MVP-ready — so aaronrene can clone/push/pull/fetch with confidence and start picking up tickets — is a small number of correctness gaps in the prebuild/index/negotiation pipeline plus a largely-disabled test suite. This issue is the result of a full read-through of both sides of the protocol. It records the architecture, the confirmed root causes (grounded in file:line), and the proposed sub-tickets.

This is a hardening pass, not a rewrite. The architecture is sound. The fixes are targeted.

Proximate triggers

  1. Clone-after-push staleness — push a repo, clone it (works), push another commit or two, clone again → the new commits/blobs are missing from the clone, yet clone exits 0.
  2. #55 — push have-negotiation ignored sibling refs, causing redundant multi-hundred-MB uploads and broken pipes when a branch lags a sibling already on the remote.

Architecture recap (for onboarding)

All four commands move data as a single binary mpack (b"MUSE" magic, section table, 32-byte SHA-256 footer). Objects are content-addressed: sha256:<hex> is the integrity check.

Command Client entry Server endpoint(s) Notes
push muse/cli/commands/push.pytransport.push_mpack_* POST /push/mpack-presign → PUT to R2 → POST /push/unpack-mpack client builds delta mpack, uploads direct to R2, server indexes
clone muse/cli/commands/clone.py POST /fetch/mpack with have=[] no dedicated clone endpoint — clone is a full fetch served from the prebuilt-mpack cache
fetch muse/cli/commands/fetch.pytransport.fetch_mpack POST /fetch/mpack have/want delta
pull muse/cli/commands/pull.py (reuses fetch_mpack) POST /fetch/mpack fetch + merge/ff

Server push pipeline (musehub_wire_push.py::wire_push_unpack_mpack): reads mpack from R2 → inserts objects/snapshots/commits → computes MusehubCommitGraph generations (step 9, synchronous) → advances branch pointer (step 10, atomic CAS) → enqueues async jobs (step 11): mpack.index, fetch.mpack.prebuild, intel.*, gc.

Server fetch pipeline (musehub_wire_fetch.py::wire_fetch_mpack): on have=[] (clone), looks up musehub_fetch_mpack_cache — HIT requires every want tip cached AND all pointing to one mpack_id; MISS raises MPackNotReadyError (HTTP 503 + Retry-After). Delta builds walk the commit graph via _walk_commit_delta.

Prebuild (process_fetch_mpack_prebuild_job): after each push, builds one combined mpack covering all branch tips and writes a cache row per tip → next clone is a sub-second cache HIT.


Confirmed root causes

RC-1 — Generation fallback to 0 silently corrupts the fetch walk (CRITICAL — the clone staleness bug)

musehub_wire_push.py:926:

_gen = (max(_parent_gens) + 1) if _parent_gens else 0

When a pushed commit's parent generation cannot be resolved — the parent is not in this mpack (it is on the remote, excluded by the have set) and its MusehubCommitGraph row is not returned by the lookup at musehub_wire_push.py:870-874 — the new commit is written with generation 0 instead of parent_gen + 1.

That single wrong generation poisons the fetch-side walk. _walk_commit_delta (musehub_wire_fetch.py:102-111) is a generation-bounded range scan:

.where(MusehubCommitGraph.generation > min_have_gen)
.where(MusehubCommitGraph.generation <= max_want_gen)

With the new tip at gen 0, max_want_gen = 0, so the scan excludes every real ancestor (gens 1..N). The BFS truncates, and the tip-snapshot resolver (musehub_wire_fetch.py:484-490, ORDER BY generation DESC LIMIT 1) selects an older commit's snapshot. The assembled mpack therefore carries the old tip's blobs but is cached against the new tip id. Clone returns 200 with a working tree missing the latest commits — exactly the reported symptom.

The fetch code already half-knows this — see the [BLOB-DEBUG] block at musehub_wire_fetch.py:495-509: "A mismatch means CommitGraph is stale for the newest push — the old snapshot's manifest_blob would be missing new blobs."

RC-2 — _walk_commit_delta has no fallback when the graph is inconsistent

musehub_wire_fetch.py:66if True: # fast path always active — makes the real parent-pointer DAG walk (the "legacy fallback" at lines 159-184) dead code. The generation range-scan is the only path. There is no safety net when generations are wrong or missing (RC-1), so a single bad row produces a silently truncated mpack instead of a correct (if slower) walk.

RC-3 — Job idempotency drops fresh payloads on back-to-back pushes

musehub_jobs.py:72-84 (enqueue_job) and :158-164 (enqueue_push_intel) dedup on (repo_id, job_type, status="pending"). If a fetch.mpack.prebuild (or mpack.index) job from push A is still pending when push B lands, push B's job is skipped. The pending job still carries push A's payload / tip set, so push B's new tip never gets a prebuild or index until yet another push displaces it. Rapid pushes ⇒ permanently stale clone cache.

RC-4 — Prebuild can run before index (foundation-job ordering race)

musehub_jobs.py:170-189: fetch.mpack.prebuild and mpack.index are both _FOUNDATION_TYPES and share the same created_at. claim_next_job (musehub_jobs.py:206-231) orders by created_at with no tiebreaker, so the prebuild may run first. wire_fetch_mpack then hits the index-coverage check (musehub_wire_fetch.py:580-593), raises FetchNotIndexedError, the prebuild swallows it (musehub_wire_fetch.py:1041), and no cache row is written. The clone then 503s until a retry/another push.

RC-5 — Client does not retry on 503 (clone-immediately-after-push fails)

The fetch route correctly returns 503 + Retry-After while the prebuild/index is in flight (wire.py:694-705). But the client (transport.py::fetch_mpack) raises TransportError(503), and clone.py:454-464 catches it, rmtrees the target, and exits INTERNAL_ERROR. There is no honor of Retry-After, no poll loop. A clone issued seconds after a push fails hard even though the server self-heals within a second or two.

RC-6 — The wire test suite is largely disabled ✅ Fixed by MWP-6

~19 wire/mpack test files are skipped, 11 with reason="muse wire protocol in flux" — including test_issue_61_clone_after_push.py, the exact clone-after-push contract. The protocol's own regression net is off, which is why these regressions reach manual testing. MVP cannot be declared with the suite dark.

Fixed: MWP-6 revived all flux-skipped wire files across 6 phases. 55 tests under pytest -m wire now run green (47 in-process + 8 slow subprocess E2E). The MWP6_00 gate (test_mwp6_wire_suite_enabled.py) enforces no-flux-skip forever.

RC-8 — Repair endpoints and force-push never invalidate musehub_fetch_mpack_cache

wire_repair_object, wire_repair_snapshot, and wire_repair_commit (musehub_wire_push.py) each end with await session.commit()return {"repaired": True} and never touch musehub_fetch_mpack_cache. Likewise, wire_push_unpack_mpack with force=True advances the branch pointer and enqueues intel jobs but never deletes old-tip cache rows.

The cache is keyed by (repo_id, tip_commit_id) with a 7-day TTL. After a repair, the prebuilt mpack cached against the tip still carries the pre-repair bytes — the repair fixed the DB row but the fetch path presigns whatever mpack_id the cache row addresses and exits 0. A clone issued within 7 days of the repair receives corrupted content.

This was the mechanism behind the gabriel/muse rc10 incident: two main-line merge commits had their signer_public_key field stamped by the rc10 migration. repair-commit fixed the musehub_commits rows — but staging clones stayed broken because the prebuilt mpack kept being served. The manual workaround was:

UPDATE musehub_fetch_mpack_cache SET expires_at = NOW() - INTERVAL '1 second' WHERE repo_id = ...

A correct fix requires active invalidation in the same DB transaction as the repair, so a cache row can never outlive its corresponding object/snapshot/commit content.

RC-7 — #55 (sibling-ref push negotiation): main path fixed, needs verification + dry-run alignment

The live-push path already uses all sibling refs as the walk boundary (push.py:819-822, branch_have = remote_branch_heads.values()), and blob dedup subtracts the sibling tip's full manifest (mpack.py:330-336, 362). Client tests exist (tests/test_push_branch_have.py, tests/test_push_have_filter.py). What remains: (a) confirm #55 is actually resolved end-to-end against staging and close it, and (b) the dry-run path still scopes have to a single branch (push.py:668-674) — this is #32 and should be aligned so --dry-run counts match the real push.


Goal — definition of MVP done

aaronrene (or any dev) can, against staging, reliably:

  1. muse clone <url> — get a complete, correct working tree every time, including immediately after a push (with bounded client retry on 503).
  2. muse push — upload only the true delta; never re-upload objects already on the remote via any ref; survive back-to-back pushes.
  3. muse fetch / muse pull — receive exactly the new commits/blobs and converge.
  4. Do all of the above repeatedly with no stale clones, no phantom large uploads, no manual cache-busting.
  5. The wire regression suite is re-enabled and green as the proof.

Proposed sub-tickets

Create as needed; each is independently shippable. Repo target in brackets.

  • MWP-1 [musehub] — Make commit-graph generations authoritative (fixes RC-1).COMPLETE → filed as musehub#106 (closed): https://staging.musehub.ai/gabriel/musehub/issues/106 Never write generation 0 for a commit that has parents. If a parent generation is unresolved at push time, resolve it (recursive lookup / on-demand backfill) or fail the push loudly. Add an invariant check + regression test for the push-child-of-remote-commit case. Shipped in 6 phases (13 tests, tests/test_mwp1_generation_authority.py): _resolve_generation_with_backfill (on-demand bottom-up backfill from musehub_commits), repair_corrupt_commit_generations (idempotent repair of existing artefacts), check_commit_graph_invariant (no gen-0-with-parents), the [MWP1_13] fetch-side repair guard, and the MWP1_14 end-to-end push→clone→push→clone regression gate. Merged to dev + main (sha256:da49a05fd62cda…).
  • MWP-2 [musehub] — Add a correctness fallback to _walk_commit_delta (fixes RC-2). ✅ CLOSED → musehub#107 closed: https://staging.musehub.ai/gabriel/musehub/issues/107 When the generation range-scan truncates (a wrong/missing generation makes the BFS dead-end), fall back to the authoritative MusehubCommit.parent_ids DAG walk so the returned commit set is never incomplete. Removed the if True: dead-code guard (line 66); promoted the legacy walk to a tested helper (_walk_commit_delta_dag) with Kahn's topo-sort (deque.popleft(), O(n)) and max_nodes=100_000 bound. 23 tests GREEN (10 MWP2 + 13 MWP1). All 6 acceptance criteria met. MWP2_12 (acceptance gate) proves fallback alone ships a complete clone even with repair patched out.
  • MWP-3 [musehub] — Fix job enqueue for back-to-back pushes (fixes RC-3). ✅ COMPLETE → musehub#108 (closed): https://staging.musehub.ai/gabriel/musehub/issues/108 fetch.mpack.prebuild / mpack.index must always reflect the latest tip set. Two-face fix shipped: (1) process_fetch_mpack_prebuild_job re-reads live MusehubBranch tips at runtime — coarse dedup becomes self-coalescing; (2) enqueue_job deduped per mpack_key via JSONB predicate — distinct mpacks are non-substitutable work and can never be dropped by a coarse pending-job check. RC-4 ordering is explicitly out of scope (→ MWP-4). 36 tests GREEN across 5 suites (14 MWP-3 + adjacent regressions fixed). MWP3_10 (two-push full drain) + MWP3_11 (N=10 stress partial drain) prove E2E correctness end-to-end.
  • MWP-4 [musehub] — Order prebuild strictly after index (fixes RC-4).COMPLETEmusehub#109 (closed): https://staging.musehub.ai/gabriel/musehub/issues/109 Three-layer fix shipped in 5 phases (12 tests, tests/test_mwp4_prebuild_ordering.py): (1) created_at stagger — prebuild gets t+1s, index stays at t+0s (defense-in-depth); (2) correlated NOT EXISTS barrier in claim_next_job — prebuild un-claimable while any mpack.index for the same repo_id is pending/running (load-bearing); (3) narrow except FetchNotIndexedError: raise in process_fetch_mpack_prebuild_job — index gaps surface as retryable fail_job failures instead of silent empty-cache. MWP4_11 (E2E claim loop) + MWP4_12 (N=10 stress) prove zero FetchNotIndexedError and a correct cache at the latest tip after any burst of back-to-back pushes. Commit range: sha256:2c523da453513sha256:46854101acdde on task/mwp-4-prebuild-after-index.
  • MWP-5 [muse] — Clone/fetch honor Retry-After with a bounded poll (fixes RC-5).COMPLETEmuse#59 (closed): https://staging.musehub.ai/gabriel/muse/issues/59 Extracted HttpTransport._fetch_mpack_once; added bounded retry loop in fetch_mpack tracking total_waited against a configurable budget (default 120s). `--retry-timeout` / `--no-retry` flags on `clone`, `fetch`, `pull`. Terminal 503 message improved; JSON envelope carries `retryable: true`. 19 tests GREEN (MWP5_00–MWP5_18 including real-urllib E2E on a live localhost server). Zero regressions across adjacent suites.
  • MWP-6 [musehub + muse] — Re-enable the wire regression suite (fixes RC-6). Un-skip test_issue_61_clone_after_push.py and the ..._flux mpack/wire tests; fix or rewrite each against the current protocol; gate MVP on green.
  • MWP-7 [muse] — Verify and close #55; align --dry-run negotiation (RC-7 / #32).COMPLETEmuse#62 (closed): https://staging.musehub.ai/gabriel/muse/issues/62 Five-phase E2E + TDD campaign on task/mwp-7-sibling-negotiation. Phase 1/2 (GT/LF): 9 unit tests confirm the live-push sibling-ref negotiation path is correct (push.py:819-822, branch_have = remote_branch_heads.values()). Phase 3 (VS): real staging push with a 17-commit dev + 1-commit main topology — commits_sent=1, objects_sent=1; clone verified integrity all_ok:true. #55 closed. Phase 4 (DR): fixed dry-run have-set: replaced get_remote_head(push_branch) with _all_known_have_anchors(root) (all sibling tracking refs) — matches live-path logic. Phase 5 (RG): removed c != local_head filter so up-to-date dry-runs report 0 correctly. 21 tests GREEN (pytest -m wire); 85 adjacent push-corpus tests GREEN. #32 closed. Commit: sha256:6ec37c93009f7c30451ea8db62a0a1b764bb79fa93231b5506bb6855251b3e5d.
  • MWP-8 [musehub] — Cache invalidation for repair and force-push (fixes RC-8).COMPLETEmusehub#111 (closed): https://staging.musehub.ai/gabriel/musehub/issues/111 Five-phase TDD campaign on task/mwp-8-cache-invalidation. Phase 0 (AU): proved the stale-HIT gap — AU_01..AU_05 go RED before any fix. Phase 1 (IV): added invalidate_fetch_mpack_cache(session, repo_id, tips=None) to musehub_gc.py; deletes rows AND best-effort-deletes their R2 objects regardless of TTL; tips=None → repo-wide (repair path), tips=[...] → tip-scoped (force-push). Does NOT commit — caller owns the transaction (D2). 6 unit tests GREEN (IV_01..IV_06). Phase 2 (RP): wired invalidation into wire_repair_object, wire_repair_snapshot, wire_repair_commit; each calls invalidate_fetch_mpack_cache then _enqueue_repair_prebuild in the same transaction before session.commit(). 4 contract tests GREEN (RP_04..RP_07 incl. atomicity rollback guard). Phase 3 (FP): wired tip-scoped invalidation into wire_push_unpack_mpack force-push path; only old+new tips busted — sibling cache rows survive (D1/D3 design decisions). 4 tests GREEN (FP_02..FP_05 incl. blast-radius and prebuild-still-enqueued proofs). Phase 4 (EX): added _enqueue_repair_prebuild helper; verified cache-bust logged via caplog; EX_02 is the E2E narrative (@pytest.mark.wire): repair → MISS → prebuild → HIT serves fresh content. 3 tests GREEN (EX_01..EX_03). Phase 5 (RG): 54 adjacent corpus tests GREEN; 16 MWP-8 tests collected + GREEN; master tracker (muse#58) updated; musehub#111 closed with audit verdict. 16 tests total in tests/test_mwp8_cache_invalidation.py.

Acceptance criteria

  • [x] Push C1→C2→C3, clone, push C4, clone again → second clone contains C4 and its blobs (RC-1, RC-2). Codified as a non-skipped regression test. ✅ Fixed by MWP-1 + MWP-2
  • [x] Two pushes within one prebuild interval both end with a correct, fresh clone cache (RC-3 ✅ fixed by MWP-3, RC-4 ✅ fixed by MWP-4).
  • [x] muse clone issued immediately after muse push succeeds via bounded retry, never hard-fails on the first 503 (RC-5). ✅ Fixed by MWP-5 (muse#59)
  • [x] Pushing main when it lags an already-pushed dev sends only the delta (one merge commit, ~zero new blobs); #55 closed with evidence (RC-7). ✅ Fixed by MWP-7
  • [x] --dry-run commit/object counts equal the real push counts (#32). ✅ Fixed by MWP-7
  • [x] ✅ The previously-skipped wire/mpack test suite is re-enabled and green (RC-6). Fixed by MWP-6.
  • [x] A repaired object/snapshot/commit is never masked by a stale prebuilt mpack — cache invalidated in the same transaction as the repair; clone issued immediately after repair returns the corrected content (RC-8). ✅ Fixed by MWP-8 (musehub#111)

Out of scope

  • Any redesign of the mpack binary format or the presigned-upload model — both are sound.
  • Per-object presign / fetch/objects fallback paths beyond what clone/fetch need for MVP.
  • Shallow-clone (--depth) hardening beyond current behavior.
  • Code-intel / structural-intel jobs (independent of correctness of the wire data path).
File History 2 commits
sha256:ac29b8ba9021514a03ab2782d92bf67671f0efa5b3b70d46f7598c5d4e923378 docs: record muse#74 Phase 4 live verification (SCR_13, SCR_14) Sonnet 5 13 days ago
sha256:2562dffa0a0822ac1bdea854f9b267843c6ce95b497a9dc5c55837c80a3ebd0a feat: domain_command_registry — Phase 1 of muse#74 (SCR_01-03) Sonnet 4.6 patch 13 days ago