Skip to content

Commit 0301aef

Browse files
author
Instinct Agent
committed
fix(batch): clear idempotency keys pointing at dead runs in batchTrigger
The single-trigger path already clears an idempotency key when the cached run reached a clearable terminal state (shouldIdempotencyKeyBeCleared: failed statuses + EXPIRED). The batch path only tested time expiry, so batchTrigger with an idempotency key pointing at a FAILED run returned the dead run as isCached forever. - PostgresRunStore.findRunsByIdempotencyKeys now selects the status column - IdempotencyKeyRunMatch gains status - batchTriggerV3 clears the key and mints a fresh run when shouldIdempotencyKeyBeCleared(status) holds, same branch as time expiry Credit: reported by @Jaimin2687 in #4819; fix shape follows @itzzdev09's analysis in the issue thread.
1 parent 6f5c49c commit 0301aef

4 files changed

Lines changed: 46 additions & 3 deletions

File tree

‎apps/webapp/app/v3/services/batchTriggerV3.server.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { mintBatchFriendlyId } from "~/v3/runOpsMigration/mintBatchFriendlyId.se
2727
import { batchTriggerWorker } from "../batchTriggerWorker.server";
2828
import { guardQueueSizeLimitsForEnv } from "../queueSizeLimits.server";
2929
import { downloadPacketFromObjectStore, uploadPacketToObjectStore } from "../objectStore.server";
30-
import { isFinalAttemptStatus, isFinalRunStatus } from "../taskStatus";
30+
import { isFinalAttemptStatus, isFinalRunStatus, shouldIdempotencyKeyBeCleared } from "../taskStatus";
3131
import { startActiveSpan } from "../tracer.server";
3232
import { BaseService, ServiceValidationError } from "./baseService.server";
3333
import { OutOfEntitlementError, TriggerTaskService } from "./triggerTask.server";
@@ -450,7 +450,13 @@ export class BatchTriggerV3Service extends BaseService {
450450
);
451451

452452
if (cachedRun) {
453-
if (cachedRun.idempotencyKeyExpiresAt && cachedRun.idempotencyKeyExpiresAt < new Date()) {
453+
// Parity with the single-trigger path (IdempotencyKeyConcern.handleExistingRun): a cached
454+
// run that reached a clearable terminal state (failed/expired) must not be returned as
455+
// isCached - clear the key and mint a fresh run, same as a time-expired one.
456+
if (
457+
(cachedRun.idempotencyKeyExpiresAt && cachedRun.idempotencyKeyExpiresAt < new Date()) ||
458+
shouldIdempotencyKeyBeCleared(cachedRun.status)
459+
) {
454460
expiredRunIds.add(cachedRun.friendlyId);
455461

456462
return {

‎internal-packages/run-store/src/PostgresRunStore.findRunsByIdempotencyKeys.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,42 @@ describe("PostgresRunStore.findRunsByIdempotencyKeys", () => {
105105
expect(byKey.get("idem-2")?.idempotencyKeyExpiresAt).toBeNull();
106106
});
107107

108+
postgresTest("returns run status so callers can reject dead cached runs", async ({ prisma }) => {
109+
const { project, environment } = await seedEnvironment(prisma);
110+
const store = new PostgresRunStore({ prisma, readOnlyPrisma: prisma });
111+
112+
await createRun(prisma, {
113+
runtimeEnvironmentId: environment.id,
114+
projectId: project.id,
115+
friendlyId: "run_crashed1",
116+
taskIdentifier: "task-a",
117+
idempotencyKey: "idem-dead",
118+
});
119+
await prisma.taskRun.update({
120+
where: { friendlyId: "run_crashed1" },
121+
data: { status: "CRASHED" },
122+
});
123+
await createRun(prisma, {
124+
runtimeEnvironmentId: environment.id,
125+
projectId: project.id,
126+
friendlyId: "run_ok1",
127+
taskIdentifier: "task-a",
128+
idempotencyKey: "idem-live",
129+
});
130+
131+
const rows = await store.findRunsByIdempotencyKeys({
132+
runtimeEnvironmentId: environment.id,
133+
taskIdentifier: "task-a",
134+
idempotencyKeys: ["idem-dead", "idem-live"],
135+
});
136+
137+
const byKey = new Map(rows.map((r) => [r.idempotencyKey, r]));
138+
// batchTriggerV3 clears the key and mints a fresh run when shouldIdempotencyKeyBeCleared(status)
139+
// holds - it can only do that if the lookup actually returns the status column.
140+
expect(byKey.get("idem-dead")?.status).toBe("CRASHED");
141+
expect(byKey.get("idem-live")?.status).toBe("PENDING");
142+
});
143+
108144
postgresTest("short-circuits on an empty key list without querying", async ({ prisma }) => {
109145
const { environment } = await seedEnvironment(prisma);
110146
const store = new PostgresRunStore({ prisma, readOnlyPrisma: prisma });

‎internal-packages/run-store/src/PostgresRunStore.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1916,7 +1916,7 @@ export class PostgresRunStore implements RunStore {
19161916
const branches = args.idempotencyKeys.map((key) => {
19171917
const base = params.length;
19181918
params.push(args.runtimeEnvironmentId, args.taskIdentifier, key);
1919-
return `SELECT "id", "createdAt", "friendlyId", "idempotencyKey", "idempotencyKeyExpiresAt" FROM "TaskRun" WHERE "runtimeEnvironmentId" = $${base + 1} AND "taskIdentifier" = $${base + 2} AND "idempotencyKey" = $${base + 3}`;
1919+
return `SELECT "id", "createdAt", "friendlyId", "idempotencyKey", "idempotencyKeyExpiresAt", "status" FROM "TaskRun" WHERE "runtimeEnvironmentId" = $${base + 1} AND "taskIdentifier" = $${base + 2} AND "idempotencyKey" = $${base + 3}`;
19201920
});
19211921
return prisma.$queryRawUnsafe<IdempotencyKeyRunMatch[]>(
19221922
branches.join(" UNION ALL "),

‎internal-packages/run-store/src/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export type IdempotencyKeyRunMatch = {
3030
friendlyId: string;
3131
idempotencyKey: string | null;
3232
idempotencyKeyExpiresAt: Date | null;
33+
status: TaskRunStatus;
3334
};
3435

3536
/**

0 commit comments

Comments
 (0)