Skip to content

Commit 45eaaa7

Browse files
authored
feat(run-store,testcontainers): execution-snapshot read comparator and shared test utilities (#4772)
## Summary Adds the read comparator for the in-progress migration of the run execution-snapshot log from Postgres to Redis. The comparator samples a single read against both stores, normalizes the two results to one shape, and reports any per-field difference with a tagged metric. It never serves a read itself: the diff layer imports only types, so it cannot hold a store client, and a test enforces that by failing if any value import appears. Also adds a combined Postgres-and-Redis test fixture and two shared test utilities (a cluster-slot assertion and a generic fault-injection harness) that the parallel Redis-store work reuses. Everything here is inert. Nothing constructs the comparator, so merging changes no runtime behavior. It becomes active only when a later change turns on compare mode. ## Notes The divergence classes separate real differences (scalar, ordering, waitpoint id set, validity, missing on one side) from two expected classes that must not be driven to zero: a rotated idempotency key, and a Redis-only surplus at a since-cursor tie. The since comparison is direction sensitive: a Postgres-only entry at the cursor is always a lost write, never an expected tie.
1 parent 036cf8d commit 45eaaa7

11 files changed

Lines changed: 850 additions & 35 deletions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@ export * from "./PostgresRunStore.js";
33
export * from "./runOpsStore.js";
44
export * from "./readReplicaClient.js";
55
export * from "./redisSnapshotStore.js";
6+
export * from "./snapshotComparator.js";

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

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Unit suite for the raw Redis execution-snapshot store. Redis-only: the store holds no Prisma
22
// reference, so no Postgres container is needed.
33
import { expect, describe, vi } from "vitest";
4-
import { redisTest } from "@internal/testcontainers";
4+
import { redisTest, slotOf } from "@internal/testcontainers";
55
import { createRedisClient } from "@internal/redis";
66
import { Logger } from "@trigger.dev/core/logger";
77
import {
@@ -1283,45 +1283,23 @@ describe("expectedCur compare-and-set", () => {
12831283
);
12841284
});
12851285

1286-
// CRC16/XMODEM over a key's hash tag, per Redis's cluster hashing rule. CLUSTER KEYSLOT is
1287-
// unavailable on this standalone container ("cluster support disabled"), so the slot is computed
1288-
// here instead. Verified against the `cluster-key-slot` package's output for our key shapes.
1289-
function crc16(str: string): number {
1290-
let crc = 0;
1291-
for (let i = 0; i < str.length; i++) {
1292-
crc ^= str.charCodeAt(i) << 8;
1293-
for (let j = 0; j < 8; j++) {
1294-
crc = crc & 0x8000 ? (crc << 1) ^ 0x1021 : crc << 1;
1295-
crc &= 0xffff;
1296-
}
1297-
}
1298-
return crc;
1299-
}
1300-
1301-
function hashSlot(key: string): number {
1302-
const start = key.indexOf("{");
1303-
const end = start === -1 ? -1 : key.indexOf("}", start + 1);
1304-
const tag = start !== -1 && end !== -1 && end > start + 1 ? key.slice(start + 1, end) : key;
1305-
return crc16(tag) % 16384;
1306-
}
1307-
13081286
describe("hash tag and keyPrefix", () => {
13091287
it("every key for one run lands in one cluster slot", () => {
13101288
// Keys come from snapshotKeys() plus the wp:<n> suffix the Lua prelude derives the same way,
13111289
// with a keyPrefix prepended by hand as ioredis would. A dropped hash tag would split the slots.
1312-
// Pin the helper itself before trusting it: the published XMODEM check value, and two known
1290+
// Pin the shared helper before trusting it: the published XMODEM check value, and two known
13131291
// slots (one matching cluster-key-slot, one a different run's tag as a negative control --
13141292
// otherwise a constant-valued crc16 would satisfy slots.size === 1 for the wrong reason).
1315-
expect(crc16("123456789")).toBe(0x31c3);
1316-
expect(hashSlot("engine:snap:{run_1}:e")).toBe(8108);
1317-
expect(hashSlot("engine:snap:{run_2}:e")).toBe(12239);
1293+
expect(slotOf("123456789")).toBe(0x31c3);
1294+
expect(slotOf("engine:snap:{run_1}:e")).toBe(8108);
1295+
expect(slotOf("engine:snap:{run_2}:e")).toBe(12239);
13181296

13191297
const k = snapshotKeys("run_1");
13201298
const base = k.e.slice(0, -2);
13211299
const keys = [k.e, k.idx, k.cur, k.seq, `${base}:wp:1`, `${base}:wp:2`].map(
13221300
(key) => `engine:${key}`
13231301
);
1324-
const slots = new Set(keys.map(hashSlot));
1302+
const slots = new Set(keys.map(slotOf));
13251303
expect(slots.size).toBe(1);
13261304
});
13271305

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Proves the Frozen rule: the comparator's VALUE-import set is empty. Every import it has is
2+
// `import type`, erased at runtime, so the compiled module pulls in no Redis or Prisma client and
3+
// cannot read. Goes red the instant any value import is added — a client, the barrel, or a dynamic
4+
// import(). The detector is pinned against redisSnapshotStore.ts (which value-imports a client) so
5+
// this cannot pass as a tautology.
6+
import { expect, it, describe } from "vitest";
7+
import { readFileSync } from "node:fs";
8+
import { fileURLToPath } from "node:url";
9+
import { dirname, resolve } from "node:path";
10+
11+
const here = dirname(fileURLToPath(import.meta.url));
12+
13+
// Returns the module specifiers a file imports FOR VALUE (i.e. that survive to runtime). `import type`
14+
// declarations and named blocks whose specifiers are all inline `type` are erased and excluded.
15+
function valueImports(sourcePath: string): string[] {
16+
const raw = readFileSync(sourcePath, "utf8");
17+
const out: string[] = [];
18+
19+
// Statements are scanned on RAW source, anchored to line start (`^\s*import`), so a `//` comment
20+
// line never matches and no stripping can hide a real import. Only the mid-line dynamic `import(`
21+
// check runs on comment-stripped source. The pin test below guarantees the scan catches a real import.
22+
const stripped = raw.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
23+
if (/(^|[^.\w])import\s*\(/.test(stripped)) out.push("<dynamic import()>");
24+
25+
const importRe = /^\s*import\b([\s\S]*?)\bfrom\s*["']([^"']+)["']/gm;
26+
for (let m = importRe.exec(raw); m !== null; m = importRe.exec(raw)) {
27+
const clause = m[1];
28+
const spec = m[2];
29+
if (/^\s*type\b/.test(clause)) continue; // `import type ... from`
30+
const named = clause.match(/\{([\s\S]*?)\}/);
31+
// Strip inline `type Foo` specifiers, including an `as Bar` alias, before checking whether any
32+
// value specifier remains.
33+
const inlineType = /\btype\s+[A-Za-z_$][\w$]*(?:\s+as\s+[A-Za-z_$][\w$]*)?/g;
34+
if (named && !/(^|,)\s*[A-Za-z_$]/.test(named[1].replace(inlineType, ""))) {
35+
continue; // every named specifier is an inline `type` — nothing left for value
36+
}
37+
out.push(spec);
38+
}
39+
40+
// Bare side-effect imports (`import "x"`) run the module.
41+
const bareRe = /^\s*import\s*["']([^"']+)["']/gm;
42+
for (let m = bareRe.exec(raw); m !== null; m = bareRe.exec(raw)) out.push(m[1]);
43+
44+
return out;
45+
}
46+
47+
describe("comparator read-isolation", () => {
48+
it("the detector flags a real value import (pin against the store)", () => {
49+
// redisSnapshotStore.ts value-imports @internal/redis, so a working detector MUST see it.
50+
const storeImports = valueImports(resolve(here, "redisSnapshotStore.ts"));
51+
expect(storeImports).toContain("@internal/redis");
52+
});
53+
54+
it("the comparator has no value imports — it is import-type-only and cannot read", () => {
55+
expect(valueImports(resolve(here, "snapshotComparator.ts"))).toEqual([]);
56+
});
57+
});
Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
import { expect, it, describe } from "vitest";
2+
import {
3+
diffLatest,
4+
diffSince,
5+
normalizeFromRedis,
6+
normalizeFromPg,
7+
SnapshotComparator,
8+
type DivergenceClass,
9+
type NormalizedSnapshot,
10+
} from "./snapshotComparator.js";
11+
import type { SnapshotRead } from "./redisSnapshotStore.js";
12+
13+
function norm(over: Partial<NormalizedSnapshot> = {}): NormalizedSnapshot {
14+
const base: NormalizedSnapshot = {
15+
id: "s1",
16+
engine: "V2",
17+
executionStatus: "RUN_CREATED",
18+
description: "d",
19+
isValid: true,
20+
error: null,
21+
previousSnapshotId: null,
22+
runId: "r1",
23+
runStatus: "PENDING",
24+
batchId: null,
25+
attemptNumber: null,
26+
environmentId: "env",
27+
environmentType: "DEVELOPMENT",
28+
projectId: "p",
29+
organizationId: "o",
30+
checkpointId: null,
31+
workerId: null,
32+
runnerId: null,
33+
createdAt: 1000,
34+
updatedAt: 1000,
35+
metadata: null,
36+
completedWaitpointOrder: [],
37+
waitpointIdSet: [],
38+
};
39+
return { ...base, ...over };
40+
}
41+
42+
describe("diffLatest", () => {
43+
it("no divergence when the two sides match", () => {
44+
expect(diffLatest(norm(), norm())).toEqual([]);
45+
});
46+
47+
it("reports a scalar difference by field", () => {
48+
expect(diffLatest(norm(), norm({ executionStatus: "EXECUTING" }))).toEqual([
49+
{ field: "executionStatus", class: "scalar", pg: "RUN_CREATED", redis: "EXECUTING" },
50+
]);
51+
});
52+
53+
it("compares createdAt and updatedAt by strict equality", () => {
54+
expect(diffLatest(norm(), norm({ createdAt: 1001 }))).toEqual([
55+
{ field: "createdAt", class: "scalar", pg: 1000, redis: 1001 },
56+
]);
57+
});
58+
59+
it("classifies a validity mismatch", () => {
60+
const d = diffLatest(norm({ isValid: true }), norm({ isValid: false, error: "boom" }));
61+
expect(d.map((x) => x.field).sort()).toEqual(["error", "isValid"]);
62+
expect(d.find((x) => x.field === "isValid")!.class).toBe("validity");
63+
});
64+
65+
it("classifies completedWaitpointOrder differences as order, repeats significant", () => {
66+
expect(
67+
diffLatest(
68+
norm({ completedWaitpointOrder: ["a", "a", "b"] }),
69+
norm({ completedWaitpointOrder: ["a", "b"] })
70+
)
71+
).toEqual([
72+
{ field: "completedWaitpointOrder", class: "order", pg: ["a", "a", "b"], redis: ["a", "b"] },
73+
]);
74+
});
75+
76+
it("classifies waitpoint id set differences, order-insensitive", () => {
77+
expect(
78+
diffLatest(norm({ waitpointIdSet: ["a", "b"] }), norm({ waitpointIdSet: ["a", "b"] }))
79+
).toEqual([]);
80+
const d2 = diffLatest(norm({ waitpointIdSet: ["a", "b"] }), norm({ waitpointIdSet: ["a"] }));
81+
expect(d2[0]).toMatchObject({ field: "waitpointIdSet", class: "waitpointIdSet" });
82+
});
83+
84+
it("does NOT emit a divergence for a rotated idempotency key — invisible at id-set granularity", () => {
85+
expect(diffLatest(norm({ waitpointIdSet: ["w1"] }), norm({ waitpointIdSet: ["w1"] }))).toEqual(
86+
[]
87+
);
88+
});
89+
90+
it("missingInRedis when the row exists only in Postgres", () => {
91+
expect(diffLatest(norm(), null)).toEqual([
92+
expect.objectContaining({ class: "missingInRedis" }),
93+
]);
94+
});
95+
96+
it("missingInPg when the row exists only in Redis", () => {
97+
expect(diffLatest(null, norm())).toEqual([expect.objectContaining({ class: "missingInPg" })]);
98+
});
99+
100+
it("raises unknownField for a key on neither the compared nor excluded list", () => {
101+
const d = diffLatest(norm(), { ...norm(), somethingNew: 1 } as NormalizedSnapshot);
102+
expect(d).toEqual([expect.objectContaining({ field: "somethingNew", class: "unknownField" })]);
103+
});
104+
105+
it("normalizeFromRedis carries an unrecognised entry field, so unknownField fires on real input", () => {
106+
const read: SnapshotRead = {
107+
id: "s1",
108+
seq: 1,
109+
isValid: true,
110+
raw: "{}",
111+
entry: {
112+
engine: "V2",
113+
executionStatus: "RUN_CREATED",
114+
description: "d",
115+
runId: "r1",
116+
runStatus: "PENDING",
117+
createdAt: "2026-08-24T00:00:00.000Z",
118+
environmentId: "env",
119+
environmentType: "DEVELOPMENT",
120+
projectId: "p",
121+
organizationId: "o",
122+
mysteryField: "surprise",
123+
},
124+
};
125+
const redis = normalizeFromRedis(read);
126+
expect(redis.mysteryField).toBe("surprise"); // not dropped by normalization
127+
const d = diffLatest(
128+
norm({ id: "s1", createdAt: redis.createdAt, updatedAt: redis.updatedAt }),
129+
redis
130+
);
131+
expect(d).toEqual([
132+
expect.objectContaining({ field: "mysteryField", class: "unknownField", redis: "surprise" }),
133+
]);
134+
});
135+
136+
it("surfaces an inherited-name key and does not pollute the prototype", () => {
137+
// JSON.parse produces OWN keys for `toString` and `__proto__` (unlike an object literal).
138+
const entry = JSON.parse(
139+
'{"engine":"V2","executionStatus":"RUN_CREATED","description":"d","runId":"r1",' +
140+
'"runStatus":"PENDING","createdAt":"2026-08-24T00:00:00.000Z","environmentId":"env",' +
141+
'"environmentType":"DEVELOPMENT","projectId":"p","organizationId":"o",' +
142+
'"toString":"surprise","__proto__":{"polluted":true}}'
143+
) as Record<string, unknown>;
144+
const read: SnapshotRead = { id: "s1", seq: 1, isValid: true, raw: "{}", entry };
145+
const n = normalizeFromRedis(read) as Record<string, unknown>;
146+
147+
expect(Object.keys(n)).toContain("toString"); // carried as an own key despite the inherited name
148+
expect(n["toString"]).toBe("surprise");
149+
expect(Object.getPrototypeOf(n)).toBe(Object.prototype); // __proto__ skipped, no pollution
150+
expect("polluted" in {}).toBe(false);
151+
152+
const d = diffLatest(
153+
norm({ id: "s1", createdAt: n.createdAt as number, updatedAt: n.updatedAt as number }),
154+
n as NormalizedSnapshot
155+
);
156+
expect(d.some((x) => x.field === "toString" && x.class === "unknownField")).toBe(true);
157+
});
158+
159+
it("normalizeFromPg's waitpointIdSet is index-bearing only, matching the Redis read surface", () => {
160+
// A non-indexed completed waitpoint is in the relation but not in completedWaitpointOrder; Redis's
161+
// distinctIds (dedupe of order) does not expose it, so the PG side must not either.
162+
const row = {
163+
id: "s1",
164+
engine: "V2",
165+
executionStatus: "EXECUTING",
166+
description: "d",
167+
isValid: true,
168+
error: null,
169+
previousSnapshotId: null,
170+
runId: "r1",
171+
runStatus: "EXECUTING",
172+
batchId: null,
173+
attemptNumber: null,
174+
environmentId: "env",
175+
environmentType: "DEVELOPMENT",
176+
projectId: "p",
177+
organizationId: "o",
178+
checkpointId: null,
179+
workerId: null,
180+
runnerId: null,
181+
createdAt: new Date(1000),
182+
updatedAt: new Date(1000),
183+
metadata: null,
184+
completedWaitpointOrder: ["w_indexed"],
185+
completedWaitpoints: [{ id: "w_indexed" }, { id: "w_nonindexed" }],
186+
};
187+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
188+
const n = normalizeFromPg(row as any);
189+
expect(n.waitpointIdSet).toEqual(["w_indexed"]);
190+
});
191+
});
192+
193+
describe("diffSince", () => {
194+
const cursor = { id: "s1", createdAtMs: 1000 };
195+
196+
it("a Postgres-only entry at the cursor ms is a lost append (missingInRedis), never a tie", () => {
197+
const pg = [norm({ id: "s2", createdAt: 1000, previousSnapshotId: "s1" })];
198+
expect(diffSince({ pg, redis: [], cursor })).toEqual([
199+
expect.objectContaining({ field: "s2", class: "missingInRedis" }),
200+
]);
201+
});
202+
203+
it("a Redis-only chain-boundary surplus at the cursor ms is expected:redisSurplusAtCursorTie", () => {
204+
const redis = [norm({ id: "s2", createdAt: 1000, previousSnapshotId: "s1" })];
205+
expect(diffSince({ pg: [], redis, cursor })).toEqual([
206+
expect.objectContaining({ field: "s2", class: "expected:redisSurplusAtCursorTie" }),
207+
]);
208+
});
209+
210+
it("a Redis-only surplus that is NOT a chain boundary is a real missingInPg", () => {
211+
const redis = [norm({ id: "s3", createdAt: 1000, previousSnapshotId: "s2" })];
212+
expect(diffSince({ pg: [], redis, cursor })).toEqual([
213+
expect.objectContaining({ field: "s3", class: "missingInPg" }),
214+
]);
215+
});
216+
217+
it("a Redis-only surplus above the cursor ms is a real missingInPg", () => {
218+
const redis = [norm({ id: "s2", createdAt: 1500, previousSnapshotId: "s1" })];
219+
expect(diffSince({ pg: [], redis, cursor })).toEqual([
220+
expect.objectContaining({ field: "s2", class: "missingInPg" }),
221+
]);
222+
});
223+
});
224+
225+
describe("SnapshotComparator", () => {
226+
it("shouldSample honours the injected rng and percent", () => {
227+
expect(new SnapshotComparator({ samplePercent: 10, rng: () => 0.05 }).shouldSample()).toBe(
228+
true
229+
);
230+
expect(new SnapshotComparator({ samplePercent: 10, rng: () => 0.5 }).shouldSample()).toBe(
231+
false
232+
);
233+
});
234+
235+
it("record emits one metric per divergence, tagged by class and op, and returns void", () => {
236+
const seen: Array<{ op: string; cls: DivergenceClass }> = [];
237+
const cmp = new SnapshotComparator({
238+
samplePercent: 100,
239+
metrics: {
240+
recordDivergence: (op, cls) => seen.push({ op, cls }),
241+
recordSample: () => {},
242+
},
243+
});
244+
const ret = cmp.record("getLatest", [
245+
{ field: "executionStatus", class: "scalar" },
246+
{ field: "idempotencyKey", class: "expected:rotatedIdempotencyKey" },
247+
]);
248+
expect(ret).toBeUndefined();
249+
expect(seen).toEqual([
250+
{ op: "getLatest", cls: "scalar" },
251+
{ op: "getLatest", cls: "expected:rotatedIdempotencyKey" },
252+
]);
253+
});
254+
});

0 commit comments

Comments
 (0)