fix(heartbeat): give provider_quota_exhausted a retry family (BLO-28924)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The heartbeat service decides, when a run fails, whether the agent is parked idle and whether a retry is scheduled to resume the work
• Those two decisions read different fields: finalizeAgentStatus keys "recoverable" on the run's error code, while the retry keys on the error family resolved by readHeartbeatRunErrorFamily
• The two lists had drifted: provider_quota_exhausted was recoverable but unmapped, so it resolved to null and readTransientRecoveryContractFromRun returned null — no retry was ever scheduled
• Since fix(quota-hook): do not re-wake an agent when the debounced hook failed #1407 stopped re-waking an agent whose debounced quota hook failed, there is no recovery wake for it either, so an interval-less event-driven agent never recovers at all
• This pull request maps the code to the provider_quota family and extracts the recoverable set into a shared constant that the invariant test iterates
• The benefit is that "recoverable" becomes a whole contract rather than half of one, and the two lists can no longer drift apart silently

Linked Issues or Issue Description

• Refs BLO-28924
• Follow-up to fix(quota-hook): do not re-wake an agent when the debounced hook failed #1407 — raised there by Ally as an Important finding (review at head a3023c93) and deliberately split out, because changing this shared contract would have moved that PR's head and dismissed a clean review during a live incident.

What Changed

• readHeartbeatRunErrorFamily now resolves provider_quota_exhausted to the provider_quota family. It shares that contract — a billing/session boundary carrying an authoritative reset instant, not a capacity estimate — so it correctly inherits provider_quota's exemption from clampTransientHorizon and the adapter's retryNotBefore is honoured verbatim instead of being clamped to a per-attempt capacity ceiling.
• Extracted the recoverable-code set from an inline three-way || in finalizeAgentStatus into an exported RECOVERABLE_AGENT_STATUS_ERROR_CODES. finalizeAgentStatus and the new invariant test now read the same list, so a fourth code added without a family arm fails CI.
• Exported readHeartbeatRunErrorFamily and readTransientRecoveryContractFromRun (previously module-private) so they can be asserted directly, matching the existing convention for pure helpers in this module (shouldScheduleAutomaticRunRetry, shouldUseRepoLessFallbackWorkspaceSource).
• Added server/src/__tests__/heartbeat-recoverable-error-family.test.ts.
• Corrected the stale in-tree comment describing provider_quota_exhausted as a "legacy/pluggable adapter signal". It is emitted by the first-party claude-local adapter (packages/adapters/claude-local/src/server/execute.ts:1364), which also populates retryNotBefore — so the emit side was already complete and only the family mapping was missing.

Verification

Regression proof — the behavioural arm was reverted while keeping the new exports in place, so the test fails on a meaningful assertion rather than an import error:

× maps provider_quota_exhausted to the provider_quota family
  AssertionError: expected null to be 'provider_quota'
× yields a transient recovery contract with no persisted errorFamily
  AssertionError: expected null not to be null
× carries the adapter-supplied retryNotBefore onto the contract
× recoverable code provider_quota_exhausted resolves to a non-null family and schedules a retry
Tests  4 failed | 4 passed (8)

The 4 that still pass are the controls (persisted-family precedence, the unrelated non-recoverable code, and the other two recoverable codes in the parameterised invariant) — confirming the suite is not simply globally broken.

With the fix applied, 372 tests pass across 8 suites:

npx vitest run server/src/__tests__/heartbeat-recoverable-error-family.test.ts   #   8 passed
npx vitest run server/src/__tests__/heartbeat-retry-scheduling.test.ts \
  server/src/__tests__/quota-exhausted-hook.test.ts \
  server/src/__tests__/heartbeat-rate-limit-retry-schedule.test.ts               #  96 passed (4 files)
npx vitest run server/src/__tests__/heartbeat-rate-limit-exhausted.test.ts \
  server/src/__tests__/recovery-classifiers.test.ts                              #  60 passed
npx vitest run server/src/__tests__/heartbeat-process-recovery.test.ts           # 208 passed
npx tsc --noEmit -p server/tsconfig.json                                         # exit 0

Risks

Low risk, and the blast radius was enumerated rather than assumed. Mapping the code to a non-null family changes behaviour at exactly two call sites:

1. shouldScheduleAutomaticRunRetry (heartbeat.ts:1138) — now returns true. This is the fix. The stale-kill evidence gate above it is unaffected: it returns before this line.
2. keepIdleOnFailure (heartbeat.ts:25550) — now computes true for these runs. This is a no-op: recoverable already includes provider_quota_exhausted, and both feed the same ? "idle" branch, so the resulting status is unchanged.

Not a behavioural change: a persisted resultJson.errorFamily still takes precedence over the code fallback (explicitly tested), and unrelated codes stay unmapped (also tested, to guard the inverse). The RECOVERABLE_AGENT_STATUS_ERROR_CODES refactor is value-identical to the || chain it replaces, including for null/undefined (?? "" matches nothing in the set).

Low live exposure: a full parked census on 2026-08-19 (n=700, truncated: false) plus a 16-run stratified sample across all 12 fleet agents found zero runs carrying this code — the fleet runs claude_k8s/opencode_k8s, not claude-local. This is a latent correctness gap that bites the first deployment using the claude-local adapter, not a live outage.

Model Used

• Claude Opus 5, model ID claude-opus-5[1m] (1M context), extended thinking, with tool use and code execution — run as the Paperclip CTO agent on the claude_k8s adapter.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — N/A, server-only
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first CI run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-28924

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 8b54b81

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The new provider_quota_exhausted mapping reuses the existing provider_quota recovery contract while preserving persisted-family precedence.
• The recoverable-code invariant test directly guards against future drift between agent-status recovery and retry-family mapping.
• The tests cover the adapter reset hint, retry scheduling, persisted metadata precedence, and the non-recoverable inverse case.

Recommended Action

1. No Critical or Important issues found. The change is suitable for the next merge decision once CI completes.