fix(heartbeat): keep a routine claim queued while its slot is held (#18933)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - A routine creates one task per tick. Only one open task of a routine
can hold an execution lock (`issues_open_routine_execution_uq`).
> - Since #18879, a failed native run that waits for a scheduled resume
keeps its task's lock. Routine dispatch counts only queued, running or
scheduled runs as live, so it can queue a newer task of the same routine
meanwhile.
> - When that newer run is claimed, stamping its execution lock violates
the unique index. The raw database error is not a deferrable claim
rejection, so it escapes the queue loop. The queue code notes that this
can stop the agent's queue and startup recovery.
> - This pull request turns that exact violation into a 409 claim
rejection, so the run stays queued until the slot is free.
> - The benefit is that a busy routine slot delays one run instead of
stopping the queue or startup.

## Linked Issues or Issue Description

Refs #18879 (keeps a failed run's lock while its resume is pending),
#18916 (defers the same unique-index conflict in restart recovery's lock
reclaim) and #18931 (open; keeps one run's restart-recovery failure from
stopping startup recovery). This is the same index conflict on a
different path: the heartbeat queue claim, not restart recovery. It does
not change #18916 or #18931. Its doc paragraph is separate from the one
#18931 edits.

**What happened?**

While auditing the lock paths from #18879 for this index, I found that a
queued run of a newer routine task can be claimed while an older task of
the same routine still holds the slot through a failed run with a
pending resume. The claim's lock stamp then raises `duplicate key value
violates unique constraint "issues_open_routine_execution_uq"`.
`isDeferrableClaimRejection` accepts only `HttpError` 4xx, so the error
escapes `startNextQueuedRunForAgent`.

**Expected behavior**

The claim is rejected as a conflict and rolls back. The run stays queued
and is claimed after the older task releases the slot. Other errors
behave as before.

**Steps to reproduce**

1. Create two open tasks of the same routine (same origin and
fingerprint).
2. Give the older task an execution lock held by a failed run.
3. Queue a run for the newer task and claim it. On `master` the claim
throws the unique violation.

The new queue test runs these steps against PostgreSQL.

**Paperclip version or commit**

`master` at `0dedbf23c35`.

**Deployment mode**

Any server with routines whose tasks run on native runners.

## What Changed

- `bindClaimedIssueExecution` in
`server/src/services/heartbeat/queue.ts`: if stamping the execution lock
violates `issues_open_routine_execution_uq`, it throws
`conflict("Another execution for this routine is already in progress")`.
The claim transaction rolls back, and the queue loop keeps the run
queued as it does for other 4xx claim rejections. Every other error is
rethrown unchanged. The issue checkout route already maps this violation
to the same 409.
- `doc/execution-semantics.md`: one paragraph on the behavior.

## Verification

- Failing first: `queue.test.ts` "keeps a claim queued while another
task of its routine holds the execution slot". On `master` the claim
raised the `issues_open_routine_execution_uq` violation. Now it rejects
with 409, the run stays `queued`, neither task's lock changes, and
nothing executes.
- Failing first, review round 1:
`heartbeat-queued-run-claim-isolation.test.ts` "keeps a routine run
queued while another task holds the routine slot, then runs it once the
slot is free" goes through `resumeQueuedRuns`. On `master` it rejected
with the unique violation. Now the routine run stays queued with no
error code while another queued run for the same agent executes. After
the slot is released, the routine run executes exactly once (executions
are counted per run, because the finished task can queue its own
follow-up run).
- `cd server && npx vitest run src/services/heartbeat/queue.test.ts
src/__tests__/heartbeat-queued-run-claim-isolation.test.ts`: 70 passed.
- `npx vitest run src/services/heartbeat/recovery.test.ts
src/__tests__/routines*.test.ts`: 4 files, 205 passed.
- `pnpm --filter @paperclipai/server typecheck`: pass.
- Not verified: the queue loop end to end on a real instance. The loop's
handling of a 4xx claim rejection is existing behavior.

## Risks

- While the older task holds the slot, the newer run stays queued and
the loop logs "queued heartbeat run claim was rejected" on each pass. It
is claimed once the older task's resume finishes or its lock is
released.
- Only this one constraint is mapped. Other database errors still escape
as before.
- No schema migration.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use
(shell, file edits, test runs). The runtime does not expose the context
window or reasoning configuration.

## 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 not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] 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

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin FoleyandPaperclip authored and GitHub committed 2026-11-03 21:33:00 -08:00
1 parent b4c69ded9f
commit 1a39dea9aa
4 files changed
+83 -1

No files matched your search

+6
View File
@@ -1265,6 +1265,12 @@ checks coordinator ownership in its terminal update, so a result recorded after
the process check is also protected. Terminal task status remains authoritative;
exhausted finalization retries do not keep an otherwise orphaned run alive.
Only one open task of a routine can hold an execution lock. A failed run that
waits for a scheduled resume keeps its task's lock, so the routine can queue a
newer task meanwhile. When that task's run is claimed while the slot is held,
the claim is rejected with a conflict, rolls back, and the run stays queued
until the slot is released.
A native failure that schedules a same-run resume marks the run `failed` but
keeps the task's execution and checkout locks for that resume. The sweep leaves
those locks in place and clears them only when the task is done or cancelled.
@@ -4,10 +4,13 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest
import {
agents,
agentWakeupRequests,
authUsers,
companies,
companyMemberships,
createDb,
heartbeatRuns,
externalAgentHolds,
issues,
} from "@paperclipai/db";
import {
getEmbeddedPostgresTestSupport,
@@ -231,6 +234,49 @@ describeEmbeddedPostgres("heartbeat queued-run claim isolation", () => {
expect(await runStatus(healthyRunId)).toMatchObject({ status: "succeeded" });
});
it("keeps a routine run queued while another task holds the routine slot, then runs it once the slot is free", async () => {
const { companyId, agentId } = await insertAgent();
const origin = { originKind: "routine_execution", originId: randomUUID(), originFingerprint: "scheduled" };
// The earlier task keeps its lock while its failed run waits for a scheduled resume.
const holderIssueId = randomUUID();
const holderRunId = randomUUID();
await db.insert(issues).values({ id: holderIssueId, companyId, title: "Earlier routine task",
status: "in_progress", assigneeAgentId: agentId, ...origin });
await db.insert(heartbeatRuns).values({ id: holderRunId, companyId, agentId, status: "failed",
finishedAt: new Date(), contextSnapshot: { issueId: holderIssueId } });
await db.update(issues).set({ executionRunId: holderRunId }).where(sql`${issues.id} = ${holderIssueId}`);
const userId = `routine-owner-${randomUUID()}`;
await db.insert(authUsers).values({ id: userId, name: "Routine owner", email: `${userId}@paperclip.test`,
createdAt: new Date(), updatedAt: new Date() });
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId,
status: "active", membershipRole: "owner" });
const routineIssueId = randomUUID();
await db.insert(issues).values({ id: routineIssueId, companyId, title: "Current routine task",
status: "todo", assigneeAgentId: agentId, responsibleUserId: userId, ...origin });
const { runId: routineRunId } = await insertQueuedRun(companyId, agentId, {
requestedByActorType: "user", requestedByActorId: userId,
payload: { manualUserWake: true, issueId: routineIssueId },
}, new Date(Date.now() - 60_000));
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: routineIssueId } })
.where(sql`${heartbeatRuns.id} = ${routineRunId}`);
const { runId: healthyRunId } = await insertClaimableRun(companyId, agentId);
await expect(heartbeat.resumeQueuedRuns()).resolves.toBeUndefined();
await heartbeat.drainActiveRunExecutions();
expect(await runStatus(routineRunId)).toMatchObject({ status: "queued", errorCode: null });
expect(await runStatus(healthyRunId)).toMatchObject({ status: "succeeded" });
expect(mockAdapterExecute).toHaveBeenCalledOnce();
await db.update(issues).set({ executionRunId: null }).where(sql`${issues.id} = ${holderIssueId}`);
await expect(heartbeat.resumeQueuedRuns()).resolves.toBeUndefined();
await heartbeat.drainActiveRunExecutions();
expect(await runStatus(routineRunId)).toMatchObject({ status: "succeeded" });
// Count executions per run: the finished task may queue its own follow-up run.
const executedRunIds = mockAdapterExecute.mock.calls.map(call => ((call as unknown[])[0] as { runId?: string }).runId);
expect(executedRunIds.filter(id => id === routineRunId)).toHaveLength(1);
expect(executedRunIds.filter(id => id === healthyRunId)).toHaveLength(1);
});
it.each(["worker", "native-effects"])("retains a %s barrier across provider changes and resumes only after reconciliation", async barrier => {
const { companyId, agentId } = await insertAgent();
const { runId } = await insertClaimableRun(companyId, agentId);
@@ -693,6 +693,27 @@ describe.skipIf(!support.supported)("heartbeat queue database wiring", () => {
expect(deps.canStartNewExternalBotWork).not.toHaveBeenCalled();
});
it("keeps a claim queued while another task of its routine holds the execution slot", async () => {
const { company, agent, issue, run } = await fixture("queued");
const origin = { originKind: "routine_execution", originId: crypto.randomUUID(), originFingerprint: "scheduled" };
await db.update(issues).set({ ...origin, executionRunId: null, executionAgentNameKey: null, executionLockedAt: null })
.where(eq(issues.id, issue.id));
// An earlier task of the same routine keeps its lock while its failed run waits for a scheduled resume.
const [holder] = await db.insert(issues).values({ companyId: company.id, title: "Earlier routine task",
status: "in_progress", assigneeAgentId: agent.id, ...origin }).returning();
const [holderRun] = await db.insert(heartbeatRuns).values({ companyId: company.id, agentId: agent.id,
status: "failed", finishedAt: new Date(), contextSnapshot: { issueId: holder.id } }).returning();
await db.update(issues).set({ executionRunId: holderRun.id }).where(eq(issues.id, holder.id));
const deps = callbacks(db);
deps.getAgent.mockResolvedValue(agent);
await expect(createHeartbeatQueue(db, deps).claimQueuedRun({ ...run, responsibleUserId: "queue-test-user" }))
.rejects.toMatchObject({ status: 409 });
expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id)))[0]).toMatchObject({ status: "queued" });
expect((await db.select().from(issues).where(eq(issues.id, issue.id)))[0]).toMatchObject({ executionRunId: null });
expect((await db.select().from(issues).where(eq(issues.id, holder.id)))[0]).toMatchObject({ executionRunId: holderRun.id });
expect(deps.executeRun).not.toHaveBeenCalled();
});
it("cancels at the daily run cap before claiming or dispatching", async () => {
const { run, agent } = await fixture("queued");
const cappedAgent = { ...agent, runtimeConfig: { heartbeat: { maxDailyRuns: 0 } } };
+10 -1
View File
@@ -112,6 +112,7 @@ import {
notFound,
} from "../../errors.js";
import { logger } from "../../middleware/logger.js";
import { isUniqueViolation } from "../../db-errors.js";
import { publishLiveEvent } from "../live-events.js";
import { allocateHeartbeatRunEventSeq } from "../heartbeat-run-events.js";
import {
@@ -1101,7 +1102,15 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende
executionAgentNameKey: normalizeAgentNameKey(agent.name),
executionLockedAt: claimedAt,
updatedAt: claimedAt,
}).where(and(eq(issues.id, issueId), eq(issues.companyId, run.companyId)));
}).where(and(eq(issues.id, issueId), eq(issues.companyId, run.companyId))).catch((error: unknown) => {
// Another open task of the same routine holds its execution slot, for
// example a failed run that waits for its scheduled resume. Roll back
// this claim and keep the run queued until that slot is released.
if (isUniqueViolation(error, "issues_open_routine_execution_uq")) {
throw conflict("Another execution for this routine is already in progress");
}
throw error;
});
}
const nativeReviewContext = readNativeReviewAssignmentContext(context);
const queuedCommentIds = queuedCommentIdsFromRunContext(context);