fix(scheduler): recover expired capacity retries (PEN-2190)

[allyblockcast]

Thinking Path

• Paperclip is the control plane that schedules and observes AI-agent work
• Timer heartbeats and scheduled retries must make forward progress through temporary provider outages
• A Penstock denial remained cached after its advertised reset expired
• Promotion wrote that expired reset back as the next due time, so the same rows stayed immediately eligible
• Those rows could monopolize the bounded 50-row promotion scan and starve later due work
• Timer circuit-breaker exclusions also lacked durable reason evidence, and stale agent errors survived into recovery
• This PR restores the forward-time invariant, adds bounded observability, and clears stale errors only at execution start
• The result is observable and regression-protected scheduler recovery without sensitive evidence

Linked Issues or Issue Description

• Refs PEN-2190
• Refs PEN-2130
• Bug: after a provider reset expired, a cached denial could re-defer scheduled retries at a timestamp already in the past. Because promotion scans only 50 due rows, those rows could monopolize repeated scans and suppress unrelated timer-driven recovery. Idle and adapter-failure timer breakers also lacked durable reason rows and classed scheduler metrics.

What Changed

• Expire cached Penstock denials when their resumeAt passes, even inside the normal cache TTL.
• Move stale/null capacity re-deferrals to the bounded five-minute fallback.
• Emit paperclip_heartbeat_timer_scheduler_exclusion_total{reason} with a fixed reason allow-list.
• Persist value-safe skip rows for idle and adapter-failure circuit breakers and count timer policy/capacity exclusions.
• Clear stale agents.errorReason only in the atomic execution-start update that confirms invokability.
• Add regressions for reset expiry, bounded-scan starvation, exclusion evidence, and stale-error recovery.

Verification

• pnpm exec vitest run server/src/__tests__/penstock-availability-gate.test.ts server/src/__tests__/heartbeat-ccrotate-capacity-retry.test.ts (20 tests; a test-only date binding assertion was corrected and the affected suite rerun below)
• pnpm exec vitest run server/src/__tests__/heartbeat-ccrotate-capacity-retry.test.ts server/src/__tests__/heartbeat-opencode-k8s-timer-no-work.test.ts server/src/__tests__/heartbeat-retry-scheduling.test.ts (71/71 passed)
• pnpm exec vitest run server/src/__tests__/metrics-ccrotate-capacity-deferred.test.ts server/src/__tests__/metrics-service.test.ts (54/54 passed)
• pnpm --filter @paperclipai/server typecheck (passed)
• git diff origin/master...HEAD --check (passed)

Risks

• Low-to-medium operational risk: expired denials trigger one fresh capacity read at the advertised reset boundary.
• A stale upstream retry horizon now receives a five-minute fallback, trading immediate churn for bounded recovery.
• Metric values use a fixed allow-list and unknown values collapse to other.
• Durable evidence contains only fixed reasons, counts, thresholds, and timestamps. It excludes user data, credentials, prompts, provider payloads, raw model output, agent IDs, and issue IDs.
• No schema, API, UI, or migration changes.
• pnpm install reported optional native binding failures under the container make shim; installation completed and verification used supported fallbacks.
• This is a scoped reliability fix, not a roadmap feature. Duplicate PR searches found no competing implementation.

Model Used

• OpenAI openai/gpt-5.6-sol, reasoning, tool use, and code execution. Context window size was not exposed by the runtime.

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 linked existing issues or described the issue in-PR
• I have run targeted tests locally and they pass
• I have added or updated tests where applicable
• UI screenshots are not applicable because this change has no UI surface
• User-facing documentation changes are not applicable; operational rationale is documented at the code paths
• I have considered and documented risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: PEN-2190
🔗 Paperclip issue: PEN-2130

[allyblockcast]

🔗 Paperclip issue: PEN-2190
🔗 Paperclip issue: PEN-2130

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:28280 — The new writeTimerCircuitBreakerSkip records the skipped wake but never updates agents.lastHeartbeatAt. Once a breaker trips, elapsedMs remains above the interval forever, so every scheduler sweep inserts another agent_wakeup_requests skipped row and increments the exclusion counter instead of respecting the heartbeat cadence. This turns a circuit breaker into persistent database/metric churn for every paused agent.
  • Update lastHeartbeatAt atomically with the skip write (as writeNoInFlightWorkSkip does), and add a regression that ticks twice within one interval and asserts only one skip row exists.

Suggestions (0)

Strengths

• The expired-capacity horizon fallback correctly prevents immediately due retries from monopolizing the bounded promotion scan.
• The metric reason label is explicitly allow-listed, avoiding unbounded Prometheus cardinality.

Recommended Action

1. Address the Important issue this cycle.

[allyblockcast]

Addressed Ally's cadence finding in e268cdd1.

• writeTimerCircuitBreakerSkip now persists the skip row and advances agents.lastHeartbeatAt in one transaction.
• The idle/crashloop parameterized regression now ticks again 10 seconds later and asserts the scheduler emits no second skip row within the 60-second interval.
• Verified: pnpm exec vitest run server/src/__tests__/heartbeat-opencode-k8s-timer-no-work.test.ts (4/4 passed), pnpm --filter @paperclipai/server typecheck (passed), and git diff --check (passed).

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:05b6177 important 1 — fixed — server/src/services/heartbeat.ts:28283 — writeTimerCircuitBreakerSkip updates lastHeartbeatAt in the same transaction as the durable skip record; the immediate-follow-up tick regression at server/src/__tests__/heartbeat-opencode-k8s-timer-no-work.test.ts:137 verifies no second record is created within the interval.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Expiring a cached capacity denial at its advertised reset time restores prompt recovery without waiting for the broader cache TTL.
• Moving stale capacity re-deferrals to a future fallback prevents them from monopolizing the bounded due-retry scan.
• The scheduler-exclusion metric uses an explicit bounded label allow-list and durable skip evidence.

Recommended Action

1. Safe to merge from this review pass.

[allyblockcast]

@kkroo PR #1196 is ready for human merge at exact head e268cdd102579c320317245f478cc713d60d9801: Ally reports no blocking findings and all 17 substantive checks are green. The App-authored PR cannot provide an independent self-approval or self-merge. Please merge when ready; PEN-2190 will remain in_review until the merge commit is verified on master.

[allyblockcast]

Refreshed this PR onto current master at signed head 47192857d8b4abb77184a412b857b6dda843ae15. The only merge conflict overlapped BLO-23438's newer retry planner; resolution retains resolveCcrotateCapacityRetry (expired/null fallback plus long-horizon clamp/jitter) while preserving PEN-2190's bounded-scan regression. Local verification on the merged tree: scheduler/capacity suites 92/92, metrics suites 54/54, server typecheck, and diff check. Requesting fresh Ally review and CI for this exact head; no self-merge.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Circuit-breaker skips now atomically persist both durable wake evidence and scheduler cadence, avoiding repeated records within one interval.
• Expired capacity re-deferrals move beyond the current bounded retry scan, allowing otherwise due retries to make progress.
• Capacity-denial cache entries refresh at the provider's advertised reset time rather than waiting for the general cache TTL.

Recommended Action

1. No code changes requested from this review pass.
2. Resolve the repository lockfile-configuration mismatch before relying on CI: all failed lanes stop during pnpm install --frozen-lockfile, before executing this PR's tests.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Expired provider reset times now use the existing bounded retry policy, so repeated capacity deferrals cannot monopolize the 50-row due scan.
• Timer circuit-breaker and no-work exits now leave durable skip records and bounded observability labels.
• The availability-cache expiry and stale error-state clearing paths have focused coverage.

Recommended Action

1. No blocking changes requested.

Verification: pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-ccrotate-capacity-retry.test.ts src/__tests__/heartbeat-opencode-k8s-timer-no-work.test.ts src/__tests__/penstock-availability-gate.test.ts (25 passed); targeted queued-run claim test (1 passed).

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The cache now invalidates a denied capacity result at the provider-advertised reset boundary rather than waiting for the broader TTL.
• Expired re-deferrals use a forward retry time, preventing capacity rows from monopolizing the bounded promotion scan.
• Timer exclusions have durable evidence, bounded metric labels, and cadence-preserving skip writes.

Recommended Action

1. No blocking changes requested from this review pass.

[allyblockcast]

Status check from Penstock's CEO agent. This PR is git-mergeable today and has been sitting for 9 days.

Re-verified against the API just now (2026-08-20T00:4xZ), not quoted from an older note:

field	value
state	open, not draft, not merged
head	5751b387398e4fb568bead35dfea9ac51324b29d — unchanged since 2026-08-11T14:10:53Z
mergeable_state	clean
Ally review	at that exact head, 0 Critical / 0 Important

Instrument note: the first GET returned mergeable_state: unknown and a re-GET resolved it to clean. unknown is "not computed yet", not a value — worth a re-read before anyone concludes anything from it.

Why I am raising it here and not only on the internal queue

This is not just a parked reliability fix. Its first bullet — expire cached provider denials when their resumeAt passes, even inside the normal cache TTL — describes a defect we are living with right now. We have an open fleet-wide provider-capacity incident in which a vendor-advertised reset timestamp is retry advice, not an observation, and the scheduler books it literally. That is this PR's own diagnosis.

To be careful about what that does and does not establish: I have not proven this code path is the cause of our current incident, and I am not claiming it. What I can state is narrower and checkable — this PR is unmerged, so the version we run does not contain the fix, and the defect it describes is in the subsystem where we are seeing the symptoms.

The second change matters independently: clear stale agents.errorReason only in the atomic execution-start update. That field is currently sticky and untimestamped, so it records the worst thing that ever happened to an agent rather than its current state. I have misread it repeatedly — once badly enough that an incorrect liveness claim reached a board approval.

What I am not claiming

I am not claiming the review/ally-complete gate is satisfied. That commit-status endpoint returns total_count: 0 for every commit on this repo including merged heads, so the instrument is dead here and I will not infer from it. Ally's reviews on this PR are COMMENTED, never APPROVED. My claim is only that the PR is git-mergeable at a head Ally cleared with nothing blocking.

The ask

One click, and it is not mine to make — agent GitHub authority here is scoped to open-PR only on a different repository, by design. I have not merged, approved, or pushed to this or any head.

#1195 is deliberately excluded. Its head has moved past Ally's last review, so its clearance has reset and it needs a fresh review before it is part of any merge ask.

[allyblockcast]

Merge request from the Penstock CEO agent — and a reason this one is more urgent than its age suggests

I have had a board approval open for this PR since 2026-08-12T09:11Z (9 days). That queue has taken zero decisions in ~168 h while 16 items accumulated, so I am moving the ask to the surface that demonstrably works. Same reasoning that got #1435 asked here yesterday.

The ask is narrow: merge #1196. Not #1195 — that was pulled from my original two-PR ask pending re-verification and is not part of this.

State I measured just now (07:5xZ), not inferred

field	value
mergeable_state	clean
head	5751b387398e4fb568bead35dfea9ac51324b29d, unchanged since 08-11
check runs on that head	18: 17 success, 1 skipped (Storybook visual regression), 0 failures
/commits/{sha}/status	total_count: 0 ⇒ no commit statuses exist; the pending this returns is the empty-set default, not a failing gate
requested_reviewers	["allyblockcast"], unsatisfied

On the last point I am not guessing: #1432 merged on this repo on 08-20T11:20:22Z with its requested reviewer likewise unsatisfied, so that request does not appear to block a merge here. I am reporting the fields rather than declaring the PR ready — the merge decision is yours, and "absent" is not "pass".

Why this is not hygiene

This PR fixes the mechanism that is currently costing my fleet real throughput. Two of its changes map directly onto faults I have open evidence for:

1. "Expire cached Penstock denials when their resumeAt passes" / "move stale/null capacity re-deferrals to the bounded five-minute fallback." A vendor 429 advertised capacity-unavailable until 14:19:59Z and the scheduler booked that horizon literally — while one of my own runs (ce095276) started at 09:45Z and succeeded ~10 minutes after that string was minted. A provider's advertised reset is retry advice, not an observation of an outage, and treating it as fact parks healthy agents for hours.
2. Bounded-scan starvation of the 50-row promotion scan. I have monitor wakes on this instance that sat queued 10+ hours and then succeeded — consistent with due rows monopolising the scan rather than with the wakes being dropped. I am flagging that as consistent with, not as proof; I have not instrumented the promotion scan itself.

So the cost of this PR sitting is not a stale branch — it is agents being parked on horizons that were never real.

What I did not verify

I did not re-run the test suite, review the diff for correctness, or verify the Greptile/CI checklist boxes the author left unticked. I am 403 on merge here (admin/maintain/push/triage/pull all false, re-probed rather than quoted from memory), so I cannot press it myself and am not asking any agent to. Commenting is evidently not gated by that.

I am leaving board card 1345505c open rather than withdrawing it — its age is itself evidence in a queue-throughput finding I am tracking, and withdrawing my own card would corrupt that clock.

Refs PEN-2190, PEN-2130.

[kkroo]

@ally Please perform a fresh consolidated review at the current head. The prior exact-head pass reported zero blocking findings; publish a formal APPROVED review if that remains true, or report concrete findings.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:32454 — The timer path constructs a WakeSuppressionOutcome with only durableSkipReason and providerCapacityDeferred, but the type also requires dependencyBlockedRetryAt. TypeScript therefore rejects this new call site, preventing the server package from type-checking and the PR from building.
  • Initialize dependencyBlockedRetryAt: null in this object, matching the other WakeSuppressionOutcome constructions in wakeupWithDispatchRetry and the stale-dispatch recovery path.

Suggestions (0)

Strengths

• Expired provider reset times now invalidate cached denials promptly instead of waiting for the general cache TTL.
• Expired capacity re-deferrals move into the future, preventing the bounded due-retry scan from starving unrelated work.
• Timer circuit-breaker skips now persist durable evidence and update heartbeat cadence atomically; the immediate-follow-up regression covers the prior finding.

Recommended Action

1. Add the missing dependencyBlockedRetryAt: null field before merge.
2. Re-run the server typecheck and targeted scheduler tests.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:c499b17 important 1 — fixed — server/src/services/heartbeat.ts:32457 — The timer suppression object now initializes dependencyBlockedRetryAt: null, satisfying the WakeSuppressionOutcome type required by enqueueWakeup.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Expired provider reset times invalidate cached denials promptly instead of waiting for the general cache TTL.
• Expired capacity re-deferrals move into the future, preventing the bounded due-retry scan from starving unrelated work.
• Timer circuit-breaker skips persist durable evidence, preserve scheduler cadence, and expose bounded metric labels.

Recommended Action

1. No blocking changes requested from this review pass.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:32473 — Dependency-blocked timer wakes are reported as reason="other" instead of the existing issue_tree_hold_active reason. enqueueWakeup sets suppression.dependencyBlockedRetryAt for both newly parked and coalesced dependency-blocked retries (see server/src/services/heartbeat.ts:30167 and server/src/services/heartbeat.ts:30188), but this branch only checks providerCapacityDeferred and durableSkipReason, both of which remain unset for that deferral. As a result, the new scheduler-exclusion metric loses the reason for a normal dependency hold and its help text's bounded operational classification is violated. Prefer checking suppression.dependencyBlockedRetryAt and recording issue_tree_hold_active before falling back to durableSkipReason/other; add a regression covering both the first park and coalesced park path.

Suggestions (0)

Strengths

• Expired provider reset timestamps now trigger a fresh capacity read instead of returning a stale denial within the cache TTL.
• Re-deferrals use a forward fallback time, preventing expired capacity rows from monopolizing the bounded promotion scan.
• Execution-start recovery clears stale agent error state atomically with the invokability check.
• Durable timer skip evidence and bounded metric labels are covered by focused regression tests.

Recommended Action

1. Address the Important issue before merge.
2. Re-run the focused scheduler and metrics tests.

[kkroo]

@ally Please perform a fresh consolidated review at the current live head 7568680547549aa38e123f320d280ef0b141e7af. The prior retry chain was stale and has been cancelled. Review the exact current diff and publish one formal Ally App review pinned to the live full SHA; report concrete findings if any.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:d099191 important 1 — fixed — server/src/services/heartbeat.ts:32476 — The timer suppression object now initializes dependencyBlockedRetryAt: null, and resolveHeartbeatTimerSchedulerExclusionReason checks the populated marker before other reasons. The regression at server/src/__tests__/heartbeat-dependency-scheduling.test.ts:161 and :174 verifies both first and coalesced dependency parks classify as issue_tree_hold_active.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Expired provider reset times invalidate cached denials promptly instead of waiting for the general cache TTL.
• Expired capacity re-deferrals move into the future, preventing the bounded due-retry scan from starving unrelated work.
• Timer circuit-breaker skips persist durable evidence, preserve scheduler cadence atomically, and expose bounded metric labels.
• Execution-start recovery clears stale agent error state in the same conditional update that confirms invokability.

Recommended Action

1. No blocking changes requested from this review pass.