# MWP-4 — Gate `fetch.mpack.prebuild` behind `mpack.index` (fixes RC-4)

> **Master tracker:** muse#58 — https://staging.musehub.ai/gabriel/muse/issues/58
> **Predecessors (all closed):**
> - musehub#106 (MWP-1, generation authority)
> - musehub#107 (MWP-2, walk fallback)
> - musehub#108 (MWP-3, job-enqueue idempotency)
>
> This ticket is written to be picked up cold by another agent. It is dense on
> purpose. Every phase is TDD: write the test, watch it fail, implement until
> green, never skip ahead. Symbol anchors point code-intelligence at the exact
> call sites — use `muse code cat "<anchor>" --json` to read each one before you
> touch it.

---

## Background

### The symptom

A `muse clone` issued shortly after a `muse push` intermittently fails with
`503 MPackNotReadyError` (or returns and then a follow-up clone 503s), even
though the push succeeded and the objects are on the remote. A second clone a
moment later works. This is the last open correctness gap in the prebuild/index
pipeline after MWP-1/2/3.

### Why it happens — RC-4 (foundation-job ordering race)

Every push enqueues a batch of background jobs in
`enqueue_push_intel` (`musehub/services/musehub_jobs.py::enqueue_push_intel`).
Two of them gate clone correctness and **both currently share the same
`created_at` timestamp**:

- `mpack.index` — writes `MusehubMPackIndex` rows so the fetch path can prove
  every object a clone needs is retrievable.
- `fetch.mpack.prebuild` — builds the combined clone mpack for all live branch
  tips and writes `MusehubFetchMPackCache` rows.

The single worker claims jobs via
`musehub/services/musehub_jobs.py::claim_next_job`, which orders **only** by
`created_at`:

```python
.order_by(MusehubBackgroundJob.created_at)
.limit(1)
.with_for_update(skip_locked=True)
```

There is **no tiebreaker**. When `mpack.index` and `fetch.mpack.prebuild` carry
identical `created_at` values, PostgreSQL is free to return either first. If the
prebuild wins the tie, it runs **before** the index has written its rows.

The prebuild then calls
`musehub/services/musehub_wire_fetch.py::wire_fetch_mpack` with
`force_build=True`. That function runs the index-coverage check at
`musehub/services/musehub_wire_fetch.py:681-695`:

```python
indexed_q = await session.execute(
    select(MusehubMPackIndex.entity_id)
    .where(MusehubMPackIndex.entity_id.in_(list(new_oids)))
    .where(MusehubMPackIndex.entity_type == "object")
)
indexed_oids = {row[0] for row in indexed_q}
missing = new_oids - indexed_oids
if missing:
    logger.warning("[wire_fetch_mpack] step=3 NOT INDEXED %d/%d objects ...")
    raise FetchNotIndexedError(len(missing))      # ← raised here
```

`FetchNotIndexedError` is defined at
`musehub/services/musehub_wire_shared.py::FetchNotIndexedError`. It propagates up
into `process_fetch_mpack_prebuild_job`, where it is **silently swallowed** by
the broad handler at `musehub/services/musehub_wire_fetch.py:1164-1167`:

```python
except Exception as exc:
    logger.error("[fetch.mpack.prebuild] FAILED: %s", exc, exc_info=True)
```

Result: `tips_built` stays `0`, **no `MusehubFetchMPackCache` row is written**,
and the job nonetheless completes "successfully" in the worker
(`musehub/worker.py:121-131` → `complete_job`). The clone cache stays empty for
that tip until *another* push displaces it, so the next clone 503s.

### Why MWP-3 did not already fix this

MWP-3 made the prebuild **self-coalescing** (it re-reads live branch tips at run
time in `process_fetch_mpack_prebuild_job`, ignoring the stale payload tip set)
and made `mpack.index` dedup **per `mpack_key`** so distinct mpacks are never
dropped. That fixed *which work gets enqueued*. RC-4 is orthogonal: it is about
*the order in which already-enqueued work is claimed*. A correct, coalesced
prebuild job that simply runs too early still produces an empty cache.

### Worker-scope reality

The worker is **single-threaded** today
(`musehub/worker.py::_process_one` claims one job, runs it to completion, loops —
`musehub/worker.py:74-166`). Only `mpack.index` and `fetch.mpack.prebuild` are
live (`musehub/worker.py:107-108`: every other type falls through `pass`). So for
the single-worker case, **claim order == run order**, and a deterministic claim
gate fully closes the race. The fix is also designed to remain correct if the
worker is ever scaled to multiple processes (see Phase 2 rationale).

---

## Goal — definition of done

1. `claim_next_job` **never** returns a `fetch.mpack.prebuild` job while an
   `mpack.index` job for the **same repo** is `pending` or `running`.
2. After any sequence of back-to-back pushes, once the queue drains, the clone
   cache covers the latest tip of every branch, with **zero**
   `FetchNotIndexedError` occurrences during the drain.
3. A genuine index gap (e.g. a permanently-failed `mpack.index`) is **surfaced**,
   not masked as a successful empty prebuild.
4. `claim_next_job` gains its first direct test coverage (it has none today).
5. The MWP-4 suite plus all adjacent suites are green with no regressions.

---

## Design — chosen fix and rationale

The master tracker proposed two options: (a) give `fetch.mpack.prebuild` a later
`created_at` than `mpack.index`, or (b) make the prebuild depend on index
completion. **This ticket implements (b) as the load-bearing fix and (a) as
defense-in-depth**, because (a) alone is a timing heuristic that breaks under
back-to-back pushes and multi-worker claim.

### Why a timestamp stagger alone is insufficient

Consider two pushes A then B. With a pure stagger, push A's prebuild gets
`T_a+δ` and push B's index gets `T_b`. If `T_a+δ < T_b`, push A's prebuild is
claimed before push B's index. But MWP-3 made the prebuild read **live** tips —
so push A's prebuild tries to build push B's not-yet-indexed tip and hits
`FetchNotIndexedError`. The stagger fixes only the within-one-push ordering, not
the cross-push interleaving. A real dependency barrier fixes both.

### The load-bearing fix — a claim-layer dependency barrier

Modify `claim_next_job` so a `fetch.mpack.prebuild` row is **not claimable** while
any `mpack.index` for the same `repo_id` is `pending` or `running`. The prebuild
simply waits in `pending` until index work drains — no re-enqueue, no defer
counter, no busy-loop, correct under any worker count. Contract:

```python
from sqlalchemy import exists, select
from sqlalchemy.orm import aliased

_idx = aliased(MusehubBackgroundJob)
_index_blocking = (
    select(_idx.job_id)
    .where(
        _idx.repo_id == MusehubBackgroundJob.repo_id,        # correlated by repo
        _idx.job_type == "mpack.index",
        _idx.status.in_(("pending", "running")),
    )
)

result = await session.execute(
    select(MusehubBackgroundJob)
    .where(
        MusehubBackgroundJob.status == "pending",
        MusehubBackgroundJob.attempt < _MAX_ATTEMPTS,
        ~(
            (MusehubBackgroundJob.job_type == "fetch.mpack.prebuild")
            & _index_blocking.exists()
        ),
    )
    .order_by(MusehubBackgroundJob.created_at)
    .limit(1)
    .with_for_update(skip_locked=True)
)
```

Behavioural contract of the barrier:

| Scenario | Outcome |
|----------|---------|
| `mpack.index` `pending` for repo R | prebuild for R **not** claimable |
| `mpack.index` `running` for repo R | prebuild for R **not** claimable |
| `mpack.index` `done` / `failed` / `quarantined` for repo R | prebuild for R **claimable** (terminal states don't gate) |
| `mpack.index` `pending` for repo X | prebuild for repo Y **claimable** (per-repo) |
| claiming the `mpack.index` job itself | **unaffected** — barrier only filters `fetch.mpack.prebuild` |

> **Implementation note — `FOR UPDATE` + correlated `EXISTS`.** The `FOR UPDATE
> SKIP LOCKED` clause locks only the outer selected row; the `EXISTS` subquery is
> a read-only existence check, not a join, so PostgreSQL applies the row lock
> cleanly. Verify with the Phase 2 tests against the real DB session
> (`db_session` fixture) — do not assert this only against a mock.

> **Starvation note.** Under a relentless push stream, fresh `mpack.index` jobs
> keep arriving and the prebuild stays blocked. This is acceptable and correct:
> MWP-3 made the prebuild read live tips, so a single prebuild after the burst
> covers every tip. The barrier delays the prebuild to exactly when it can
> succeed; it never drops it. Steady-state push bursts are finite, so the queue
> drains and the prebuild runs once. Document this; do not add a timeout that
> would re-introduce the early-run race.

### Defense-in-depth — `created_at` stagger

Mirror the existing subtype stagger (`musehub/services/musehub_jobs.py:216-221`,
`created_at` vs `created_at_subtype = created_at + 2s`). Add a middle tier so the
prebuild sorts after the index foundation jobs even before the barrier is
consulted:

```
t+0s   mpack.index, intel.code, intel.structural, gc, ...   (foundation)
t+1s   fetch.mpack.prebuild                                  (NEW middle tier)
t+2s   intel.code.* subtypes
```

This keeps the common case race-free at the ordering layer and makes the barrier
a guarantee rather than the sole line of defence.

### Hardening — stop masking the index gap

Narrow the swallow in `process_fetch_mpack_prebuild_job`
(`musehub/services/musehub_wire_fetch.py:1164-1167`). Catch
`FetchNotIndexedError` **specifically**, log a structured warning, and **re-raise**
so the worker records it via `fail_job` (retry up to `_MAX_ATTEMPTS`). With the
barrier in place this path is only reachable when an `mpack.index` has terminally
failed — exactly the case that *should* surface as a failed job, not a silent
empty cache. Keep the broad `except Exception` for genuinely unexpected errors,
but it must no longer convert a missing index into a "successful" no-op.

---

## Phases

Each phase must be fully green before the next begins. Test file:
`tests/test_mwp4_prebuild_ordering.py`. Fixtures: reuse `db_session`,
`tests.factories.create_repo`, `tests.factories.create_branch`, and the MWP-3
E2E push helpers (`_e2e_push`, `_drain_index_jobs`) — copy/adapt from
`tests/test_mwp3_job_idempotency.py` rather than re-inventing.

### Phase 0 — RED reproduction (no production change)

- [x] **MWP4_01** RED — `claim_next_job` returns a `fetch.mpack.prebuild` ahead of
      a still-`pending` `mpack.index` for the same repo. Make it deterministic by
      inserting the prebuild with an **earlier** `created_at` than the index
      (simulates the cross-push interleaving). Pre-fix: claim returns the
      prebuild. This is the exact race. ✅ FAILING RED as expected.
- [x] **MWP4_02** RED — handler-level: with `mpack.index` not yet processed (index
      rows deleted after push to simulate the pre-index state), call
      `process_fetch_mpack_prebuild_job`; assert `wire_fetch_mpack` raises
      `FetchNotIndexedError`, the handler swallows it, and **no**
      `MusehubFetchMPackCache` row exists for the tip (documents the broken
      silent-no-cache outcome). ✅ PASSING — bug is proven.

### Phase 1 — `created_at` stagger (defense-in-depth) ✅ COMPLETE

Anchor: `musehub/services/musehub_jobs.py::enqueue_push_intel`.

- [x] **MWP4_03** — after one `enqueue_push_intel`, the `fetch.mpack.prebuild`
      row's `created_at` is strictly greater than every `mpack.index` row's
      `created_at`, and strictly less than the `intel.code.*` subtype tier.
      ✅ PASSING.
- [x] Implement the middle tier: removed `fetch.mpack.prebuild` from
      `_FOUNDATION_TYPES`; introduced `created_at_prebuild = created_at + 1s`
      via `_job_created_at(job_type)` helper. The three tiers are now:
      foundation (t+0s) → prebuild (t+1s) → subtypes (t+2s). ✅ SHIPPED.

### Phase 2 — claim-layer dependency barrier (load-bearing) ✅ COMPLETE

Anchor: `musehub/services/musehub_jobs.py::claim_next_job`.

- [x] **MWP4_04** — prebuild NOT claimed while an `mpack.index` is `pending` for
      the same repo (even when the prebuild has the earlier `created_at` from
      MWP4_01). `claim_next_job` returns the index (or `None` if only the blocked
      prebuild remains). ✅ PASSING.
- [x] **MWP4_05** — after the `mpack.index` is set `done`, the prebuild becomes
      claimable on the next `claim_next_job`. ✅ PASSING.
- [x] **MWP4_06** — an `mpack.index` in `running` state also blocks the prebuild.
      ✅ PASSING — claim returns `None` when only a blocked prebuild is pending.
- [x] **MWP4_07** — `failed` and `quarantined` `mpack.index` rows do **not** block
      the prebuild (terminal states are not gates). ✅ PASSING.
- [x] **MWP4_08** — per-repo isolation: `mpack.index` pending in repo X does not
      block a prebuild in repo Y. ✅ PASSING.
- [x] **MWP4_09** — the barrier filters only `fetch.mpack.prebuild`: a pending
      `mpack.index` is still claimable, and other job types are unaffected.
      ✅ PASSING.
- [x] Implemented correlated `~and_(job_type == 'fetch.mpack.prebuild', exists(...))`
      filter in `claim_next_job`. `FOR UPDATE SKIP LOCKED` locks only the outer
      row; the `EXISTS` subquery is read-only. Verified against real `db_session`.
      MWP4_01 (the original RED test) now passes. ✅ SHIPPED.

### Phase 3 — stop masking the index gap ✅ COMPLETE

Anchor: `musehub/services/musehub_wire_fetch.py::process_fetch_mpack_prebuild_job`.

- [x] **MWP4_10** — when `wire_fetch_mpack` raises `FetchNotIndexedError`, the
      handler logs and **re-raises** (does not return a success result with
      `tips_built=0`). Assert the exception propagates so the worker's `fail_job`
      path records it. Other exceptions still hit the broad handler unchanged.
      ✅ PASSING.
- [x] Narrowed `except Exception` at `musehub_wire_fetch.py:1164-1167` — added a
      specific `except FetchNotIndexedError: raise` guard before the broad handler.
      MWP4_02 (which previously documented the bug as RED passing) now confirms the
      error propagates correctly. ✅ SHIPPED.

### Phase 4 — integration + stress (real worker claim loop) ✅ COMPLETE

- [x] **MWP4_11** E2E — push once via the MWP-3 `_e2e_push` helper (enqueues real
      `mpack.index` + `fetch.mpack.prebuild`), then drive a claim loop
      (`claim_next_job` → dispatch → `complete_job`, mirroring
      `musehub/worker.py::_process_one`). Assert the index is claimed and
      completed **before** the prebuild is ever claimed, and a
      `MusehubFetchMPackCache` row lands at the pushed tip. ✅ PASSING.
- [x] **MWP4_12** stress — N=10 back-to-back pushes (no intermediate drain), then
      drain via the claim loop. Assert: (a) every `fetch.mpack.prebuild` claim
      happens only when zero `mpack.index` jobs are `pending`/`running` for the
      repo; (b) zero `FetchNotIndexedError` raised during the drain; (c) the final
      cache covers the latest tip (`len(mpack_ids) == 1` across all live tips).
      ✅ PASSING.
- [x] `_drain_via_claim_loop` helper added to test file — mirrors `_process_one`
      using `claim_next_job` → dispatch → `complete_job`, so MWP4_11/12 exercise
      the real barrier under end-to-end claim semantics. ✅ SHIPPED.

### Phase 5 — docs + close ✅ COMPLETE

- [x] Tick every checkbox above and every acceptance criterion below in this body.
      ✅ All Phase 0–4 checkboxes and all acceptance criteria ticked.
- [x] Update master tracker muse#58: MWP-4 → ✅ COMPLETE; flip the RC-4 half of
      the "two pushes within one prebuild interval" acceptance criterion to ✅.
      ✅ Pushed to staging.musehub.ai.
- [x] Post a closing comment summarizing the fix + commit range; close musehub
      issue with `--status closed`. ✅ Done.

---

## Acceptance criteria

- [x] `claim_next_job` never returns a `fetch.mpack.prebuild` while an
      `mpack.index` for the same repo is `pending` or `running` (MWP4_04, _06).
- [x] Terminal `mpack.index` states (`done`/`failed`/`quarantined`) do not gate
      the prebuild (MWP4_05, _07).
- [x] The barrier is per-repo and filters only prebuild jobs (MWP4_08, _09).
- [x] `fetch.mpack.prebuild` is enqueued with a `created_at` strictly after every
      `mpack.index` foundation job (MWP4_03).
- [x] A `FetchNotIndexedError` in the prebuild handler is surfaced (re-raised /
      failed job), never masked as a successful empty cache (MWP4_10).
- [x] N back-to-back pushes drain with zero `FetchNotIndexedError` and a final
      clone cache covering the latest tip (MWP4_12).
- [x] `claim_next_job` has direct test coverage where it had none.
- [x] MWP-4 suite + adjacent suites green, no regressions (see below).

### Adjacent suites to run green (no regressions)

- `tests/test_mwp4_prebuild_ordering.py` (new)
- `tests/test_mwp3_job_idempotency.py`
- `tests/test_fetch_mpack_prebuild.py`
- `tests/test_process_mpack_index_job.py`
- `tests/test_mpack_index_always_enqueued.py`

---

## Out of scope

- RC-3 job-enqueue idempotency — shipped in MWP-3 (musehub#108).
- RC-5 client-side 503 retry/`Retry-After` honoring — MWP-5 (muse repo).
- RC-6 re-enabling the skipped wire/mpack suite — MWP-6.
- The ~17 deliberately-disabled intel jobs (`musehub/worker.py:107-108`) — they
  remain no-ops; do not revive them here.
- The mpack binary format and presigned-upload model — both sound, untouched.
- Multi-worker deployment itself — the fix is *compatible* with it, but scaling
  the worker is a separate operational decision.

---

## Reference — key symbol anchors

| Symbol | Purpose |
|--------|---------|
| `musehub/services/musehub_jobs.py::claim_next_job` | add the dependency barrier (Phase 2) |
| `musehub/services/musehub_jobs.py::enqueue_push_intel` | add the `created_at` stagger tier (Phase 1) |
| `musehub/services/musehub_wire_fetch.py::process_fetch_mpack_prebuild_job` | narrow the swallow (Phase 3) |
| `musehub/services/musehub_wire_fetch.py::wire_fetch_mpack` | raises `FetchNotIndexedError` at the index-coverage check (lines 681-695) |
| `musehub/services/musehub_wire_shared.py::FetchNotIndexedError` | the exception type |
| `musehub/worker.py::_process_one` | single-threaded claim/dispatch loop to mirror in integration tests |
| `musehub/services/musehub_jobs.py::complete_job` / `fail_job` | status transitions the tests drive |