mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 02:44:50 +01:00
Fix stale checkout release race
This commit is contained in:
1 parent
0b86889b6f
commit
d012443148
2 files changed
+142
-48
No files matched your search
@@ -3112,4 +3112,96 @@ describeEmbeddedPostgres("issueService.clearExecutionRunIfTerminal", () => {
|
||||
.then((rows) => rows[0]);
|
||||
expect(row).toEqual({ executionRunId: null, executionLockedAt: null });
|
||||
});
|
||||
|
||||
it("does not let stale release clobber a successor checkout lock", async () => {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
const failedRunId = randomUUID();
|
||||
const releasingRunId = randomUUID();
|
||||
const successorRunId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
await db.insert(heartbeatRuns).values([
|
||||
{
|
||||
id: failedRunId,
|
||||
companyId,
|
||||
agentId,
|
||||
status: "failed",
|
||||
invocationSource: "manual",
|
||||
finishedAt: new Date("2026-06-10T10:05:00.000Z"),
|
||||
},
|
||||
{
|
||||
id: releasingRunId,
|
||||
companyId,
|
||||
agentId,
|
||||
status: "running",
|
||||
invocationSource: "manual",
|
||||
startedAt: new Date("2026-06-10T10:06:00.000Z"),
|
||||
},
|
||||
{
|
||||
id: successorRunId,
|
||||
companyId,
|
||||
agentId,
|
||||
status: "running",
|
||||
invocationSource: "manual",
|
||||
startedAt: new Date("2026-06-10T10:07:00.000Z"),
|
||||
},
|
||||
]);
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Race stale release",
|
||||
status: "in_progress",
|
||||
priority: "high",
|
||||
assigneeAgentId: agentId,
|
||||
checkoutRunId: failedRunId,
|
||||
executionRunId: failedRunId,
|
||||
executionAgentNameKey: "codexcoder",
|
||||
executionLockedAt: new Date("2026-06-10T10:00:00.000Z"),
|
||||
});
|
||||
|
||||
const [releaseResult, checkoutResult] = await Promise.allSettled([
|
||||
svc.release(issueId, agentId, releasingRunId),
|
||||
svc.checkout(issueId, agentId, ["todo", "in_progress"], successorRunId),
|
||||
]);
|
||||
|
||||
expect(checkoutResult.status).toBe("fulfilled");
|
||||
if (releaseResult.status === "rejected") {
|
||||
expect(releaseResult.reason).toMatchObject({ status: 409 });
|
||||
}
|
||||
|
||||
const row = await db
|
||||
.select({
|
||||
status: issues.status,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
checkoutRunId: issues.checkoutRunId,
|
||||
executionRunId: issues.executionRunId,
|
||||
})
|
||||
.from(issues)
|
||||
.where(eq(issues.id, issueId))
|
||||
.then((rows) => rows[0]);
|
||||
expect(row).toMatchObject({
|
||||
status: "in_progress",
|
||||
assigneeAgentId: agentId,
|
||||
checkoutRunId: successorRunId,
|
||||
executionRunId: successorRunId,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -3256,8 +3256,8 @@ export function issueService(db: Db) {
|
||||
);
|
||||
}
|
||||
|
||||
async function isTerminalOrMissingHeartbeatRun(runId: string) {
|
||||
const run = await db
|
||||
async function isTerminalOrMissingHeartbeatRun(runId: string, dbOrTx: DbReader = db) {
|
||||
const run = await dbOrTx
|
||||
.select({ status: heartbeatRuns.status })
|
||||
.from(heartbeatRuns)
|
||||
.where(eq(heartbeatRuns.id, runId))
|
||||
@@ -4867,55 +4867,57 @@ export function issueService(db: Db) {
|
||||
});
|
||||
},
|
||||
|
||||
release: async (id: string, actorAgentId?: string, actorRunId?: string | null) => {
|
||||
await clearExecutionRunIfTerminal(id);
|
||||
await clearCheckoutRunIfTerminal(id);
|
||||
const existing = await db
|
||||
.select()
|
||||
.from(issues)
|
||||
.where(eq(issues.id, id))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
release: async (id: string, actorAgentId?: string, actorRunId?: string | null) =>
|
||||
db.transaction(async (tx) => {
|
||||
await tx.execute(
|
||||
sql`select ${issues.id} from ${issues} where ${issues.id} = ${id} for update`,
|
||||
);
|
||||
const existing = await tx
|
||||
.select()
|
||||
.from(issues)
|
||||
.where(eq(issues.id, id))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
if (!existing) return null;
|
||||
if (actorAgentId && existing.assigneeAgentId && existing.assigneeAgentId !== actorAgentId) {
|
||||
throw conflict("Only assignee can release issue");
|
||||
}
|
||||
if (
|
||||
actorAgentId &&
|
||||
existing.status === "in_progress" &&
|
||||
existing.assigneeAgentId === actorAgentId &&
|
||||
existing.checkoutRunId &&
|
||||
!sameRunLock(existing.checkoutRunId, actorRunId ?? null)
|
||||
) {
|
||||
const stale = await isTerminalOrMissingHeartbeatRun(existing.checkoutRunId);
|
||||
if (!stale) {
|
||||
throw conflict("Only checkout run can release issue", {
|
||||
issueId: existing.id,
|
||||
assigneeAgentId: existing.assigneeAgentId,
|
||||
checkoutRunId: existing.checkoutRunId,
|
||||
actorRunId: actorRunId ?? null,
|
||||
});
|
||||
if (!existing) return null;
|
||||
if (actorAgentId && existing.assigneeAgentId && existing.assigneeAgentId !== actorAgentId) {
|
||||
throw conflict("Only assignee can release issue");
|
||||
}
|
||||
if (
|
||||
actorAgentId &&
|
||||
existing.status === "in_progress" &&
|
||||
existing.assigneeAgentId === actorAgentId &&
|
||||
existing.checkoutRunId &&
|
||||
!sameRunLock(existing.checkoutRunId, actorRunId ?? null)
|
||||
) {
|
||||
const stale = await isTerminalOrMissingHeartbeatRun(existing.checkoutRunId, tx);
|
||||
if (!stale) {
|
||||
throw conflict("Only checkout run can release issue", {
|
||||
issueId: existing.id,
|
||||
assigneeAgentId: existing.assigneeAgentId,
|
||||
checkoutRunId: existing.checkoutRunId,
|
||||
actorRunId: actorRunId ?? null,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const updated = await db
|
||||
.update(issues)
|
||||
.set({
|
||||
status: "todo",
|
||||
assigneeAgentId: null,
|
||||
checkoutRunId: null,
|
||||
executionRunId: null,
|
||||
executionAgentNameKey: null,
|
||||
executionLockedAt: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(issues.id, id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!updated) return null;
|
||||
const [enriched] = await withIssueLabels(db, [updated]);
|
||||
return enriched;
|
||||
},
|
||||
const updated = await tx
|
||||
.update(issues)
|
||||
.set({
|
||||
status: "todo",
|
||||
assigneeAgentId: null,
|
||||
checkoutRunId: null,
|
||||
executionRunId: null,
|
||||
executionAgentNameKey: null,
|
||||
executionLockedAt: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(issues.id, id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!updated) return null;
|
||||
const [enriched] = await withIssueLabels(tx, [updated]);
|
||||
return enriched;
|
||||
}),
|
||||
|
||||
adminForceRelease: async (id: string, options: { clearAssignee?: boolean } = {}) =>
|
||||
db.transaction(async (tx) => {
|
||||
|
||||
Reference in new issue
Block a user