fix(recovery): adapter-aware stale-run remedy, and stop gating evidence on logBytes (PEN-2106)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The recovery service watches active heartbeat runs and mints a stale_active_run_evaluation wrapper issue when a run goes silent, so a human or manager agent can triage it
• That path was written against the BLO-4467 fault: an external-lifecycle agent (claude_k8s, opencode_k8s) whose pod/Job died while its run row stayed running, holding the agent's clamp-to-1 concurrency slot until reaped
• It applies that story to every adapter. On a sessioned-local adapter (claude_local, codex_local) there is no pod, no Job, and no lock — so the remedy text is false in every clause, and the card that carries it has an empty evidence block on top, because the log read is gated on a column that is null for exactly this population
• This pull request branches the remedy text on adapter lifecycle and stops gating the evidence read on run.logBytes
• The benefit is that a stale-run card is disposable at a glance instead of requiring a manual log-read, and it no longer sends the reader after a pod that does not exist

Linked Issues or Issue Description

Refs PEN-2106 (Penstock-side record of the observation), BLO-4467 (completed-run-not-reaped), BLO-7113 (the dedupe/reopen path this text lives in).

No Blockcast/paperclip issue exists for this; describing it here per path (B).

Bug. Two defects on server/src/services/recovery/service.ts, both observed in production on agent Summarizer (claude_local):

(1) Adapter-boundary generalization. The re-fire reopen comment read, verbatim:

Likely the canonical BLO-4467-family wedge: the run row is running but the pod/Job is gone. Force-finish the run (reaper) so the agent's concurrency lock releases — do not just re-close this wrapper.

Every clause is inapplicable to a sessioned-local adapter:

• EXTERNAL_LIFECYCLE_ADAPTER_TYPES = ["claude_k8s", "opencode_k8s"] (packages/shared/src/validators/agent.ts:27). claude_local is not in it.
• There is no pod and no Job: the run's workspace is provider: local, transport: local, externalRunId: null.
• There is no concurrency lock to release. The clamp-to-1 in resolveExternalLifecycleConcurrency and the orphaned-Job early return at the dispatch gate are both guarded by externalLifecycle; and runningCount filters to rows whose signal is within RUN_STALE_SILENCE_MS (15 min), so a multi-hour-silent row contributes 0 to concurrency regardless.

This is actively misleading remediation rather than noise: it invites a reader to "free" a lock by cancelling, or to escalate a scheduling fault, on a false premise. The same false premise appeared a second time in the suppression note (the orphaned running row is the canonical BLO-4467-family wedge (pod already reaped)), so the two could drift apart independently.

(2) The card's evidence block is blind for its entire population. readRunLogTailForEvidence opened with:

if (!run.logStore || !run.logRef || !run.logBytes) return "";

logStore/logRef are written immediately after runLogStore.begin() — productivity-review.ts:1466-1467 relies on exactly that property. logBytes is only written back on finalize. This detector fires only on rows that are still running, i.e. never finalized, so logBytes is null-or-0 for 100% of the population and the read returned "" every time. Production symptom: a card reporting "No run-log tail was available" and Last output sequence: 1 while a readable 682-byte, 3-line log sat at the path the row's own logRef names — with the single most diagnostic line, stderr: Adapter execution timeout: timeoutSec=900, invisible on both the row and the card.

The gate was also unnecessary: runLogStore.read → readLocalRange already fs.stats the file and clamps start/end to its real size, so logBytes can only ever be a seek hint.

What Changed

• Added staleRunOrphanedRowRemedy(adapterType), which returns both the remedy sentence and the short mechanism phrase. External-lifecycle adapters keep the existing BLO-4467 pod/Job/reaper text unchanged. Sessioned-local adapters get text that states what is actually true: nothing to reap, no lock held, cancelling frees nothing, the row is simply orphaned running — and the only route to terminal is POST /heartbeat-runs/:runId/cancel, which is board-gated, so escalate rather than re-close.
• Routed both sites through that one helper: the re-fire reopen comment in escalateStaleRunRefire, and the suppression note in suppressOrEscalateStaleRunRefire. Previously two independent string literals asserting the same mechanism.
• readRunLogTailForEvidence no longer requires run.logBytes. It treats it as a seek hint, and when the hint is absent or stale-low it walks forward in 256 KiB chunks using the store's own nextOffset, keeping only the trailing 8 KiB window. Bounded at 4 MiB scanned per call so a pathologically large log cannot turn a detector sweep into a multi-MB read; a partial tail is returned on read error instead of discarding it.

Verification

Run in a durable clone (/paperclip/work/pen-2106), base master @ d92989b8b, head 956ebfe81. master has advanced 3 commits since the base; none of them touch either file in this PR, so no rebase was needed.

Typecheck — pnpm --filter @paperclipai/server typecheck → exit 0.

This caught one real defect in the first commit, now fixed: EXTERNAL_LIFECYCLE_ADAPTER_TYPES was imported from @paperclipai/db, which does not export it. It lives in packages/shared/src/validators/agent.ts, and the import now resolves it the same way heartbeat.ts and services/agents.ts do.

Suite — vitest run server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts --no-file-parallelism --maxWorkers=1:

Test Files  1 passed (1)
     Tests  31 passed (31)

27 pre-existing + 4 added, so nothing in the existing suite regressed.

Fail-first check

Each new test was run against the pre-fix source before the fix was credited: git checkout d92989b8b -- server/src/services/recovery/service.ts, tests left in place, then -t "PEN-2106".

Tests  3 failed | 1 passed | 27 skipped (31)

Test	Pre-fix	Assertion it failed on
sessioned-local reopen drops pod/Job/lock language	❌ fails	body still contained "the run row is running but the pod/Job is gone. Force-finish the run (reaper) so the agent's concurrency lock releases"
suppression note branches its mechanism phrase	❌ fails	tally still contained "the canonical BLO-4467-family wedge (pod already reaped)"
evidence tail collected when logBytes is unwritten	❌ fails	card body did not contain Adapter execution timeout: timeoutSec=900; the whole tail was absent
external-lifecycle reopen KEEPS the pod/Job reaper remedy	✅ passes	— by design, see below

Each of the three reproduces the reported defect on the exact production string, not on a setup error.

The fourth passing pre-fix is intended and is not a gap. It is a positive control, not a regression test: it asserts that behaviour on claude_k8s is unchanged, so it must pass on both sides. Its job is to fail if the adapter branch ever blanks both arms — i.e. if someone "fixes" the sessioned-local text by deleting the pod/Job remedy outright, the three above would still pass and only this one would catch it. A branch guarded in one direction only is how a text fix silently becomes a text deletion.

Not run locally: the full test:run matrix and e2e. Those are running in CI on this head; the General tests (server 1-4) shards are the relevant ones for this file.

Risks

Low, with two things worth a reviewer's attention.

• Behavioural surface is comment text plus one read path. No schema, no migration, no change to which runs are detected, no threshold moved. Deliberately: raising the 1h/4h thresholds would suppress false positives and delay reporting a genuinely dead run — claude_local has an observed 43.2h recovery tail — so that is a desirability decision with a real cost, not a defect fix, and it is not bundled here.
• The read is no longer free. The old gate made it a no-op; now a card mint reads up to 4 MiB. collectStaleRunEvidence is called once per non-skipped detector evaluation (sweeps run ~10–15 min apart), so a live wedge with a large log re-reads on each sweep. The observed population is 682-byte logs (one 8 KiB chunk), and the walk short-circuits immediately whenever logBytes is populated and sane, but the cap is the thing holding the worst case — flag if 4 MiB is judged too high.

Two things I deliberately did not change, both arguably in the same family:

• issues.ts:5347 (readRunLogText) carries the same logBytes <= 0 gate. I left it alone: it derives issue-comment metadata for terminal runs, where logBytes is written, so it is not blind on its population the way this one was. Worth a separate look, not a drive-by here.
• recovery/service.ts:4783 / :5882 — the stranded_assigned_issue remedy ("fix the runtime/adapter failure") reproduced against PEN-2106 itself twice, labelling a provider 429 (BYOS provider capacity … retry in 16803s) as job_failed — External lifecycle Job failed: BackoffLimitExceeded, and a CephFS mount fault as k8s_pod_schedule_failed. That is a real defect but a different one — an outer container verdict masking the body's terminal result, which is fix(heartbeat): recover orphaned runs' own terminal result instead of failing them job_missing (PEN-2421) #1443's shape, not an adapter-lifecycle branch. Fixing it here would mean changing failure classification, not remedy wording.

Defect (2) in the linked issue — that the prescribed action is board-gated while the wrapper is reopened onto an agent — is addressed at the text layer only: the sessioned-local branch now names the grant boundary and says escalate. Actually re-routing the reopen to a board approval instead of an agent assignee is a routing change with its own design questions, and I have left it out rather than smuggle it in.

Model Used

Claude Opus 4.5 (claude-opus-4-5), 1M context, extended thinking, with tool use / code execution (Claude Code agent run).

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, no UI surface
• I have updated relevant documentation to reflect my changes — n/a, no documented behaviour changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

Test boxes are now done (see Verification). The remaining two unchecked boxes are outcomes I cannot assert myself: CI is still running on this head, and Greptile has not reviewed it yet.

[allyblockcast]

🔗 Paperclip issue: PEN-2106
🔗 Paperclip issue: BLO-4467
🔗 Paperclip issue: BLO-7113

[allyblockcast]

🔗 Paperclip issue: PEN-2106
🔗 Paperclip issue: BLO-4467
🔗 Paperclip issue: BLO-7113

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Empty section: ## Verification
• No test files detected in this PR — please include a test that verifies the bug fix or new behavior. If this PR genuinely doesn't need a test (e.g. a refactor), please retitle with refactor: prefix.

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

— commitperclip

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Empty section: ## Verification

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

— commitperclip

[allyblockcast]

Marked ready for review. Head unchanged at 956ebfe81.

Heads-up on the red General tests (server 4/4) / verify: it is not this diff, and a re-run is in flight.

Failing step is #8 Run grouped general test suites (a real execution, not a runner eviction), and it is 1 failed | 1826 passed:

FAIL src/__tests__/heartbeat-dispatch-priority-sort.test.ts
  > heartbeat dispatch priority sort (BLO-12990)
  > advances emergency keysets past default-generated sub-millisecond timestamps

AssertionError: expected '2026-08-23 13:50:59.331+00' to match /\.\d{4,}/
  at heartbeat-dispatch-priority-sort.test.ts:5199

This PR touches only server/src/services/recovery/service.ts and server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts. Neither feeds dispatch-priority keyset sorting.

The assertion looks like a latent repo-wide flake rather than a real precision bug. It is a precondition guard checking that the DB-default created_at is sub-millisecond, but it reads the value through created_at::text — and Postgres trims trailing zeros rendering timestamptz. When the microsecond component is an exact multiple of 1000, .331000 renders as .331, three digits, and /\.\d{4,}/ fails. That is roughly 1 run in 1000, on any PR. Asserting on the numeric microsecond component instead of the rendered string would fix it.

Not bundling that here — unrelated file, and I would rather not move this head. Flagging it so the red is not read as a finding against this change.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [native-codex] server/src/services/recovery/service.ts:2953 — consider adding a focused test for a multibyte UTF-8 log crossing a chunk boundary, since the reader advances in byte offsets while the returned content is decoded to strings.

Strengths

• The adapter-specific remediation is centralized and preserves the external-lifecycle behavior while avoiding false pod/Job guidance for local adapters.
• The evidence reader now handles the still-running population where logBytes is unset, uses the store's forward offsets, bounds scanning, and preserves partial evidence on read failure.
• The added tests include both adapter directions and a fail-first regression for the previously blind log-tail path.

Recommended Action

1. No Critical or Important issues found; the PR is suitable for merge after CI completes.
2. Consider the UTF-8 boundary regression test opportunistically.