Skip to content

Commit 8891394

Browse files
committed
fix(run-store): fall back to the other store when a routed run read misses
RoutingRunStore.findRun routed a classifiable id to its single owning store by id shape and returned null on a miss, so a run whose physical residency diverges from its id shape would 404 even though it exists. Read the owning store first, then fall back to the other store only on a miss, mirroring the unrouted fan-out. The found-in-owning-store path is unchanged (single read).
1 parent 32e46e9 commit 8891394

2 files changed

Lines changed: 306 additions & 4 deletions

File tree

Lines changed: 278 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
1+
// RED→GREEN lock for RoutingRunStore.findRun's ON-MISS FAN-OUT for a CLASSIFIABLE id.
2+
//
3+
// THE BUG: findRun for a classifiable id routed to the SINGLE owning store (by id-shape
4+
// classification) and returned whatever it gave — no on-miss fallback. When a run's PHYSICAL
5+
// residency does not match its id-shape classification (e.g. a pre-#4154 27-char base62 run that
6+
// lives on the NEW store but now classifies LEGACY), findRun routed to the wrong store, missed,
7+
// and returned null → a spurious 404 — even though the run is physically present on the OTHER DB
8+
// (and runs.list surfaces it). The unclassifiable path already fanned out NEW→LEGACY; this makes
9+
// the classifiable path equally robust.
10+
//
11+
// Uses the REAL two-physical-DB split (heteroRunOpsPostgresTest: prisma14 = full/legacy on PG14,
12+
// prisma17 = dedicated run-ops subset on PG17). NEVER mocked. The residency/classification MISMATCH
13+
// is simulated deterministically by injecting a custom `classify` fn into the RoutingRunStore
14+
// constructor — the physical row is written to the NEW store while `classify` reports its id LEGACY.
15+
16+
import { heteroRunOpsPostgresTest } from "@internal/testcontainers";
17+
import { ownerEngine, type Residency } from "@trigger.dev/core/v3/isomorphic";
18+
import type { PrismaClient } from "@trigger.dev/database";
19+
import type { RunOpsPrismaClient } from "@internal/run-ops-database";
20+
import { describe, expect } from "vitest";
21+
import { PostgresRunStore } from "./PostgresRunStore.js";
22+
import { RoutingRunStore } from "./runOpsStore.js";
23+
import type { CreateRunInput, RunStore } from "./types.js";
24+
25+
// ownerEngine classifies by internal-id LENGTH/version char: 25 chars → cuid → LEGACY,
26+
// a v1 body (version "1" at index 25) → run-ops id → NEW.
27+
function cuidLegacy(seed: string): string {
28+
return (seed + "c".repeat(25)).slice(0, 25);
29+
}
30+
function runOpsNew(seed: string): string {
31+
return (seed.replace(/[^0-9a-v]/g, "0") + "k".repeat(24)).slice(0, 24) + "01";
32+
}
33+
34+
function makeDedicatedStore(prisma17: RunOpsPrismaClient) {
35+
return new PostgresRunStore({
36+
prisma: prisma17 as never,
37+
readOnlyPrisma: prisma17 as never,
38+
schemaVariant: "dedicated",
39+
});
40+
}
41+
42+
function makeLegacyStore(prisma14: PrismaClient) {
43+
return new PostgresRunStore({ prisma: prisma14, readOnlyPrisma: prisma14, schemaVariant: "legacy" });
44+
}
45+
46+
// Wrap a real store so findRun/findRunOnPrimary calls are COUNTED while every method still delegates
47+
// to the REAL PostgresRunStore (this is instrumentation, not a behavior mock — the underlying reads,
48+
// writes, getters all run for real). Lets us assert the FAST PATH does not touch the other store.
49+
function countingReads(inner: RunStore, counts: { findRun: number; findRunOnPrimary: number }): RunStore {
50+
return new Proxy(inner, {
51+
get(target, prop) {
52+
// Read via target[prop] so getters (e.g. primaryReadClient) run with `this` = the real store.
53+
const value = (target as unknown as Record<string | symbol, unknown>)[prop];
54+
if (typeof value !== "function") return value;
55+
if (prop === "findRun" || prop === "findRunOnPrimary") {
56+
return (...args: unknown[]) => {
57+
counts[prop as "findRun" | "findRunOnPrimary"] += 1;
58+
return (value as (...a: unknown[]) => unknown).apply(target, args);
59+
};
60+
}
61+
return (value as (...a: unknown[]) => unknown).bind(target);
62+
},
63+
}) as unknown as RunStore;
64+
}
65+
66+
async function seedLegacyEnvironment(prisma14: PrismaClient, suffix: string) {
67+
const organization = await prisma14.organization.create({
68+
data: { title: `Org ${suffix}`, slug: `org-${suffix}` },
69+
});
70+
const project = await prisma14.project.create({
71+
data: {
72+
name: `Project ${suffix}`,
73+
slug: `project-${suffix}`,
74+
externalRef: `proj_${suffix}`,
75+
organizationId: organization.id,
76+
},
77+
});
78+
const environment = await prisma14.runtimeEnvironment.create({
79+
data: {
80+
type: "DEVELOPMENT",
81+
slug: "dev",
82+
projectId: project.id,
83+
organizationId: organization.id,
84+
apiKey: `tr_dev_${suffix}`,
85+
pkApiKey: `pk_dev_${suffix}`,
86+
shortcode: `short_${suffix}`,
87+
},
88+
});
89+
return {
90+
organizationId: organization.id,
91+
projectId: project.id,
92+
runtimeEnvironmentId: environment.id,
93+
environmentId: environment.id,
94+
};
95+
}
96+
97+
function buildCreateRunInput(params: {
98+
runId: string;
99+
friendlyId: string;
100+
organizationId: string;
101+
projectId: string;
102+
runtimeEnvironmentId: string;
103+
}): CreateRunInput {
104+
return {
105+
data: {
106+
id: params.runId,
107+
engine: "V2",
108+
status: "PENDING",
109+
friendlyId: params.friendlyId,
110+
runtimeEnvironmentId: params.runtimeEnvironmentId,
111+
environmentType: "DEVELOPMENT",
112+
organizationId: params.organizationId,
113+
projectId: params.projectId,
114+
taskIdentifier: "my-task",
115+
payload: "{}",
116+
payloadType: "application/json",
117+
traceContext: {},
118+
traceId: `trace_${params.runId}`,
119+
spanId: `span_${params.runId}`,
120+
queue: "task/my-task",
121+
isTest: false,
122+
taskEventStore: "taskEvent",
123+
depth: 0,
124+
},
125+
snapshot: {
126+
engine: "V2",
127+
executionStatus: "RUN_CREATED",
128+
description: "Run was created",
129+
runStatus: "PENDING",
130+
environmentId: params.runtimeEnvironmentId,
131+
environmentType: "DEVELOPMENT",
132+
projectId: params.projectId,
133+
organizationId: params.organizationId,
134+
},
135+
};
136+
}
137+
138+
// Insert a TaskRun row DIRECTLY onto the NEW (dedicated) store, bypassing routing, so we can force a
139+
// residency/classification MISMATCH: the row is physically on #new while `classify` calls its id LEGACY.
140+
async function insertRunOnNewStore(
141+
prisma17: RunOpsPrismaClient,
142+
params: { runId: string; friendlyId: string; environmentId: string; organizationId: string; projectId: string }
143+
) {
144+
await prisma17.taskRun.create({
145+
data: {
146+
id: params.runId,
147+
engine: "V2",
148+
status: "PENDING",
149+
friendlyId: params.friendlyId,
150+
runtimeEnvironmentId: params.environmentId,
151+
environmentType: "DEVELOPMENT",
152+
organizationId: params.organizationId,
153+
projectId: params.projectId,
154+
taskIdentifier: "my-task",
155+
payload: "{}",
156+
payloadType: "application/json",
157+
traceContext: {},
158+
traceId: `trace_${params.runId}`,
159+
spanId: `span_${params.runId}`,
160+
queue: "task/my-task",
161+
isTest: false,
162+
taskEventStore: "taskEvent",
163+
depth: 0,
164+
},
165+
});
166+
}
167+
168+
describe("RoutingRunStore.findRun — on-miss fan-out for a classifiable id (residency ≠ classification)", () => {
169+
// ── THE BUG: a run physically on #new whose id classifies LEGACY must still be found ──
170+
// Without the on-miss fallback, findRun routes to #legacy (per classification), misses, returns null.
171+
heteroRunOpsPostgresTest(
172+
"returns a #new-resident run whose id classifies LEGACY (owning-store miss → other-store fallback)",
173+
async ({ prisma14, prisma17 }) => {
174+
const env = await seedLegacyEnvironment(prisma14, "mm1");
175+
const newStore = makeDedicatedStore(prisma17);
176+
const legacyStore = makeLegacyStore(prisma14);
177+
178+
// A run-ops-shaped id (so ownerEngine would say NEW), but we FORCE classify → LEGACY to model
179+
// a residency/classification mismatch: physically on #new, classified LEGACY.
180+
const mismatchId = runOpsNew("mm1");
181+
const classify = (id: string): Residency => (id === mismatchId ? "LEGACY" : ownerEngine(id));
182+
const router = new RoutingRunStore({ new: newStore, legacy: legacyStore, classify });
183+
184+
await insertRunOnNewStore(prisma17, {
185+
runId: mismatchId,
186+
friendlyId: "run_mm1",
187+
environmentId: env.environmentId,
188+
organizationId: env.organizationId,
189+
projectId: env.projectId,
190+
});
191+
192+
// Physical residency sanity: on #new only.
193+
expect(await prisma17.taskRun.findUnique({ where: { id: mismatchId } })).not.toBeNull();
194+
expect(await prisma14.taskRun.findUnique({ where: { id: mismatchId } })).toBeNull();
195+
196+
// classify → LEGACY routes to #legacy (miss); the fix falls back to #new and finds the run.
197+
const byId = (await router.findRun({ id: mismatchId }, { select: { id: true } })) as Record<
198+
string,
199+
unknown
200+
> | null;
201+
expect(byId?.id).toBe(mismatchId);
202+
203+
// Same on the read-your-writes primary variant (a caller-passed writer → findRunOnPrimary).
204+
const byIdPrimary = (await router.findRun(
205+
{ id: mismatchId },
206+
{ select: { id: true } },
207+
prisma14
208+
)) as Record<string, unknown> | null;
209+
expect(byIdPrimary?.id).toBe(mismatchId);
210+
}
211+
);
212+
213+
// ── FAST PATH: a run found in its CLASSIFIED store is a SINGLE read (no second-store probe) ──
214+
heteroRunOpsPostgresTest(
215+
"does NOT read the other store when the classified (owning) store hits",
216+
async ({ prisma14, prisma17 }) => {
217+
const env = await seedLegacyEnvironment(prisma14, "mm2");
218+
219+
// NEW-resident run-ops-id run: owning store = #new. Wrap #legacy to catch any stray probe.
220+
const newCounts = { findRun: 0, findRunOnPrimary: 0 };
221+
const legacyCounts = { findRun: 0, findRunOnPrimary: 0 };
222+
const newStore = countingReads(makeDedicatedStore(prisma17), newCounts);
223+
const legacyStore = countingReads(makeLegacyStore(prisma14), legacyCounts);
224+
const router = new RoutingRunStore({ new: newStore, legacy: legacyStore });
225+
226+
const newId = runOpsNew("mm2n"); // classifies NEW
227+
await router.createRun(
228+
buildCreateRunInput({
229+
runId: newId,
230+
friendlyId: "run_mm2_new",
231+
organizationId: env.organizationId,
232+
projectId: env.projectId,
233+
runtimeEnvironmentId: env.runtimeEnvironmentId,
234+
})
235+
);
236+
237+
const hit = (await router.findRun({ id: newId }, { select: { id: true } })) as Record<
238+
string,
239+
unknown
240+
> | null;
241+
expect(hit?.id).toBe(newId);
242+
// Owning store read exactly once; the other store NOT touched (fast path preserved).
243+
expect(newCounts.findRun).toBe(1);
244+
expect(legacyCounts.findRun).toBe(0);
245+
expect(legacyCounts.findRunOnPrimary).toBe(0);
246+
247+
// Symmetric: a cuid run whose owning store is #legacy must not probe #new on a hit.
248+
const legacyId = cuidLegacy("mm2l"); // classifies LEGACY
249+
await router.createRun(
250+
buildCreateRunInput({
251+
runId: legacyId,
252+
friendlyId: "run_mm2_legacy",
253+
organizationId: env.organizationId,
254+
projectId: env.projectId,
255+
runtimeEnvironmentId: env.runtimeEnvironmentId,
256+
})
257+
);
258+
newCounts.findRun = 0;
259+
legacyCounts.findRun = 0;
260+
261+
const hitLegacy = (await router.findRun({ id: legacyId }, { select: { id: true } })) as Record<
262+
string,
263+
unknown
264+
> | null;
265+
expect(hitLegacy?.id).toBe(legacyId);
266+
expect(legacyCounts.findRun).toBe(1);
267+
expect(newCounts.findRun).toBe(0);
268+
}
269+
);
270+
271+
// ── A genuine miss on BOTH stores still returns null (fan-out exhausted) ──
272+
heteroRunOpsPostgresTest("returns null when the run is on neither store", async ({ prisma14, prisma17 }) => {
273+
const newStore = makeDedicatedStore(prisma17);
274+
const legacyStore = makeLegacyStore(prisma14);
275+
const router = new RoutingRunStore({ new: newStore, legacy: legacyStore });
276+
expect(await router.findRun({ id: cuidLegacy("ghost") }, { select: { id: true } })).toBeNull();
277+
});
278+
});

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,17 +239,41 @@ export class RoutingRunStore implements RunStore {
239239
const onPrimary = readYourWrites(argsOrClient, _client);
240240
const id = idFromWhere(where);
241241
if (id !== undefined) {
242-
// Residency-classifiable (id/friendlyId): route to the owning store.
243-
const store = this.#routeOrNew(id);
244-
const method = onPrimary ? "findRunOnPrimary" : "findRun";
245-
return (store[method] as (...rest: unknown[]) => Promise<unknown>)(where, args);
242+
// Residency-classifiable (id/friendlyId): route to the owning store, then fall back to the
243+
// OTHER store on a miss.
244+
return this.#findRunRouted(id, where, args, onPrimary);
246245
}
247246
// Unclassifiable where (e.g. spanId, idempotencyKey): the run may live on either DB,
248247
// so fan out NEW-first then LEGACY rather than defaulting to NEW — defaulting silently
249248
// misses legacy-resident runs (span detail, idempotency-dedup probe, etc.).
250249
return this.#findRunUnrouted(where, args, onPrimary);
251250
}
252251

252+
// A classifiable id names its OWNING store by id-shape, but physical residency can diverge from
253+
// classification (a pre-#4154 base62 run lives on #new yet classifies LEGACY). Read the owning
254+
// store first — a hit is a SINGLE read (the fast path) — then, ONLY on a miss, probe the OTHER
255+
// store before returning null, so a run whose residency ≠ its id-shape is found rather than 404'd.
256+
// Both legs run the SAME index-covered TaskRun lookup; the fan-out cost is paid only on the (rare)
257+
// miss. Mirrors #findRunUnrouted's shape but keyed on the classified owner.
258+
async #findRunRouted(
259+
id: string,
260+
where: Prisma.TaskRunWhereInput,
261+
args: unknown,
262+
onPrimary: boolean
263+
): Promise<unknown> {
264+
const method = onPrimary ? "findRunOnPrimary" : "findRun";
265+
const owning = this.#routeOrNew(id);
266+
const fromOwning = await (owning[method] as (...rest: unknown[]) => Promise<unknown>)(
267+
where,
268+
args
269+
);
270+
if (fromOwning != null) {
271+
return fromOwning;
272+
}
273+
const other = owning === this.#new ? this.#legacy : this.#new;
274+
return (other[method] as (...rest: unknown[]) => Promise<unknown>)(where, args);
275+
}
276+
253277
async #findRunUnrouted(
254278
where: Prisma.TaskRunWhereInput,
255279
args: unknown,

0 commit comments

Comments
 (0)