diff --git a/apps/webapp/app/v3/services/batchTriggerV3.server.ts b/apps/webapp/app/v3/services/batchTriggerV3.server.ts index b86e5a40a64..c2b091f7f1d 100644 --- a/apps/webapp/app/v3/services/batchTriggerV3.server.ts +++ b/apps/webapp/app/v3/services/batchTriggerV3.server.ts @@ -27,7 +27,11 @@ import { mintBatchFriendlyId } from "~/v3/runOpsMigration/mintBatchFriendlyId.se import { batchTriggerWorker } from "../batchTriggerWorker.server"; import { guardQueueSizeLimitsForEnv } from "../queueSizeLimits.server"; import { downloadPacketFromObjectStore, uploadPacketToObjectStore } from "../objectStore.server"; -import { isFinalAttemptStatus, isFinalRunStatus } from "../taskStatus"; +import { + isFinalAttemptStatus, + isFinalRunStatus, + shouldIdempotencyKeyBeCleared, +} from "../taskStatus"; import { startActiveSpan } from "../tracer.server"; import { BaseService, ServiceValidationError } from "./baseService.server"; import { OutOfEntitlementError, TriggerTaskService } from "./triggerTask.server"; @@ -450,7 +454,24 @@ export class BatchTriggerV3Service extends BaseService { ); if (cachedRun) { - if (cachedRun.idempotencyKeyExpiresAt && cachedRun.idempotencyKeyExpiresAt < new Date()) { + // Clear the idempotency key and mint a fresh run when either: + // (a) the key's time-based expiry has passed, OR + // (b) the previous run ended in a terminal failure state the + // single-trigger path treats as "retry allowed" + // (CRASHED, SYSTEM_FAILURE, INTERRUPTED, COMPLETED_WITH_ERRORS, + // EXPIRED, TIMED_OUT_WITH_ERRORS — see shouldIdempotencyKeyBeCleared). + // + // Before this fix, batchTrigger only checked (a). A failed run with + // the same idempotency key was returned as `isCached: true`, silently + // handing the caller back a dead run that would never produce output + // (issue #4819). This mirrors the single-trigger behaviour at + // IdempotencyKeyConcern.handleExistingRun (idempotencyKeys.server.ts:463). + const expired = + cachedRun.idempotencyKeyExpiresAt && + cachedRun.idempotencyKeyExpiresAt < new Date(); + const failed = shouldIdempotencyKeyBeCleared(cachedRun.status); + + if (expired || failed) { expiredRunIds.add(cachedRun.friendlyId); return { diff --git a/internal-packages/run-store/src/PostgresRunStore.findRunsByIdempotencyKeys.test.ts b/internal-packages/run-store/src/PostgresRunStore.findRunsByIdempotencyKeys.test.ts index bc108cd1122..dff3e3ca663 100644 --- a/internal-packages/run-store/src/PostgresRunStore.findRunsByIdempotencyKeys.test.ts +++ b/internal-packages/run-store/src/PostgresRunStore.findRunsByIdempotencyKeys.test.ts @@ -1,5 +1,5 @@ import { postgresTest } from "@internal/testcontainers"; -import type { PrismaClient } from "@trigger.dev/database"; +import type { PrismaClient, TaskRunStatus } from "@trigger.dev/database"; import { describe, expect } from "vitest"; import { PostgresRunStore } from "./PostgresRunStore.js"; @@ -38,6 +38,7 @@ async function createRun( taskIdentifier: string; idempotencyKey: string; idempotencyKeyExpiresAt?: Date; + status?: TaskRunStatus; } ) { await prisma.taskRun.create({ @@ -46,6 +47,7 @@ async function createRun( taskIdentifier: params.taskIdentifier, idempotencyKey: params.idempotencyKey, idempotencyKeyExpiresAt: params.idempotencyKeyExpiresAt ?? null, + status: params.status ?? "PENDING", payload: "{}", payloadType: "application/json", runtimeEnvironmentId: params.runtimeEnvironmentId, @@ -117,4 +119,62 @@ describe("PostgresRunStore.findRunsByIdempotencyKeys", () => { expect(rows).toEqual([]); }); + + // Regression for #4819 — batchTrigger needs the row's status to decide + // whether to re-trigger (same semantics as single trigger via + // shouldIdempotencyKeyBeCleared). Before this fix the SQL omitted `status`, + // so batchTrigger could only inspect time-based expiry and silently handed + // callers back dead CRASHED / COMPLETED_WITH_ERRORS runs as `isCached: true`. + postgresTest( + "returns run status so callers can honour shouldIdempotencyKeyBeCleared", + async ({ prisma }) => { + const { project, environment } = await seedEnvironment(prisma); + const store = new PostgresRunStore({ prisma, readOnlyPrisma: prisma }); + + await createRun(prisma, { + runtimeEnvironmentId: environment.id, + projectId: project.id, + friendlyId: "run_pending", + taskIdentifier: "task-x", + idempotencyKey: "key-pending", + status: "PENDING", + }); + await createRun(prisma, { + runtimeEnvironmentId: environment.id, + projectId: project.id, + friendlyId: "run_crashed", + taskIdentifier: "task-x", + idempotencyKey: "key-crashed", + status: "CRASHED", + }); + await createRun(prisma, { + runtimeEnvironmentId: environment.id, + projectId: project.id, + friendlyId: "run_completed_with_errors", + taskIdentifier: "task-x", + idempotencyKey: "key-cwe", + status: "COMPLETED_WITH_ERRORS", + }); + await createRun(prisma, { + runtimeEnvironmentId: environment.id, + projectId: project.id, + friendlyId: "run_completed", + taskIdentifier: "task-x", + idempotencyKey: "key-ok", + status: "COMPLETED_SUCCESSFULLY", + }); + + const rows = await store.findRunsByIdempotencyKeys({ + runtimeEnvironmentId: environment.id, + taskIdentifier: "task-x", + idempotencyKeys: ["key-pending", "key-crashed", "key-cwe", "key-ok"], + }); + + const byKey = new Map(rows.map((r) => [r.idempotencyKey, r])); + expect(byKey.get("key-pending")?.status).toBe("PENDING"); + expect(byKey.get("key-crashed")?.status).toBe("CRASHED"); + expect(byKey.get("key-cwe")?.status).toBe("COMPLETED_WITH_ERRORS"); + expect(byKey.get("key-ok")?.status).toBe("COMPLETED_SUCCESSFULLY"); + } + ); }); diff --git a/internal-packages/run-store/src/PostgresRunStore.ts b/internal-packages/run-store/src/PostgresRunStore.ts index e66c9237a21..5714f9c9da0 100644 --- a/internal-packages/run-store/src/PostgresRunStore.ts +++ b/internal-packages/run-store/src/PostgresRunStore.ts @@ -1924,7 +1924,9 @@ export class PostgresRunStore implements RunStore { const branches = args.idempotencyKeys.map((key) => { const base = params.length; params.push(args.runtimeEnvironmentId, args.taskIdentifier, key); - return `SELECT "id", "createdAt", "friendlyId", "idempotencyKey", "idempotencyKeyExpiresAt" FROM "TaskRun" WHERE "runtimeEnvironmentId" = $${base + 1} AND "taskIdentifier" = $${base + 2} AND "idempotencyKey" = $${base + 3}`; + // "status" is required so callers can honour shouldIdempotencyKeyBeCleared + // (closes #4819 — batchTrigger previously returned stale failed runs as cached). + return `SELECT "id", "createdAt", "friendlyId", "idempotencyKey", "idempotencyKeyExpiresAt", "status" FROM "TaskRun" WHERE "runtimeEnvironmentId" = $${base + 1} AND "taskIdentifier" = $${base + 2} AND "idempotencyKey" = $${base + 3}`; }); return prisma.$queryRawUnsafe( branches.join(" UNION ALL "), diff --git a/internal-packages/run-store/src/types.ts b/internal-packages/run-store/src/types.ts index 660994eb95e..6233a6f7b24 100644 --- a/internal-packages/run-store/src/types.ts +++ b/internal-packages/run-store/src/types.ts @@ -30,6 +30,14 @@ export type IdempotencyKeyRunMatch = { friendlyId: string; idempotencyKey: string | null; idempotencyKeyExpiresAt: Date | null; + /** + * Terminal run status is needed so callers can honour the same + * "clear the idempotency key if the previous run failed" semantics the + * single-trigger path already applies via `shouldIdempotencyKeyBeCleared`. + * Without it, `batchTrigger` silently returned stale failed runs as + * `isCached: true` (issue #4819). + */ + status: TaskRunStatus; }; /**