diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index ba6efe59..a7a5fae6 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -4079,6 +4079,179 @@ describeEmbeddedPostgres("issueService.clearExecutionRunIfTerminal", () => { executionRunId: successorRunId, }); }); + + it("checkout refuses to promote a 'done' issue when 'done' is not in expectedStatuses, even with a lingering executionRunId pointer", async () => { + // Regression for PR #2482 checkout-adoption review finding: the original + // patch's stale-executionRunId adoption SQL set `status: 'in_progress'` + // unconditionally, bypassing the caller's expectedStatuses guard. With the + // guard restored, attempting to take over a 'done' issue with + // expectedStatuses=['todo'] must fail and leave the row untouched. + const companyId = randomUUID(); + const agentId = randomUUID(); + const issueId = randomUUID(); + const failedRunId = 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: 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: "Stale lock on done issue", + status: "done", + priority: "medium", + assigneeAgentId: agentId, + executionRunId: failedRunId, + executionAgentNameKey: "codexcoder", + executionLockedAt: new Date("2026-06-10T10:00:00.000Z"), + completedAt: new Date("2026-06-10T10:01:00.000Z"), + }); + + await expect(svc.checkout(issueId, agentId, ["todo"], successorRunId)) + .rejects.toMatchObject({ status: 409 }); + + const row = await db + .select({ + status: issues.status, + assigneeAgentId: issues.assigneeAgentId, + checkoutRunId: issues.checkoutRunId, + }) + .from(issues) + .where(eq(issues.id, issueId)) + .then((rows) => rows[0]); + expect(row).toMatchObject({ + status: "done", + assigneeAgentId: agentId, + checkoutRunId: null, + }); + }); + + it("checkout adoption of a stale checkoutRunId preserves the issue's assigneeUserId", async () => { + // Regression for PR #2482 checkout-adoption review finding: any adoption + // helper that re-locks an existing in_progress issue (e.g. when the prior + // checkout/execution run is terminal) must not strip the row's + // assigneeUserId. We exercise this via the adoptStaleCheckoutRun path, + // which fires when checkoutRunId points at a terminal run while + // executionRunId still points at a different, non-terminal run. + const companyId = randomUUID(); + const agentId = randomUUID(); + const userId = randomUUID(); + const issueId = randomUUID(); + const failedCheckoutRunId = randomUUID(); + const queuedExecutionRunId = 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: failedCheckoutRunId, + companyId, + agentId, + status: "failed", + invocationSource: "manual", + finishedAt: new Date("2026-06-10T10:05:00.000Z"), + }, + { + id: queuedExecutionRunId, + companyId, + agentId, + status: "queued", + invocationSource: "manual", + }, + { + 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: "Stale checkout lock with user co-assignee", + status: "in_progress", + priority: "medium", + assigneeAgentId: agentId, + assigneeUserId: userId, + checkoutRunId: failedCheckoutRunId, + executionRunId: queuedExecutionRunId, + executionAgentNameKey: "codexcoder", + executionLockedAt: new Date("2026-06-10T10:00:00.000Z"), + }); + + const result = await svc.checkout(issueId, agentId, ["todo", "in_progress"], successorRunId); + expect(result).toBeTruthy(); + + const row = await db + .select({ + status: issues.status, + assigneeAgentId: issues.assigneeAgentId, + assigneeUserId: issues.assigneeUserId, + 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, + assigneeUserId: userId, + checkoutRunId: successorRunId, + executionRunId: successorRunId, + }); + }); }); describeEmbeddedPostgres("accepted plan decomposition", () => { diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index f77744d2..93e72ddb 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -5098,7 +5098,6 @@ export function issueService(db: Db) { } if (issueData.status && issueData.status !== "in_progress") { patch.checkoutRunId = null; - // Fix B: also clear the execution lock when leaving in_progress patch.executionRunId = null; patch.executionAgentNameKey = null; patch.executionLockedAt = null; @@ -5108,7 +5107,6 @@ export function issueService(db: Db) { (issueData.assigneeUserId !== undefined && issueData.assigneeUserId !== existing.assigneeUserId) ) { patch.checkoutRunId = null; - // Fix B: clear execution lock on reassignment, matching checkoutRunId clear patch.executionRunId = null; patch.executionAgentNameKey = null; patch.executionLockedAt = null; @@ -5496,6 +5494,50 @@ export function issueService(db: Db) { } } + // Adopt stale executionRunId — if the execution lock points to a terminal/missing run, clear it and proceed. + // Only adopts when the caller's expectedStatuses guard still holds; preserves any existing assigneeUserId + // and preserves the original startedAt when the issue is already in_progress. + if ( + checkoutRunId && + current.executionRunId && + current.executionRunId !== checkoutRunId && + (current.assigneeAgentId === agentId || current.assigneeAgentId == null) + ) { + const stale = await isTerminalOrMissingHeartbeatRun(current.executionRunId); + if (stale) { + const now = new Date(); + const adoptionSet: Record = { + assigneeAgentId: agentId, + checkoutRunId, + executionRunId: checkoutRunId, + executionAgentNameKey: null, + executionLockedAt: now, + status: "in_progress", + updatedAt: now, + }; + if (current.status !== "in_progress") { + adoptionSet.startedAt = now; + } + const adopted = await db + .update(issues) + .set(adoptionSet) + .where( + and( + eq(issues.id, id), + inArray(issues.status, expectedStatuses), + eq(issues.executionRunId, current.executionRunId), + or(isNull(issues.assigneeAgentId), eq(issues.assigneeAgentId, agentId)), + ), + ) + .returning() + .then((rows) => rows[0] ?? null); + if (adopted) { + const [enriched] = await withIssueLabels(db, [adopted]); + return enriched; + } + } + } + // If this run already owns it and it's in_progress, return it (no self-409) if ( current.assigneeAgentId === agentId &&