feat(heartbeat): auto-retry ccrotate capacity defers via scheduled_retry (PEN-382)

[kkroo]

What

Auto-retry agent wakes that the ccrotate capacity gate defers on pool exhaustion, by reusing paperclip's existing scheduled_retry machinery. Implements the design in penstock-paperclip-plugin#2 / PEN-382 (BLO-4728).

Why

enqueueWakeup (heartbeat.ts) used to write a terminal status:"skipped" wake and drop it on capacity defer — resumeAt was decorative, so a human had to @ally-re-ping after the pool reset (and a re-ping before reset just re-deferred). Live evidence 2026-06-05: Ally's review wake stranded at resumeAt 03:02Z with no autonomous recovery.

How

At the gate-deny site, instead of dropping, persist a status:"scheduled_retry" heartbeat run:

• scheduledRetryReason = "ccrotate_capacity", scheduledRetryAt = resumeAt
• errorFamily = "rate_limit_exhausted" + retryNotBefore = resumeAt so the existing rate-limit bounded backoff/exhaust honors resumeAt as the floor

The existing promoteDueScheduledRetries sweep (server tick + startup recovery) claims it by (status, scheduledRetryAt <= now) and promotes it when due. No new status enum (status is a text column), no new sweep, no DB migration.

Two correctness properties:

• The resume claim is keyed on (status, scheduledRetryAt), not idempotencyKey — so the retry cannot coalesce into its own deferred row (the crux flagged in eng review).
• A null resumeAt falls back to CCROTATE_CAPACITY_DEFAULT_RETRY_DELAY_MS (5 min) so the row is never stranded with a null scheduledRetryAt the sweep would skip.

Tests

server/src/__tests__/heartbeat-ccrotate-capacity-retry.test.ts (embedded-postgres), all TDD'd red→green:

1. capacity defer creates a scheduled_retry run with scheduledRetryAt = resumeAt + rate-limit family (and does not write a terminal skipped drop)
2. null resumeAt falls back to a bounded future delay (not null)
3. the row promotes to queued when due via the existing sweep

tsc --noEmit clean. No regression to the three adjacent gate tests (they all inject an allow:true gate; only the deny path changed).

Follow-up (not in this PR)

Re-check the capacity gate at promotion time (a ccrotate_capacity branch in ScheduledRetryGate) so a still-exhausted pool re-defers with backoff instead of dispatching a run that immediately 429s. Today it self-corrects via the post-run rate-limit path, so this is an optimization, not a correctness gap.

🤖 Generated with Claude Code

[kkroo]

Split: the adapter-resolution guard (the "Process adapter missing command" / Ally adapter-down fix) was moved to its own PR off master — #302. This branch is now purely the PEN-382 scheduled_retry feature.

[kkroo]

@ally please review this PR.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + gstack/codex (folded).

Solid, well-tested change. The core mechanism is correct: the resume claim is keyed on (status, scheduledRetryAt<=now) (not idempotencyKey), the optimistic-UPDATE guard is reused consistently, and the null resumeAt → bounded-delay fallback closes the stranded-row gap. CI Typecheck + General tests (server) + all four serialized server suites are green, so the new embedded-Postgres suite (all 5 cases) passes and the heartbeatRuns insert is schema-valid. One real concern below.

Important Issues (1)

• [code] server/src/services/heartbeat.ts:11481 — Capacity defers accumulate one promotable scheduled_retry row per timer tick; on recovery they all promote into a burst of redundant runs, re-creating the capacity waste the gate exists to prevent.
  • The old deny path wrote a terminal, inert skipped row. The new path inserts a scheduled_retry heartbeatRuns row with no idempotencyKey and no dedup against an existing capacity row for the agent. The defer also does not bump lastHeartbeatAt, and neither the cooldown check (heartbeat.ts:11423, keyed on lastHeartbeatAt) nor the idle circuit-breaker (tickTimers, keyed on consecutiveTimerIdleRuns which only increments after a run executes) suppresses a deferred wake. So during a prolonged pool exhaustion, tickTimers (heartbeat.ts:12780) re-fires heartbeat_timer every interval and each tick mints a fresh row (all with scheduledRetryAt ≈ resumeAt).
  • When capacity returns, promoteDueScheduledRetries claims up to 50 due rows and the new re-check promotes each that the gate now allows → N queued runs for one agent. With maxConcurrentRuns:1 they serialize rather than overload, but they still each dispatch a full heartbeat — exactly the "burning per-account cap on redundant requests" the long gate comment at heartbeat.ts:11437 is protecting against, just shifted to recovery time (and able to re-exhaust the pool).
  • Recommendation: make the capacity defer reason-aware-idempotent for fungible wakes — before inserting, look for an existing scheduled_retry capacity row for this agent and update its scheduledRetryAt to the new resumeAt instead of inserting a second row (at minimum for heartbeat_timer; distinct issue-scoped wakes can legitimately each survive). This collapses a 30-tick exhaustion into one recovery run.

Suggestions (3)

• [comments] PR body is stale and under-describes scope. The "Follow-up (not in this PR)" section says re-checking the capacity gate at promotion time is deferred, but this PR implements exactly that (heartbeat.ts:6354, the ccrotate_capacity re-check → re-defer-with-backoff / terminate-at-cap) and tests it (tests 4 & 5 at heartbeat-ccrotate-capacity-retry.test.ts:219 and :253). The "Tests" section also lists only 3 of the 5 cases. Update the body so reviewers/history aren't misled about what landed.
• [tests] No test exercises the multi-row accumulation path (several gated wakes → several scheduled_retry rows → multiple promotions). That gap is why the amplification above isn't visible. A test asserting that repeated heartbeat_timer defers collapse to a single promotable row would guard the recommended fix.
• [code] e2e CI is red, but it's the browser/relay end-to-end suite with no touchpoint on server-side heartbeat.ts scheduling (Typecheck + server + serialized suites all pass). Looks like the usual e2e flake — a re-run to get a clean board before merge would remove ambiguity.

Strengths

• Concurrency is handled correctly throughout: every state transition (promote, re-defer, terminate) reuses the same (id, status="scheduled_retry", scheduledRetryAt<=now) guarded UPDATE with .returning(), so concurrent sweeps can't double-promote or double-increment the attempt counter.
• The CCROTATE_CAPACITY_MAX_RETRY_ATTEMPTS backstop bounds the re-defer loop per row and terminates to cancelled with a clear operator-facing error + lifecycle event — no infinite loop.
• appendRunEvent is correctly emitted only when the guarded update actually returned a row (if (exhausted) / if (rescheduled)), so no event fires on a no-op update.
• Promoted rows carry the full enrichedContextSnapshot (issueId/taskKey preserved), so a recovered run has richer context than the old skipped drop. SQL is fully parameterized via drizzle — no injection surface; no LLM trust-boundary in this diff.

Recommended Action

1. Address the Important accumulation issue before merge (reason-aware dedup on the capacity defer), or consciously accept the recovery-burst trade-off and note it.
2. Fix the stale PR body (the promotion-time re-check is in this PR).
3. Re-run e2e to confirm the failure is the unrelated flake.