Skip to content

Commit 178472e

Browse files
committed
fix(run-store,run-engine): attach the frozen record type to Redis, close the output union
- RedisSnapshotStore.append's cycle.records now takes a typed CompletedWaitpointRecord[] and serializes it, instead of accepting an opaque pre-serialized string with no compile-time link to the frozen type. Adds a round-trip test reading the cycle hash's records field back with a raw client. - The test file's referenceResolver output discrimination is now exhaustive: an explicit deriveFromRun branch plus a `never` fallback, so a future output variant fails to compile here instead of silently resolving through a TaskRun re-read. - tsconfig.freeze-test.json documents that it resolves @internal/run-store from dist, so the gate must run through turbo rather than directly inside run-engine against a stale build. - assertParity now asserts pointer.count === order.length on every parity case, binding the frozen count-is-order.length rule instead of leaving it decorative.
1 parent d638b41 commit 178472e

4 files changed

Lines changed: 72 additions & 13 deletions

File tree

internal-packages/run-engine/src/engine/systems/completedWaitpointFreeze.test.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,13 @@ async function referenceResolver(
9494
output = record.output.inline;
9595
} else if ("ref" in record.output) {
9696
output = record.output.ref;
97-
} else {
97+
} else if ("deriveFromRun" in record.output) {
9898
output = record.completedByTaskRunId
9999
? await lookupRunOutput(record.completedByTaskRunId)
100100
: undefined;
101+
} else {
102+
const _never: never = record.output;
103+
throw new Error(`unknown record output variant: ${JSON.stringify(_never)}`);
101104
}
102105

103106
for (const index of indexes) {
@@ -156,16 +159,17 @@ async function assertParity(
156159
runOutputs: Record<string, string> = {}
157160
) {
158161
const enhanced = enhanceExecutionSnapshotWithWaitpoints(makeSnapshot(batchId), waitpoints, order);
159-
const resolved = await referenceResolver(
160-
{
161-
runId: "run_1",
162-
batchId: batchId ?? undefined,
163-
pointer: { cycleSeq: 1, count: order.length },
164-
order,
165-
records: waitpoints.map(toRecord),
166-
},
167-
async (id) => runOutputs[id]
168-
);
162+
const args: ResolveCompletedWaitpointsArgs = {
163+
runId: "run_1",
164+
batchId: batchId ?? undefined,
165+
pointer: { cycleSeq: 1, count: order.length },
166+
order,
167+
records: waitpoints.map(toRecord),
168+
};
169+
// The frozen rule: count is order.length, NOT the record count. Binding it here means every
170+
// parity case enforces it, not only the dedicated "the frozen pointer shape" cases.
171+
expect(args.pointer.count).toBe(order.length);
172+
const resolved = await referenceResolver(args, async (id) => runOutputs[id]);
169173
expect(resolved).toEqual(enhanced.completedWaitpoints);
170174
return { enhanced, resolved };
171175
}

internal-packages/run-engine/tsconfig.freeze-test.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
// Typechecks completedWaitpointFreeze.test.ts, which tsconfig.build.json otherwise excludes
2+
// (src/**/*.test.ts) and vitest's esbuild transform never checks. This config has no
3+
// "@triggerdotdev/source" customCondition, so it resolves @internal/run-store from its built
4+
// `dist`, not from source -- same as tsconfig.build.json. That means this gate only sees a
5+
// source change in run-store once run-store has been rebuilt, so it MUST be run through turbo
6+
// (`pnpm run typecheck --filter @internal/run-engine`), whose `typecheck` task declares
7+
// `dependsOn: ["^build"]`. Running `tsc -p tsconfig.freeze-test.json` (or `pnpm run typecheck`)
8+
// directly inside this package, against a stale dist/, passes green while the frozen type has
9+
// already drifted in source. Do not "fix" this with customConditions: that pulls
10+
// @trigger.dev/core's source in too, which fails to typecheck here on `lib: ES2020`.
111
{
212
"extends": "./tsconfig.build.json",
313
"include": ["src/engine/systems/completedWaitpointFreeze.test.ts"],

internal-packages/run-store/src/redisSnapshotStore.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
RedisSnapshotStore,
1212
type SnapshotEntryInput,
1313
type CompletedWaitpointsPointer,
14+
type CompletedWaitpointRecord,
1415
} from "./redisSnapshotStore.js";
1516

1617
describe("snapshotKeys", () => {
@@ -237,6 +238,46 @@ describe("append", () => {
237238
}
238239
);
239240

241+
redisTest(
242+
"round-trips a typed records array through the cycle hash's records field",
243+
async ({ redisOptions }) => {
244+
// The only place CompletedWaitpointRecord[] physically enters Redis. If the writer ever
245+
// serializes a different envelope, this is where that would show up as a broken round trip.
246+
const store = new RedisSnapshotStore({ redisOptions, completedTtlMs: 1000 });
247+
const raw = createRedisClient(redisOptions);
248+
const records: CompletedWaitpointRecord[] = [
249+
{
250+
id: "w_a",
251+
friendlyId: "waitpoint_a",
252+
type: "RUN",
253+
completedAt: "2026-01-01T00:00:00.000Z",
254+
outputType: "application/json",
255+
outputIsError: false,
256+
output: { deriveFromRun: true },
257+
completedByTaskRunId: "run_child",
258+
},
259+
];
260+
try {
261+
await store.append({
262+
entry: entry({ id: "snap_1" }),
263+
kind: "birth",
264+
isTerminal: false,
265+
cycle: {
266+
kind: "new",
267+
completedWaitpoints: [{ id: "w_a", index: 0 }],
268+
records,
269+
},
270+
});
271+
272+
const storedRaw = await raw.hget("snap:{run_1}:wp:1", "records");
273+
expect(JSON.parse(storedRaw!)).toEqual(records);
274+
} finally {
275+
raw.disconnect();
276+
await store.quit();
277+
}
278+
}
279+
);
280+
240281
redisTest(
241282
"reports a duplicate id without overwriting the original entry",
242283
async ({ redisOptions }) => {

internal-packages/run-store/src/redisSnapshotStore.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,11 @@ export class RedisSnapshotStore {
246246
isTerminal: boolean;
247247
expectedCur?: string;
248248
cycle?:
249-
| { kind: "new"; completedWaitpoints: CompletedWaitpointRef[]; records?: string }
249+
| {
250+
kind: "new";
251+
completedWaitpoints: CompletedWaitpointRef[];
252+
records?: CompletedWaitpointRecord[];
253+
}
250254
| { kind: "carryForward"; cycleSeq: number };
251255
}): Promise<AppendResult> {
252256
if (args.entry.completedWaitpoints !== undefined) {
@@ -270,7 +274,7 @@ export class RedisSnapshotStore {
270274
const order = deriveOrder(args.cycle.completedWaitpoints);
271275
cycleMode = "new";
272276
orderJson = JSON.stringify(order);
273-
records = args.cycle.records ?? "";
277+
records = args.cycle.records ? JSON.stringify(args.cycle.records) : "";
274278
orderCount = String(order.length);
275279
} else if (args.cycle?.kind === "carryForward") {
276280
cycleMode = "carry";

0 commit comments

Comments
 (0)