fix(issues): hydrate activeRun by the terminal complement, not an enumeration (BLO-25410)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agent runs take an execution lock on an issue, and two read paths report that lock: POST /api/issues/{id}/checkout decides whether work can be claimed, and GET /api/issues/{id}.activeRun reports who holds it
• BLO-19749 / fix(heartbeat): gate timer skip predicate on checkout availability (BLO-19749) #1108 unified "holds the execution lock" into issue-execution-lock.ts and pinned holding/terminal as exact complements
• But the consumers still express that notion in two different forms — a terminal complement in checkout(), an enumeration in the activeRun hydration — and those forms agree only inside the canonical HEARTBEAT_RUN_STATUSES union
• heartbeat_runs.status is plain text() with no enum and no check constraint, and error / adapter_failed are already documented as occurring in the column without being in the union, so out-of-union statuses are a demonstrated occurrence rather than a hypothesis
• For a status that is out-of-union and non-terminal, checkout() 409s naming a run that GET /api/issues/{id} reports as activeRun: null — the exact disagreement BLO-19749 set out to remove, reproduced for the unknown-status case
• This pull request converts the activeRun hydration to the same terminal-complement predicate, and documents the four remaining enumerations that are deliberately not lock predicates
• The benefit is that the two read paths cannot disagree again for the next status added to the column

Linked Issues or Issue Description

Closes BLO-25410. Follow-up to BLO-19749 / #1108 — Ally's review landed 2m28s after that PR merged, so the finding could not be folded in.

The defect

BLO-19749 unified "holds the execution lock" into issue-execution-lock.ts and pinned holding/terminal as exact complements. But the consumers express that notion in two different forms, which agree inside HEARTBEAT_RUN_STATUSES and disagree outside it:

site	form	out-of-union non-terminal status
checkout()	!TERMINAL.has(status)	holding → 409
hasActionableTimerWork	notInArray(status, TERMINAL_…_VALUES)	holding → wake suppressed
activeRun hydration	inArray(status, ACTIVE_RUN_STATUSES)	not matched → activeRun: null

ACTIVE_RUN_STATUSES = [...ISSUE_EXECUTION_LOCK_HOLDING_RUN_STATUSES], and that constant is HEARTBEAT_RUN_STATUSES.filter(s => !TERMINAL.has(s)) — an enumeration over the canonical union only. A status outside the union cannot appear in it, so GET /api/issues/{id} returned activeRun: null while POST /checkout 409'd naming that exact run.

heartbeat_runs.status is text("status") — no enum, no check constraint. issue-execution-lock.ts already records that error and adapter_failed occur in the column without being in the union, so out-of-union statuses are a demonstrated occurrence, not a hypothesis.

Severity: latent, not live. No current status is both out-of-union and non-terminal. The exposure is the next one that isn't.

What Changed

• activeRunMapForIssues (server/src/services/issues.ts) now selects holders by notInArray(heartbeatRuns.status, TERMINAL_HEARTBEAT_RUN_STATUS_VALUES) instead of inArray(..., ACTIVE_RUN_STATUSES). This is a no-op for every status in the canonical union and a guard for the next one added to the column.
• Audited all four remaining inArray-over-active-statuses sites. None is a lock predicate, so none was converted; each now carries a comment recording why. The point is that these predicates must fail in opposite directions:
  • the lock predicate fails toward "held" (an unknown status defers; being wrong costs one deferral instead of two runs in one worktree)
  • BLOCKER_ATTENTION_ACTIVE_RUN_STATUSES / BLOCKED_INBOX_ACTIVE_RUN_STATUSES fail toward "not covered" (an unknown status raises attention; being wrong costs a spurious nudge instead of silently swallowing a stuck issue)
  • converting the latter would flip that safety direction and make every future non-terminal status suppress attention by default
  • issue-tree-control.ts drives pause/cancel control (and re-narrows to queued/running in memory anyway); productivity-review.ts only counts runs for a report
• Fixed a latent test bug this surfaced (below), and added the regression coverage in issue-execution-lock.test.ts.

A latent test bug this surfaced

issue-detail-active-run-routes.test.ts asserted its "terminalized" case over ["completed", "failed", "cancelled"]. "completed" is never written to heartbeat_runs.status — finalizeRun in routines.ts writes it to routine_runs, and it is a real status only on agent_wakeup_requests (which is where the vocabulary was borrowed from). The case passed because the enumeration dropped any unknown string, never because the system classified it as terminal.

That is itself corroboration for this issue: a developer already reached for an out-of-union status and got the disagreement, silently. The test now asserts over the real TERMINAL_HEARTBEAT_RUN_STATUS_VALUES, plus a new case pinning that an unknown non-terminal status reads as held — matching checkout(), which 409s on such a run because clearExecutionRunIfTerminal shares the same terminal set and will never clear the pointer. Reporting null there is what made a permanently un-checkoutable issue look unowned.

Verification

Regression criterion confirmed by reverting the production change and re-running:

× hydrates a run whose non-terminal status is absent from the canonical union
    AssertionError: expected null not to be null
× names the same run id that checkout conflicts on
    AssertionError: expected undefined to be 'bf376429-…'
Tests  2 failed | 9 passed (11)

Both pass after the change; the 9 unchanged-behaviour cases (3 union holding statuses hydrate, 7 terminal statuses do not) pass both ways.

The second test drives the real checkout() and asserts the 409's details.executionRunId equals the hydrated activeRun.id — the two read paths agreeing, which is the acceptance criterion.

• 130 tests passed (130) across 9 suites: issue-execution-lock, issue-detail-active-run-routes, issue-blocker-attention, issue-run-holding, issue-tree-control-service, issue-tree-control-routes, agent-inbox-lite-status-contract, issue-liveness, execution-lock-orphan-cleanup
• tsc --noEmit clean

The new DB-backed block is gated on getEmbeddedPostgresTestSupport() in a separate describe, so the existing pure-unit assertions in that file keep running on hosts without embedded postgres.

Risks

Low risk. The change widens which rows hydrate as activeRun, and it widens it to exactly the set checkout() already treats as holding, so the two paths converge rather than diverge.

• No behaviour change for any status in the canonical union — this is pinned by the 9 unchanged-behaviour cases, which pass both before and after the change.
• No migration, no schema change, no API shape change; heartbeat_runs.status stays text().
• The one behavioural shift is intended: a run whose status is non-terminal and out-of-union now reports as activeRun rather than null. That direction is the safe one — it reports the issue as held (matching checkout's 409) instead of advertising a permanently un-checkoutable issue as available.
• The four enumerations that were deliberately left alone are the residual risk surface; they are documented in place so a future reader does not "finish the job" and silently flip their failure direction.

Model Used

Claude Opus 5 (claude-opus-5[1m], 1M context), extended thinking, running as the CTO agent in the Paperclip agent harness with tool use.

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 only)
• I have updated relevant documentation to reflect my changes (in-code comments on the four enumerations)
• 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

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-19749
🔗 Paperclip issue: BLO-25410

[allyblockcast]

🔗 Paperclip issue: BLO-19749
🔗 Paperclip issue: BLO-25410

[allyblockcast]

Ally — Consolidated PR Review

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

Looks good. The active-run hydration now uses the terminal-status complement used by checkout locking, so unrecognized non-terminal persisted statuses remain visible rather than appearing unowned.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The regression coverage exercises an out-of-union non-terminal status and verifies that the detail route and checkout behavior identify the same run.
• The narrow enumerations that intentionally model execution or attention, rather than lock ownership, are explicitly documented to preserve their safer behavior.

Recommended Action

1. Merge after the pending CI checks pass.

[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