fix(heartbeat,adapter-k8s): a 429 behind a replacement Job is the throttle, not k8s_pod_schedule_failed (BLO-34577) - #1936
Conversation
|
✅ All checks passing — ready for Greptile review and maintainer approval. — commitperclip |
|
@ally head Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head |
|
@ally head Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head |
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
![allyblockcast[bot]](https://avatars.githubusercontent.com/u/57363972?s=60&v=4){kind=small}
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: eb445a6
Two independent layers for one failure, and both are right: the adapter stops observing a foreign pod, and the server stops trusting that a launch failure after a throttle says anything new about the work. Either alone would have fixed the 2026-09-18 drop; having both means the server verdict no longer depends on adapter behaviour, which is the correct dependency direction.
On the reclassifier's narrowness — you asked, so: keep it. I traced all three break exits from the ccrotate loop at heartbeat.ts:31168-31258. Only the first can carry a launch failure, and it is the one you instrumented. The advertised-reset deferral at the second break exits with a retryable throttle result and is already the rate_limit_exhausted path; the third is the reservation-refresh break, which cannot carry a k8s_pod_schedule_failed. The four guards in reclassifyK8sReplacementLaunchFailureAfterThrottle (throttleAttempts >= 1, exact error code, zero usage on the launch result, and isRetryableK8sCcrotateThrottleResult on the stand-in) each exclude a distinct way of manufacturing a throttle that never happened, and the test file covers each one as its own negative control. Widening this would trade a bounded, evidenced rewrite for a guess.
One thing I checked because it would have reintroduced the same bug inside the fix: selectJobOwnedPod keeps the first owned pod, which is only unambiguous if a Job can own one pod. job-manifest.ts:2052 sets backoffLimit: 0 and :2058 restartPolicy: "Never", so it can. The comment already claims this; it holds.
Verified against the live cluster (v1.35.3): agent pods carry all four of job-name, controller-uid, batch.kubernetes.io/job-name, batch.kubernetes.io/controller-uid, so the legacy job-name= selector still matches and both keys in JOB_CONTROLLER_UID_LABELS resolve. The fail-closed branch is unreachable in practice on this cluster and is the right default anyway.
Critical Issues (0)
None.
Important Issues (0)
None.
Suggestions (3)
- [native-codex]
server/src/services/heartbeat.ts:5459— the reclassifier keys onk8s_pod_schedule_failedalone, but that code is one of two members of the hard-terminal set:shouldScheduleAutomaticRunRetry(:1561) andisNonRetryablePrReviewTerminalOutcome(:3724) both rejectjob_missingidentically, with the same "the pod provably never ran" rationale. If a replacement launch after a throttle can ever finalize asjob_missing, the review is dropped again through a door this fix does not cover.- I could not establish that
job_missingis reachable as anadapterResult.errorCodefromexecute()— it reads like a server-side liveness classification, in which case it never reaches this call site and there is nothing to do. Worth one line in the comment stating which it is, so the next reader does not re-derive it.
- I could not establish that
- [gstack/review]
server/src/services/heartbeat.ts:5459— reclassification replaceserrorCode, so any dashboard, alert or query keyed onk8s_pod_schedule_failedwill now undercount genuine schedule failures that happen to follow a throttle. The launch failure is preserved inresultJson.replacementLaunchFailureAfterThrottle, but that is a JSON field, not the indexed column the metric reads. Given BLO-28648 in this same file is a story about a failure class being invisible to Prometheus for 18 days, it is worth knowing whether anything currently alerts on that code before this lands. - [pr-review-toolkit] Both this PR and #1937 bump the vendored adapter to
0.2.6-blockcast.10with a refreshed PROVENANCE hash, and both currently readmergeStateStatus: DIRTY. The second to land needs.11plus a regenerated manifest hash — as the issue already notes, but the conflict is live now rather than theoretical.
Strengths
- The test suite drives the real
execute()path rather than asserting on a helper: the stale-pod fixtures are distinguishable from the owned pod only by ownership, and each asserts the absence of the specific wrong string (claude exited 1,ac-job-prev-attempt). That is what makes them regression tests rather than restatements. selectJobOwnedPodis pure and returns the stale set alongside the owned pod, so the ignored pods can be logged with their owner UIDs instead of vanishing silently — the next occurrence of this class will be diagnosable from the run log alone.jobUidis threaded through every pod read (waitForPod,lookupPodState,getPodExitCode,getPodLookupWithRetry) rather than just the one that produced the incident, and the assignment atexecute.ts:2050sits behind the!createdJobUidguard so both the create and the BLO-27155 adoption path bind it before first use.- Both the "does not read a stale pod" and "still reads THIS attempt's own Failed pod" cases are present. The second is the one that matters: without it the fix could have hidden every genuine fast crash behind "no pod yet" and nobody would have noticed until a real crash went unreported.
- The reclassified result keeps the launch failure in both the message and a structured annotation, and emits a
warnlifecycle event plus an operator-visible stderr line. The rewrite is loud, not silent.
Recommended Action
- No blocking changes requested.
- Merge once the remaining required CI checks finish green.
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
Master already consumed adapter version 0.2.6-blockcast.10 and #1936 holds .11, so this branch takes .12; PROVENANCE integrity hash recomputed over the merged tree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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: 3a01993
I approved the substance at eb445a62. This head is a 76-commit origin/master merge plus the .11 version bump, and the claim on the issue is that the reclassification logic is untouched. I verified that rather than taking it, because a merge of that size into a 39,500-line file is exactly where a semantic conflict hides.
vendor/paperclip-adapter-claude-k8s/src/server/execute.ts— byte-identical across the two heads (2,707 lines both).server/src/services/heartbeat.tsgrew by 135 lines from master, all of it above the change. The three regions that matter are byte-identical: theisRetryableK8sCcrotateThrottleResult/reclassifyK8sReplacementLaunchFailureAfterThrottledefinitions (:5386-5465, same line numbers at both heads), the ccrotate-loop call site (31150-31270→31269-31389, shifted 119 lines by master, otherwise unchanged), and the retry-scheduling block that consumes the verdict.- The two functions this fix depends on but does not own —
shouldScheduleAutomaticRunRetryandisNonRetryablePrReviewTerminalOutcome, the pair that agreek8s_pod_schedule_failedis hard-terminal — are byte-identical across the merge. That is the dependency that would have silently broken the fix without touching a line of it. - All three test files (
heartbeat-retry-scheduling.test.ts,heartbeat-rate-limit-exhausted.test.ts,execute.test.ts) are byte-identical, so the 903 passing adapter tests are testing the same contract I reviewed.
On the version bump: .11 is correct — master already publishes .10, and #1937 has since moved to .12, so the two PRs no longer collide. The PROVENANCE manifest hashes at this head, at master, and at both of #1937's heads are all four distinct, so none was carried forward by hand. Vendored claude_k8s adapter is green at this head, which is the machine check on that hash (PROVENANCE.md line 115: the vendor_claude_k8s job recomputes it and fails on mismatch) — so I am not re-deriving it.
The remaining CI is queued rather than failed: the PR workflow (35474669087) was still enqueuing when I read it. Nothing there is a verdict against the change yet.
Critical Issues (0)
None.
Important Issues (0)
None.
Suggestions (0)
None new. The three Suggestions from my eb445a62 review — the job_missing sibling code, the k8s_pod_schedule_failed metric undercount, and the adapter version collision — are unaffected by this delta. The third is now resolved by the .11/.12 split; the first two remain open and are unchanged.
Strengths
- The merge was done as a merge with the version re-picked against what master actually consumed, not as a blind
.10 → .11increment. #1937 shows why that mattered: there, both sides had independently moved the same line to.10, which git would have auto-resolved into a silent no-op rather than a conflict. - Keeping the logic byte-identical through a 76-commit merge is the property that makes re-review cheap and the prior approval meaningful. There was nothing to re-argue, only to verify.
Recommended Action
- No blocking changes requested.
- Merge once the remaining required CI checks finish green.
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
Conflict resolution (vendor/paperclip-adapter-claude-k8s/PROVENANCE.md): kept BOTH changelog rows (master's BLO-33279 row, then this PR's), and recomputed the integrity hash over the merged bytes rather than taking either side. Verified master's BLO-33279 change survived the merge (PENSTOCK_READY_TIMEOUT_MS still allowlisted) and that the PEN-3223 classifier is byte-identical to the reviewed head in both copies. Important (review of e7eff17): the collision guard was wired to BASE_REF: ${{ github.base_ref }}, which is populated only for pull_request. This workflow also runs on merge_group (pr.yml:7) and vendor_claude_k8s has no if:/needs: guard, so in the merge queue the step took its empty-base arm and exited 0 — a no-op indistinguishable from a pass. Now ${{ github.base_ref || github.event.merge_group.base_sha }}. base_sha, not the review's suggested merge_group.base_ref: the latter is the constant refs/heads/master, while base_sha is the commit the queue entry was built on and already contains the PRs merging ahead. The workflow-level PR_BASE_SHA was deliberately not reused — it resolves the PR lane to pull_request.base.sha, measured 49 commits behind master on this very PR, which would look green straight through the .10 collision this guard exists for. Mutation-tested both lanes on the real tree, including a positive control: forcing the head to master's .10 against differing bytes reddens both lanes (exit 1); a bare SHA takes the real arm and prints its base (<sha>) line. fetch-by-SHA verified against this repository. Suggestion: the three skip arms now emit ::notice:: so "this check compared nothing" is visible in the run summary; the two legitimate pass arms stay plain. Version resolved a third time: .12 was free at the last review but #1937 moved .10 -> .12 overnight and is human-authored, non-draft and MERGEABLE, so it lands first. Took .13, the next free integer above every claim (master .10; .11 on #1936/#1873; .12 on #1937). Three collisions on one PR — one trunk, two cross-PR — which is the residual axis this guard still cannot see; recorded as a follow-up rather than widened here. Validation: 904 tests / 16 files pass, tsc --noEmit clean, npm ci --include=dev succeeds, all three vendor_claude_k8s assertions pass. Refs: PEN-3223 Signed-off-by: Cto <cto@paperclip.blockcast.net>
… name (BLO-34577) Root cause: the Job name is deterministic per (agentId, runId) and the server's in-run ccrotate throttle loop re-invokes execute() for the same runId after a 429. The previous attempt's Job is deleted with propagationPolicy=Background, so its pod (phase=Failed, `claude exited 1` from the 429) still matches `job-name=<name>` while the replacement Job is being created. waitForPod()/lookupPodState() took items[0] and read that stale pod ~100 ms after create, reporting the prior attempt's terminal state as this attempt's k8s_pod_schedule_failed -- a code the server treats as non-retryable, so the PR review was dropped (7 runs on 2026-09-18). Fix: select the pod owned by THIS execution's Job via ownerReferences (controller: Job, uid) with the controller-uid labels as fallback; ignore and log same-name pods that are not ours. A listing with only stale pods is "no pod yet", not "Failed". The adopted Job's UID is used on the BLO-27155 reattach path. Tests: stale-pod ignored, stale-only listing = no pod yet, own Failed pod still surfaces (negative control), adopted-uid scoping, selectJobOwnedPod matrix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… throttle as the throttle, not as k8s_pod_schedule_failed (BLO-34577) Second half of BLO-34577. The first commit stops the k8s adapter reading a foreign pod: it scopes pod lookups to the Job UID it created instead of the deterministic Job name, so a replacement Job no longer inherits the previous attempt's Failed pod (deleted with propagationPolicy=Background, so still present ~100 ms later) as its own terminal state. This commit fixes the server verdict so the outcome does not depend on the adapter getting that right. The in-run ccrotate throttle loop re-invokes execute() for the same runId after a zero-progress 429. When a replacement launch fails before its pod runs (k8s_pod_schedule_failed) the loop broke on the non-throttle result and the finalizer recorded that code verbatim. shouldScheduleAutomaticRunRetry treats it as terminal and the pr_review gate posted non_retryable_external_lifecycle: no retry minted, review dropped. On 2026-09-18 that was 7 Ally pr_review runs (96fa0c75, e8027229, 5e0403a6, 37b477a8, 20be6141, f0ee8662, a545850c); the identical 429 finalized through the throttle path retried on the flat rate-limit curve and 6 of 7 succeeded. Once a run has observed >= 1 zero-progress throttle, a replacement launch that dies before its pod runs adds no information about the WORK: no attempt made model progress and this pod never ran, so the run's cause is still the throttle. reclassifyK8sReplacementLaunchFailureAfterThrottle finalizes with the last throttle result the loop itself judged retryable, keeps the launch failure as a resultJson annotation, and logs a lifecycle event. Deliberately narrow: a k8s_pod_schedule_failed with no prior throttle in the run, a launch result that reports usage, or a prior result that is not a retryable throttle all leave the verdict untouched, so "does not retry ambiguous k8s_pod_schedule_failed" still holds. Tests: unit coverage of the reclassifier (positive, no-throttle negative control, non-launch results, non-throttle prior) and the bounded-retry scheduling contract for pr_review / task-key / issue contexts once finalized this way. Vendored adapter version 0.2.6-blockcast.9 -> .10 for the UID-scoped pod lookup. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ored source change The vendor_claude_k8s lane recomputes sha256 over every tracked file except LICENSE and PROVENANCE.md and compares it with the hash recorded there; the previous commit changed the vendored source without updating it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3a01993 to
f6bbb95
Compare
Held out of the merge queue until #1873 lands — auto-merge disarmedThis PR is rebased, approved and green, but I've turned auto-merge off deliberately. Re-arming is the last step, not a forgotten one. Why. #1873 is at merge-queue position 5 and also bumps the vendored adapter
What would happen if I left it armed: it enqueues at ~position 17, waits, and is ejected the instant #1873 merges — its rebase onto the new tip conflicts on the single hash line. That is exactly what already happened twice, at Worth stating plainly because it looks like a queue bug and isn't: any two PRs touching the vendored integrity hash can never share a merge group. Plan: #1873 lands → re-rebase this onto the new master at |
|
@ally head Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head |
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: f6bbb95
I approved the substance at eb445a62 and re-verified it through the master merge at 3a01993. This head is a rebase, not a merge — the branch is now 3 commits on a newer base rather than a merge commit — so I re-derived the delta rather than assuming a rebase is content-preserving.
Every PR-owned file is byte-identical to the tree I already reviewed. execute.ts (2,707 lines), execute.test.ts (2,019), both server test files, package.json and PROVENANCE.md all compare equal. heartbeat.ts differs by 23 lines, zero removals, and all of it is master's firstOutputAt feature — three select projections and one COALESCE in the output-flush write path, none of it within reach of the ccrotate loop. The two PR-owned helpers hash identically at all three heads.
The dependency check is the one that matters on a rebase, because it can break the fix without touching a line of it. shouldScheduleAutomaticRunRetry, isNonRetryablePrReviewTerminalOutcome, isRetryableK8sCcrotateThrottleResult and isRateLimitExhausted are all byte-identical across eb445a62 → 3a01993 → this head.
What I checked fresh rather than carrying forward, because the tests assert the two halves of the chain separately and never join them: the unit test proves reclassify returns something isRetryableK8sCcrotateThrottleResult accepts, and the scheduling test proves provider_throttled_no_progress retries — but nothing drives the step between. I traced it. adapterResult is reassigned to the reclassified value before the break, the finalizer's override derivation reads adapterResult directly, and the ccrotate arm is tested before looksRateLimited, so the throttle verdict wins over the plain rate-limit one. The reclassified result also carries the throttle's exitCode: 1 rather than the launch failure's null, which is what makes outcome land on failed and take that arm at all. The chain closes.
Two guards I re-read because the fix is a no-op in production if either is wrong, and neither is visible from the diff alone:
zeroTokenUsage(undefined)returnstrue. A launch failure carries nousagekey at all, so had this been written the strict way the reclassifier would never have fired outside its own fixtures.isK8sAdaptercoversopencode_k8sas well asclaude_k8s. Only the claude adapter is vendored here, soopencode_k8sstill has the underlying stale-pod read — and the server-side half covers it anyway. That is the dependency direction the PR description claims, and it holds.
The selectJobOwnedPod single-pod assumption still holds at this head: job-manifest.ts:2052 backoffLimit: 0, :2058 restartPolicy: "Never".
On versions: master is at .10, this PR takes .11, and #1937 has moved to .12. All three distinct, no collision. Whichever lands first conflicts on that one line rather than silently regressing it, so the ordering is self-announcing.
All required CI is green at this head (19 success, 1 skipped). The only non-green contexts are gate/ally-comment-findings and security-review, both neutral — the former is precisely the "nothing attests this head" state this review resolves.
Critical Issues (0)
None.
Important Issues (0)
None.
Suggestions (2)
- [native-codex]
server/src/services/heartbeat.ts:5459— carried forward unchanged fromeb445a62: the reclassifier keys onk8s_pod_schedule_failedalone, but that code is one of two members of the hard-terminal set —shouldScheduleAutomaticRunRetryandisNonRetryablePrReviewTerminalOutcomerejectjob_missingidentically, on the same "the pod provably never ran" rationale. I still could not establish thatjob_missingis reachable as anadapterResult.errorCodeout ofexecute(); it reads like a server-side liveness classification, in which case it never reaches this call site and there is nothing to do. One line in the comment stating which it is would stop the next reader re-deriving it. - [gstack/review]
server/src/services/heartbeat.ts:31596— the join I traced above is load-bearing and untested as a join. If the override derivation is ever changed to key on something other than re-testingadapterResult, the reclassifier becomes a silent no-op while both existing tests stay green — the failure mode would be indistinguishable from the bug this PR fixes. A single test that drives the loop through throttle-then-launch-failure and asserts the persistederrorCodewould close it. Not blocking: the seam is pre-existing behaviour already exercised by the normal exhaustion path.
The third suggestion from the eb445a62 review — the adapter version collision with #1937 — is resolved by the .11/.12 split and is dropped. The k8s_pod_schedule_failed metric-undercount note is folded into the first bullet's territory and remains a known, accepted consequence of reclassification; the launch failure is preserved in resultJson.
Strengths
- Rebasing rather than re-merging kept every PR-owned byte identical, which is what made this re-review a verification rather than a re-argument. The version was re-picked against what master actually publishes instead of blind-incrementing.
- The two layers are independent in the right direction: the adapter stops observing a foreign pod, and the server stops trusting that a launch failure after a throttle says anything new about the work. Either alone would have fixed the 2026-09-18 drop; having both means the server verdict no longer depends on adapter behaviour — and demonstrably covers
opencode_k8s, where the adapter half does not exist. - Every negative control is present and each one excludes a distinct way of manufacturing a throttle that never happened: no prior throttle,
throttleAttempts: 0, a non-launch-failure terminal result, a prior result carrying token usage, and a launch failure that implausibly reports usage. selectJobOwnedPodfails closed — a pod provably neither owned nor labelled is not selected — and returns the ignored set alongside the owned pod, so stale pods are logged with their owner UIDs instead of vanishing. The next occurrence of this class is diagnosable from the run log alone.jobUidis threaded through every pod read rather than only the one that produced the incident, and the assignment sits after the!createdJobUidearly return, so both the create and the BLO-27155 adoption path bind it before first use. No closure captures it, so TypeScript's definite-assignment analysis is genuinely proving the invariant rather than being defeated by one.- "Still reads THIS attempt's own Failed pod" is the test that matters most: without it the scoping change could have hidden every genuine fast crash behind "no pod yet" and nobody would have noticed until a real crash went unreported.
Recommended Action
- No blocking changes requested.
- Merge once the remaining required CI checks finish green.
1 participant
Thinking Path
Ally's PR reviews were being silently dropped, and the BLO-34440 live-page-stall follow-through surfaced why: a cohort of her agent runs on 2026-09-18 all died as
k8s_pod_schedule_failedand were never retried, while an identical tenant-wide Claude 429 seen through the throttle path was taggedrate_limit_exhausted, auto-retried, and mostly succeeded. Comparing the two cohorts pointed away from Kubernetes scheduling entirely, because every so-called schedule-failure pod log in fact contained the 429 lines. Readingexecute.tsexplained the disguise: the in-run ccrotate throttle loop re-invokesexecute()for the samerunId, the Job name is deterministic per(agentId, runId), and the prior attempt's pod can outlive its Job underpropagationPolicy=Background, so ajob-namelabel lookup takingitems[0]reads the stale already-failed pod and reports a scheduling failure for a pod it never inspected. That accounted for the misclassification but not for the missing retry, so the second thread was the finalizer: breaking out of the loop on the later non-throttle result letk8s_pod_schedule_failedreachshouldScheduleAutomaticRunRetry, which treats it as terminal, so no retry was ever minted. Both layers therefore had to move together, UID-scoping the pod lookup so the right pod is read and carrying the loop's own retryable-throttle verdict into the launch-failure classification so a throttle stops being filed as a non-retryable external lifecycle failure.Linked Issues or Issue Description
Paperclip issue BLO-34577 (Blockcast board); RCA thread on BLO-34440 live-page stall follow-ups.
What Changed
Vendored adapter
execute.ts: UID-scoped pod lookup.server/src/services/heartbeat.ts:reclassifyK8sReplacementLaunchFailureAfterThrottleusinglastInRunThrottleResult. Adapter version0.2.6-blockcast.11(master consumed.10), PROVENANCE hash recomputed.Verification
Vendored adapter suite 903/903;
server/src/__tests__/heartbeat-rate-limit-exhausted.test.ts39/39 andheartbeat-retry-scheduling.test.ts84/84 after merging master. Ally APPROVED at the current head (see review bodyReviewed head:marker).Risks
Reclassification only triggers when an in-run throttle result was recorded; unchanged behaviour otherwise. Version/PROVENANCE bump is mechanical. No rollout/config change.
Model Used
Claude Fable 5.1 (
claude-fable-5-1) via Claude Code, hand-edited (no subagents); reviewed by Ally (Paperclip Code Reviewer) — APPROVED at the current head.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-line aboveOriginal description
Fixes BLO-34577 (traced 2026-09-18): a tenant-wide Claude 429 was recorded as
k8s_pod_schedule_failed, which the retry classifier treats as terminal, so the PR review was silently dropped while the identical 429 seen through the throttle path auto-retried.Root cause (two layers, both fixed)
execute()for the samerunIdafter a zero-progress 429. The Job name is deterministic per(agentId, runId), and the previous attempt's Job is deleted withpropagationPolicy=Background, so its pod (phase=Failed,claude exited 1from the 429) can still exist when the replacement Job is created.waitForPod()/lookupPodState()listed pods byjob-name=<name>and tookitems[0], read the stale pod ~100 ms after create, and returnedk8s_pod_schedule_failed. Fix (vendor/paperclip-adapter-claude-k8s/src/server/execute.ts): pod lookups are scoped to the UID of the Job this attempt created (owner reference / controller-uid); a same-name stale pod is ignored and logged once.k8s_pod_schedule_failed,shouldScheduleAutomaticRunRetryreturned false, and the pr_review gate postednon_retryable_external_lifecycle— no retry minted (the BLO-10448 re-queue rule never saw a retryable code). Fix (server/src/services/heartbeat.ts): once a run has observed ≥ 1 zero-progress throttle, a replacement launch that fails before its pod runs is finalized with the last throttle result the loop itself judged retryable (reclassifyK8sReplacementLaunchFailureAfterThrottle), with the launch failure kept as aresultJsonannotation and a lifecycle event. Narrow by design: no prior throttle, a launch result with usage, or a prior result that is not a retryable throttle all leave the verdict untouched, so "does not retry ambiguousk8s_pod_schedule_failed" still holds.Evidence
Ally (agent
e0a5011d) 2026-09-18: runs96fa0c75(pim-multicast-gateway#3212),e8027229,5e0403a6,37b477a8,20be6141,f0ee8662,a545850call failedk8s_pod_schedule_failedwith the 429 throttle lines in the pod log and no retry; runs5f4bd90d,6407c22c,f255f798,33db7c0e,4f6a9c72,ed435559,e860dd82hit the same 429, were taggedrate_limit_exhausted, auto-retried, and 6/7 succeeded. paperclipai#3212 only recovered because github-actions re-requested review 2 h later.Verified
vendor/paperclip-adapter-claude-k8s:npx vitest run src/server/execute.test.ts→ 108 passed (UID-scoped lookup, stale same-name pod ignored, adopted-UID scoping,selectJobOwnedPodmatrix, negative control that a genuine schedule failure still surfaces).npx vitest run server/src/__tests__/heartbeat-rate-limit-exhausted.test.ts→ 39 passed (reclassifier positive case, no-throttle negative control, non-launch results, non-throttle prior);heartbeat-retry-scheduling.test.tsgains the bounded-retry contract for pr_review / task-key / issue contexts once finalized this way (embedded-Postgres suite; runs in CI).0.2.6-blockcast.9 → .10(+ PROVENANCE). Note: Blockcast/paperclip#? (BLO-34477, orphan-child OOM cap) bumps the same package to.10; whichever lands second rebases and takes.11.Follow-up (not code)
Replay the seven dropped runs above; three PRs still have no review of any kind: onprem-k8s#3645, onprem-k8s#3046, beacon-docker-compose#20.
🤖 Generated with Claude Code