fix(heartbeat): stop a launch-stalled run absorbing its agent's wakes (PEN-1995)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents run on a heartbeat: a timer wake mints a heartbeat_runs row, an adapter is spawned, the agent does a pass. Cadence is the product's pulse — an agent that stops getting wakes is offline, silently
• enqueueWakeup coalesces a wake into an existing same-task-scope run so a burst of triggers doesn't mint N duplicate runs. The guard on that is filterZombieCoalesceTarget, which refuses to merge into a running row that has no live in-memory execution
• That guard has a blind spot. executeRun registers the run in activeRunExecutions at claim, before the adapter invoke — so a run that stalls in the claim→spawn window is tracked in memory (not a zombie) while having produced nothing
• Such a row is also excluded from reapOrphanedRuns at any age, so nothing finalizes it. Every subsequent __heartbeat__ timer wake is then absorbed into it by a bare UPDATE that mints no run and stamps no lastHeartbeatAt — the wake is simply lost — and that UPDATE refreshes updatedAt, which re-arms the shield. The stall suppresses the heartbeats and the suppressed heartbeats keep the stall alive
• Measured on the Summarizer built-in: a run wedged at lastOutputSeq: 1 for 11.87 h with zero runs started behind it; normal hourly cadence resumed exactly one interval after a worker restart cleared the Set
• This pull request stops a launch-stalled run from speaking for wakes it cannot service, so the wake mints its own run instead
• The benefit is that a single stalled spawn costs one run instead of taking the agent 100% offline until the worker restarts

Linked Issues or Issue Description

Refs PEN-1995 ([Operator] Contain and root-cause claude_local stillborn heartbeat windows), parent PEN-1990.

No GitHub issue exists; the investigation lives in Paperclip. Bug summary in the issue-template shape:

• What happened: two claude_local built-ins (Summarizer 9d5bc03e, Reflection Coach 83da1f23) intermittently stop running heartbeats for many hours.
• Expected: a stalled run fails or recovers; the agent's hourly cadence continues either way.
• Actual: cadence stops entirely for the lifetime of the stall. Two specimens: 236ca044 (11.87 h, 0 runs started behind it) and c1348962 (117 min and counting at time of measurement, 2 full intervals missed).
• Recovery today: only a worker restart.

What Changed

• isLaunchStalledRun(run, nowMs, staleMs?) — new exported predicate: a running row that has never flushed output past the pre-exec prefix (lastOutputSeq <= 1) and whose newest of lastUsefulActionAt/lastOutputAt/startedAt is older than the stale floor.
• filterLaunchStalledCoalesceTarget(target, nowMs, staleMs?) — the matching coalesce filter, composed with the existing filterZombieCoalesceTarget at the one call site in enqueueWakeup. The two are complementary: the zombie filter catches a running row with no in-memory execution (post-restart); this one catches a running row whose in-memory execution never reached the adapter.
• Tests: server/src/__tests__/heartbeat-launch-stalled-coalesce.test.ts (7 unit + 3 embedded-Postgres integration).

Two deliberate choices worth reviewing:

The stale floor is RUN_STALE_SILENCE_MS (15 min), reused rather than invented. BLO-12990/BLO-20775 already drop a >15-min-silent row from runningCount so it cannot starve new work. Freeing that slot accomplishes nothing while this path still swallows the wake that would fill it — the two halves of dispatch have to agree, and this makes them agree. The predicate is strictly narrower than the slot gate, because it also requires the run to have never flushed past the pre-exec prefix.

This is not a terminating fix, and that is the point. PEN-1995 measured a wall-clock kill ceiling at an 85.7–96.2% false-kill rate against recoverable runs (p99 session establishment 3.3 h, max 43.20 h) and recorded a decision not to add one. That objection does not transfer here: the stalled run is left entirely alone — still tracked, still awaited, still free to finish and deliver. All that changes is that it stops absorbing wakes. Firing early costs one extra queued run, not destroyed work.

Verification

Fail-first was verified by reverting only the call-site wiring and keeping the exported helpers, so the failure is behavioral rather than an unresolved import:

AssertionError: expected 'b88228cf-…' not to be 'b88228cf-…'
 ❯ expect(run?.id).not.toBe(runningRunId);

That is the production defect exactly: the timer wake comes back as the same run id as the wedged run. With the fix, all 10 pass.

npx vitest run server/src/__tests__/heartbeat-launch-stalled-coalesce.test.ts
  → 10 passed

# coalescing-adjacent suites, for regression
npx vitest run \
  server/src/__tests__/heartbeat-launch-stalled-coalesce.test.ts \
  server/src/__tests__/heartbeat-zombie-guard.test.ts \
  server/src/__tests__/heartbeat-timer-wake-coalescing.test.ts \
  server/src/__tests__/heartbeat-pr-review-request-coalescing.test.ts \
  server/src/__tests__/heartbeat-comment-wake-batching.test.ts \
  server/src/__tests__/plugin-agent-invoke-wake-fanout.test.ts
  → 6 files, 48 passed

cd server && npx tsc --noEmit   → clean

Note on scope of the fail-first: only the first integration test fails against unfixed code. The other two integration cases are controls — a quiet-but-productive run (lastOutputSeq: 12) and a still-launching run (2 min old) must keep coalescing, and they pass both before and after. They are regression guards, not evidence of the fix.

Risks

Low-to-moderate, and bounded by being non-destructive.

• Extra runs while a stall persists. At most one additional run per heartbeat interval — i.e. the agent's normal cadence, which is the intended outcome. Growth is self-limiting: activeRuns is ordered createdAt DESC, so the next wake finds the newest same-scope running run, which is young and therefore not launch-stalled, and coalesces into it as usual.
• A slow-but-live run past 15 min now gets a sibling wake instead of absorbing it. Session establishment p95 is 17.9 min, so this will fire on genuinely healthy slow launches. Absorbing is the worse outcome there — the absorbed wake is lost outright — and the slow run is not disturbed.
• No new concurrency contention. These are issueless __heartbeat__ runs, so there is no issue/worktree lock to contend; isIssueHeldByForeignRun still guards self-selection. The stalled row is already excluded from runningCount, so the new run has a slot by the existing rule.
• Not touched: the issue-scoped coalesce site (heartbeat.ts ~28824). Splitting a wake off a live issue-scoped run risks double-driving an issue, and the measured defect is entirely on the issueless timer path. Deliberately out of scope.
• Does not address the multi-hour launch stall itself (PEN-1990 / fix(acpx): make stalled ACP session establishment observable (PEN-1995) #1396 observability), only its cadence blast radius.

Model Used

Claude Opus 5 (claude-opus-5), 1M context window, extended thinking, with tool use and code execution. Run as the Paperclip CTO agent (claude_k8s adapter).

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• 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
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• 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 — N/A, server-side scheduling only
• I have updated relevant documentation to reflect my changes — the rationale lives in the doc comments on the new predicates
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first CI run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-12990
🔗 Paperclip issue: PEN-1990
🔗 Paperclip issue: BLO-20775
🔗 Paperclip issue: PEN-1995

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:408 — filterLaunchStalledCoalesceTarget is applied to every same-scope running target, regardless of wake type or task scope. Any quiet running issue/PR execution whose lastOutputSeq is 0 or 1 and whose newest timestamp is older than the 15-minute floor is treated as launch-stalled, so its wake bypasses the coalescing path and mints a second concurrent run. That can duplicate work in the same issue/worktree and is not limited to the heartbeat launch-stall scenario described by the PR.
  • Restrict this filter to the intended __heartbeat__ timer target, or add an explicit pre-adapter/launch-stall signal to the run and require that signal before bypassing coalescing. Add a regression test for a stale quiet issue-scoped running run with lastOutputSeq <= 1 and verify its wake still coalesces.

Suggestions (0)

Strengths

• The helper uses the newest activity timestamp and preserves the existing zombie filter composition.
• The tests cover stale, young, progressed, and non-running signals, plus the intended timer coalescing behavior.

Recommended Action

1. Fix the Important issue before merge.

[allyblockcast]

⛔ Do not merge as written — I have refuted this PR's own predicate against n=50 of the detector's alert history, and the review's proposed fix makes it worse

I authored this PR. New evidence from today's specimen (c1348962, PEN-2484) sent me back to measure the predicate rather than argue about it, and it does not survive. Posting here rather than only on PEN-1995 because this PR is CLEAN / MERGEABLE and one press from landing.

What the predicate is

filterLaunchStalledCoalesceTarget treats a running target as launch-stalled — and bypasses coalescing, minting a second concurrent run — when lastOutputSeq <= 1 and the newest activity timestamp is older than a 15-minute floor.

Measurement

The stale_active_run_evaluation cards are a ready-made labelled sample: each one records the run's lastOutputSeq at alert time, and the run row records where it ended up. I took every card whose run was at seq == 1 when alerted (so the predicate would have fired), joined it to its run's terminal record, and asked whether the run went on to produce output anyway.

population	would-be FALSE POS	true pos	n	precision
taskKey == __heartbeat__	13	3	16	19%
taskKey not recorded (older rows)	14	20	34	59%
ALL	27	23	50	46%

The 13 __heartbeat__ false positives went on to reach final lastOutputSeq of 23, 39, 43, 48, 52, 68, 94, 101, 109, 121, 123, 150 and 203 — eleven of them succeeded with exitCode 0. They are PEN-2204, 2253, 2262, 2270, 2273, 2279, 2291, 2384, 2399, 2409, 2442, 2484, 2492.

The review's recommended fix inverts

"Restrict this filter to the intended __heartbeat__ timer target"

That is the worst available restriction. Scoping to __heartbeat__ moves the false-positive rate from 54% to 81% (precision 46% → 19%), because the heartbeat timer population is exactly where the slow-session-establishment runs live. The scope concern in the review is real, but fixing it does not rescue the predicate — it concentrates the harm.

Why no at-alert-time variant of this predicate can work

Sorting the same sample by terminal errorCode shows where the real signal sits:

• True positives (seq never left 1): job_missing ×11, process_lost ×5, cancelled ×2, external_lifecycle_stale_killed ×1, job_failed ×1 — all infrastructure loss.
• False positives: None (i.e. succeeded) ×24, process_lost ×2, acpx_turn_failed ×1.

The discriminating fact is the terminal record, which by definition does not exist at alert time. Today's specimen is the cleanest demonstration: c1348962 was seq 1 / 60 min stale at 04:36Z — squarely inside this predicate — then produced 52 lines / 16,430 B and executed a real turn, dying at 07:40:44Z only on 429 · BYOS provider capacity. Its sibling dde22de3 sat silent 4 h 15 m before its acpx.session line and then exited 0. Note also that 2 false positives ended in process_lost after producing 43 and 121 lines — so even that code is a statement about timing, not about the run being stillborn.

Second-order harm

The binding constraint on this fleet right now is provider capacity, not scheduling: c1348962 and five later Summarizer runs today died acpx_turn_failed on BYOS 429s. A change whose failure mode is "mint an extra concurrent run" pushes directly against that constraint, so the 81% of cases where it fires wrongly are not merely wasted — they worsen the condition producing the alerts.

Disposition

Converting to draft so this cannot land on a green mergeable state while refuted. Not closing it: the underlying problem (a launch-stalled run absorbing its agent's wakes) is real and is documented at n=1 — 236ca044, process_lost, seq 1, 257 B, 11.87 h, ended only by the 19:13:35Z worker restart.

What would actually discriminate is a positive liveness signal from the adapter — a keepalive from the child process that separates "alive but slow inside ensureSession" from "process gone" — or fixing the multi-hour session-establishment tail itself (PEN-1990). Both are real work; neither is this diff. Re-planning on PEN-1995.