fix(heartbeat): stop reaper from killing live pre-adapter runs (BLO-13176) - #576
Merged
Merged
Conversation
…3176)
The executor's orphaned-heartbeat reaper was force-killing live agent runs
before they ever invoked the model adapter, surfacing fleet-wide as
`process_lost` ("k8s job terminated ... before external adapter invocation").
Confirmed blocker for BLO-12825 (critical): its run 6d4843b2 was reaped at
~5 min with a `2/2 Running` pod — penstock logs show zero model relays in the
death window, and the Job spec has activeDeadlineSeconds: null, so the executor
itself deleted the Job. Follow-on to BLO-12996 (the reaper that added
force-reap of live-but-silent Jobs).
Two defects, both fixed:
1. Over-reaping pre-adapter runs. Once the 5-min pre-adapter grace expired, a
run with no adapter.invoke event fell straight through to the process_lost
finalize WITHOUT ever consulting kube Job liveness — so a pod still
provisioning (image pull, repo clone, opencode/claude cold boot) past 5 min
was killed even though it was alive. Now a pre-adapter run past the grace
consults the kube signal the reaper already gathered: only POSITIVE evidence
of a live Job protects it (with a 45-min hard-stale escape hatch for a
genuinely wedged setup). "dead"/"unknown" keep the pre-existing fall-through,
so real orphans and the kube-unavailable degraded path reap exactly as
before. Mirrors the 2026-05-23 guard that already protects live-but-quiet
STARTED runs.
2. deletedJobs:0 force-kill thrash. The hard-stale force-kill deleted the Job
before claiming the run terminally, so overlapping reaper passes could both
select the still-`running` row and both delete + warn. The staleKill path now
compare-and-swaps status=running -> failed FIRST (setRunStatusIfRunning); only
the winner deletes the Job, losers no-op.
Tests: 4 new cases covering the live-Job-protect (status + list snapshot), the
confirmed-gone orphan still reaping, and the hard-stale pre-adapter force-kill +
idempotency. Full heartbeat-process-recovery suite green (125/125), incl. the
BLO-12996 guards.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 |
kkroo
added a commit
that referenced
this pull request
Jul 2, 2026
…droom (BLO-12563) (#577) Per-merge helm deploys were recreating paperclip-0 ~hourly (5 image rolls in the 8h before 07:51Z on 2026-07-02, STS revs 670-674), and every roll hard-killed whatever agent run was mid-setup: the pod granted the Node SIGTERM handler only the k8s default 30s grace minus the 5s preStop sleep, while the handler's SSE drain alone is bounded at 25s - so the graceful drain was truncated by SIGKILL on essentially every rollout. Deploy batching (docker.yml): - push events still build+push every merge, but the deploy job now debounces: it skips the rollout while paperclip-0 is younger than 6h (directly encoding BLO-12563's acceptance metric). - a new 6-hourly scheduled window deploys master tip, catching up any debounced commits; it verifies the per-commit Harbor tag exists (with retries) before rolling, so a window firing mid-build fails loud instead of rolling a missing tag. - urgent escape hatches: workflow_dispatch, or "[deploy]" in the head commit message, bypass the debounce. - deploy now runs when build-and-push was skipped (schedule/dispatch) but never after a failed build. Drain headroom (chart): - pod.terminationGracePeriodSeconds value (default 120) wired into both the worker StatefulSet and the api Deployment, replacing the implicit 30s default, so the existing graceful-drain sequence (SSE drain 25s, server.close, telemetry/OTel flush, Linear tunnel teardown) actually completes. Full run-reattach across restarts remains BLO-12564; the reaper fix for live pre-adapter runs landed separately as BLO-13176 (#576). Co-authored-by: Claude Fable 5 <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.
Looks good. I traced the full reapOrphanedRuns control flow (not just the diff) against the current file at head c51732e2 to check the new pre-adapter liveness branch interacts correctly with the pre-existing STARTED-run liveness branch below it, and it's internally consistent:
resolveExternalLifecycleJobLivenesscorrectly falls back to "unknown" when bothjobRunStatusesandliveJobRunIdsare null (kube fully unavailable), which preserves the pre-existing silence-floor fallthrough — matches the PR's stated intent that only "dead"/"unknown" reap as before.- The CAS-first finalize (
setRunStatusIfRunning) correctly gates the destructivedeleteAgentJobsForRuncall behind winning therunning → failedtransition, and reuses an existing helper (already used elsewhere for the STARTED hard-stale guard) rather than inventing new locking. - Verified the 45-min hard-stale math against the test fixtures:
seedRunFixturepinsstartedAt/createdAtto a fixed historical date but letslastOutputAtdefault to real wall-clocknew Date(), andexternalLifecycleRecentRefTimetakes the max — so the 6-min and 50-min test cases correctly land on either side ofEXTERNAL_LIFECYCLE_HARD_STALE_MSagainst the realnowthe reaper uses. No off-by-fixture-date bug. - The "STILL reaps... confirmed gone by name" test correctly exercises the one path where a pre-adapter run remains genuinely reapable (BLO-8827 regression guard).
Suggestions (2)
- [code]
server/src/services/heartbeat.ts(finalizeExternalLifecycleTerminalRunappendRunEvent payload, ~line 10327) — the force-kill event payload forstaleKillruns includesjobPhase/jobReason/jobMessagebut noexternalLifecyclePreAdaptermarker, unlike the plainprocess_lostpath a few lines down which does stamp it (line ~10896). An operator triagingexternal_lifecycle_stale_killedevents can't tell "wedged before ever invoking the adapter" apart from "wedged after invoking it" without cross-referencinghasAdapterInvocationEventseparately. Worth threading the same flag through for consistency. - [perf]
resolveExternalLifecycleJobLivenessissues an extrareadAgentJobRunStatusByNamekube call for every pre-adapter run that misses the snapshot, on every reaper tick until it resolves. This mirrors an existing pattern in the STARTED-run branch, so it's not a new class of cost, but worth keeping an eye on fleet-wide kube QPS if the pre-adapter population grows (e.g. an image-pull incident stalling many runs at once).
Strengths
- Root-caused with concrete evidence (executor logs, pod phase, penstock silence) rather than a speculative fix.
- 4 new tests cover exactly the four branch outcomes (live+protected, live via list-fallback, confirmed-dead still reaps, wedged past hard ceiling + idempotent second pass) — the idempotency test is a good catch given the
deletedJobs:0thrash this PR also fixes. - The CAS fix for the second defect (
deletedJobs:0thrash) is a real bug and the ordering fix (claim before delete) is the correct direction.
Recommended Action
Nothing blocking. Consider the payload-marker suggestion opportunistically.
kkroo
added a commit
that referenced
this pull request
Jul 2, 2026
… follow-on) (#578) Follow-on to #576. With the pre-adapter reaper fixed, BLO-12825 still could not complete: its runs kept dying "before external adapter invocation" — but NOT from the reaper. Root cause is over-dispatch. An external-lifecycle (k8s Job) agent can only run ONE Job at a time — the `runningCount>0` and `hasActiveJobForAgent` gates in startNextQueuedRunForAgent reject any second dispatch while one is active. But on an IDLE agent (runningCount 0), `availableSlots = maxConcurrentRuns` (e.g. 3), so the claim loop claimed and `executeRun`'d up to 3 queued runs (one per distinct issue) CONCURRENTLY. Only the first to reach Job creation won the single slot; the losers sat pre-adapter with no Job and were correctly reaped as process_lost. The race is first-lease-wins, not priority-ordered — so on 2026-07-02 the `critical` BLO-12825 repeatedly lost the slot to a sibling that leased ~5s earlier (confirmed in prod: run 49ca8e50 leased 17:32:11, never got a Job, reaped 17:36:17, while a normal-priority sibling that leased 17:32:06 got the Job and ran fine). Fix: cap external-lifecycle dispatch to a single run (`hasExternalLifecycle(adapterType) ? 1 : maxConcurrentRuns`). This (a) stops leasing work we cannot immediately give a Job — no more doomed surplus pre-adapter runs — and (b) gives the one slot to the top of the existing priority sort, so a critical issue wins instead of the fastest leaser. Local (child-process) adapters are unaffected and keep full concurrency. Test: new case — an idle opencode_k8s agent with maxConcurrentRuns=3 and three queued distinct-issue runs (critical is the newest, so createdAt/first-lease would NOT pick it) claims exactly ONE run, and it is the critical one; the other two stay queued. Full heartbeat-process-recovery + dispatch-priority suites green (128/128). Server typecheck clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
13 tasks
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.
Summary
The executor's orphaned-heartbeat reaper was force-killing live agent runs before they ever invoked the model adapter, surfacing fleet-wide as
process_lost("k8s job terminated … before external adapter invocation"). This is the confirmed blocker for BLO-12825 (critical — could not complete a single run for days) and it lines up with the dashboard's chronic run-failure counts.Follow-on to BLO-12996 / #575 (which added
reapOrphanedRunsforce-reap of live-but-silent Jobs). BLO-12996 correctly reaps truly-dead Jobs — but the reaper was also killing live-but-pre-adapter runs whose setup exceeded the grace window.Evidence (executor's own logs,
paperclip-0, 2026-07-02)6d4843b2reaped at 08:58:17 (reaped orphaned heartbeat runs), ~5 min after preRun, with a2/2 Runningpod.activeDeadlineSeconds: null,restartPolicy: Never— the ~5-min death is the executor deleting the Job, not a k8s time cap.Two defects, both fixed
1. Over-reaping pre-adapter runs. Once the 5-min pre-adapter grace expired, a run with no
adapter.invokeevent fell straight through to theprocess_lostfinalize without ever consulting kube Job liveness — so a pod still provisioning (image pull, repo clone, opencode/claude cold boot) past 5 min was killed while alive. Now a pre-adapter run past the grace consults the kube signal the reaper already gathered this pass: only positive evidence of a live Job protects it (with a 45-min hard-stale escape hatch for a genuinely wedged setup).dead/unknownkeep the pre-existing fall-through, so real orphans and the kube-unavailable degraded path reap exactly as before. Mirrors the 2026-05-23 guard that already protects live-but-quiet started runs.2.
deletedJobs:0force-kill thrash. The hard-stale force-kill deleted the Job before claiming the run terminally, so overlapping reaper passes could both select the still-runningrow and both delete + warn (observed: 5 consecutiveforce-killed live-but-silent … deletedJobs:0warns for one runId). ThestaleKillpath now compare-and-swapsstatus=running → failedfirst (setRunStatusIfRunning); only the winner deletes the Job, losers no-op. A failed delete is reaped next pass bycleanupTerminalExternalLifecycleJobs.Tests
4 new cases in
heartbeat-process-recovery.test.ts:Full suite green: 125/125, including the three BLO-12996 hard-stale guards and the 2026-05-23 live-but-quiet guard. Server typecheck clean.
🤖 Generated with Claude Code