deef1f479d
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Each issue can hold an execution lock via `issues.execution_run_id`,
so concurrent wakes for the same task either coalesce into the active
run or wait deferred
> - When the issue is reassigned to a *different* agent (e.g. board
operator changes `assigneeAgentId` from Coder → Reviewer + flips
`status` to `in_review`), the new assignee's wake is correctly sent down
the assignment-wakeup path
> - But the lookup `activeExecutionRun` still finds the previous holder
run as long as it is in `{queued, running, scheduled_retry}` — and
`enqueueAssignmentWakeup` falls through to the deferred-wake branch when
the holder agent does not match the new assignee
> - The trouble is the **queued** holder for the old assignee will never
start (the issue's status / target now belongs to someone else, the
relevant assignment trigger was the original one), so the lock is never
released, the deferred wake is never promoted, and the new assignee
silently never wakes
> - This pull request detects that situation right next to the existing
`cancelStaleScheduledRetry` cleanup: if `activeExecutionRun.status !==
'running'` AND the holder agent differs from `issue.assigneeAgentId`,
cancel the holder run, release the lock, and proceed with a normal
queued wake instead of deferring
> - The benefit is hand-offs across agents become reliable — no more
silent stalls that operators have to unstick by manually cancelling a
queued run
## Linked Issues or Issue Description
- Closes #4058
## What Changed
- One new check in `reapOrphanedRuns()`'s peer function — the
`enqueueAssignmentWakeup` defer-detection block in
`server/src/services/heartbeat.ts` (around the lock-resolution code
immediately following `cancelStaleScheduledRetry`):
- If `activeExecutionRun` exists, its `status !== 'running'`, and
`activeExecutionRun.agentId !== issue.assigneeAgentId`, mark the holder
run as `cancelled` with errorCode `lock_released_on_reassignment`,
cancel its corresponding wakeup request if any, and null
`activeExecutionRun` so the lock-clear branch right below proceeds to
release `executionRunId` / `executionAgentNameKey` / `executionLockedAt`
and the wake gets enqueued normally.
- `running` runs still defer (legitimate concurrency).
- Same-agent queued/scheduled holders still defer (legitimate coalesce).
- Total +37 lines, no API change, no schema change.
## Verification
```sh
# Existing reaper tests still pass — exercises the lock-resolution path
pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts --no-coverage
# expected: Tests 39 passed (39)
# New regression test for the cross-agent lock-release race
pnpm exec vitest run server/src/__tests__/heartbeat-lock-release-on-reassignment.test.ts --no-coverage
```
Manual reproduction (matches an incident we hit running a small Coder +
Reviewer company):
1. Coder pickup heartbeat schedule fires; paperclip queues a Coder run
and pre-allocates the lock by recording `issues.execution_run_id =
<queued-coder-run-id>` for the pickup issue.
2. The Coder run sits in `queued` because the agent's slot is busy
elsewhere (`maxConcurrentRuns: 1`).
3. Operator (or CEO) PATCHes the issue: `assigneeAgentId: <coder>` →
`<reviewer>` together with `status: in_progress` → `in_review`.
4. Paperclip creates the Reviewer assignment wakeup, but stores it as
`deferred_issue_execution` because `activeExecutionRun` is the queued
Coder run.
5. **Before this PR**: Reviewer never wakes; the deferred wakeup waits
for the queued Coder lock holder which never starts (the issue is no
longer the Coder's). Operator has to `POST
/api/heartbeat-runs/<queued-coder>/cancel` manually to unstick the
chain.
6. **After this PR**: paperclip recognizes the holder is non-running and
belongs to a now-foreign agent, cancels it inline, releases the lock,
and queues the Reviewer wake normally — Reviewer wakes on the next
heartbeat tick.
## Risks
- **Low**. The new branch only fires when both conditions are true:
- The holder run is **not** `running` — `running` runs still defer (we
never interrupt active work).
- `activeExecutionRun.agentId` is different from the issue's *current*
`assigneeAgentId` — i.e. the assignee was just changed, the old holder
is bound to the prior owner.
- The cancel uses errorCode `lock_released_on_reassignment` so operators
can grep for it; the corresponding wakeup is also cancelled in the same
transaction so we do not leave an orphan wakeup request.
- No DB schema change, no public API change, no UI change.
- Sits next to the existing `cancelStaleScheduledRetry` cleanup pattern,
so the behavior is locally consistent with how stale schedule retries
are already cleared.
## Model Used
- Claude Opus 4.7 (`claude-opus-4-7`), 1M-context build, extended
thinking + tool use enabled. Used to trace the lock-acquire / defer /
promote paths in `heartbeat.ts` from the live incident, design the
minimal-blast-radius fix next to `cancelStaleScheduledRetry`, and
produce this PR description.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have run tests locally and they pass (39 in the directly
affected suite, plus the new regression test)
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots — N/A (server-side wakeup routing)
- [x] I have updated relevant documentation to reflect my changes —
in-line code comment explains the new branch
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge
## Cross-references and status (maintainer)
- `Closes #4058`
### Maintainer-added changes on top of the original commit
A second commit was added on top of @vbalko-claimate's original to pin
the cancel `UPDATE` for the queued/scheduled holder to the exact
non-running status read just above it. Without that predicate, a worker
that flipped the holder from `queued` → `running` between the `SELECT`
and the `UPDATE` could have its freshly-claimed `running` row silently
clobbered to `cancelled`. The new commit also gates the wakeup-request
cancellation and the `activeExecutionRun = null` assignment on a
non-empty `RETURNING`, so neither fires when the predicate misses. A
dedicated regression test
(`heartbeat-lock-release-on-reassignment.test.ts`) covers both paths:
the legitimate-running-holder defer case and the queued→running race.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Devin Foley <devin@devinfoley.com>