# MWP-5 — Clone/fetch/pull honor `Retry-After` with a bounded poll (fixes RC-5)

> **Master tracker:** muse#58 — https://staging.musehub.ai/gabriel/muse/issues/58
> **Predecessors (all closed, server side):**
> - musehub#106 (MWP-1, generation authority — fixes RC-1)
> - musehub#107 (MWP-2, walk fallback — fixes RC-2)
> - musehub#108 (MWP-3, job-enqueue idempotency — fixes RC-3)
> - musehub#109 (MWP-4, prebuild-after-index ordering — fixes RC-4)
>
> **Repo target: `muse` (the client).** Every predecessor hardened the *server*
> so the 503 window is now as small as it can be. MWP-5 is the **client** half:
> when the server legitimately answers `503 + Retry-After` during the brief
> prebuild/index window, the client must wait and retry instead of destroying the
> half-created clone and exiting hard.
>
> 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 — run `muse code cat "<anchor>" --json` to read each one before you
> touch it.

---

## Background

### The symptom

A `muse clone <url>` (or `muse fetch` / `muse pull`) issued **seconds after a
push** fails hard:

```
❌ Fetch failed: Objects not yet indexed: 412 missing. Retry shortly.
```

The clone then `rmtree`s the half-created target directory and exits `3`
(`INTERNAL_ERROR`). A second clone a moment later works — because by then the
server's `mpack.index` + `fetch.mpack.prebuild` jobs have completed. The data is
never missing; the client just refuses to wait the second or two the server
explicitly asked it to.

### The server is behaving correctly — RC-5 is a client bug

The fetch endpoint `musehub/api/routes/wire.py::fetch_mpack` (the single endpoint
`clone`, `fetch`, and `pull` all hit) returns a **well-formed 503 with a
`Retry-After` header** while the async prebuild/index pipeline is still in flight.
There are exactly two retryable 503 shapes, both confirmed in
`musehub/api/routes/wire.py:694-705`:

| Server exception | Status | `Retry-After` | Body |
|------------------|--------|---------------|------|
| `MPackNotReadyError` | 503 | `30` | `mpack not ready — prebuild in progress. Retry shortly.` |
| `FetchNotIndexedError` | 503 | `60` | `Objects not yet indexed: N missing. Retry shortly.` |

This is RFC 7231 §6.6.4 + §7.1.3 textbook behavior: "service temporarily
unavailable, come back after N seconds." The client is supposed to honor it.

### Where the client throws the contract away — two precise defects

**Defect A — the `Retry-After` header is silently dropped at the transport seam.**

`muse/core/transport.py::_urllib_do` catches the 503 and converts it to a
`TransportError`, but reads only `err.code` — never `err.headers`:

```python
    except urllib.error.HTTPError as err:
        raise TransportError(_http_error_message(err), err.code) from err   # ← Retry-After lost
```

`muse/core/transport.py::_execute_fetch` has the identical pattern. So even
though `TransportError` carries `status_code = 503`, the number of seconds the
server asked us to wait is gone before any caller can see it.

`muse/core/transport.py::TransportError.__init__` has no field for it either:

```python
    def __init__(self, message: str, status_code: int) -> None:
        super().__init__(message)
        self.status_code = status_code
```

**Defect B — every caller treats the first 503 as terminal.**

The POST in `muse/core/transport.py::HttpTransport.fetch_mpack` goes through
`_urllib_do`; a 503 propagates straight out as `TransportError(…, 503)` with no
retry. Each command then hard-fails on the first one:

- `muse/cli/commands/clone.py::run` — `except TransportError` →
  `shutil.rmtree(target, ignore_errors=True)` → `raise SystemExit(INTERNAL_ERROR)`.
  **This deletes the freshly-created repo on a transient, self-healing 503.**
- `muse/cli/commands/fetch.py::_fetch_one` — `except TransportError` → print
  `❌ Fetch failed` → `raise SystemExit(INTERNAL_ERROR)`.
- `muse/cli/commands/pull.py::run` (around line 149-156) — same pattern.

None of them inspect `status_code == 503`, none honor `Retry-After`, none poll.

---

## Goal — definition of done

`muse clone`, `muse fetch`, and `muse pull`, issued immediately after a push,
**succeed via a bounded retry loop** that honors the server's `Retry-After`:

1. On a 503 from `/fetch/mpack`, the client waits `Retry-After` seconds (clamped
   to a sane range) and retries, up to a **total wall-clock budget** (default
   120s — see *Budget math* below), emitting clear progress to **stderr**.
2. A clone is **never** `rmtree`d on a retryable 503 — the target survives across
   retries and is removed only after the budget is genuinely exhausted, with a
   message that says so ("remote still preparing data after Ns").
3. Non-503 errors (401/404/422/network) still fail **immediately** — zero
   behavioral change, zero added latency.
4. `--json` mode keeps **stdout** clean: only the final envelope is printed there;
   all retry chatter goes to stderr.
5. The retry budget is configurable (env + optional flag) and a `--no-retry`
   escape hatch restores single-attempt behavior for deterministic CI.
6. The whole thing is provable with mocked seams *and* one real-urllib E2E that
   exercises actual `HTTPError` header parsing.

---

## Design

### D1 — `TransportError` gains `retry_after`

`muse/core/transport.py::TransportError`:

```python
class TransportError(Exception):
    def __init__(
        self,
        message: str,
        status_code: int,
        *,
        retry_after: int | None = None,   # NEW — seconds, parsed from Retry-After
    ) -> None:
        super().__init__(message)
        self.status_code = status_code
        self.retry_after = retry_after
```

Keyword-only and defaulted to `None` → every existing `TransportError(msg, code)`
call site stays valid. No call site needs editing except the two that learn the
header (D2).

### D2 — capture `Retry-After` at both HTTPError seams

Add a module-level parser and use it in `_urllib_do` **and** `_execute_fetch`:

```python
def _parse_retry_after(headers) -> int | None:
    """Parse an RFC 7231 Retry-After header into whole seconds.

    Accepts the delta-seconds form (``"60"``). The HTTP-date form is rare for
    this endpoint and is intentionally treated as ``None`` (caller falls back to
    its default backoff) rather than risking a brittle date parse. Returns None
    on a missing or non-integer value — never raises.
    """
    if headers is None:
        return None
    raw = headers.get("Retry-After")
    if raw is None:
        return None
    try:
        secs = int(str(raw).strip())
    except (TypeError, ValueError):
        return None
    return secs if secs >= 0 else None
```

```python
    except urllib.error.HTTPError as err:
        raise TransportError(
            _http_error_message(err), err.code,
            retry_after=_parse_retry_after(err.headers),
        ) from err
```

### D3 — bounded retry helper + test seams

Two module-level seams so tests never sleep for real and budgets are tunable:

```python
_FETCH_RETRY_BUDGET_DEFAULT_S: float = 120.0
_RETRY_AFTER_CEILING_S: float = 60.0   # never sleep longer than this per attempt
_RETRY_AFTER_FLOOR_S: float = 1.0      # never busy-spin if server says 0
_RETRY_DEFAULT_BACKOFF_S: float = 5.0  # used when 503 carries no Retry-After

def _sleep(seconds: float) -> None:
    """Indirection over time.sleep — patched in tests to assert wait schedule."""
    time.sleep(seconds)

def _resolve_retry_budget(explicit: float | None) -> float:
    """explicit arg > MUSE_FETCH_RETRY_BUDGET_S env > module default."""
    if explicit is not None:
        return max(0.0, explicit)
    env = os.environ.get("MUSE_FETCH_RETRY_BUDGET_S")
    if env:
        try:
            return max(0.0, float(env))
        except ValueError:
            pass
    return _FETCH_RETRY_BUDGET_DEFAULT_S

def _next_wait(retry_after: int | None) -> float:
    base = float(retry_after) if retry_after is not None else _RETRY_DEFAULT_BACKOFF_S
    return min(max(base, _RETRY_AFTER_FLOOR_S), _RETRY_AFTER_CEILING_S)
```

### D4 — the loop lives in `HttpTransport.fetch_mpack` (single implementation)

`make_transport` always returns `HttpTransport` (confirmed —
`muse/core/transport.py::make_transport`), so putting the loop in
`HttpTransport.fetch_mpack` gives all three commands the fix for free with **zero
duplication**. Extract the current single-attempt body into
`_fetch_mpack_once(...)` and wrap it:

```python
def fetch_mpack(self, url, signing, want, have, *, ttl_seconds=3600,
                retry_budget_s: float | None = None) -> FetchMPackResult:
    budget = _resolve_retry_budget(retry_budget_s)
    start = time.monotonic()
    attempt = 0
    while True:
        attempt += 1
        try:
            return self._fetch_mpack_once(url, signing, want, have, ttl_seconds=ttl_seconds)
        except TransportError as exc:
            if exc.status_code != 503:
                raise                      # non-retryable — fail immediately
            elapsed = time.monotonic() - start
            wait = _next_wait(exc.retry_after)
            if elapsed + wait > budget:
                raise                      # budget exhausted — terminal 503
            print(
                f"⏳ remote preparing fetch data (server busy, attempt {attempt}); "
                f"retrying in {int(wait)}s … (waited {int(elapsed)}s / {int(budget)}s)",
                file=sys.stderr, flush=True,
            )
            _sleep(wait)
```

`MuseTransport.fetch_mpack` is an abstract/base variant **not used in
production** (`make_transport` never returns it). Do **not** duplicate the loop
there; add a one-line note that retry lives in the concrete `HttpTransport`.

> **Budget math — why 120s, not the "~60s" the tracker hand-waved.** A single
> `FetchNotIndexedError` carries `Retry-After: 60`. With a 60s budget the first
> sleep (60s) already meets the cap, so `elapsed + wait > budget` trips and the
> client gives up after **one** attempt — no real retry. 120s permits: attempt 1
> at t=0 → 503(60) → sleep 60 → attempt 2 at t=60 → on a second 503(60) the next
> sleep would reach t=120 and we stop cleanly. That is two genuine attempts plus
> a guaranteed retry of the common single-503 case. The default is configurable;
> document the deviation in the closing comment.

### D5 — command-layer integration

The retry now lives below the commands, so the commands' existing
`except TransportError` blocks become the **terminal** path (budget already
exhausted). Required changes:

- `clone.py::run` — keep the `rmtree` + `INTERNAL_ERROR`, but only as the
  genuinely-terminal outcome. Improve the message when `exc.status_code == 503`
  to: `❌ Remote still preparing clone data after {budget}s — try again shortly.`
  The directory survived every retry; it is removed only here. JSON error
  envelope: add `"retryable": true` / a `timed_out` marker on the 503 branch.
- `fetch.py::_fetch_one` and `pull.py::run` — same terminal-message refinement
  and JSON marker. No `rmtree` (they mutate an existing repo).
- **Optional CLI surface** (add if cheap, else env-only): `--retry-timeout
  SECONDS` (maps to `retry_budget_s`) and `--no-retry` (maps to
  `retry_budget_s=0`, i.e. single attempt) on `clone`, `fetch`, `pull`. `register`
  is the arg-parser hook in each command module.

---

## Phases — load-bearing, TDD, never skip ahead

All tests live in **`tests/test_mwp5_clone_retry.py`** (muse repo). Test IDs
appear in both this plan and the test file. The network seam is patched exactly
like the existing suite does it —
`unittest.mock.patch("muse.core.transport._urllib_do", side_effect=…)` (see
`tests/test_core_transport.py:348-436` for the canonical helper) — and `_sleep`
is patched to a recorder so no test waits in real time.

### Phase 0 — RED harness ✅
- [x] Create `tests/test_mwp5_clone_retry.py` with a `_seq_urllib_do(...)` helper
      that raises `TransportError(msg, 503, retry_after=R)` for the first *k*
      calls then returns a valid mpack POST body, plus a `_record_sleeps()`
      fixture patching `muse.core.transport._sleep`.
- [x] **MWP5_00** — characterization: with current code, one 503 from
      `_urllib_do` makes `HttpTransport().fetch_mpack(...)` raise `TransportError`
      immediately and `_sleep` is never called. **Asserts the bug. RED→stays red
      until Phase 3, then flips to assert the new behavior.**
      Commits: `sha256:11eb9c76826dc95e3b1cd109b04f538a6739f1b50c897cc0d7b19db5d2bec9bb`

### Phase 1 — capture `Retry-After` (Defect A) ✅
Anchors: `muse/core/transport.py::TransportError`,
`muse/core/transport.py::_urllib_do`, `muse/core/transport.py::_execute_fetch`,
`muse/core/transport.py::_http_error_message`.
- [x] Add `retry_after` to `TransportError.__init__` (D1).
- [x] Add `_parse_retry_after` and call it in both HTTPError handlers (D2).
- [x] **MWP5_01** — `_parse_retry_after({"Retry-After": "60"})` → `60`. ✅
- [x] **MWP5_02** — missing header → `None`; non-integer (`"Wed, 21 Oct"`) → `None`. ✅
- [x] **MWP5_03** — a real `HTTPError(code=503, headers={"Retry-After":"30"})`
      driven through `_urllib_do` yields `TransportError` with
      `status_code==503` and `retry_after==30`. ✅
- [x] **MWP5_04** — a 404 `HTTPError` yields `retry_after is None` and is
      unaffected (regression guard for non-retryable codes). ✅
      Commits: `sha256:8be791449fd7d6c466a1ebd875c55866c8c41d81eae6778b1c5a46eda3470dff`

### Phase 2 — retry primitives (D3) ✅
Anchors: new module-level symbols in `muse/core/transport.py`.
- [x] Add `_sleep`, `_resolve_retry_budget`, `_next_wait`, the four module
      constants.
- [x] **MWP5_05** — `_next_wait(0)` floors to `1.0`; `_next_wait(999)` ceilings to
      `60.0`; `_next_wait(None)` → `_RETRY_DEFAULT_BACKOFF_S`. ✅
- [x] **MWP5_06** — `_resolve_retry_budget`: explicit arg wins; else
      `MUSE_FETCH_RETRY_BUDGET_S` env; else `120.0`; negatives clamp to `0.0`;
      junk env falls through to default. ✅
      Commits: `sha256:0fdd6beaff2a25b2eda7d4c8b3034444bbc124619339ed6d91a836ffaafb5608`

### Phase 3 — wire the loop into `fetch_mpack` (Defect B core) ✅
Anchors: `muse/core/transport.py::HttpTransport.fetch_mpack` (extract
`_fetch_mpack_once`).
- [x] Extract the existing single-attempt body verbatim into
      `_fetch_mpack_once`; add the loop per D4.
- [x] **MWP5_07** — one 503(`Retry-After:30`) then success: returns the
      `FetchMPackResult`, `_sleep` called once with `30.0`. ✅
- [x] **MWP5_08** — 503(30) then 503(60) then success: two sleeps `[30.0, 60.0]`,
      result returned. ✅
- [x] **MWP5_09** — 503 with no `Retry-After`: sleeps `_RETRY_DEFAULT_BACKOFF_S`. ✅
- [x] **MWP5_10** — budget exhausted (503 forever, budget=120): raises
      `TransportError(status_code=503)`; total slept `<= 120`; sleep count
      bounded; the final raise is the server's 503, not a synthetic one. ✅
- [x] **MWP5_11** — a 404 mid-loop raises immediately; `_sleep` never called
      (no added latency for real errors). ✅
- [x] **MWP5_12** — `retry_budget_s=0` (== `--no-retry`): exactly one attempt,
      zero sleeps, original behavior preserved. ✅
- [x] Flipped **MWP5_00** to assert the success-after-retry behavior. ✅
      Commits: `sha256:1df101e06206778511bc1b961d78270ac494c2986bfdf12104bb1d7d8a0474b2`

### Phase 4 — command integration (D5) ✅
Anchors: `muse/cli/commands/clone.py::run`,
`muse/cli/commands/fetch.py::_fetch_one`, `muse/cli/commands/pull.py::run`,
and each module's `register`.
- [x] Refined terminal 503 messaging in all three; added JSON `retryable`
      marker. Added `--retry-timeout` / `--no-retry` flags to all three commands.
- [x] **MWP5_13** — CLI clone, transport patched to 503-then-success: exits `0`,
      target directory **exists** and is populated, was never `rmtree`d. ✅
- [x] **MWP5_14** — CLI clone, 503-forever with budget=0 (`--no-retry`): exits
      `INTERNAL_ERROR`, target **is** removed, stderr says "still preparing …
      after Ns", JSON envelope (when `--json`) carries the retryable marker. ✅
- [x] **MWP5_15** — CLI fetch, 503-then-success: exits `0`, commits applied. ✅
- [x] **MWP5_16** — CLI pull, 503-then-success: exits `0`, ff/merge proceeds. ✅
- [x] **MWP5_17** — `--json` clone with retries: **stdout** contains exactly one
      JSON object (the final envelope); all `⏳` lines are on **stderr**. ✅
      Commits: `sha256:1df101e06206778511bc1b961d78270ac494c2986bfdf12104bb1d7d8a0474b2`

### Phase 5 — real-urllib E2E + docs + close ✅
- [x] **MWP5_18** (E2E, highest value) — stood up a throwaway
      `http.server.BaseHTTPRequestHandler` on `127.0.0.1:0` that answers the
      first POST with `503` + `Retry-After: 1` and the second with a valid
      mpack body. Drives a **real** `HttpTransport().fetch_mpack` (real
      `_urllib_do`, real `urllib.error.HTTPError`, real header parsing) with
      `_sleep` patched to no-op. Proves the header survives the actual urllib
      path end-to-end — not just a mocked `side_effect`. ✅
      Commits: `sha256:1df101e06206778511bc1b961d78270ac494c2986bfdf12104bb1d7d8a0474b2`
- [x] Update `muse#58`: tick RC-5 in *Confirmed root causes* and the acceptance
      criterion *"`muse clone` issued immediately after `muse push` succeeds via
      bounded retry, never hard-fails on the first 503 (RC-5)"*. (Edit the local
      `docs/issues/mwp-mvp-master.md` first, then push `--body-file` per the
      local-file rule.) ✅
- [x] Post a closing comment on this issue documenting the final budget default,
      the deviation from the tracker's "~60s", and the commit range; close it. ✅

---

## Acceptance criteria

- [x] `muse clone` immediately after a push succeeds via bounded retry and never
      `rmtree`s the target on a transient 503 (MWP5_13). ✅
- [x] `muse fetch` / `muse pull` immediately after a push succeed via the same
      loop (MWP5_15, MWP5_16). ✅
- [x] Both server 503 shapes are honored: `Retry-After: 30`
      (`MPackNotReadyError`) and `Retry-After: 60` (`FetchNotIndexedError`)
      (MWP5_07, MWP5_08). ✅
- [x] Budget exhaustion ends in a clear terminal failure that slept ≤ budget and
      surfaced the server's real 503 (MWP5_10, MWP5_14). ✅
- [x] Non-503 errors fail immediately with zero added sleeps (MWP5_04, MWP5_11). ✅
- [x] `--no-retry` / `retry_budget_s=0` restores exact single-attempt behavior
      (MWP5_12). ✅
- [x] `--json` stdout carries only the final envelope during retries (MWP5_17). ✅
- [x] The real-urllib E2E proves `Retry-After` survives the actual HTTPError path
      (MWP5_18). ✅
- [x] muse#58 RC-5 + its acceptance criterion are ticked; this issue closed with
      evidence. ✅

## Out of scope

- 429 rate-limit retry. The same `retry_after` plumbing makes a future 429 loop
  trivial, but `WIRE_FETCH_LIMIT` tuning and 429 policy are a separate concern.
  Capturing the header for 429 (Phase 1 does this incidentally) is fine; **acting**
  on 429 is not part of MWP-5.
- The HTTP-date form of `Retry-After`. The endpoint only emits delta-seconds;
  date parsing is deliberately `None`-and-fall-back.
- Any change to the server 503 contract (that surface is frozen by MWP-1..4).
- Retrying the R2 **GET** (post-presign download) or integrity failures — those
  are not 503 and not part of this ticket.
- `fetch_remote_info` retry — ref listing needs no prebuild and never 503s here.

---

## Symbol-anchor appendix — read before you touch

```bash
# Client transport (all edits land here except the command-layer messaging)
muse code cat "muse/core/transport.py::TransportError" --json
muse code cat "muse/core/transport.py::_urllib_do" --json
muse code cat "muse/core/transport.py::_execute_fetch" --json
muse code cat "muse/core/transport.py::_http_error_message" --json
muse code cat "muse/core/transport.py::HttpTransport.fetch_mpack" --json
muse code cat "muse/core/transport.py::make_transport" --json

# Command layer (terminal-path messaging + optional flags)
muse code cat "muse/cli/commands/clone.py::run" --json
muse code cat "muse/cli/commands/fetch.py::_fetch_one" --json
muse code cat "muse/cli/commands/pull.py::run" --json

# Blast radius before editing TransportError / fetch_mpack
muse code impact "muse/core/transport.py::TransportError" --json
muse code impact "muse/core/transport.py::HttpTransport.fetch_mpack" --json

# Test seam reference (copy the _urllib_do patch idiom)
muse code cat "tests/test_core_transport.py" --all --json
```

**Server-side contract (read-only; do NOT edit — frozen by MWP-1..4):**
`musehub/api/routes/wire.py:684-706` — the `fetch_mpack` route's two 503 branches
and their exact `Retry-After` values.