diff --git a/internal-packages/run-engine/src/engine/systems/enqueueSystem.ts b/internal-packages/run-engine/src/engine/systems/enqueueSystem.ts index 38c681c511e..850f3714239 100644 --- a/internal-packages/run-engine/src/engine/systems/enqueueSystem.ts +++ b/internal-packages/run-engine/src/engine/systems/enqueueSystem.ts @@ -4,7 +4,7 @@ import type { TaskRun, TaskRunExecutionStatus, } from "@trigger.dev/database"; -import type { RunStore } from "@internal/run-store"; +import type { CompletedWaitpointRecord, RunStore } from "@internal/run-store"; import { parseNaturalLanguageDuration } from "@trigger.dev/core/v3/isomorphic"; import type { MinimalAuthenticatedEnvironment } from "../../shared/index.js"; import { QUEUED_SNAPSHOT_DESCRIPTION, QUEUED_SNAPSHOT_STATUS } from "../consts.js"; @@ -34,6 +34,7 @@ export class EnqueueSystem { batchId, checkpointId, completedWaitpoints, + completedWaitpointRecords, workerId, runnerId, skipRunLock, @@ -57,6 +58,7 @@ export class EnqueueSystem { id: string; index?: number; }[]; + completedWaitpointRecords?: CompletedWaitpointRecord[]; workerId?: string; runnerId?: string; skipRunLock?: boolean; @@ -108,6 +110,7 @@ export class EnqueueSystem { organizationId: env.organization.id, checkpointId, completedWaitpoints, + completedWaitpointRecords, workerId, runnerId, }, diff --git a/internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts b/internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts index 81c41d2c2ae..6cf830cc140 100644 --- a/internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts +++ b/internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts @@ -10,7 +10,7 @@ import type { TaskRunStatus, Waitpoint, } from "@trigger.dev/database"; -import type { RunStore } from "@internal/run-store"; +import type { CompletedWaitpointRecord, RunStore } from "@internal/run-store"; import { ExecutionSnapshotNotFoundError, ServiceValidationError } from "../errors.js"; import type { HeartbeatTimeouts } from "../types.js"; import type { SystemResources } from "./systems.js"; @@ -449,6 +449,7 @@ export class ExecutionSnapshotSystem { workerId, runnerId, completedWaitpoints, + completedWaitpointRecords, error, }: { run: { id: string; status: TaskRunStatus; attemptNumber?: number | null }; @@ -470,6 +471,7 @@ export class ExecutionSnapshotSystem { id: string; index?: number; }[]; + completedWaitpointRecords?: CompletedWaitpointRecord[]; error?: string; }, // When set (inside runStore.runInTransaction), the snapshot write goes through the owning store @@ -492,6 +494,7 @@ export class ExecutionSnapshotSystem { workerId, runnerId, completedWaitpoints, + completedWaitpointRecords, error, }, prisma diff --git a/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts b/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts index 3dbed999445..7b4d39e80b8 100644 --- a/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts +++ b/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts @@ -1,4 +1,6 @@ import { timeoutError } from "@trigger.dev/core/v3"; +import { parseWaitpointId } from "@trigger.dev/core/v3/isomorphic"; +import type { CompletedWaitpointRecord } from "@internal/run-store"; import type { PrismaClientOrTransaction, TaskRun, @@ -10,7 +12,8 @@ import { assertNever } from "assert-never"; import { sendNotificationToWorker } from "../eventBus.js"; import { isFinalRunStatus } from "../statuses.js"; import { LegacyPostgresWaitpointCoordinator } from "../waitpointCoordinator/legacyPostgresCoordinator.js"; -import type { WaitpointCoordinator } from "../waitpointCoordinator/types.js"; +import { buildCompletedWaitpointRecords } from "../waitpointCoordinator/completedWaitpointRecords.js"; +import type { RunBlockEdge, WaitpointCoordinator } from "../waitpointCoordinator/types.js"; import type { EnqueueSystem } from "./enqueueSystem.js"; import type { ExecutionSnapshotSystem } from "./executionSnapshotSystem.js"; import { getLatestExecutionSnapshot } from "./executionSnapshotSystem.js"; @@ -484,6 +487,14 @@ export class WaitpointSystem { }; } + // The record set rides the wait cycle's key once per resume, so build it here rather + // than at each append site. Nothing mints a store-format waitpoint yet, so + // #completedWaitpointRecordsFor returns undefined on every live path today. + const completedWaitpointRecords = await this.#completedWaitpointRecordsFor( + runId, + blockingWaitpoints + ); + // 3. Get the run (run-ops scalars) + resolve its environment via the control-plane resolver, // so the run-ops DB can split without a cross-provider join. const run = await this.$.runStore.findRun( @@ -623,6 +634,7 @@ export class WaitpointSystem { id: b.waitpoint.id, index: b.batchIndex ?? undefined, })), + ...(completedWaitpointRecords && { completedWaitpointRecords }), } ); @@ -682,6 +694,7 @@ export class WaitpointSystem { id: b.waitpoint.id, index: b.batchIndex ?? undefined, })), + ...(completedWaitpointRecords && { completedWaitpointRecords }), checkpointId: snapshot.checkpointId ?? undefined, }); @@ -728,6 +741,37 @@ export class WaitpointSystem { return this.coordinator.mintAssociatedWaitpointData({ projectId, environmentId }); } + /** + * The record set for one resume, or undefined when this wait has no store-resident half. + * + * The classification gate is what keeps this inert. `parseWaitpointId` reports legacy for + * every id minted today, so no live resume reads an envelope or writes a record until a + * waitpoint mints in store format. + */ + async #completedWaitpointRecordsFor( + runId: string, + blockingWaitpoints: RunBlockEdge[] + ): Promise { + const storeResidentIds = [ + ...new Set( + blockingWaitpoints + .map((b) => b.waitpoint.id) + .filter((id) => parseWaitpointId(id).format === "b32hexW") + ), + ]; + + if (storeResidentIds.length === 0) { + return undefined; + } + + const sources = await this.coordinator.readCompletionEnvelopes({ + runId, + waitpointIds: storeResidentIds, + }); + + return buildCompletedWaitpointRecords(sources); + } + /** * Builds the waitpoint output payload from a completed run's stored output/error. */ diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointEquivalence.test.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointEquivalence.test.ts new file mode 100644 index 00000000000..b5420806074 --- /dev/null +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointEquivalence.test.ts @@ -0,0 +1,330 @@ +// The resolver must produce what the executor already consumes, so the oracle is the +// existing hydration and not a hand-written literal. A literal cannot catch a drift in +// enhanceExecutionSnapshotWithWaitpoints itself; this can. +import type { Waitpoint } from "@trigger.dev/database"; +import { describe, expect, it } from "vitest"; +import { enhanceExecutionSnapshotWithWaitpoints } from "../systems/executionSnapshotSystem.js"; +import { buildCompletedWaitpointRecords } from "./completedWaitpointRecords.js"; +import { createCompletedWaitpointResolver } from "./completedWaitpointResolver.js"; +import type { CompletionEnvelopeSource } from "./types.js"; + +const COMPLETED_AT = new Date("2026-08-25T00:00:00.000Z"); +const RUN_ID = "run_0123456789abcdefghijklm"; +const CHILD_RUN_ID = "run_zyxwvutsrqponmlkjihgfe"; +const BATCH_ID = "batch_0123456789abcdefghijk"; + +/** + * One waitpoint, in both shapes, from one description. Keeping them in one factory is what + * makes the comparison meaningful: a field added to only one shape shows up as a diff. + */ +function pair(overrides: { + id: string; + type: Waitpoint["type"]; + output?: string | null; + outputType?: string; + outputIsError?: boolean; + completedByTaskRunId?: string | null; + completedByBatchId?: string | null; + completedAfter?: Date | null; + idempotencyKey?: string; + userProvidedIdempotencyKey?: boolean; + inactiveIdempotencyKey?: string | null; +}): { row: Waitpoint; source: CompletionEnvelopeSource } { + const outputType = overrides.outputType ?? "application/json"; + const outputIsError = overrides.outputIsError ?? false; + const output = overrides.output ?? null; + const isRef = outputType === "application/store"; + + const row = { + id: overrides.id, + friendlyId: `waitpoint_${overrides.id}`, + type: overrides.type, + status: "COMPLETED", + completedAt: COMPLETED_AT, + output, + outputType, + outputIsError, + completedByTaskRunId: overrides.completedByTaskRunId ?? null, + completedByBatchId: overrides.completedByBatchId ?? null, + completedAfter: overrides.completedAfter ?? null, + idempotencyKey: overrides.idempotencyKey ?? "internal", + userProvidedIdempotencyKey: overrides.userProvidedIdempotencyKey ?? false, + inactiveIdempotencyKey: overrides.inactiveIdempotencyKey ?? null, + } as unknown as Waitpoint; + + const source: CompletionEnvelopeSource = { + id: overrides.id, + friendlyId: `waitpoint_${overrides.id}`, + type: overrides.type, + completedAt: COMPLETED_AT, + outputType, + outputIsError, + ...(output !== null ? (isRef ? { outputRef: output } : { output }) : {}), + ...(overrides.completedByTaskRunId && { + completedByTaskRunId: overrides.completedByTaskRunId, + }), + ...(overrides.completedByBatchId && { completedByBatchId: overrides.completedByBatchId }), + ...(overrides.completedAfter && { completedAfter: overrides.completedAfter }), + ...(overrides.userProvidedIdempotencyKey && + !overrides.inactiveIdempotencyKey && + overrides.idempotencyKey + ? { idempotencyKey: overrides.idempotencyKey } + : {}), + }; + + return { row, source }; +} + +function snapshot(batchId: string | null) { + return { id: "snap_1", runId: RUN_ID, batchId } as never; +} + +function sortEntries(entries: T[]): T[] { + return [...entries].sort((a, b) => a.id.localeCompare(b.id) || (a.index ?? -1) - (b.index ?? -1)); +} + +/** + * Run one description through both paths and assert the results match. + * + * `deriveFromRun` is the one case where the two paths cannot be identical by construction: + * the row carries the value and the record carries a marker. Feeding the row's own output + * back as the run's output is what makes them comparable, which is exactly the claim the + * variant makes — that TaskRun.output holds the same string. + */ +async function bothPaths( + pairs: ReturnType[], + order: string[], + batchId: string | null = null +) { + const outputsByRunId = new Map(); + for (const { row } of pairs) { + if (row.completedByTaskRunId && row.output !== null) { + outputsByRunId.set(row.completedByTaskRunId, row.output); + } + } + + const expected = enhanceExecutionSnapshotWithWaitpoints( + snapshot(batchId), + pairs.map((p) => p.row), + order + ).completedWaitpoints; + + const actual = await createCompletedWaitpointResolver({ + readRunOutput: async (taskRunId) => outputsByRunId.get(taskRunId), + })({ + runId: RUN_ID, + ...(batchId ? { batchId } : {}), + pointer: { cycleSeq: 1, count: order.length }, + order, + records: buildCompletedWaitpointRecords(pairs.map((p) => p.source)), + }); + + return { expected: sortEntries(expected), actual: sortEntries(actual) }; +} + +describe("the resolver reproduces the existing hydration", () => { + it("for a single MANUAL waitpoint with an inline output", async () => { + const { expected, actual } = await bothPaths( + [pair({ id: "wp_manual", type: "MANUAL", output: '{"token":1}' })], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for a MANUAL waitpoint with a user-provided idempotency key", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_manual", + type: "MANUAL", + output: '{"token":1}', + idempotencyKey: "user-key", + userProvidedIdempotencyKey: true, + }), + ], + [] + ); + + expect(actual).toEqual(expected); + expect(actual[0]?.idempotencyKey).toBe("user-key"); + }); + + it("for an idempotency key the user provided but that went inactive", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_manual", + type: "MANUAL", + output: '{"token":1}', + idempotencyKey: "user-key", + userProvidedIdempotencyKey: true, + inactiveIdempotencyKey: "old", + }), + ], + [] + ); + + expect(actual).toEqual(expected); + expect(actual[0]?.idempotencyKey).toBeUndefined(); + }); + + it("for a DATETIME waitpoint", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_datetime", + type: "DATETIME", + completedAfter: new Date("2026-08-26T00:00:00.000Z"), + }), + ], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for a RUN waitpoint outside a batch", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_run", + type: "RUN", + output: '{"ok":true}', + completedByTaskRunId: CHILD_RUN_ID, + }), + ], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for a RUN waitpoint read under a batch", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_run", + type: "RUN", + output: '{"ok":true}', + completedByTaskRunId: CHILD_RUN_ID, + }), + ], + ["wp_run"], + BATCH_ID + ); + + expect(actual).toEqual(expected); + expect(actual[0]?.completedByTaskRun?.batch?.id).toBe(BATCH_ID); + }); + + it("for a RUN waitpoint whose output is an error", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_run", + type: "RUN", + output: '{"message":"boom"}', + outputIsError: true, + completedByTaskRunId: CHILD_RUN_ID, + }), + ], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for a BATCH waitpoint", async () => { + const { expected, actual } = await bothPaths( + [pair({ id: "wp_batch", type: "BATCH", completedByBatchId: BATCH_ID })], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for an already-offloaded output", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_manual", + type: "MANUAL", + output: "store-key-1", + outputType: "application/store", + }), + ], + [] + ); + + expect(actual).toEqual(expected); + }); + + it("for one run present at two batch indexes", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_run", + type: "RUN", + output: '{"ok":true}', + completedByTaskRunId: CHILD_RUN_ID, + }), + ], + ["wp_run", "wp_run"], + BATCH_ID + ); + + expect(actual).toEqual(expected); + expect(actual.map((w) => w.index)).toEqual([0, 1]); + }); + + it("for an index-less waitpoint sitting beside indexed ones", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ id: "wp_indexless", type: "MANUAL", output: '{"token":1}' }), + pair({ + id: "wp_run", + type: "RUN", + output: '{"ok":true}', + completedByTaskRunId: CHILD_RUN_ID, + }), + ], + ["wp_run"], + BATCH_ID + ); + + expect(actual).toEqual(expected); + expect(actual.find((w) => w.id === "wp_indexless")?.index).toBeUndefined(); + }); + + it("for every type at once, under a batch", async () => { + const { expected, actual } = await bothPaths( + [ + pair({ + id: "wp_run", + type: "RUN", + output: '{"ok":true}', + completedByTaskRunId: CHILD_RUN_ID, + }), + pair({ id: "wp_batch", type: "BATCH", completedByBatchId: BATCH_ID }), + pair({ + id: "wp_datetime", + type: "DATETIME", + completedAfter: new Date("2026-08-26T00:00:00.000Z"), + }), + pair({ + id: "wp_manual", + type: "MANUAL", + output: '{"token":1}', + idempotencyKey: "user-key", + userProvidedIdempotencyKey: true, + }), + ], + ["wp_run", "wp_batch", "wp_datetime"], + BATCH_ID + ); + + expect(actual).toEqual(expected); + expect(actual).toHaveLength(4); + }); +}); diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.test.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.test.ts new file mode 100644 index 00000000000..42d54ce71af --- /dev/null +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it } from "vitest"; +import { buildCompletedWaitpointRecords } from "./completedWaitpointRecords.js"; +import type { CompletionEnvelopeSource } from "./types.js"; + +const COMPLETED_AT = new Date("2026-08-25T00:00:00.000Z"); + +function source(overrides: Partial = {}): CompletionEnvelopeSource { + return { + id: "wp_1", + friendlyId: "waitpoint_wp_1", + type: "MANUAL", + completedAt: COMPLETED_AT, + outputType: "application/json", + outputIsError: false, + ...overrides, + }; +} + +describe("buildCompletedWaitpointRecords", () => { + it("emits one record per distinct id", () => { + const records = buildCompletedWaitpointRecords([source(), source()]); + + expect(records).toHaveLength(1); + }); + + it("emits one record for each of several distinct ids", () => { + const records = buildCompletedWaitpointRecords([ + source({ id: "wp_1" }), + source({ id: "wp_2" }), + ]); + + expect(records.map((r) => r.id)).toEqual(["wp_1", "wp_2"]); + }); + + it("writes completedAt as an ISO string", () => { + const [record] = buildCompletedWaitpointRecords([source()]); + + expect(record?.completedAt).toBe("2026-08-25T00:00:00.000Z"); + }); + + it("omits every absent optional field rather than writing undefined", () => { + const [record] = buildCompletedWaitpointRecords([source()]); + + expect("completedByTaskRunId" in record!).toBe(false); + expect("completedByBatchId" in record!).toBe(false); + expect("completedAfter" in record!).toBe(false); + expect("idempotencyKey" in record!).toBe(false); + }); + + it("carries the fields the executor shape needs", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ + completedAfter: new Date("2026-08-26T00:00:00.000Z"), + idempotencyKey: "user-key", + }), + ]); + + expect(record).toMatchObject({ + id: "wp_1", + friendlyId: "waitpoint_wp_1", + type: "MANUAL", + outputType: "application/json", + outputIsError: false, + completedAfter: "2026-08-26T00:00:00.000Z", + idempotencyKey: "user-key", + }); + }); + + describe("the output variant", () => { + it("keeps an already-offloaded value as a ref", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ outputRef: "store-key-1", outputType: "application/store" }), + ]); + + expect(record?.output).toEqual({ ref: "store-key-1" }); + }); + + it("prefers a ref over an inline value when both are somehow present", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ output: '{"ok":true}', outputRef: "store-key-1" }), + ]); + + expect(record?.output).toEqual({ ref: "store-key-1" }); + }); + + it("marks a plain RUN output as derivable from the run", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ type: "RUN", output: '{"ok":true}', completedByTaskRunId: "run_1" }), + ]); + + expect(record?.output).toEqual({ deriveFromRun: true }); + }); + + // TaskRun.error is jsonb and does not round-trip to the same string, so a RUN error can + // never be re-read from the run row. + it("keeps a RUN error inline", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ + type: "RUN", + output: '{"message":"boom"}', + outputIsError: true, + completedByTaskRunId: "run_1", + }), + ]); + + expect(record?.output).toEqual({ inline: '{"message":"boom"}' }); + }); + + // The back-reference is onDelete: SetNull, so an orphaned RUN waitpoint has no run row + // left to derive from. + it("keeps an orphaned RUN inline", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ type: "RUN", output: '{"ok":true}' }), + ]); + + expect(record?.output).toEqual({ inline: '{"ok":true}' }); + }); + + it("omits a BATCH output, because the runtime discards it at source", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ type: "BATCH", completedByBatchId: "batch_1", output: '{"ignored":true}' }), + ]); + + expect(record?.output).toBeNull(); + }); + + it("keeps a MANUAL output inline", () => { + const [record] = buildCompletedWaitpointRecords([source({ output: '{"token":1}' })]); + + expect(record?.output).toEqual({ inline: '{"token":1}' }); + }); + + it("keeps a DATETIME output inline", () => { + const [record] = buildCompletedWaitpointRecords([ + source({ type: "DATETIME", output: '{"at":1}' }), + ]); + + expect(record?.output).toEqual({ inline: '{"at":1}' }); + }); + + it("writes null when there is no output at all", () => { + const [record] = buildCompletedWaitpointRecords([source()]); + + expect(record?.output).toBeNull(); + }); + + it("keeps an empty-string output inline, because empty is a value and not an absence", () => { + const [record] = buildCompletedWaitpointRecords([source({ output: "" })]); + + expect(record?.output).toEqual({ inline: "" }); + }); + }); + + it("returns an empty set for no sources", () => { + expect(buildCompletedWaitpointRecords([])).toEqual([]); + }); +}); diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.ts new file mode 100644 index 00000000000..91cdaaf6781 --- /dev/null +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointRecords.ts @@ -0,0 +1,60 @@ +import type { CompletedWaitpointRecord, CompletedWaitpointRecordOutput } from "@internal/run-store"; +import type { CompletionEnvelopeSource } from "./types.js"; + +/** + * Turn sourced envelope fields into the frozen record set that rides one wait cycle's key. + * + * One record per DISTINCT id. The cycle's ordered id list carries multiplicity, and the + * resolver expands one record into one entry per position of its id. That list holds only + * batch-indexed ids, because its positions ARE the indexes, so this set — not the list — is + * authoritative for membership. + */ +export function buildCompletedWaitpointRecords( + sources: CompletionEnvelopeSource[] +): CompletedWaitpointRecord[] { + const byId = new Map(); + + for (const source of sources) { + if (byId.has(source.id)) { + continue; + } + + byId.set(source.id, { + id: source.id, + friendlyId: source.friendlyId, + type: source.type, + completedAt: source.completedAt.toISOString(), + outputType: source.outputType, + outputIsError: source.outputIsError, + output: chooseOutput(source), + ...(source.completedByTaskRunId && { completedByTaskRunId: source.completedByTaskRunId }), + ...(source.completedByBatchId && { completedByBatchId: source.completedByBatchId }), + ...(source.completedAfter && { completedAfter: source.completedAfter.toISOString() }), + ...(source.idempotencyKey && { idempotencyKey: source.idempotencyKey }), + }); + } + + return [...byId.values()]; +} + +function chooseOutput(source: CompletionEnvelopeSource): CompletedWaitpointRecordOutput { + if (source.outputRef !== undefined) { + return { ref: source.outputRef }; + } + + // A plain RUN output is re-readable from TaskRun.output verbatim. Two RUN cases are not, + // and both must stay inline: an ERROR, because TaskRun.error is jsonb and does not + // round-trip to the same string, and an ORPHAN, because the back-reference is + // onDelete: SetNull so the completing row may be gone. + if (source.type === "RUN" && !source.outputIsError && source.completedByTaskRunId) { + return { deriveFromRun: true }; + } + + // The runtime discards a batch output at source, so there is nothing to carry. + if (source.type === "BATCH") { + return null; + } + + // An empty string is a value, not an absence, so this checks undefined only. + return source.output === undefined ? null : { inline: source.output }; +} diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.test.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.test.ts new file mode 100644 index 00000000000..834550cb958 --- /dev/null +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.test.ts @@ -0,0 +1,332 @@ +import type { CompletedWaitpointRecord } from "@internal/run-store"; +import { BatchId, RunId } from "@trigger.dev/core/v3/isomorphic"; +import { describe, expect, it } from "vitest"; +import { + createCompletedWaitpointResolver, + UnresolvableWaitpointId, +} from "./completedWaitpointResolver.js"; + +function record(overrides: Partial = {}): CompletedWaitpointRecord { + return { + id: "wp_1", + friendlyId: "waitpoint_wp_1", + type: "MANUAL", + completedAt: "2026-08-25T00:00:00.000Z", + outputType: "application/json", + outputIsError: false, + output: { inline: '{"ok":true}' }, + ...overrides, + }; +} + +const noRunOutput = { readRunOutput: async () => undefined }; + +function resolver(readRunOutput?: (taskRunId: string) => Promise) { + return createCompletedWaitpointResolver(readRunOutput ? { readRunOutput } : noRunOutput); +} + +const CYCLE = { cycleSeq: 1, count: 0 }; + +describe("the index expansion", () => { + it("emits one entry per position of the id in the order", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 2 }, + order: ["wp_1", "wp_1"], + records: [record()], + }); + + expect(result).toHaveLength(2); + expect(result.map((w) => w.index)).toEqual([0, 1]); + }); + + it("gives a run at two batch indexes its two real positions", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 3 }, + order: ["wp_other", "wp_1", "wp_1"], + records: [record(), record({ id: "wp_other", friendlyId: "waitpoint_wp_other" })], + }); + + expect(result.filter((w) => w.id === "wp_1").map((w) => w.index)).toEqual([1, 2]); + }); + + // Every wait.for, every single triggerAndWait and every token has no batch index, so it + // is absent from the order. Dropping it here loses the run's results on resume. + it("keeps a record with no position, with an undefined index", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record()], + }); + + expect(result).toHaveLength(1); + expect(result[0]?.index).toBeUndefined(); + }); + + it("keeps an index-less record alongside an indexed one", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 1 }, + order: ["wp_indexed"], + records: [record(), record({ id: "wp_indexed", friendlyId: "waitpoint_wp_indexed" })], + }); + + expect(result).toHaveLength(2); + expect(result.find((w) => w.id === "wp_1")?.index).toBeUndefined(); + expect(result.find((w) => w.id === "wp_indexed")?.index).toBe(0); + }); +}); + +describe("the executor shape", () => { + it("reproduces the scalar fields", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ idempotencyKey: "user-key" })], + }); + + expect(entry).toMatchObject({ + id: "wp_1", + friendlyId: "waitpoint_wp_1", + type: "MANUAL", + completedAt: new Date("2026-08-25T00:00:00.000Z"), + idempotencyKey: "user-key", + output: '{"ok":true}', + outputType: "application/json", + outputIsError: false, + }); + }); + + it("builds completedByTaskRun for a RUN record", async () => { + const childRunId = RunId.fromFriendlyId(RunId.generate().friendlyId); + + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ type: "RUN", completedByTaskRunId: childRunId, output: null })], + }); + + expect(entry?.completedByTaskRun).toEqual({ + id: childRunId, + friendlyId: RunId.toFriendlyId(childRunId), + }); + }); + + // The cycle is minted once, but a later entry in the resume chain can be read under a + // different batch. The batch shown must be the reading entry's, never the minting one's. + it("takes batch{} from the reading entry's batchId", async () => { + const childRunId = RunId.fromFriendlyId(RunId.generate().friendlyId); + const batchId = BatchId.fromFriendlyId(BatchId.generate().friendlyId); + + const [entry] = await resolver()({ + runId: "run_1", + batchId, + pointer: CYCLE, + order: [], + records: [record({ type: "RUN", completedByTaskRunId: childRunId, output: null })], + }); + + expect(entry?.completedByTaskRun?.batch).toEqual({ + id: batchId, + friendlyId: BatchId.toFriendlyId(batchId), + }); + }); + + it("omits batch{} when the reading entry has no batch", async () => { + const childRunId = RunId.fromFriendlyId(RunId.generate().friendlyId); + + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ type: "RUN", completedByTaskRunId: childRunId, output: null })], + }); + + expect(entry?.completedByTaskRun?.batch).toBeUndefined(); + }); + + it("builds completedByBatch for a BATCH record", async () => { + const batchId = BatchId.fromFriendlyId(BatchId.generate().friendlyId); + + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ type: "BATCH", completedByBatchId: batchId, output: null })], + }); + + expect(entry?.completedByBatch).toEqual({ + id: batchId, + friendlyId: BatchId.toFriendlyId(batchId), + }); + }); + + it("carries completedAfter as a Date", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ type: "DATETIME", completedAfter: "2026-08-26T00:00:00.000Z" })], + }); + + expect(entry?.completedAfter).toEqual(new Date("2026-08-26T00:00:00.000Z")); + }); +}); + +describe("the output hydration", () => { + it("returns an inline value as-is", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ output: { inline: '{"v":1}' } })], + }); + + expect(entry?.output).toBe('{"v":1}'); + }); + + it("returns a ref as the output, so the executor resolves it the existing way", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ output: { ref: "store-key-1" }, outputType: "application/store" })], + }); + + expect(entry?.output).toBe("store-key-1"); + }); + + it("reads a deriveFromRun output from the run", async () => { + const [entry] = await resolver(async (id) => + id === "run_child" ? '{"derived":true}' : undefined + )({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [ + record({ type: "RUN", completedByTaskRunId: "run_child", output: { deriveFromRun: true } }), + ], + }); + + expect(entry?.output).toBe('{"derived":true}'); + }); + + it("leaves the output undefined when the run row is gone", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [ + record({ type: "RUN", completedByTaskRunId: "run_gone", output: { deriveFromRun: true } }), + ], + }); + + expect(entry?.output).toBeUndefined(); + }); + + it("reads the run once for a record that expands to several entries", async () => { + const reads: string[] = []; + const result = await resolver(async (id) => { + reads.push(id); + return '{"derived":true}'; + })({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 2 }, + order: ["wp_1", "wp_1"], + records: [ + record({ type: "RUN", completedByTaskRunId: "run_child", output: { deriveFromRun: true } }), + ], + }); + + expect(result).toHaveLength(2); + expect(reads).toEqual(["run_child"]); + }); + + it("leaves the output undefined when the record carries none", async () => { + const [entry] = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record({ output: null })], + }); + + expect(entry?.output).toBeUndefined(); + }); +}); + +// The id classifier is total and never throws: an unrecognised shape classifies as legacy, +// finds no row, and would otherwise vanish from the resumed run's completed set with no +// error. These are the tests that make that impossible. +describe("the coverage check", () => { + it("throws when the order names an id no half resolved", async () => { + await expect( + resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 1 }, + order: ["wp_missing"], + records: [record()], + }) + ).rejects.toThrow(UnresolvableWaitpointId); + }); + + it("names the offending id and the reason", async () => { + const error = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 1 }, + order: ["wp_missing"], + records: [record()], + }).catch((caught: unknown) => caught as UnresolvableWaitpointId); + + expect(error.waitpointId).toBe("wp_missing"); + expect(error.reason).toBe("no-source"); + }); + + it("accepts an ordered id that the caller resolved from a row", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 1 }, + order: ["wp_legacy"], + records: [record()], + resolvedElsewhere: ["wp_legacy"], + }); + + expect(result.map((w) => w.id)).toEqual(["wp_1"]); + }); + + it("throws when both halves claim the same id", async () => { + const error = await resolver()({ + runId: "run_1", + pointer: CYCLE, + order: [], + records: [record()], + resolvedElsewhere: ["wp_1"], + }).catch((caught: unknown) => caught as UnresolvableWaitpointId); + + expect(error).toBeInstanceOf(UnresolvableWaitpointId); + expect(error.waitpointId).toBe("wp_1"); + expect(error.reason).toBe("two-sources"); + }); + + it("returns only its own half, leaving the legacy half to the caller", async () => { + const result = await resolver()({ + runId: "run_1", + pointer: { cycleSeq: 1, count: 2 }, + order: ["wp_legacy", "wp_1"], + records: [record()], + resolvedElsewhere: ["wp_legacy"], + }); + + expect(result.map((w) => w.id)).toEqual(["wp_1"]); + expect(result[0]?.index).toBe(1); + }); + + it("resolves an empty cycle to nothing", async () => { + await expect( + resolver()({ runId: "run_1", pointer: CYCLE, order: [], records: [] }) + ).resolves.toEqual([]); + }); +}); diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.ts new file mode 100644 index 00000000000..3430e96279d --- /dev/null +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/completedWaitpointResolver.ts @@ -0,0 +1,149 @@ +import type { CompletedWaitpointRecord, ResolveCompletedWaitpointsArgs } from "@internal/run-store"; +import { BatchId, RunId } from "@trigger.dev/core/v3/isomorphic"; +import type { CompletedWaitpoint } from "@trigger.dev/core/v3/schemas"; + +/** + * A waitpoint id that no half of a snapshot can account for, or that both halves claim. + * + * This exists because the id classifier is total and never throws: an unrecognised shape + * classifies as legacy, finds no row, and would otherwise disappear from the resumed run's + * completed set with no error at all. + */ +export class UnresolvableWaitpointId extends Error { + readonly waitpointId: string; + readonly reason: "no-source" | "two-sources"; + + constructor(waitpointId: string, reason: "no-source" | "two-sources") { + super( + reason === "no-source" + ? `Waitpoint ${waitpointId} has neither a cycle record nor a fetched row. Refusing to resume without it.` + : `Waitpoint ${waitpointId} resolved twice, from a cycle record and from a fetched row.` + ); + this.name = "UnresolvableWaitpointId"; + this.waitpointId = waitpointId; + this.reason = reason; + } +} + +export type CompletedWaitpointResolverDeps = { + /** Reads TaskRun.output. Returns undefined when the row is gone. */ + readRunOutput(taskRunId: string): Promise; +}; + +export type ResolveArgs = ResolveCompletedWaitpointsArgs & { + /** Ids the caller resolved from Postgres rows. Read by the coverage check only. */ + resolvedElsewhere?: string[]; +}; + +/** + * Rebuild `CompletedWaitpoint[]` from one wait cycle's records. + * + * Field-for-field equivalent to `enhanceExecutionSnapshotWithWaitpoints`, which is what the + * executor already consumes. It iterates the RECORDS, not the order: the order holds only + * batch-indexed ids, so iterating it would silently drop every index-less wait. + * + * Returns the store-resident half only. A mixed snapshot's legacy half arrives as Postgres + * rows and is expanded by the existing path, and the caller concatenates. Both halves read + * their index from the same order, so the positions agree with no coordination. + */ +export function createCompletedWaitpointResolver(deps: CompletedWaitpointResolverDeps) { + return async function resolveCompletedWaitpoints( + args: ResolveArgs + ): Promise { + const recordIds = new Set(args.records.map((record) => record.id)); + const resolvedElsewhere = new Set(args.resolvedElsewhere ?? []); + + for (const id of resolvedElsewhere) { + if (recordIds.has(id)) { + throw new UnresolvableWaitpointId(id, "two-sources"); + } + } + + for (const id of args.order) { + if (!recordIds.has(id) && !resolvedElsewhere.has(id)) { + throw new UnresolvableWaitpointId(id, "no-source"); + } + } + + const out: CompletedWaitpoint[] = []; + + for (const record of args.records) { + const indexes = positionsOf(record.id, args.order); + // Hydrated once per record, not once per position, so a run at several batch indexes + // costs one read rather than one per index. + const output = await hydrateOutput(record, deps); + + for (const index of indexes) { + out.push({ + id: record.id, + index, + friendlyId: record.friendlyId, + type: record.type, + completedAt: new Date(record.completedAt), + ...(record.idempotencyKey && { idempotencyKey: record.idempotencyKey }), + ...(record.completedByTaskRunId && { + completedByTaskRun: { + id: record.completedByTaskRunId, + friendlyId: RunId.toFriendlyId(record.completedByTaskRunId), + // The reading entry's batch, never the entry that minted the cycle. + ...(args.batchId && { + batch: { id: args.batchId, friendlyId: BatchId.toFriendlyId(args.batchId) }, + }), + }, + }), + ...(record.completedAfter && { completedAfter: new Date(record.completedAfter) }), + ...(record.completedByBatchId && { + completedByBatch: { + id: record.completedByBatchId, + friendlyId: BatchId.toFriendlyId(record.completedByBatchId), + }, + }), + ...(output !== undefined && { output }), + outputType: record.outputType, + outputIsError: record.outputIsError, + }); + } + } + + return out; + }; +} + +// An id with no position yields one entry with an undefined index, matching what the +// existing hydration does for a wait that carried no batch index. +function positionsOf(waitpointId: string, order: string[]): (number | undefined)[] { + const indexes: (number | undefined)[] = []; + + for (let i = 0; i < order.length; i++) { + if (order[i] === waitpointId) { + indexes.push(i); + } + } + + return indexes.length === 0 ? [undefined] : indexes; +} + +async function hydrateOutput( + record: CompletedWaitpointRecord, + deps: CompletedWaitpointResolverDeps +): Promise { + if (record.output === null) { + return undefined; + } + + if ("inline" in record.output) { + return record.output.inline; + } + + // A ref is handed back as the output verbatim: the executor already resolves an + // application/store output the same way it does for a Postgres-served snapshot. + if ("ref" in record.output) { + return record.output.ref; + } + + if (!record.completedByTaskRunId) { + return undefined; + } + + return deps.readRunOutput(record.completedByTaskRunId); +} diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/legacyPostgresCoordinator.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/legacyPostgresCoordinator.ts index d1e48fa4f8d..473f3de50a8 100644 --- a/internal-packages/run-engine/src/engine/waitpointCoordinator/legacyPostgresCoordinator.ts +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/legacyPostgresCoordinator.ts @@ -16,6 +16,8 @@ import type { CreateWaitpointResult, RegisterBlocksLocklessParams, RegisterBlocksParams, + CompletionEnvelopeSource, + ReadCompletionEnvelopesParams, RunBlockEdge, WaitpointCoordinator, } from "./types.js"; @@ -82,6 +84,73 @@ export class LegacyPostgresWaitpointCoordinator implements WaitpointCoordinator ); } + /** + * Source the envelope fields from the waitpoint rows. + * + * `runId` is unused here and that is correct: a routing store needs it to pick the owning + * database, a single store does not. It stays in the signature so both arms share one + * shape and the caller never branches. + * + * A row whose `outputType` is already a store reference carries `outputRef`, so the + * record build never re-offloads a value that object storage already holds. + */ + async readCompletionEnvelopes({ + waitpointIds, + }: ReadCompletionEnvelopesParams): Promise { + if (waitpointIds.length === 0) { + return []; + } + + const rows = await this.runStore.findManyWaitpoints( + { + where: { id: { in: boundedIn(waitpointIds) } }, + select: { + id: true, + friendlyId: true, + type: true, + completedAt: true, + output: true, + outputType: true, + outputIsError: true, + completedByTaskRunId: true, + completedByBatchId: true, + completedAfter: true, + idempotencyKey: true, + userProvidedIdempotencyKey: true, + inactiveIdempotencyKey: true, + }, + }, + this.prisma + ); + + return rows.map((row) => { + const isRef = row.outputType === "application/store"; + + return { + id: row.id, + friendlyId: row.friendlyId, + type: row.type, + // A completed waitpoint always has this set. The fallback keeps the shape total + // rather than emitting an invalid Date, and mirrors the same fallback the snapshot + // hydration already applies. + completedAt: row.completedAt ?? new Date(), + outputType: row.outputType, + outputIsError: row.outputIsError, + ...(row.output !== null && row.output !== undefined + ? isRef + ? { outputRef: row.output } + : { output: row.output } + : {}), + ...(row.completedByTaskRunId && { completedByTaskRunId: row.completedByTaskRunId }), + ...(row.completedByBatchId && { completedByBatchId: row.completedByBatchId }), + ...(row.completedAfter && { completedAfter: row.completedAfter }), + ...(row.userProvidedIdempotencyKey && !row.inactiveIdempotencyKey && row.idempotencyKey + ? { idempotencyKey: row.idempotencyKey } + : {}), + }; + }); + } + async registerBlocks({ client, ...edge diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.test.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.test.ts index f0e9c0c297d..b422bc43ac7 100644 --- a/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.test.ts +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.test.ts @@ -1835,3 +1835,141 @@ describe("genuine concurrency", () => { } ); }); + +describe("readCompletionEnvelopes", () => { + redisTest("returns the completion and the immutable half together", async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + await store.createIfAbsent({ + record: record("w_env", { + idempotencyKey: "user-key", + userProvidedIdempotencyKey: true, + }), + status: "PENDING", + }); + await store.complete({ waitpointId: "w_env", completion: completion() }); + + const envelopes = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_env"], + }); + + expect(envelopes).toEqual([ + { + id: "w_env", + friendlyId: "waitpoint_w_env", + type: "MANUAL", + completedAt: new Date(NOW), + outputType: "application/json", + outputIsError: false, + output: '{"ok":true}', + idempotencyKey: "user-key", + }, + ]); + } finally { + await store.quit(); + } + }); + + redisTest( + "carries an offloaded value as a ref, not as an inline value", + async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + await store.createIfAbsent({ record: record("w_ref"), status: "PENDING" }); + await store.complete({ + waitpointId: "w_ref", + completion: completion({ + outputType: "application/store", + output: { ref: "store-key-1" }, + }), + }); + + const [envelope] = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_ref"], + }); + + expect(envelope?.outputRef).toBe("store-key-1"); + expect(envelope?.output).toBeUndefined(); + } finally { + await store.quit(); + } + } + ); + + // The omission is the contract. A pending waitpoint has no envelope, and defaulting one + // here would hand the resolver a record it must not have. The caller's coverage check is + // what turns the gap into a loud failure. + redisTest("omits a waitpoint that is not completed", async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + await store.createIfAbsent({ record: record("w_pending"), status: "PENDING" }); + + const envelopes = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_pending"], + }); + + expect(envelopes).toEqual([]); + } finally { + await store.quit(); + } + }); + + redisTest("omits an id that has no record at all", async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + const envelopes = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_absent"], + }); + + expect(envelopes).toEqual([]); + } finally { + await store.quit(); + } + }); + + redisTest("suppresses an idempotency key the user did not provide", async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + await store.createIfAbsent({ + record: record("w_internal", { + idempotencyKey: "internal-key", + userProvidedIdempotencyKey: false, + }), + status: "PENDING", + }); + await store.complete({ waitpointId: "w_internal", completion: completion() }); + + const [envelope] = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_internal"], + }); + + expect(envelope?.idempotencyKey).toBeUndefined(); + } finally { + await store.quit(); + } + }); + + redisTest("reads many ids in one pass", async ({ redisOptions }) => { + const store = coordinator(redisOptions); + try { + for (const id of ["w_m1", "w_m2", "w_m3"]) { + await store.createIfAbsent({ record: record(id), status: "PENDING" }); + await store.complete({ waitpointId: id, completion: completion() }); + } + + const envelopes = await store.readCompletionEnvelopes({ + runId: "run_env", + waitpointIds: ["w_m1", "w_m2", "w_m3"], + }); + + expect(envelopes.map((e) => e.id).sort()).toEqual(["w_m1", "w_m2", "w_m3"]); + } finally { + await store.quit(); + } + }); +}); diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.ts index 723552c57ab..966a7e33a05 100644 --- a/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.ts +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.ts @@ -10,6 +10,7 @@ import { watcherField, } from "./keys.js"; import { registerWaitpointCommands } from "./scripts.js"; +import type { CompletionEnvelopeSource, ReadCompletionEnvelopesParams } from "./types.js"; /** The values written into a record's `status` field. Uppercase, and never a token. */ export type WaitpointStatus = "PENDING" | "COMPLETED"; @@ -505,6 +506,62 @@ export class WaitpointStoreCoordinator { return { pendingIds, deliveredIds, edges }; } + /** + * Source the envelope fields for a run's COMPLETED waitpoints. + * + * Reads `wp:{id}` and nothing else. Both halves live under that one key — `r` holds the + * immutable record, `c` holds the completion — so this needs no run-scoped key and no + * script. One HMGET per id, pipelined: each command touches a single key, so nothing can + * span two cluster slots and there is no #call guard to route through. + * + * The run's delivered hash carries the same envelope, but `wp:{id}` is the record of + * origin, and reading it keeps this independent of whether the run's edges were already + * reconciled. + * + * An id with no record, or a record with no completion, is OMITTED rather than defaulted. + * The omission is the contract: the caller's coverage check turns a gap into a loud + * failure, which a defaulted envelope would hide. + */ + async readCompletionEnvelopes({ + waitpointIds, + }: ReadCompletionEnvelopesParams): Promise { + if (waitpointIds.length === 0) { + return []; + } + + const pipeline = this.redis.pipeline(); + for (const id of waitpointIds) { + pipeline.hmget(waitpointKeys(id).record, "r", "c"); + } + const replies = await pipeline.exec(); + + const out: CompletionEnvelopeSource[] = []; + + for (let i = 0; i < waitpointIds.length; i++) { + const id = waitpointIds[i]!; + const reply = replies?.[i]; + + // A pipelined command reports its own error in slot 0. Surface it rather than reading + // slot 1, because an errored command's value is not a result. + const error = reply?.[0]; + if (error) { + throw error; + } + + const fields = reply?.[1] as (string | null)[] | undefined; + const record = parseJson(fields?.[0] ?? undefined); + const completion = parseJson(fields?.[1] ?? undefined); + + if (!record || !completion) { + continue; + } + + out.push(toEnvelopeSource(id, record, completion)); + } + + return out; + } + /** * Drain one cycle's edges, or clear the run entirely when no edge ids are given. * @@ -536,3 +593,37 @@ export class WaitpointStoreCoordinator { return { outcome: reply[0] as "cleared" | "drained" }; } } + +/** + * Map the store's two halves onto the arm-independent source shape. + * + * The idempotency key is suppressed unless the user provided it, matching the rule the + * snapshot hydration applies today. The store never sets an inactive flag, so + * `userProvidedIdempotencyKey` alone decides it here. + */ +function toEnvelopeSource( + id: string, + record: WaitpointRecordInput, + completion: WaitpointCompletion +): CompletionEnvelopeSource { + const output = completion.output; + const inline = output && "inline" in output ? output.inline : undefined; + const ref = output && "ref" in output ? output.ref : undefined; + + return { + id, + friendlyId: record.friendlyId, + type: record.type, + completedAt: new Date(completion.completedAt), + outputType: completion.outputType, + outputIsError: completion.outputIsError, + ...(inline !== undefined && { output: inline }), + ...(ref !== undefined && { outputRef: ref }), + ...(record.completedByTaskRunId && { completedByTaskRunId: record.completedByTaskRunId }), + ...(record.completedByBatchId && { completedByBatchId: record.completedByBatchId }), + ...(record.completedAfter && { completedAfter: new Date(record.completedAfter) }), + ...(record.userProvidedIdempotencyKey && record.idempotencyKey + ? { idempotencyKey: record.idempotencyKey } + : {}), + }; +} diff --git a/internal-packages/run-engine/src/engine/waitpointCoordinator/types.ts b/internal-packages/run-engine/src/engine/waitpointCoordinator/types.ts index 8a50abb7d1c..8611a361b42 100644 --- a/internal-packages/run-engine/src/engine/waitpointCoordinator/types.ts +++ b/internal-packages/run-engine/src/engine/waitpointCoordinator/types.ts @@ -16,6 +16,9 @@ import type { PrismaClientOrTransaction, Waitpoint } from "@trigger.dev/database export type WaitpointCoordinator = { clearRunBlockState(params: ClearRunBlockStateParams): Promise<{ count: number }>; readRunBlockState(runId: string): Promise; + readCompletionEnvelopes( + params: ReadCompletionEnvelopesParams + ): Promise; registerBlocks(params: RegisterBlocksParams): Promise<{ pendingCount: number }>; registerBlocksLockless(params: RegisterBlocksLocklessParams): Promise; complete(params: CompleteParams): Promise; @@ -31,6 +34,38 @@ export type WaitpointCoordinator = { }): Promise; }; +export type ReadCompletionEnvelopesParams = { + runId: string; + /** The DISTINCT completed waitpoint ids to source. Result order is not meaningful. */ + waitpointIds: string[]; +}; + +/** + * One completed waitpoint's fields, sourced from whichever arm owns it. + * + * Deliberately NOT the frozen record type. This is the raw material; the record build + * decides which output variant a record carries. Both arms return this same shape, so the + * record build never branches on residency, which is what makes a mixed wait work. + * + * `output` is the literal stored value. `outputRef` is set instead when the value was + * already offloaded to object storage. At most one of the two is set. + */ +export type CompletionEnvelopeSource = { + id: string; + friendlyId: string; + type: "RUN" | "BATCH" | "DATETIME" | "MANUAL"; + completedAt: Date; + outputType: string; + outputIsError: boolean; + output?: string; + outputRef?: string; + completedByTaskRunId?: string; + completedByBatchId?: string; + completedAfter?: Date; + /** Already resolved by the arm: userProvidedIdempotencyKey && !inactiveIdempotencyKey. */ + idempotencyKey?: string; +}; + export type ClearRunBlockStateParams = { runId: string; /** Edge ids to delete. Omit to clear every edge for the run. */ diff --git a/internal-packages/run-store/src/redisSnapshotStore.recordsOnCarryRefusal.test.ts b/internal-packages/run-store/src/redisSnapshotStore.recordsOnCarryRefusal.test.ts new file mode 100644 index 00000000000..422b90770ce --- /dev/null +++ b/internal-packages/run-store/src/redisSnapshotStore.recordsOnCarryRefusal.test.ts @@ -0,0 +1,132 @@ +// A refused carry-forward mints a replacement cycle inside the same call. That replacement must +// carry the records, not just the ids: the resolver's coverage check requires every distinct id to +// resolve through exactly one half, so a cycle holding ids with no records makes a legitimate +// resume fail loud. +import { createRedisClient } from "@internal/redis"; +import { redisTest } from "@internal/testcontainers"; +import { describe, expect } from "vitest"; +import { + RedisSnapshotStore, + type CompletedWaitpointRecord, + type SnapshotEntryInput, +} from "./redisSnapshotStore.js"; + +function entry(over: Partial = {}): SnapshotEntryInput { + return { + id: "snap_1", + engine: "V2", + executionStatus: "RUN_CREATED", + description: "created", + runId: "run_1", + runStatus: "PENDING", + createdAt: "2026-08-21T00:00:00.000Z", + environmentId: "env_1", + environmentType: "PRODUCTION", + projectId: "proj_1", + organizationId: "org_1", + ...over, + }; +} + +function record(id: string, output: string): CompletedWaitpointRecord { + return { + id, + friendlyId: `waitpoint_${id}`, + type: "MANUAL", + completedAt: "2026-08-25T00:00:00.000Z", + outputType: "application/json", + outputIsError: false, + output: { inline: output }, + }; +} + +async function recordsAt( + raw: ReturnType, + cycleSeq: number +): Promise { + const stored = await raw.hget(`snap:{run_1}:wp:${cycleSeq}`, "records"); + return stored ? (JSON.parse(stored) as CompletedWaitpointRecord[]) : undefined; +} + +describe("a refused carry-forward", () => { + redisTest("mints a replacement that carries the records", async ({ redisOptions }) => { + const store = new RedisSnapshotStore({ redisOptions, completedTtlMs: 60_000 }); + const raw = createRedisClient(redisOptions, { onError: () => {} }); + try { + await store.append({ + entry: entry({ id: "snap_1" }), + kind: "birth", + isTerminal: false, + cycle: { + kind: "new", + completedWaitpoints: [{ id: "w_a", index: 0 }], + records: [record("w_a", "first")], + }, + }); + + // Lose everything except the cycle key, as under maxmemory eviction. The carried pointer is + // now untrustworthy, so the store refuses it. + await raw.del("snap:{run_1}:e", "snap:{run_1}:idx", "snap:{run_1}:cur", "snap:{run_1}:seq"); + + const carried = await store.append({ + entry: entry({ id: "snap_2" }), + kind: "birth", + isTerminal: false, + cycle: { + kind: "carryForward", + cycleSeq: 1, + completedWaitpoints: [{ id: "w_b", index: 0 }], + records: [record("w_b", "second")], + }, + }); + + expect(carried).toMatchObject({ outcome: "written", cycleMismatch: true }); + + // The replacement holds the CARRIED records, not the dead incarnation's. + const read = await store.getLatest("run_1"); + const mintedSeq = read?.cycle?.cycleSeq; + expect(mintedSeq).toBeDefined(); + + const records = await recordsAt(raw, mintedSeq!); + expect(records).toHaveLength(1); + expect(records?.[0]?.id).toBe("w_b"); + expect(records?.[0]?.output).toEqual({ inline: "second" }); + } finally { + await Promise.all([store.quit(), raw.quit().catch(() => {})]); + } + }); + + // Without refs there is nothing to mint from, so the entry is written with no pointer. That is + // the older behaviour and it stays: no pointer is safe, a pointer with no records is not. + redisTest("writes no pointer when the caller carried no refs", async ({ redisOptions }) => { + const store = new RedisSnapshotStore({ redisOptions, completedTtlMs: 60_000 }); + const raw = createRedisClient(redisOptions, { onError: () => {} }); + try { + await store.append({ + entry: entry({ id: "snap_1" }), + kind: "birth", + isTerminal: false, + cycle: { + kind: "new", + completedWaitpoints: [{ id: "w_a", index: 0 }], + records: [record("w_a", "first")], + }, + }); + + await raw.del("snap:{run_1}:e", "snap:{run_1}:idx", "snap:{run_1}:cur", "snap:{run_1}:seq"); + + const carried = await store.append({ + entry: entry({ id: "snap_2" }), + kind: "birth", + isTerminal: false, + cycle: { kind: "carryForward", cycleSeq: 1 }, + }); + + expect(carried).toMatchObject({ outcome: "written", cycleMismatch: true }); + const read = await store.getLatest("run_1"); + expect(read?.cycle).toBeUndefined(); + } finally { + await Promise.all([store.quit(), raw.quit().catch(() => {})]); + } + }); +}); diff --git a/internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts b/internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts index c6bc145a91d..c3b8e393075 100644 --- a/internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts +++ b/internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts @@ -15,6 +15,7 @@ import { Logger } from "@trigger.dev/core/logger"; import { generateInternalId } from "@trigger.dev/core/v3/isomorphic"; import { DelegatingRunStore } from "./delegatingRunStore.js"; import type { + CompletedWaitpointRecord, CompletedWaitpointRef, RedisSnapshotStore, SnapshotEntryInput, @@ -114,6 +115,7 @@ export type StagedAppend = { */ expectedCur?: string; completedWaitpoints?: CompletedWaitpointRef[]; + completedWaitpointRecords?: CompletedWaitpointRecord[]; }; export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { @@ -177,7 +179,8 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { "runInTransaction", item.entry, item.expectedCur, - item.completedWaitpoints + item.completedWaitpoints, + item.completedWaitpointRecords ); } @@ -420,7 +423,8 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { "createExecutionSnapshot", entryFromCreateExecutionSnapshot(ctx, input), input.previousSnapshotId, - input.completedWaitpoints + input.completedWaitpoints, + input.completedWaitpointRecords ); return created; } @@ -503,7 +507,8 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { site: string, entry: SnapshotEntryInput, expectedCur?: string, - completedWaitpoints?: CompletedWaitpointRef[] + completedWaitpoints?: CompletedWaitpointRef[], + completedWaitpointRecords?: CompletedWaitpointRecord[] ): Promise { if (this.staging) { // Inside a transaction the append cannot run until the Postgres side commits, or a rollback @@ -512,6 +517,7 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { entry, ...(expectedCur !== undefined && { expectedCur }), ...(completedWaitpoints && { completedWaitpoints }), + ...(completedWaitpointRecords && { completedWaitpointRecords }), }); return; } @@ -523,7 +529,11 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { snapshotId: entry.id, }); - const cycle = await this.#resolveCycle(entry.runId, completedWaitpoints); + const cycle = await this.#resolveCycle( + entry.runId, + completedWaitpoints, + completedWaitpointRecords + ); const result = await this.redis.append({ entry, @@ -572,15 +582,28 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { * The extra read only happens for an append that actually carries waitpoints, which is the resume * path rather than the hot path. * - * `records` is deliberately left unset. The record envelope belongs to the waitpoint lane and - * ships empty in this build, so dual-write never re-versions the entry when it arrives. + * `records` rides every arm that can mint. A carryForward normally writes no key, but the + * store may refuse the pointer and mint a replacement inside the same call, and that + * replacement needs the records or the resolver's coverage check rejects the cycle later. + * A legacy-only wait supplies none at all, which is what keeps a Postgres-resident resume + * byte-identical to before. */ async #resolveCycle( runId: string, - completedWaitpoints?: CompletedWaitpointRef[] + completedWaitpoints?: CompletedWaitpointRef[], + records?: CompletedWaitpointRecord[] ): Promise< - | { kind: "new"; completedWaitpoints: CompletedWaitpointRef[] } - | { kind: "carryForward"; cycleSeq: number; completedWaitpoints: CompletedWaitpointRef[] } + | { + kind: "new"; + completedWaitpoints: CompletedWaitpointRef[]; + records?: CompletedWaitpointRecord[]; + } + | { + kind: "carryForward"; + cycleSeq: number; + completedWaitpoints: CompletedWaitpointRef[]; + records?: CompletedWaitpointRecord[]; + } | undefined > { if (!completedWaitpoints || completedWaitpoints.length === 0) { @@ -607,6 +630,7 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { kind: "carryForward", cycleSeq: head.cycle.cycleSeq, completedWaitpoints, + ...(records && { records }), }; } } catch (error) { @@ -616,7 +640,7 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore { this.logger.warn("snapshot cycle probe failed, minting a new cycle", { runId, error }); } - return { kind: "new", completedWaitpoints }; + return { kind: "new", completedWaitpoints, ...(records && { records }) }; } /** diff --git a/internal-packages/run-store/src/taskRunExecutionSnapshotStore.waitpointRecords.test.ts b/internal-packages/run-store/src/taskRunExecutionSnapshotStore.waitpointRecords.test.ts new file mode 100644 index 00000000000..b039c718971 --- /dev/null +++ b/internal-packages/run-store/src/taskRunExecutionSnapshotStore.waitpointRecords.test.ts @@ -0,0 +1,219 @@ +// The record set's journey from a caller's input to the wait cycle's key. +// +// The raw store already pins that a records array round-trips through the cycle hash. What is +// untested without this file is the decorator leg: that `completedWaitpointRecords` on a +// snapshot input reaches `cycle.records`, that a mint carries it, and that a copy-forward and +// a legacy-only wait carry none — which is what keeps a Postgres-resident resume unchanged. +import { createRedisClient } from "@internal/redis"; +import { containerTest } from "@internal/testcontainers"; +import { generateInternalId } from "@trigger.dev/core/v3/isomorphic"; +import { describe, expect } from "vitest"; +import { PostgresRunStore } from "./PostgresRunStore.js"; +import { RedisSnapshotStore, type CompletedWaitpointRecord } from "./redisSnapshotStore.js"; +import { entryFromCreateRun } from "./snapshotEntry.js"; +import { TaskRunExecutionSnapshotStore } from "./taskRunExecutionSnapshotStore.js"; +import { + buildCreateRunData, + seedSnapshotEnvironment, + seedSnapshotWaitpoints, + type SnapshotFixtureEnv, +} from "./testFixtures/snapshotIdFixture.js"; +import type { RunStore } from "./types.js"; + +const COMPLETED_TTL_MS = 72 * 60 * 60 * 1000; + +function build(prisma: never, redisOptions: never) { + const redis = new RedisSnapshotStore({ redisOptions, completedTtlMs: COMPLETED_TTL_MS }); + const decorated = new TaskRunExecutionSnapshotStore( + new PostgresRunStore({ prisma, readOnlyPrisma: prisma }) as unknown as RunStore, + { + store: redis, + mode: "redis-read", + readPercent: 100, + metrics: { + recordWrite: () => {}, + recordAppendFailed: () => {}, + recordRead: () => {}, + }, + } + ); + return { decorated, redis }; +} + +async function seedRun( + decorated: TaskRunExecutionSnapshotStore, + redis: RedisSnapshotStore, + env: SnapshotFixtureEnv +): Promise { + const runId = generateInternalId(); + const snapshot = { + id: generateInternalId(), + engine: "V2" as const, + executionStatus: "RUN_CREATED" as const, + description: "Run was created", + runStatus: "PENDING" as const, + environmentId: env.id, + environmentType: env.type, + projectId: env.projectId, + organizationId: env.organizationId, + }; + await redis.append({ + entry: entryFromCreateRun({ id: snapshot.id, runId, createdAt: new Date() }, snapshot), + kind: "birth", + isTerminal: false, + }); + await decorated.createRun({ data: buildCreateRunData(runId, env), snapshot }); + return runId; +} + +function resumeInput( + runId: string, + env: SnapshotFixtureEnv, + completedWaitpoints: { id: string; index?: number }[], + completedWaitpointRecords?: CompletedWaitpointRecord[] +) { + return { + id: generateInternalId(), + run: { id: runId, status: "EXECUTING" as const, attemptNumber: 1 }, + snapshot: { executionStatus: "EXECUTING" as const, description: "Run resumed" }, + completedWaitpoints, + ...(completedWaitpointRecords && { completedWaitpointRecords }), + environmentId: env.id, + environmentType: env.type, + projectId: env.projectId, + organizationId: env.organizationId, + }; +} + +function record(id: string, overrides: Partial = {}) { + return { + id, + friendlyId: `waitpoint_${id}`, + type: "MANUAL" as const, + completedAt: "2026-08-25T00:00:00.000Z", + outputType: "application/json", + outputIsError: false, + output: { inline: '{"ok":true}' }, + ...overrides, + } satisfies CompletedWaitpointRecord; +} + +async function readRecords( + probe: ReturnType, + runId: string +): Promise { + const [cycleKey] = await probe.keys(`snap:{${runId}}:wp:*`); + if (!cycleKey) return undefined; + const raw = await probe.hget(cycleKey, "records"); + return raw ? (JSON.parse(raw) as CompletedWaitpointRecord[]) : undefined; +} + +describe("the completed-waitpoint record set", () => { + containerTest( + "a mint writes the records the caller supplied", + async ({ prisma, redisOptions }) => { + const { decorated, redis } = build(prisma as never, redisOptions as never); + const probe = createRedisClient(redisOptions, { onError: () => {} }); + try { + const env = await seedSnapshotEnvironment(prisma); + const runId = await seedRun(decorated, redis, env); + const [wpA, wpB] = await seedSnapshotWaitpoints(prisma, env, 2); + + await decorated.createExecutionSnapshot( + resumeInput( + runId, + env, + [ + { id: wpA!, index: 0 }, + { id: wpB!, index: 1 }, + ], + [record(wpA!), record(wpB!)] + ) + ); + + const records = await readRecords(probe, runId); + + expect(records).toHaveLength(2); + expect(records?.map((r) => r.id).sort()).toEqual([wpA, wpB].sort()); + expect(records?.[0]?.output).toEqual({ inline: '{"ok":true}' }); + } finally { + await Promise.all([redis.quit(), probe.quit().catch(() => {})]); + } + } + ); + + // The inertness guarantee. A wait with no store-resident half supplies no records, and the + // cycle key must then hold none — a Postgres-resident resume is unchanged. + containerTest("a mint with no records supplied writes none", async ({ prisma, redisOptions }) => { + const { decorated, redis } = build(prisma as never, redisOptions as never); + const probe = createRedisClient(redisOptions, { onError: () => {} }); + try { + const env = await seedSnapshotEnvironment(prisma); + const runId = await seedRun(decorated, redis, env); + const [wpA] = await seedSnapshotWaitpoints(prisma, env, 1); + + await decorated.createExecutionSnapshot(resumeInput(runId, env, [{ id: wpA!, index: 0 }])); + + expect(await readRecords(probe, runId)).toBeUndefined(); + } finally { + await Promise.all([redis.quit(), probe.quit().catch(() => {})]); + } + }); + + // One record set per wait cycle, not one per entry in the resume chain. That is the write + // amplification the pointer model exists to remove. + containerTest("a copy-forward writes no second record set", async ({ prisma, redisOptions }) => { + const { decorated, redis } = build(prisma as never, redisOptions as never); + const probe = createRedisClient(redisOptions, { onError: () => {} }); + try { + const env = await seedSnapshotEnvironment(prisma); + const runId = await seedRun(decorated, redis, env); + const [wpA] = await seedSnapshotWaitpoints(prisma, env, 1); + + const waitpoints = [{ id: wpA!, index: 0 }]; + await decorated.createExecutionSnapshot(resumeInput(runId, env, waitpoints, [record(wpA!)])); + await decorated.createExecutionSnapshot(resumeInput(runId, env, waitpoints, [record(wpA!)])); + + const cycleKeys = await probe.keys(`snap:{${runId}}:wp:*`); + + expect(cycleKeys).toHaveLength(1); + expect(await readRecords(probe, runId)).toHaveLength(1); + } finally { + await Promise.all([redis.quit(), probe.quit().catch(() => {})]); + } + }); + + containerTest( + "a record set survives beside a repeat-preserving order", + async ({ prisma, redisOptions }) => { + const { decorated, redis } = build(prisma as never, redisOptions as never); + const probe = createRedisClient(redisOptions, { onError: () => {} }); + try { + const env = await seedSnapshotEnvironment(prisma); + const runId = await seedRun(decorated, redis, env); + const [wpA] = await seedSnapshotWaitpoints(prisma, env, 1); + + const created = await decorated.createExecutionSnapshot( + resumeInput( + runId, + env, + [ + { id: wpA!, index: 0 }, + { id: wpA!, index: 1 }, + ], + [record(wpA!)] + ) + ); + + const ids = await redis.getSnapshotWaitpointIds(runId, created.id); + + // One record, two positions. The record set carries membership, the order carries + // multiplicity. + expect(await readRecords(probe, runId)).toHaveLength(1); + expect(ids.order).toEqual([wpA, wpA]); + } finally { + await Promise.all([redis.quit(), probe.quit().catch(() => {})]); + } + } + ); +}); diff --git a/internal-packages/run-store/src/types.ts b/internal-packages/run-store/src/types.ts index 9ea39473e5b..7e7a548db35 100644 --- a/internal-packages/run-store/src/types.ts +++ b/internal-packages/run-store/src/types.ts @@ -13,6 +13,7 @@ import type { } from "@trigger.dev/database"; import type { TaskRunError } from "@trigger.dev/core/v3/schemas"; import type { Residency } from "@trigger.dev/core/v3/isomorphic"; +import type { CompletedWaitpointRecord } from "./redisSnapshotStore.js"; /** * Client accepted by the read methods. Reads route through the replica by @@ -352,6 +353,10 @@ export type CreateExecutionSnapshotInput = { workerId?: string; runnerId?: string; completedWaitpoints?: { id: string; index?: number }[]; + /** One envelope per DISTINCT completed waitpoint id. Owned by the waitpoint lane; the + * snapshot store only carries it into the wait cycle's key. Absent for a legacy-only + * wait, which is what keeps a Postgres-resident resume unchanged. */ + completedWaitpointRecords?: CompletedWaitpointRecord[]; error?: string; };