diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 077332c61c2..4d6cf1f88f1 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -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, + }); + }); }); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index e0839371d59..8e8ca0d8546 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -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) => {