gabriel / musehub public
mwp-6-reenable-wire-suite.md markdown
397 lines 23.2 KB
Raw
sha256:73f24973678ceefd56628ee17c6d0535d86e33533828af6c63348f50941515ad Merge branch 'fix/smoke-test-tag-label' into dev Human 15 days ago

MWP-6 — Re-enable the wire regression suite (fixes RC-6)

Master tracker: muse#58 — https://staging.musehub.ai/gabriel/muse/issues/58 Predecessors (all closed):

  • musehub#106 (MWP-1, generation authority — RC-1)
  • musehub#107 (MWP-2, walk fallback — RC-2)
  • musehub#108 (MWP-3, job-enqueue idempotency — RC-3)
  • musehub#109 (MWP-4, prebuild-after-index ordering — RC-4)
  • muse#59 (MWP-5, client Retry-After bounded poll — RC-5)

Repo target: musehub (the server), with one subprocess-E2E that also drives the muse client. MWP-1..5 rewrote the wire protocol from under a large body of tests that were mass-skipped with pytest.mark.skip(reason="muse wire protocol in flux") while the protocol was unstable. The protocol is now frozen (MWP-1..5 shipped). This ticket turns the regression net back on so the MVP can be declared with proof, not by hand.

This ticket is written to be picked up cold by another agent. It is dense on purpose. Every phase is TDD. Every test file is pre-classified (REVIVE / REWRITE / DELETE) with the contract reasoning. Symbol anchors point code-intelligence at the exact current call sites — run muse -C ~/ecosystem/musehub code cat "<anchor>" --json to read each before you touch it. Do not un-skip blindly — a skip whose premise contradicts the shipped architecture must be deleted, not revived (see Phase 2).


⚠️ Sub-agent / environment preamble — read first

This project uses Muse, not git.

  • Never run git, gh, or any git subcommand. No GitHub.
  • Repo paths: ~/ecosystem/muse, ~/ecosystem/musehub, ~/ecosystem/agentception.
  • Use muse -C ~/ecosystem/musehub <command> when CWD differs.
  • Use muse code grep / symbols / cat / impact / deps before reading files raw. Every command accepts --json.
  • Never run the full test suite. Run one file at a time: python3 -m pytest tests/test_xxx.py -q --tb=short. The wire suite needs a running Postgres (see Test infrastructure below); the storage backend is faked in-process, so MinIO/R2 is not needed except for the Phase 5 E2E.
  • Commit with full provenance: muse commit -m "..." --agent-id claude-code --model-id <model> --sign.
  • Work on a feature branch off dev. Never commit to dev/main directly. Never merge, reset, or delete anything without gabriel's explicit per-action permission.

Background

The symptom (RC-6)

A large slice of the wire/mpack regression suite is dark. The protocol's own safety net — the tests that would have caught RC-1 through RC-5 before they reached manual testing — is switched off. MVP cannot be declared while the suite that proves it is skipped.

Why it happened

While MWP-1..5 were in flight the wire protocol changed shape repeatedly (synchronous push → async jobs → prebuild/index pipeline → generation authority). Rather than fix tests against a moving target, they were mass-annotated pytest.mark.skip(reason="muse wire protocol in flux"). That was the right call then. It is the wrong state now that MWP-1..5 froze the contract.

The trap that makes this load-bearing, not mechanical

Some skipped files encode mutually contradictory design eras:

  • tests/test_mpack_push_async.py asserts *"mpack push is async: fast sync path
    • background indexing."*
  • tests/test_mpack_phase3.py asserts "Phase 3: make push fully synchronous (issue #69) — wire_push_unpack_mpack must NOT enqueue a mpack.index job."

These cannot both be true. The shipped architecture (MWP-3/MWP-4) is async with background jobsenqueue_job, process_mpack_index_job, process_fetch_mpack_prebuild_job, claim_next_job all exist and are load bearing. Therefore test_mpack_phase3.py's premise is obsolete and the file must be deleted, not revived. Un-skipping it blindly would re-assert a contract the protocol deliberately abandoned.

The core skill this ticket demands: judge each skipped test against the frozen current contract and decide revive / rewrite / delete — with a written reason.


Current contract — the frozen surface you are testing against

Read these before classifying anything. All anchors are current (verified at filing).

Symbol Location Role
wire_push_unpack_mpack musehub/services/musehub_wire_push.py:508 reads mpack from backend → inserts objects/snapshots/commits → computes generations (sync) → advances branch (CAS) → enqueues async jobs
wire_push_mpack_presign musehub/services/musehub_wire_push.py:496 issues the presigned upload URL
process_mpack_index_job(session, job_id) -> JSONObject musehub/services/musehub_wire_push.py:1537 the mpack.index job — object/ref writes, validation, content scan
wire_fetch_mpack musehub/services/musehub_wire_fetch.py:414 the single endpoint clone/fetch/pull hit; cache HIT or 503 + Retry-After
process_fetch_mpack_prebuild_job(...) musehub/services/musehub_wire_fetch.py:1034 builds the combined prebuilt mpack per branch tip
process_mpack_gc_job(session, repo_id) -> MPackGCResult musehub/services/musehub_wire_fetch.py:913 consolidates per-push index rows into one mpack
_walk_commit_delta musehub/services/musehub_wire_fetch.py generation range-scan + DAG fallback (MWP-2)
enqueue_job(...) musehub/services/musehub_jobs.py:58 dedup per mpack_key (MWP-3)
enqueue_push_intel(...) musehub/services/musehub_jobs.py:116 enqueues the intel job family
claim_next_job(session) musehub/services/musehub_jobs.py:284 created_at order + prebuild-after-index barrier (MWP-4)

Known architectural facts that invalidate some old test premises:

  1. Push is async, not synchronous. Any test asserting "push must NOT enqueue a job" or "fetch succeeds with zero indexing latency" is testing the abandoned issue-#69 design → DELETE.
  2. Intel jobs are deliberately no-ops on the worker. The musehub worker runs only mpack.index + fetch.mpack.prebuild; the ~17 intel.* jobs are intentionally disabled. A revived test that asserts an intel job executes work is obsolete. (The mpack.index job's own content-scan/abuse checks do run — those are not "intel" jobs. Distinguish carefully.)
  3. Generations are authoritative (MWP-1). No commit with parents may carry generation 0. Tests may now assert this invariant rather than working around it.
  4. _walk_commit_delta has a DAG fallback (MWP-2). The if True: dead-code guard is gone; the fallback is live.
  5. Fetch on 503 carries Retry-After (server) and the client honors it (MWP-5). A revived fetch test may assert the 503 + header shape directly.

Test infrastructure — how the suite actually runs

Confirmed by reading tests/conftest.py:

  • DB: real PostgreSQL. _db_schema (session-scoped, conftest.py:203) does one DDL pass per session; db_session (conftest.py:282) gives each test a transaction rolled back at teardown. Postgres must be running for any wire test. The pipeline uses PG-specific constructs (pg_insert, JSONB) — do not attempt to retarget to sqlite.
  • Storage: MemoryBackend() (conftest.py:84-98) is monkeypatched in for get_backend across every wire module (_wire_svc, _wire_fetch_svc, _wire_push_svc, _wire_shared_svc, _gc_svc, _wire_route, repair). So in-process tests need no MinIO/R2.
  • In-process ASGI tier: client fixture (conftest.py:357) = httpx AsyncClient over ASGITransport(app) with the DB dependency overridden. 14 of the 15 flux files use this tier.
  • Subprocess-E2E tier: tests/test_issue_61_clone_after_push.py shells out to the real muse CLI against a live https://localhost:1337 hub. This needs the container running and must be marked slow.
  • Markers (pyproject.toml:59): slow (external infra), tier2 (integration, real DB). There is no wire marker yet — Phase 6 adds one.

Full inventory — every skipped wire/mpack test, pre-classified

pytest.mark.skip(reason="muse wire protocol in flux") unless noted. Tier = in-process unless marked E2E. Classification is the filer's recommendation; the pick-up agent must confirm against the current contract and record the final call in the deliverables.

Module-level "in flux" skips (11 files — matches RC-6's count)

# File tests tier recommended why
1 test_issue_61_clone_after_push.py 8 E2E REVIVE (Phase 5) the named RC-6 contract; re-validate root cause vs MWP-1 schema
2 test_mpack_index_job.py 6 in-proc REVIVE process_mpack_index_job exists @ push.py:1537; re-anchor
3 test_mpack_index_phase2.py 4 in-proc REVIVE index covers commits+snapshots+objects; contract intact
4 test_mpack_index_job_phase3.py 3 in-proc REVIVE/REWRITE "synchronous & inline" header is stale wording; the invariants (idempotency, rollback) still hold — rewrite the framing, keep the asserts
5 test_mpack_index_job_phase4.py 5 in-proc REVIVE observability timing dict from process_mpack_index_job; verify result keys
6 test_mpack_validation_phase2.py 8 in-proc REVIVE mpack content validation in the index job
7 test_mpack_fetch_phase3.py 4 in-proc REVIEW→REWRITE PF-1..3 valid; PF-4 "legacy inline-bytes fallback" is likely dead — confirm + drop that case
8 test_mpack_gc_phase4.py 6 in-proc REVIVE process_mpack_gc_job(session, repo_id) exists @ fetch.py:913
9 test_mpack_push_async.py 3 in-proc DELETE or REVIVE — pick ONE vs #10 async-push premise; cannot coexist with test_mpack_phase3.py
10 test_mpack_phase3.py 3 in-proc DELETE "fully synchronous push, must NOT enqueue job" (issue #69) contradicts shipped async pipeline
11 test_push_ff_check.py 5 in-proc REVIVE server-side fast-forward check; collects clean today (5 coroutines)

Function-level "in flux" skips (4 files)

# File skipped / total recommended why
12 test_mpack_delta_e2e.py 1 / 2 REVIVE delta fetch end-to-end
13 test_mpack_content_scanning_phase3.py 5 / 10 REVIVE content scan runs inside process_mpack_index_job (not an "intel" job)
14 test_mpack_size_gates.py 3 / 6 REVIVE mpack size limits
15 test_mpack_rate_limiting_phase4.py 2 / 15 REVIVE push/fetch rate limiting

Stale skipif guards — already auto-running, need cleanup (4 files)

These were guarded on process_mpack_index_job being importable. It now is (re-exported from musehub.services.musehub_wire), so the guard is dead and the tests already run. Confirm green and remove the dead try/except + skipif.

# File guard action
16 test_mpack_phase2.py _PROCESS_JOB_MISSING (3 fns) remove dead guard; confirm green
17 test_commit_graph_phase2.py _PROCESS_JOB_MISSING (4 fns) remove dead guard; confirm green
18 test_mpack_index_phase1.py module skip "process_mpack_index_job removed — revisit with MWP pack index wiring" the job is back; REVIVE or DELETE if superseded by index_phase2
19 test_clone_xs_unit.py / test_fetch_xs_unit.py skipif (bench unit) verify guard condition; un-skip if stale

Total: ~19 files (RC-6's "~19").


Goal — definition of done

  1. Zero pytest.mark.skip(reason="muse wire protocol in flux") remain in the suite. Enforced by a meta-test (Phase 0) that greps the test tree and fails if any such marker survives.
  2. Every revived file is green against the current contract, run one file at a time. Every deleted file has a one-line written justification tying it to a superseded design (issue #69 sync-push, dead inline-bytes fallback, etc.).
  3. A wire pytest marker exists; every revived wire file carries it; the suite is selectable as a gate (pytest -m wire).
  4. The subprocess-E2E clone-after-push contract (issue #61) passes against a live localhost hub, marked slow, and its root-cause claim is re-validated against the MWP-1 generation-authority schema (either the bug is gone and the test proves it, or the test is rewritten to the current dedup semantics).
  5. muse#58 RC-6 + its acceptance criterion "The previously-skipped wire/mpack test suite is re-enabled and green" are ticked, with the green run as evidence.

Phases — load-bearing, TDD, never skip ahead

Each phase must be fully green before the next begins. Ordering is by cost and blast radius: cheapest/safest first (dead-guard removal), then deletions (reduce surface), then fast in-process revivals, then the slow E2E, then the gate.

Phase 0 — Triage harness + the master anti-skip gate (RED)

Anchors: none (new meta-test).

  • [ ] Create tests/test_mwp6_wire_suite_enabled.py.
  • [ ] MWP6_00 — meta-test that walks tests/*.py and asserts no pytest.mark.skip(reason="muse wire protocol in flux") string appears in any file. Starts RED (15 files still carry it). Flips GREEN only when the last file is revived or deleted in Phase 6. This is the master gate.
  • [ ] MWP6_01 — produce a written triage table (REVIVE/REWRITE/DELETE + reason per file) as a module docstring or a committed docs/issues/mwp-6-triage.md. This is the load-bearing decision record.
  • [ ] Confirm Postgres is reachable for the suite (db_session smoke).

Phase 1 — Remove dead skipif guards (cheapest; no behavior change)

Anchors: tests/test_mpack_phase2.py, tests/test_commit_graph_phase2.py, tests/test_mpack_index_phase1.py, tests/test_clone_xs_unit.py, tests/test_fetch_xs_unit.py.

  • [ ] MWP6_10 — confirm from musehub.services.musehub_wire import process_mpack_index_job succeeds (it does today); remove the try/except ImportError + _PROCESS_JOB_MISSING + the three/four skipif decorators in test_mpack_phase2.py and test_commit_graph_phase2.py. Run each file — must stay green.
  • [ ] MWP6_11 — resolve test_mpack_index_phase1.py (module skip "job removed"): the job is back. Either REVIVE (re-anchor to push.py:1537) or DELETE if test_mpack_index_phase2.py fully supersedes it. Record which.
  • [ ] MWP6_12 — inspect the skipif conditions in test_clone_xs_unit.py / test_fetch_xs_unit.py; un-skip if the guard is stale, else document why it legitimately remains.

Phase 2 — Delete obsolete-premise tests (shrink the surface before reviving)

Anchors: tests/test_mpack_phase3.py, tests/test_mpack_push_async.py, tests/test_mpack_fetch_phase3.py::PF-4, musehub/services/musehub_wire_push.py::wire_push_unpack_mpack, musehub/services/musehub_jobs.py::enqueue_job.

  • [ ] MWP6_20 — confirm the shipped contract: wire_push_unpack_mpack does enqueue mpack.index via enqueue_job (read push.py:508 + jobs.py:58). This proves issue-#69 "fully synchronous, no job" is abandoned.
  • [ ] MWP6_21DELETE test_mpack_phase3.py (premise contradicts the shipped async pipeline). muse rm tests/test_mpack_phase3.py. Justify in the triage record.
  • [ ] MWP6_22 — decide test_mpack_push_async.py vs the deleted phase3: if its async-push asserts match the current contract, REVIVE it (Phase 3); if it duplicates newer coverage, DELETE. Exactly one of phase3/push_async may survive; document the call.
  • [ ] MWP6_23 — in test_mpack_fetch_phase3.py, confirm whether the "legacy inline-bytes fallback" path (PF-4) still exists in wire_fetch_mpack (fetch.py:414). If gone, delete that one test case; keep PF-1..3 for Phase 4.

Phase 3 — Revive the index/validation job family (in-process, fast)

Anchors: musehub/services/musehub_wire_push.py::process_mpack_index_job, tests/test_mpack_index_job.py, tests/test_mpack_index_phase2.py, tests/test_mpack_index_job_phase3.py, tests/test_mpack_index_job_phase4.py, tests/test_mpack_validation_phase2.py.

  • [ ] MWP6_30test_mpack_index_phase2.py: un-skip; re-anchor to the current process_mpack_index_job(session, job_id) signature; assert every commit/snapshot/object id in the mpack gets a row. GREEN.
  • [ ] MWP6_31test_mpack_index_job.py: object writes + MusehubObjectRef + idempotency (running the job twice is a no-op). GREEN.
  • [ ] MWP6_32test_mpack_index_job_phase3.py: rewrite stale "synchronous & inline" framing; keep the idempotency/rollback-safety invariants. GREEN.
  • [ ] MWP6_33test_mpack_index_job_phase4.py: assert the result dict keys a monitoring dashboard consumes (verify against the actual return at push.py:1537). GREEN.
  • [ ] MWP6_34test_mpack_validation_phase2.py: malformed-mpack rejection inside the index job. GREEN.

Phase 4 — Revive fetch / gc / ff / delta / scan / size / rate (in-process)

Anchors: musehub/services/musehub_wire_fetch.py::wire_fetch_mpack, ::process_mpack_gc_job, musehub/services/musehub_wire_push.py::wire_push_unpack_mpack, and the seven test files below.

  • [ ] MWP6_40test_mpack_fetch_phase3.py (PF-1..3): fetch returns presigned mpack URL(s) + correct commit/snapshot metadata. GREEN.
  • [ ] MWP6_41test_mpack_gc_phase4.py: process_mpack_gc_job merges per-push index rows into one; idempotent; post-GC fetch returns exactly one mpack_url. GREEN.
  • [ ] MWP6_42test_push_ff_check.py: ff advances, non-ff rejected without force, allowed with force, new branch ok, same-head no-op (5 tests). GREEN.
  • [ ] MWP6_43test_mpack_delta_e2e.py: the one skipped delta test. GREEN.
  • [ ] MWP6_44test_mpack_content_scanning_phase3.py (5 fns): content scan inside process_mpack_index_job (NOT an intel job — it runs). GREEN.
  • [ ] MWP6_45test_mpack_size_gates.py (3 fns): oversize mpack rejection. GREEN.
  • [ ] MWP6_46test_mpack_rate_limiting_phase4.py (2 fns): push/fetch rate limits. GREEN.

Phase 5 — Revive the subprocess-E2E clone-after-push contract (issue #61, slow)

Anchors: tests/test_issue_61_clone_after_push.py, musehub/services/musehub_wire_push.py::wire_push_unpack_mpack, musehub/services/musehub_wire_fetch.py::_walk_commit_delta.

  • [ ] MWP6_50 — re-validate the file's root-cause claim (musehub_commits.commit_id sole PK ⇒ cross-repo dedup drops Repo B's fetch) against the current schema. MWP-1 made generations authoritative and added repair/backfill — determine whether cross-repo dedup is now correct. If the bug is fixed, the test should PASS as-is (proving it); if the schema changed shape, rewrite the assertions to the current dedup model.
  • [ ] MWP6_51 — mark the module @pytest.mark.slow (it needs a live localhost:1337 container) and remove the flux skip. Document the run command and container prerequisite in the file docstring.
  • [ ] MWP6_52 — run against a live hub: tracking-ref hygiene (3), clone-after-push-from-copytree (5), cross-repo dedup (1). All GREEN, or the surviving subset GREEN with deletions justified.

Phase 6 — wire marker, gate, and close

Anchors: pyproject.toml ([tool.pytest.ini_options].markers), tests/test_mwp6_wire_suite_enabled.py.

  • [ ] MWP6_60 — register a wire marker in pyproject.toml markers list; tag every revived wire file (pytestmark = pytest.mark.wire, composed with slow where needed). pytest -m wire --collect-only lists the full set.
  • [ ] MWP6_61 — MWP6_00 (the master anti-skip gate) is now GREEN — no flux skip remains anywhere.
  • [ ] MWP6_62 — update muse#58: tick RC-6 in Confirmed root causes and the acceptance criterion "The previously-skipped wire/mpack test suite is re-enabled and green". Edit the local ~/ecosystem/muse/docs/issues/mwp-mvp-master.md first, then push --body-file per the local-file rule.
  • [ ] MWP6_63 — post a closing comment: per-file final disposition (revived/rewritten/deleted + reason), total test count re-enabled, the green run evidence, and the commit range. Close this issue.

Acceptance criteria

  • [ ] No pytest.mark.skip(reason="muse wire protocol in flux") anywhere in tests/ (MWP6_00 green).
  • [ ] Every revived file passes run individually (pytest tests/test_xxx.py -q).
  • [ ] Every deleted file has a written justification tied to a superseded design.
  • [ ] test_mpack_phase3.py is deleted (issue-#69 sync-push premise) OR a written argument is recorded for why it survives over test_mpack_push_async.py (not both).
  • [ ] A wire marker exists and selects the full revived suite.
  • [ ] The issue-#61 clone-after-push E2E passes against a live hub (marked slow), root cause re-validated against the MWP-1 schema.
  • [ ] muse#58 RC-6 + its acceptance criterion are ticked; this issue closed with evidence.

Out of scope

  • Re-enabling the intel.* job family. Those are deliberately no-ops on the worker (only mpack.index + fetch.mpack.prebuild run). Any test asserting an intel job executes work is obsolete and should be deleted, not revived.
  • New protocol features. This is a test-revival pass against the frozen MWP-1..5 contract — no production wire-path behavior changes except those a revived test legitimately reveals as a regression (escalate those separately).
  • The non-wire skipped tests (test_proposal_type_badge.py → musehub#96, test_bench_cli_seed.py "hub-side seed push not implemented", test_genesis_ids.py CLI-genesis guard). Different root causes; not RC-6.
  • sqlite retargeting. The suite is Postgres-only by design (JSONB, pg_insert).

Symbol-anchor appendix — read before you touch

# Frozen server contract (read-only unless a revived test reveals a real bug)
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_wire_push.py::wire_push_unpack_mpack" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_wire_push.py::process_mpack_index_job" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_wire_fetch.py::wire_fetch_mpack" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_wire_fetch.py::process_mpack_gc_job" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_wire_fetch.py::process_fetch_mpack_prebuild_job" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_jobs.py::enqueue_job" --json
muse -C ~/ecosystem/musehub code cat "musehub/services/musehub_jobs.py::claim_next_job" --json

# Blast radius before changing any production symbol a revived test touches
muse -C ~/ecosystem/musehub code impact "musehub/services/musehub_wire_push.py::process_mpack_index_job" --json
muse -C ~/ecosystem/musehub code impact "musehub/services/musehub_wire_fetch.py::wire_fetch_mpack" --json

# Test infra to mirror
#   tests/conftest.py:282  db_session   (per-test PG transaction)
#   tests/conftest.py:357  client       (httpx AsyncClient over ASGITransport)
#   tests/conftest.py:84   MemoryBackend monkeypatch (no MinIO in-process)
File History 3 commits
sha256:cfefc25a166c3c3eed8ea3529aee19ea350bc05f2954d007420e924133b7d8ce chore: pivot to nightly channel — bump version to 0.2.0.dev… Sonnet 5 patch 13 days ago
sha256:d035733f21ccff27735fddebfbbe0ed24565a32a22db8de5885402262671ecd2 chore: bump version to 0.2.0rc15 for musehub#113 fix release Sonnet 4.6 patch 15 days ago
sha256:0032d6cfa33bc3c8367436ad768e7dd0e339b4332153160247da8266cb5fa352 Merge branch 'task/version-tags-phase3-server' into dev Human 17 days ago