fix(webapp): serialise expired/failed global-scope recreate through the claim

Devin review follow-up. Only the claim-LOSER cleared path (reacquireClearedGlobalWinner) was
claim-serialised; the INITIAL existingRun probe's expired/failed path cleared the key and fell
through to an UNSERIALISED recreate. Under the split, two concurrent cross-residency triggers on an
expired/failed global-scope key could each recreate on their own DB (the per-DB unique index can't
dedup cross-residency). Route that initial-probe recreate through the same reacquireClearedGlobalWinner
claim serialisation when global-under-split; non-split / non-global keeps the plain recreate.

Author: d-cs

apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts

@@ -181,6 +181,14 @@ export class IdempotencyKeyConcern {
       }
     );
 
+    // `global`-scope (or scope-absent) keys under the split have no per-run salt, so the Redis claim is
+    // their only cross-DB dedup mutex. Computed here (not just in the claim block below) because the
+    // expired/failed clear-and-recreate path must serialise through it too.
+    const idempotencyKeyScope = request.body.options?.idempotencyKeyOptions?.scope;
+    const globalUnderSplit =
+      (idempotencyKeyScope === "global" || idempotencyKeyScope === undefined) &&
+      (await isSplitEnabled());
+
     const existingRun = idempotencyKey
       ? await runStore.findRun(
           {
@@ -217,11 +225,37 @@ export class IdempotencyKeyConcern {
     }
 
     if (existingRun) {
-      return await this.handleExistingRun(request, parentStore, existingRun, {
+      const handled = await this.handleExistingRun(request, parentStore, existingRun, {
         idempotencyKey,
         idempotencyKeyExpiresAt,
         dedupClient,
       });
+      // A LIVE cached hit (or andWait waitpoint wiring) is terminal.
+      if (handled.isCached) {
+        return handled;
+      }
+      // isCached === false → the existing run was EXPIRED/FAILED, so handleExistingRun cleared its key
+      // and we must recreate. For a global-scope key under split that recreate has to be claim-
+      // serialised too — otherwise two concurrent cross-residency recreates each create a run the
+      // per-DB unique index can't dedup (the same hole reacquireClearedGlobalWinner closes on the
+      // claim-loser path). Non-split / non-global: the plain unserialised recreate is safe.
+      if (globalUnderSplit) {
+        return await this.reacquireClearedGlobalWinner(request, parentStore, {
+          idempotencyKey,
+          idempotencyKeyExpiresAt,
+          dedupClient,
+          ttlSeconds: computeClaimTtlSeconds({
+            keyExpiresAt: idempotencyKeyExpiresAt,
+            now: Date.now(),
+            minTtlSeconds: env.TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS,
+            maxTtlSeconds: env.TRIGGER_MOLLIFIER_CLAIM_TTL_SECONDS,
+          }),
+          clearedRunId: existingRun.friendlyId,
+          safetyNetMs: env.TRIGGER_MOLLIFIER_CLAIM_WAIT_MS,
+          pollStepMs: env.TRIGGER_MOLLIFIER_CLAIM_POLL_MS,
+        });
+      }
+      return handled;
     }
 
     // Pre-gate claim — closes the PG+buffer race during gate transition.
@@ -248,11 +282,8 @@ export class IdempotencyKeyConcern {
     // older SDK) is treated conservatively as possibly-global — harmless for a
     // real run/attempt key, whose hash already embeds the parent id so two
     // parents mint DISTINCT keys that never share a claim slot.
-    const idempotencyKeyScope = request.body.options?.idempotencyKeyOptions?.scope;
-    const globalUnderSplit =
-      (idempotencyKeyScope === "global" || idempotencyKeyScope === undefined) &&
-      (await isSplitEnabled());
-
+    // (idempotencyKeyScope / globalUnderSplit are computed above — they also gate the expired/failed
+    // recreate serialisation.)
     const claimEligible =
       !request.body.options?.debounce &&
       !request.options?.oneTimeUseToken &&

apps/webapp/test/idempotencyExpiredRecreateReserialize.test.ts

@@ -0,0 +1,93 @@
+import { describe, expect, it, vi } from "vitest";
+
+// Devin #2: a global-scope (or scope-absent) key whose existing run is EXPIRED/FAILED gets its key
+// cleared and recreated. Under the run-ops split that recreate must serialise through the claim (same
+// cross-DB dup risk as the claim-loser cleared path) rather than fall through to an unserialised create.
+vi.mock("~/db.server", () => ({
+  prisma: {},
+  $replica: {},
+  runOpsNewPrisma: {},
+  runOpsLegacyPrisma: {},
+  runOpsNewReplica: {},
+  runOpsLegacyReplica: {},
+}));
+
+const h = vi.hoisted(() => ({ existingRun: null as unknown, splitEnabled: true }));
+
+vi.mock("~/v3/runStore.server", () => ({
+  runStore: { findRun: vi.fn(async () => h.existingRun) },
+}));
+vi.mock("~/v3/mollifier/mollifierBuffer.server", () => ({ getMollifierBuffer: () => null }));
+vi.mock("~/v3/mollifier/mollifierGate.server", () => ({
+  makeResolveMollifierFlag: () => async () => false,
+}));
+vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({
+  isSplitEnabled: async () => h.splitEnabled,
+}));
+vi.mock("~/runEngine/concerns/idempotencyResidency.server", () => ({
+  resolveIdempotencyDedupClient: async () => ({}),
+}));
+
+import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
+import type { TriggerTaskRequest } from "~/runEngine/types";
+
+function makeRequest(): TriggerTaskRequest {
+  return {
+    taskId: "my-task",
+    environment: { id: "env_a", organizationId: "org_1", organization: { featureFlags: {} } },
+    options: {},
+    body: { options: { idempotencyKey: "k-1" } }, // scope absent → treated as global
+  } as unknown as TriggerTaskRequest;
+}
+
+// handleExistingRun's documented contract for an expired/failed run: clear the key, return isCached:false.
+const CLEARED = {
+  isCached: false as const,
+  idempotencyKey: "k-1",
+  idempotencyKeyExpiresAt: new Date(Date.now() + 60_000),
+};
+
+describe("IdempotencyKeyConcern · expired/failed recreate re-serialisation", () => {
+  it("routes the cleared recreate through the claim under global-scope split (no unserialised create)", async () => {
+    h.existingRun = { id: "run_internal", friendlyId: "run_friendly" };
+    h.splitEnabled = true;
+    const concern = new IdempotencyKeyConcern({} as never, {} as never, {} as never);
+    vi.spyOn(
+      concern as never as { handleExistingRun: () => unknown },
+      "handleExistingRun"
+    ).mockResolvedValue(CLEARED);
+    const SENTINEL = { ...CLEARED, claim: { token: "t" } };
+    const reacquire = vi
+      .spyOn(
+        concern as never as { reacquireClearedGlobalWinner: () => unknown },
+        "reacquireClearedGlobalWinner"
+      )
+      .mockResolvedValue(SENTINEL);
+
+    const result = await concern.handleTriggerRequest(makeRequest(), undefined);
+
+    expect(reacquire).toHaveBeenCalledOnce();
+    expect(result).toBe(SENTINEL);
+  });
+
+  it("does NOT re-serialise when the split is off — plain recreate", async () => {
+    h.existingRun = { id: "run_internal", friendlyId: "run_friendly" };
+    h.splitEnabled = false;
+    const concern = new IdempotencyKeyConcern({} as never, {} as never, {} as never);
+    vi.spyOn(
+      concern as never as { handleExistingRun: () => unknown },
+      "handleExistingRun"
+    ).mockResolvedValue(CLEARED);
+    const reacquire = vi
+      .spyOn(
+        concern as never as { reacquireClearedGlobalWinner: () => unknown },
+        "reacquireClearedGlobalWinner"
+      )
+      .mockResolvedValue({} as never);
+
+    const result = await concern.handleTriggerRequest(makeRequest(), undefined);
+
+    expect(reacquire).not.toHaveBeenCalled();
+    expect(result).toBe(CLEARED);
+  });
+});