Merge 6113ed70b3 into c9e3bb7ca4

2026-09-13 17:20:41 +08:00 · 2026-09-13 17:20:41 +08:00 · e29d9b9694
parent c9e3bb7ca4 6113ed70b3
commit e29d9b9694
5 changed files with 388 additions and 3 deletions
--- a/packages/shared/src/validators/issue.test.ts
+++ b/packages/shared/src/validators/issue.test.ts
@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
 import { MAX_ISSUE_REQUEST_DEPTH } from "../index.js";
 import {
  addIssueCommentSchema,
+  checkoutIssueSchema,
  createIssueSchema,
  issueBlockedInboxAttentionSchema,
  resolveIssueRecoveryActionSchema,
@ -14,6 +15,39 @@ import {
 import { createAgentSchema } from "./agent.js";

 describe("issue validators", () => {
+  it("rejects terminal issue statuses as checkout expectations", () => {
+    const agentId = "11111111-1111-4111-8111-111111111111";
+
+    expect(
+      checkoutIssueSchema.safeParse({
+        agentId,
+        expectedStatuses: [
+          "backlog",
+          "todo",
+          "in_progress",
+          "in_review",
+          "blocked",
+        ],
+      }).success,
+    ).toBe(true);
+    expect(
+      checkoutIssueSchema.safeParse({ agentId, expectedStatuses: ["done"] })
+        .success,
+    ).toBe(false);
+    expect(
+      checkoutIssueSchema.safeParse({
+        agentId,
+        expectedStatuses: ["cancelled"],
+      }).success,
+    ).toBe(false);
+    expect(
+      checkoutIssueSchema.safeParse({
+        agentId,
+        expectedStatuses: ["todo", "done"],
+      }).success,
+    ).toBe(false);
+  });
+
  it("uses the same bounded unique upload ID contract for comment and update requests", () => {
    const id = "9af8228f-0be7-45ae-a104-6fbe0af6f1d3";
    expect(
--- a/packages/shared/src/validators/issue.ts
+++ b/packages/shared/src/validators/issue.ts
@ -894,7 +894,13 @@ export type StalledReviewDecision = z.infer<typeof stalledReviewDecisionSchema>;

 export const checkoutIssueSchema = z.object({
  agentId: z.string().guid(),
-  expectedStatuses: z.array(z.enum(ISSUE_STATUSES)).nonempty(),
+  expectedStatuses: z.array(z.enum([
+    "backlog",
+    "todo",
+    "in_progress",
+    "in_review",
+    "blocked",
+  ])).nonempty(),
 });

 export type CheckoutIssue = z.infer<typeof checkoutIssueSchema>;
--- a/server/src/tests/issue-stale-execution-lock-routes.test.ts
+++ b/server/src/tests/issue-stale-execution-lock-routes.test.ts
@ -486,9 +486,10 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => {
    // was cleared by releaseIssueExecutionAndPromote, but checkoutRunId stayed
    // pinned to the dead run. The new agent's POST /checkout would 409 forever
    // without the clearCheckoutRunIfTerminal helper in svc.checkout.
-    const { companyId, agentId, failedRunId, currentRunId } = await seedCompanyAgentAndRuns();
+    const { companyId, failedRunId } = await seedCompanyAgentAndRuns();
    const issueId = randomUUID();
    const otherAgentId = randomUUID();
+    const currentRunId = randomUUID();
    await db.insert(agents).values({
      id: otherAgentId,
      companyId,
@ -500,6 +501,14 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => {
      runtimeConfig: {},
      permissions: {},
    });
+    await db.insert(heartbeatRuns).values({
+      id: currentRunId,
+      companyId,
+      agentId: otherAgentId,
+      status: "running",
+      invocationSource: "manual",
+      startedAt: new Date(),
+    });
    await db.insert(issues).values({
      id: issueId,
      companyId,
--- a/server/src/tests/issues-service.test.ts
+++ b/server/src/tests/issues-service.test.ts
@ -6076,6 +6076,229 @@ describeEmbeddedPostgres("issueService.clearExecutionRunIfTerminal", () => {
    });
  });

+  it("rejects the exact late checkout from a succeeded run without reopening the done issue", async () => {
+    const companyId = randomUUID();
+    const agentId = randomUUID();
+    const issueId = randomUUID();
+    const succeededRunId = 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: succeededRunId,
+      companyId,
+      agentId,
+      status: "succeeded",
+      invocationSource: "manual",
+      finishedAt: new Date("2026-08-26T11:16:18.729Z"),
+    });
+    await db.insert(issues).values({
+      id: issueId,
+      companyId,
+      title: "Completed issue with a late tool call",
+      status: "done",
+      priority: "critical",
+      assigneeAgentId: agentId,
+      checkoutRunId: null,
+      executionRunId: null,
+      executionAgentNameKey: null,
+      executionLockedAt: null,
+      completedAt: new Date("2026-08-26T11:16:18.000Z"),
+    });
+
+    await expect(svc.checkout(issueId, agentId, ["done"], succeededRunId))
+      .rejects.toMatchObject({ status: 422 });
+
+    const row = await db
+      .select({
+        status: issues.status,
+        checkoutRunId: issues.checkoutRunId,
+        executionRunId: issues.executionRunId,
+        executionAgentNameKey: issues.executionAgentNameKey,
+        executionLockedAt: issues.executionLockedAt,
+      })
+      .from(issues)
+      .where(eq(issues.id, issueId))
+      .then((rows) => rows[0]);
+    expect(row).toEqual({
+      status: "done",
+      checkoutRunId: null,
+      executionRunId: null,
+      executionAgentNameKey: null,
+      executionLockedAt: null,
+    });
+  });
+
+  it.each([
+    ["succeeded", "succeeded"],
+    [null, "missing"],
+  ])("rejects a %s checkout run before acquiring an active issue lock", async (runStatus, expectedRunStatus) => {
+    const companyId = randomUUID();
+    const agentId = randomUUID();
+    const issueId = randomUUID();
+    const checkoutRunId = 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: {},
+    });
+    if (runStatus) {
+      await db.insert(heartbeatRuns).values({
+        id: checkoutRunId,
+        companyId,
+        agentId,
+        status: runStatus,
+        invocationSource: "manual",
+        finishedAt: new Date("2026-08-26T11:16:18.729Z"),
+      });
+    }
+    await db.insert(issues).values({
+      id: issueId,
+      companyId,
+      title: "Active issue with an invalid checkout run",
+      status: "todo",
+      priority: "critical",
+      assigneeAgentId: agentId,
+    });
+
+    await expect(svc.checkout(issueId, agentId, ["todo"], checkoutRunId))
+      .rejects.toMatchObject({
+        status: 409,
+        details: {
+          code: "checkout_run_not_active",
+          checkoutRunId,
+          runStatus: expectedRunStatus,
+        },
+      });
+
+    const row = await db
+      .select({
+        status: issues.status,
+        checkoutRunId: issues.checkoutRunId,
+        executionRunId: issues.executionRunId,
+      })
+      .from(issues)
+      .where(eq(issues.id, issueId))
+      .then((rows) => rows[0]);
+    expect(row).toEqual({
+      status: "todo",
+      checkoutRunId: null,
+      executionRunId: null,
+    });
+  });
+
+  it("rejects checkout when the run becomes terminal before the issue mutation", async () => {
+    const companyId = randomUUID();
+    const agentId = randomUUID();
+    const issueId = randomUUID();
+    const checkoutRunId = 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: checkoutRunId,
+      companyId,
+      agentId,
+      status: "running",
+      invocationSource: "manual",
+      startedAt: new Date("2026-08-26T11:16:18.000Z"),
+    });
+    await db.insert(issues).values({
+      id: issueId,
+      companyId,
+      title: "Checkout racing run completion",
+      status: "todo",
+      priority: "critical",
+      assigneeAgentId: agentId,
+    });
+
+    const terminalWriteReady = deferred<void>();
+    const allowTerminalCommit = deferred<void>();
+    const terminalWrite = db.transaction(async (tx) => {
+      await tx
+        .update(heartbeatRuns)
+        .set({
+          status: "succeeded",
+          finishedAt: new Date("2026-08-26T11:16:19.000Z"),
+        })
+        .where(eq(heartbeatRuns.id, checkoutRunId));
+      terminalWriteReady.resolve();
+      await allowTerminalCommit.promise;
+    });
+    await terminalWriteReady.promise;
+
+    const checkout = svc.checkout(issueId, agentId, ["todo"], checkoutRunId);
+    const checkoutAssertion = expect(checkout).rejects.toMatchObject({
+      status: 409,
+      details: {
+        code: "checkout_run_not_active",
+        checkoutRunId,
+        runStatus: "succeeded",
+      },
+    });
+    await new Promise<void>((resolve) => setImmediate(resolve));
+    allowTerminalCommit.resolve();
+    await terminalWrite;
+    await checkoutAssertion;
+    const row = await db
+      .select({
+        status: issues.status,
+        checkoutRunId: issues.checkoutRunId,
+        executionRunId: issues.executionRunId,
+      })
+      .from(issues)
+      .where(eq(issues.id, issueId))
+      .then((rows) => rows[0]);
+    expect(row).toEqual({
+      status: "todo",
+      checkoutRunId: null,
+      executionRunId: 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
@ -7094,6 +7317,42 @@ describeEmbeddedPostgres("issueService.assertCheckoutOwner stale checkout adopti
    });
  });

+  it("serializes concurrent checkout and unowned ownership assertion without a deadlock", async () => {
+    const seeded = await seedOwnershipIssue({ checkoutStatus: "failed" });
+    await db
+      .update(issues)
+      .set({
+        checkoutRunId: null,
+        executionRunId: null,
+        executionLockedAt: null,
+        executionAgentNameKey: null,
+      })
+      .where(eq(issues.id, seeded.issueId));
+
+    const [checkedOut, ownership] = await Promise.all([
+      svc.checkout(seeded.issueId, seeded.actorAgentId, ["in_progress"], seeded.actorRunId),
+      svc.assertCheckoutOwner(seeded.issueId, seeded.actorAgentId, seeded.actorRunId),
+    ]);
+
+    expect(checkedOut.checkoutRunId).toBe(seeded.actorRunId);
+    expect(checkedOut.executionRunId).toBe(seeded.actorRunId);
+    expect(ownership.checkoutRunId).toBe(seeded.actorRunId);
+    expect(ownership.executionRunId).toBe(seeded.actorRunId);
+
+    const row = await db
+      .select({
+        checkoutRunId: issues.checkoutRunId,
+        executionRunId: issues.executionRunId,
+      })
+      .from(issues)
+      .where(eq(issues.id, seeded.issueId))
+      .then((rows) => rows[0]);
+    expect(row).toEqual({
+      checkoutRunId: seeded.actorRunId,
+      executionRunId: seeded.actorRunId,
+    });
+  });
+
 });

 describeEmbeddedPostgres("issueService.addComment createdByRunId", () => {
--- a/server/src/services/issues.ts
+++ b/server/src/services/issues.ts
@ -7365,6 +7365,50 @@ export function issueService(db: Db) {
    return heartbeatRunIsTerminalOrMissing(dbOrTx, runId);
  }

+  async function withActiveCheckoutRun<T>(input: {
+    issueId: string;
+    companyId: string;
+    agentId: string;
+    checkoutRunId: string;
+    operation: (tx: DbTransaction) => Promise<T>;
+  }): Promise<T> {
+    return db.transaction(async (tx) => {
+      await tx.execute(
+        sql`select ${issues.id} from ${issues} where ${issues.id} = ${input.issueId} for update`,
+      );
+      await tx.execute(
+        sql`select ${heartbeatRuns.id} from ${heartbeatRuns} where ${heartbeatRuns.id} = ${input.checkoutRunId} for update`,
+      );
+      const checkoutRun = await tx
+        .select({
+          status: heartbeatRuns.status,
+          companyId: heartbeatRuns.companyId,
+          agentId: heartbeatRuns.agentId,
+        })
+        .from(heartbeatRuns)
+        .where(eq(heartbeatRuns.id, input.checkoutRunId))
+        .then((rows) => rows[0] ?? null);
+      const isOwnedRun = checkoutRun
+        && checkoutRun.companyId === input.companyId
+        && checkoutRun.agentId === input.agentId;
+      if (!isOwnedRun) {
+        throw conflict("Issue checkout requires an active heartbeat run", {
+          code: "checkout_run_not_active",
+          checkoutRunId: input.checkoutRunId,
+          runStatus: "missing",
+        });
+      }
+      if (TERMINAL_HEARTBEAT_RUN_STATUSES.has(checkoutRun.status)) {
+        throw conflict("Issue checkout requires an active heartbeat run", {
+          code: "checkout_run_not_active",
+          checkoutRunId: input.checkoutRunId,
+          runStatus: checkoutRun.status,
+        });
+      }
+      return input.operation(tx);
+    });
+  }
+
  async function adoptStaleCheckoutRun(input: {
    issueId: string;
    actorAgentId: string;
@ -7474,6 +7518,10 @@ export function issueService(db: Db) {
    actorRunId: string;
  }) {
    return db.transaction(async (tx) => {
+      // Keep the issue -> heartbeat lock order aligned with checkout and stale-lock cleanup.
+      await tx.execute(
+        sql`select ${issues.id} from ${issues} where ${issues.id} = ${input.issueId} for update`,
+      );
      await tx.execute(
        sql`select ${heartbeatRuns.id} from ${heartbeatRuns} where ${heartbeatRuns.id} = ${input.actorRunId} for update`,
      );
@ -11222,6 +11270,16 @@ export function issueService(db: Db) {
      expectedStatuses: string[],
      checkoutRunId: string | null,
    ) => {
+      const terminalExpectedStatuses = expectedStatuses.filter(
+        (status) => status === "done" || status === "cancelled",
+      );
+      if (terminalExpectedStatuses.length > 0) {
+        throw unprocessable(
+          "Issue checkout cannot expect terminal issue statuses",
+          { terminalExpectedStatuses },
+        );
+      }
+
      const issueCompany = await db
        .select({ companyId: issues.companyId })
        .from(issues)
@ -11258,6 +11316,16 @@ export function issueService(db: Db) {
        });
      }

+      if (checkoutRunId) {
+        await withActiveCheckoutRun({
+          issueId: id,
+          companyId: issueCompany.companyId,
+          agentId,
+          checkoutRunId,
+          operation: async () => undefined,
+        });
+      }
+
      await clearExecutionRunIfTerminal(id);
      await clearCheckoutRunIfTerminal(id);

@ -11300,7 +11368,7 @@ export function issueService(db: Db) {
            eq(issues.executionRunId, checkoutRunId),
          )
        : isNull(issues.executionRunId);
-      const updated = await db
+      const updateIssue = (dbOrTx: Db | DbTransaction) => dbOrTx
        .update(issues)
        .set({
          assigneeAgentId: agentId,
@ -11321,6 +11389,15 @@ export function issueService(db: Db) {
        )
        .returning()
        .then((rows) => rows[0] ?? null);
+      const updated = checkoutRunId
+        ? await withActiveCheckoutRun({
+            issueId: id,
+            companyId: issueCompany.companyId,
+            agentId,
+            checkoutRunId,
+            operation: updateIssue,
+          })
+        : await updateIssue(db);

      if (updated) {
        const [enriched] = await withIssueLabels(db, [updated]);