Surface Pi usage-limit errors
Child C3 of epic 7425 (ACP runtime: every provider lane on ACP). Planned by Task 7428.
## Requirement (verbatim)
Epic goal: "Goal: every provider and auth lane can run on jaah-acp-host. On it, ... a usage limit triggers spillover ... Each fix removes its catalogue gap or legacy-only row in `apps/jaahd/agent_providers.py`, so Settings › Providers tells the truth. While a legacy path still exists, it keeps working."
Outcome 2 (A6): "Limits are seen. Pi's 429/usage-limit error reaches `classify_error` as `usage_limit` with `resets_at`, so spillover moves the run. Prefer an upstream pi-acp change, else a pinned fork. This depends on Pi's catalogue row (Task 7276) having landed." (Dependency met: Task 7276 merged as 6bf33ffad; the PI row is in `apps/jaahd/agent_providers.py`.)
Current behaviour (PRD §3): Fixture `pi_rate_limit_silent.json` (pi 1.0.2 / pi-acp 0.0.34): pi retries the 429 and pi-acp ends the turn with `end_turn` and no error frame. `test_replay_pi.py:81-89` asserts nothing is classified and gap `usage-limit-not-reported` exists. `classify_error` (`events.py:210-228`) would classify a JSON-RPC error with code 429, `data.errorKind`, or `_meta["jaah.ai"].error_class`, taking `resets_at` via `_resets_at_from` (`:196-207`). There is no `packages/acp-forks`. The pin is `packaging/platform-pins.toml:329-333`. PI row: `rate_limit_reset=False`.
Decided (PRD): build against a pinned fork unless an upstream release with the fix exists, because an upstream merge is outside our control. Architecture evidence: `npm view pi-acp versions` shows 0.0.34 as the latest (2026-09-24), so the fork path applies. Re-check at build time; if an upstream release with the fix exists, pin it instead and record that.
## Approach (architecture §3, verbatim)
**Chose:** a patch over the pinned upstream tarball, **over** a full source fork built and published to npm (publishing needs an npm credential that is not on the roster; a patch keeps the upstream integrity chain plus a reviewable diff of a few lines).
- `packages/acp-forks/pi-acp/` holds: `README.md` (why the fork exists, the upstream issue to file — a deferred follow-up — and how to drop it), and `0001-surface-final-error.patch` against the **published dist JS** of pi-acp 0.0.34.
- **Patch behaviour:** when pi's final assistant message has `stopReason: "error"`, answer `session/prompt` with a JSON-RPC error carrying `message`: pi's `errorMessage`; `data._meta["jaah.ai"]`: `{error_class: "usage_limit", resets_at}` when the message or status is 429 or a usage limit; `resets_at` taken in order from (1) any reset or retry-after time pi exposes, (2) an ISO or epoch time in the message, (3) **now + 1 hour**. The host's `classify_error` already honours an explicit `_meta["jaah.ai"].error_class` with `resets_at` (`events.py:214-216`), so **the host needs no change**, and no other provider's classification moves. Chose the 1-hour fallback inside the fork, over a host-wide default, because a host default would change resets for Claude and Codex too.
- **Pins:** the `[[npm]]` entry for pi-acp in `packaging/platform-pins.toml` gains `patches = ["packages/acp-forks/pi-acp/0001-surface-final-error.patch"]`. `apps/jaahd/runner_cli_pins.py` applies the listed patches after `verify_tarball` and before unpack (`:304-313`), and fails closed if a patch does not apply. The lock in `packaging/runner-cli-locks/pi-acp.json` records the post-patch tree hash. `tools/build_jaahd_bundle.py` must ship the patch file. Extend `tools/tests/test_runner_cli_pins_table.py` for the `patches` key, which must resolve to an existing file.
- **Replay:** a new fixture `pi_rate_limit_error.json`, next to `pi_rate_limit_silent.json`. It must be **recorded from the patched pi-acp**, never hand-built. Commit the recording script (e.g. `apps/jaahd/tests/acp_replay/record_pi_rate_limit.py`): it installs the pinned tarball, applies the patch, and runs `pi-acp` with pi pointed at a local stub HTTP server through pi's provider base-URL setting. The stub answers every request with 429, once with a `retry-after` header and once without. The script captures the ACP frames. A separate unit test asserts the patch applies cleanly to the pinned tarball, so a stale fixture cannot hide a broken patch. Delete or re-scope the silent fixture's assertion.
- **Catalogue:** PI `usage.rate_limit_reset=True`. Delete gap `usage-limit-not-reported` and the test row `(48, (PI,), "usage-limit-not-reported")`.
Seam (architecture §9): the `packages/acp-forks/` patch mechanism is owned here; C6 may reuse it as a fallback only. Contract: `[[npm]] patches = [<repo path>]`, applied after integrity verify, fail closed — keep it generic (not pi-specific).
Credentials & reach:
- Replay, unit, API and golden tests; `check_provider_branches.py`; `./dc check full` — Nothing beyond the worktree — verified
- Pi usage-limit replay (R3.1) — none (a fixture; a stub 429 server for recording) — verified (no live Anthropic key needed)
Out of scope: Do NOT touch "Pi's question path (`jaah-call input.request` from the shell, `ask_tool`). Do not change it." Do NOT change `classify_error` or any host-wide reset default (would move Claude/Codex). Do NOT publish anything to npm. Do NOT touch billing rules, the chat ↔ terminal hand-off (Task 7370), registry and SDK v2 agents (Task 7429), Settings › Providers layout, Antigravity's row. "No secret in any persisted surface". "No new provider-name branches (`tools/provider_branches.toml` allowlist growth)."
Done means:
- R3.1 When Anthropic answers pi's request with 429 or a usage limit and pi gives up, the host receives an error that `classify_error` classifies as `usage_limit` with a non-null `resets_at`. — verify: a replay test over `pi_rate_limit_error.json`, recorded from the patched pi-acp by the committed script. Observe exactly one `session.error` event with class `usage_limit`, `resets_at` set, and `request_replace` true.
- R3.2 Delivery mechanism: an upstream pi-acp release if one exists at build time, else a fork pinned in-repo at `packages/acp-forks/pi-acp`, built and pinned in `packaging/platform-pins.toml` with integrity, like the other runner CLIs. — verify: `packaging/platform-pins.toml` names the version that `node` resolves, the pin-integrity check in `./dc check full` passes, a unit test asserts the patch applies cleanly to the pinned tarball, and a non-applying patch fails closed (test).
- R3.3 When pi's error carries no reset time, `resets_at` still comes out non-null. — verify: a replay case with no reset in the error (the no-`retry-after` recording). Observe non-null `resets_at`.
- R3.4 The PI row: gap `usage-limit-not-reported` is deleted, and `usage.rate_limit_reset` becomes True. The test row `(48, (PI,), "usage-limit-not-reported")` and `test_replay_pi.py:81-89` are updated. — verify: `pytest tools/tests/test_agent_providers.py apps/jaahd/tests/acp_replay/test_replay_pi.py` passes, and `grep -rn usage-limit-not-reported apps tools docs` finds nothing.
- R3.5 (states) A Pi turn that ends normally still settles normally (no spurious error). Pi's question-by-command and settle-by-MCP replays still pass. — verify: existing `test_replay_pi.py` cases pass.
- D.1 Every living doc that states this gap (e.g. `apps/jaahd/README.md` §"The ACP host runtime" / Pi paragraphs) is updated in place; `packages/acp-forks/pi-acp/README.md` is reachable from a router/README per the context-architecture policy. — verify: grep for the gap key returns nothing; `python3 tools/check_context.py` passes.
- T.3 No new gap is claimed fixed unless it is. — verify: review in the PR, plus the catalogue test.
- T.5 `python3 tools/check_provider_branches.py` passes, and no provider-name branch is added to its allowlist. — verify: run it and observe "OK"; `git diff tools/provider_branches.toml` adds no `[[allow]]`.
Required checks: ./dc check full
Outcome
Pi 429/usage limit now reaches the host as usage_limit with resets_at, via a patch over pinned pi-acp 0.0.34 and a generic [[npm]] patches step that fails closed. PI gap removed. Quick gate green; PR #6058 merged. Deploy was superseded by a later push. Upstream issue drafted as Task 7510.
Artifacts
Activity
- Draft
- Ready
- Runner started
- Runner ended (done)