fix(heartbeat): force-reap live-but-silent external-lifecycle Jobs (BLO-12996)

[kkroo]

What & why

A k8s Job that is still phase: active but has emitted no useful action for a long time blocks all dispatch for its agent. startNextQueuedRunForAgent's hasActiveJobForAgent gate refuses to start newly-queued work while the zombie's Job is alive — even though BLO-12990 Fix #1 already excludes stale runs from the slot-count gate. Net effect: a single silent run starves high-priority work for hours. Observed repeatedly on 2026-07-01/02 (90–120 min zombies on MulticastEngineer / Staff Engineer); only manual kubectl delete pod unblocked them.

The trap this avoids

The naive fix — reap any silent active Job at EXTERNAL_LIFECYCLE_STALE_MS (15 min) — would reintroduce the 2026-05-23 regression: killing live-but-quiet Jobs at that floor produced ~6.5/hr fleet-wide false process_lost on healthy long-streaming agents (the guard is commented in reapOrphanedRuns, and pinned by the existing 16-min "does not reap … live but silent" tests).

Approach — a separate hard ceiling

• New EXTERNAL_LIFECYCLE_HARD_STALE_MS = 45 min gates only the destructive kill. The 15-min soft floor is unchanged (slot-gate counting + reaping absent/terminal Jobs).
• reapOrphanedRuns now force-kills a run whose Job is still phase: active but silent ≥ 45 min — in both the kube-status-snapshot and kube-list branches — reusing finalizeExternalLifecycleTerminalRun's teardown (lease release, issue-execution promote, dispatch trigger, bounded retry) with errorCode external_lifecycle_stale_killed.
• Below 45 min a live-but-quiet Job is still left alone → 2026-05-23 guard preserved.

Keyed on lastUsefulActionAt (not stdout), so a genuinely working agent that streams progress within 45 min is never killed; observed zombies (no useful action for 90–120 min) are.

Tests

server/src/__tests__/heartbeat-process-recovery.test.ts — 121 passed locally (vitest run), including:

• force-kill via kube-status-snapshot branch (50 min silent) → failed + external_lifecycle_stale_killed + Job deleted
• force-kill via kube-list branch (50 min silent) → same
• 30-min boundary case → NOT killed (guards the 2026-05-23 fix)
• existing 16-min "live but silent → not reaped" guards remain green

Review note (please do not fast-merge)

This touches the exact reaper path that caused the 2026-05-23 fleet incident. The 45-min threshold is a deliberate, conservative choice — worth a CTO eye before merge. Let CI's serialized server suites run.

Closes BLO-12996.

🤖 Generated with Claude Code

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

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

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:508-550 — The new external_lifecycle_stale_killed errorCode is not added to shouldScheduleAutomaticRunRetry's allowlist, so force-killed zombie runs get no automatic retry, even for pr_review wakes.
  • finalizeExternalLifecycleTerminalRun (heartbeat.ts:10282) calls shouldScheduleAutomaticRunRetry(finalizedRun) after every finalize, including the new staleKill: true path. That function only retries for transient_upstream/rate_limit_exhausted families, pr_review_auth_expired, k8s_concurrent_run_blocked, k8s_pod_schedule_failed, job_missing, adapter_failed, or process_lost — external_lifecycle_stale_killed matches none of these, so it always falls through to return false.
  • This is the exact failure class the codebase has patched three times already for adjacent codes (BLO-8215 pr_review_auth_expired, BLO-9147 k8s_concurrent_run_blocked, BLO-10448 job_missing/k8s_pod_schedule_failed) — each comment explicitly cites "silently dropped a PR review with no retry" as the bug being fixed. A run silenced for 45+ min while doing a pr_review task (e.g. an Ally review job itself, stuck on a slow CI wait or big diff) will now hit the same silent-drop bug those fixes closed.
  • Recommend gating external_lifecycle_stale_killed through isPrReviewRetryContext the same way job_missing is, unless intentionally excluded — in which case a one-line comment explaining why a hard-stale kill is not eligible for the same retry treatment as a vanished Job would remove the ambiguity.

Suggestions (2)

• [pr-review-toolkit:tests] New tests (heartbeat-process-recovery.test.ts:9-76) assert reaped, status, errorCode, and the Job-delete call, but never assert retry behavior (e.g. scheduleBoundedRetryForRun called/not-called). A test asserting the retry decision for a pr_review-context run would have caught the Important issue above.
• [pr-review-toolkit:tests] Coverage jumps straight from 30-min (not stale) to 50-min (stale); there's no test at the actual 45-min EXTERNAL_LIFECYCLE_HARD_STALE_MS boundary. Given the 2026-05-23 incident was itself an off-by-threshold regression, a boundary-exact test would harden this constant against future drift.

Strengths

• Excellent PR description with RCA context and an explicit "don't fast-merge" flag on a historically sensitive reaper path.
• Every new constant/branch has a comment explaining the why (2026-05-23 regression, BLO-12990 slot-gate interaction), not just the what.
• Tests cover both liveness-check branches (kube-status-snapshot and kube-list) plus the negative case guarding the prior incident.
• Correctly reuses finalizeExternalLifecycleTerminalRun's existing teardown (lease release, issue-execution promote, dispatch trigger, bounded-retry hook) instead of duplicating it — the retry-hook gap above is a missing allowlist entry, not a structural bypass.

Recommended Action

1. Resolve the retry-eligibility gap for external_lifecycle_stale_killed before merge, or document why it's deliberately excluded.
2. Suggestions are opportunistic — a boundary test is cheap insurance given this constant's incident history.