Skip to content

fix(heartbeat): re-decide the whole capacity park, and make parked agents observable (BLO-24011) - #1286

Merged
kkroo merged 1 commit into
masterfrom
fix/blo-24011-capacity-park-observability
Aug 11, 2026
Merged

fix(heartbeat): re-decide the whole capacity park, and make parked agents observable (BLO-24011)#1286
kkroo merged 1 commit into
masterfrom
fix/blo-24011-capacity-park-observability

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown

Thinking Path

  • BLO-24011 reported a ~64-minute provider Retry-After becoming a ~4-day agent freeze, taking PlatformSREEngineer offline across an active production incident.
  • The horizon clamp landed in fix(heartbeat): bound provider-advertised retry horizons (BLO-23438) #1225, which satisfies the "retry on the order of N" criterion. Reviewing the incident row against that fix showed the report's core confusion was not the clamp at all.
  • retryNotBefore: 08:00Z (from a denial advertising 3834s) and scheduledRetryAt: Aug 13 (from a later denial advertising ~395987s) were two different gate decisions sharing one row. The promotion-time re-defer updated attempt/due and left every descriptive field behind.
  • So an operator reading that row could not tell a superseded 64-minute figure from the park actually in force — which is exactly how it read as a two-orders-of-magnitude scheduler bug on attempt 1.
  • The third acceptance criterion (fleet-level visibility) had no implementation at all: nothing in the codebase answered "who is parked?" without waking each agent.

Linked Issues or Issue Description

Refs BLO-24011. Follows #1225 (BLO-23438), which clamped the horizon; this closes the two acceptance criteria that fix did not reach.

What Changed

  • Re-decide the whole park, not just when it lands. Both the initial capacity gate and the promotion-time re-defer now write resultJson through a shared applyCcrotateCapacityDecision, which clears the previous decision's keys before writing the current one. A superseded advertised value can no longer linger beside a park it does not explain, and the two writers cannot drift.
  • GET /api/companies/:companyId/parked-agents — agents on a scheduled_retry, soonest-due first, with reason, attempt, and (for capacity parks) what the provider advertised beside what was actually booked. overdueMs separates "parked a long time" from "due but the sweep is not draining it", which previously looked identical.
  • paperclipListParkedAgents MCP tool wrapping it. Authorization reuses the same per-agent read filter as the agents list rather than gating board-only — an agent asking "who is parked?" is the primary caller, and board-only would defeat the criterion.

Verification

  • pnpm exec vitest run over ccrotate-capacity-retry, heartbeat-ccrotate-capacity-retry, heartbeat-retry-scheduling, heartbeat-rate-limit-retry-schedule, parked-agents-routes, openapi-routes, metrics-ccrotate-capacity-deferred, mcp-server/tools133 passed.
  • pnpm --filter @paperclipai/server typecheck — passed. pnpm --filter @paperclipai/mcp-server typecheck — passed.
  • git diff --check — clean.

The new regression replays the incident end to end through the real persistence path: initial denial advertising 3834s, then a promotion-time denial advertising ~4.6 days. It asserts attempt 1 never parks past 24h, that the row's retry floor and its scheduled instant agree, and that the superseded 3834s figure is gone.

Negative control: reverting only the resultJson write on the re-defer fails that test (expected '…T00:34:09Z' to be '…T00:51:30Z'), so the assertion is pinned to the defect rather than passing incidentally.

Acceptance Criteria

Criterion Status
Retry-After of N → retry on the order of N Already met by #1225; now also covered by an end-to-end regression
scheduledRetryAttempt: 1 never produces a multi-day delay Asserted in the new two-gate regression
Parked agents are observable without invoking a heartbeat on each New endpoint + MCP tool

Risks

The endpoint is a read over heartbeat_runs filtered to status='scheduled_retry'; the existing (company_id, status, …) index prefixes serve it, with scheduledRetryAt ordering as a post-filter sort on a small result set. No schema or migration. The resultJson change is additive in shape — it rewrites capacity-decision keys on a path that previously left them stale, and touches no other consumer of that column.

Not addressed here (deliberately, and worth a separate ticket): BLO-24011 notes that the frozen run was created inside the BLO-23456 vault-wedge window, so that wedge poisoned agent scheduling for days after the 11.7-hour customer-facing outage. That blast-radius linkage raises the cost of leaving BLO-23456 unfixed and is not recorded on any ticket; it is a scoping decision rather than a code change.

Model Used

Claude Opus 5 (claude-opus-5[1m]), with repo-local test execution and a negative-control run to confirm the regression fails without the fix.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used
  • 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 (fix(heartbeat): bound provider-advertised retry horizons (BLO-23438) #1225, fix(heartbeat): bound provider-advertised retry horizons (BLO-23438) #1186)
  • I have linked the issue with Refs
  • 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 (not applicable — API + scheduler)
  • I have updated relevant documentation (mcp-server README tool list)
  • I have considered and documented any risks above
  • All Paperclip CI gates are green (pending)
  • Greptile is 5/5 with no open P2s (pending)
  • I will address all Greptile and reviewer comments before requesting merge

🤖 Generated with Claude Code

…ents observable (BLO-24011)

A `ccrotate_capacity` park is re-decided every time the run comes due and the
pool is still exhausted, but the promotion-time re-defer updated only
`scheduledRetryAttempt`/`scheduledRetryAt`. Every descriptive field stayed
behind from the *first* denial, so the incident row read
`penstockRetryAfterSeconds: 3834` and `retryNotBefore: 08:00Z` beside a
`scheduledRetryAt` four days out — two decisions wearing one row, two orders of
magnitude apart, on attempt 1, with nothing explaining why.

The horizon clamp itself landed in #1225. This closes the remaining gap:

- Route both writers through `applyCcrotateCapacityDecision` so a re-defer
  clears the previous decision wholesale before writing the current one, and
  the two writers cannot drift.
- Add `GET /api/companies/:companyId/parked-agents` + the
  `paperclipListParkedAgents` MCP tool, answering "which agents cannot run, and
  until when?" without invoking a heartbeat on each one to find out. Reuses the
  same per-agent read filter as the agents list, so an agent caller sees exactly
  what it is already allowed to see.
- `overdueMs` separates "parked a long time" from "due but the sweep is not
  draining it" — different failures that looked identical before.

Regression replays the incident end to end: initial denial advertising 3834s,
promotion-time denial advertising ~4.6 days. Asserts attempt 1 never parks past
24h, that the row's retry floor and scheduled instant agree, and that the
superseded 3834s figure is gone rather than sitting beside a park it no longer
explains. Verified as a negative control: reverting the resultJson write fails
the test.

Co-Authored-By: Claude <noreply@anthropic.com>
@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-24011
🔗 Paperclip issue: BLO-23456
🔗 Paperclip issue: BLO-23438

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-24011
🔗 Paperclip issue: BLO-23456
🔗 Paperclip issue: BLO-23438

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

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

Looks good. The re-defer path now replaces all capacity-decision metadata with the current denial, and the parked-agents route provides the intended observable, access-filtered fleet view.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The two-gate regression test directly covers the stale-metadata incident and validates the clamped retry horizon.
  • The new route covers ordering, overdue state, reason filtering, same-company access, and tenant isolation.

Recommended Action

  1. Merge when the existing CI checks pass.

@kkroo
kkroo merged commit 953fd2e into master Aug 11, 2026
5 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant