fix(db): repair exit-zero timeouts that 0214 missed after adapter migration (BLO-22922)

[allyblockcast]

Issue: https://paperclip.blockcast.net/BLO/issues/BLO-22922

Problem

Migration 0214_repair_successful_runs_mislabeled_timed_out.sql shipped to production on 2026-08-18 at 11:08Z and was a silent no-op. It repaired zero rows while reporting success.

The repair was gated on a live join to mutable current agent state:

FROM "agents" AS "agent"
WHERE "agent"."id" = "run"."agent_id"
  AND "agent"."adapter_type" = 'opencode_k8s'

The only affected agent (Ally) was moved off opencode_k8s to claude_k8s on 2026-08-12 — six days before 0214 finally cleared its deploy gate. By the time the statement ran, no agent row still read opencode_k8s, so nothing matched.

Evidence it did not run

Sampled heartbeat_runs rows from the affected window (2026-08-06 → 08-08) via /api/heartbeat-runs/:runId. 10 of 10 sampled rows still read:

field	value
status	timed_out
exit_code	0
error_code	timeout
error	Timed out after {600,3600,7200}s
result_json keys	configFreshness, effectiveTimeoutSec, stopReason, timeoutConfigured, timeoutFired, timeoutSource
result_json.outcomeCorrection	absent

Every row satisfies each of 0214's row-level clauses — exit_code = 0, error_code = 'timeout', error ~ '^Timed out after [0-9]+s$', and no excluded payload key (stopReason ≠ stop_reason, so the exclusion array is not tripped). The sole unsatisfied clause is the adapter join, and no row carries the outcomeCorrection stamp 0214 would have written.

Change

0221 retains 0214's row-level predicate verbatim and drops only the adapter join.

A historical data repair must not depend on present-day agent configuration. The remaining predicate is self-identifying: exit_code = 0 alongside status = 'timed_out' AND error_code = 'timeout' is contradictory on its face — a process that exited 0 did not hit its deadline. This is exactly the invariant the issue states: "A run with exitCode === 0 must not produce status='timed_out'."

Blast radius is bounded by the retained guards:

• The payload guard still leaves any row carrying a real adapter or publication error (error, is_error, subtype, message, …) as-is for an operator.
• A genuine over-deadline kill exits 137, not 0, so real timeouts are untouched.
• Idempotent with respect to 0214: rows it repaired are already succeeded and no longer match.

No forward-looking behaviour changes. The runtime guard isFalseAdapterTimeoutResult (server/src/services/heartbeat.ts:9325) keys purely on result shape and was already adapter-agnostic, so the fix that prevents new mislabels is correct fleet-wide. Only the historical repair was mis-scoped.

Verification

packages/db/src/heartbeat-exit-zero-timeout-repair-migration.test.ts — asserts the repair lands when the agent's current adapter_type is claude_k8s (the production shape 0214 missed), and that a 137 exit and a structured-failure payload both retain timed_out.

Falsified rather than assumed: reintroducing the adapter join makes the new test fail with - "status": "succeeded" / + "status": "timed_out", and removing it makes it pass.

✓ src/heartbeat-exit-zero-timeout-repair-migration.test.ts  (new)
✓ src/heartbeat-timeout-outcome-migration.test.ts           (0214, unbroken)
  Test Files  2 passed (2)

check:migration-numbering  pass
check:migration-safety     pass — 23 historical findings covered by baseline

Guardrail this adds

The class of defect is "a migration that runs successfully but matches zero rows is indistinguishable from one that repaired everything." 0214's own test seeded adapter_type = 'opencode_k8s', hard-coding the very condition production failed to satisfy — so it could never have caught this. The new test pins the post-adapter-migration shape instead.

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-22922

[allyblockcast]

🔗 Paperclip issue: BLO-22922

[allyblockcast]

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

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• 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.
Reviewed head: 310d559

Looks good. The core claim checks out under verification rather than assertion:

• Predicate is verbatim. Diffed against 0214_repair_successful_runs_mislabeled_timed_out.sql at this head — all five row-level clauses (status, exit_code, error_code, the anchored error regex, and the 16-element ?| exclusion array) are byte-identical. Only the agents join is dropped, exactly as the description states.
• The broadened scope matches deployed runtime policy. isFalseAdapterTimeoutResult (server/src/services/heartbeat.ts:9325) gates on timedOut && exitCode === 0 && errorCode === 'timeout' && /^Timed out after [0-9]+s$/ && !hasStructuredAdapterFailureEvidence(...) — field-for-field equivalent to the SQL, and already adapter-agnostic fleet-wide via isSuccessfulAdapterResult. So 0221 makes history agree with the policy the runtime already applies going forward, rather than inventing a new one.
• The exit 137 negative fixture is a real production shape, not a strawman: vendor/paperclip-adapter-claude-k8s/src/server/execute.ts:1117 maps exitCode === 137 to SIGKILL/OOMKilled and 143 to SIGTERM, which is how the k8s terminated-state surfaces a genuine over-deadline kill. exit_code = 0 also excludes NULL, so hard-killed runs are untouched on both paths.
• Mechanics are sound. No numbering collision (master ends at 0220); journal entry idx:221 / version:"7" is unique, tag-unique, and monotonic against 0220 (the 17 pre-existing when inversions elsewhere are untouched). The full-table-mutation-large-table ignore id is a real rule in check-migration-safety.ts:17. RETURNING wakeup_request_id yields NULL for run rows without a wakeup, which the wake.id = ... join discards correctly.

Critical Issues (0)

Important Issues (0)

Suggestions (3)

• [gstack/review] packages/db/src/migrations/0221_repair_exit_zero_timeouts_after_adapter_migration.sql:39 — the outcomeCorrection stamp is byte-identical to 0214's, so after this lands there is no way to tell which migration repaired a given row. In production that is moot (0214 matched zero rows), but any staging/dev database where an agent was still on opencode_k8s on 2026-08-18 will hold two indistinguishable populations. Given that this PR exists because 0214's silent no-op was hard to detect, a discriminator such as "migration":"0221" in the stamp turns the next post-deploy audit into one query instead of a row-shape inference.

• [native-codex] packages/db/src/migrations/0221_repair_exit_zero_timeouts_after_adapter_migration.sql:50 — the exclusion array carries 'stop_reason' but not camelCase 'stopReason', while the runtime's hasStructuredAdapterFailureEvidence checks both. A row with result_json.stopReason = 'error' (or failed/cancelled) would be held back by the runtime guard but repaired here — and line 36 then strips stopReason, removing the evidence. Note the obvious fix is wrong: adding 'stopReason' to this key-presence array would exclude every production row (they all carry stopReason: "timeout") and re-break the repair exactly as 0214 did. Parity would need a value-based test mirroring the runtime regex, e.g. NOT (result_json->>'stopReason' ~* '^(error([_-].*)?|failed|failure|cancelled|canceled|timed[_-]?out)$'). Reasonable to defer given the deliberate verbatim-inheritance, but worth recording as a known divergence.

• [pr-review-toolkit/errors] packages/db/src/migrations/0221_repair_exit_zero_timeouts_after_adapter_migration.sql:43 — the runtime compares result.errorMessage.trim() against the same regex; the SQL applies no trim, and Postgres ~ anchors $ at end-of-string (not end-of-line). A stored 'Timed out after 3600s\n' therefore passes the runtime check but fails here and stays mislabelled. This errs conservative (under-repair, not over-repair), but under-repairing silently is the precise failure mode 0214 exhibited — btrim("run"."error") ~ '...' would close it.

Strengths

• The root-cause analysis is the valuable part: identifying that a historical repair was gated on mutable current agent state, and that the fix is to make the predicate self-identifying, is a generalizable lesson worth the header comment it got. That comment will save the next reader real time.
• The falsification step — reintroducing the join to confirm the new test fails, then removing it to confirm it passes — is the difference between a test that documents behavior and one that actually pins the regression. 0214 shipped without it and was a no-op for six days.
• Negative fixtures are well chosen and non-redundant: exit 137 (real kill), is_error/subtype payload (ambiguous, operator-owned), NULL error, plus the kept: "yes" sentinel proving unrelated result_json keys survive the - operators. The wakeup cascade is asserted in both directions.
• Retaining the row predicate verbatim, rather than "improving" it while fixing the scope, keeps the blast-radius argument auditable against 0214 — and the near-identical measured counts (691 → ~689) are good corroboration that the join was the only thing excluding rows.

Recommended Action

1. No Critical or Important issues — this is mergeable as-is.
2. Consider the stamp discriminator (Suggestion 1) before merge; it is a one-line change to a file that becomes immutable once applied, and it directly serves the post-deploy verification this PR's own history shows is necessary.
3. Suggestions 2 and 3 are guard-parity drift between the SQL and heartbeat.ts; both are safe to defer, but worth a follow-up so the two predicates do not diverge further.

Posted as a formal COMMENT review: GitHub bars a pull request's author from approving its own PR, and this PR is authored by the Ally App. reviewDecision is empty (no required-review protection on master), so no approval identity is needed to land it.