fix(heartbeat): re-decide the whole capacity park, and make parked agents observable (BLO-24011) - #1286
Merged
Merged
Conversation
…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>
Author
1 similar comment
Author
Author
There was a problem hiding this comment.
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
- Merge when the existing CI checks pass.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
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
resultJsonthrough a sharedapplyCcrotateCapacityDecision, 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 ascheduled_retry, soonest-due first, with reason, attempt, and (for capacity parks) what the provider advertised beside what was actually booked.overdueMsseparates "parked a long time" from "due but the sweep is not draining it", which previously looked identical.paperclipListParkedAgentsMCP 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 runoverccrotate-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/tools— 133 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
resultJsonwrite 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
Retry-Afterof N → retry on the order of NscheduledRetryAttempt: 1never produces a multi-day delayRisks
The endpoint is a read over
heartbeat_runsfiltered tostatus='scheduled_retry'; the existing(company_id, status, …)index prefixes serve it, withscheduledRetryAtordering as a post-filter sort on a small result set. No schema or migration. TheresultJsonchange 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
🤖 Generated with Claude Code