Skip to content

Commit aa9b888

Browse files
authored
refactor(run-store): hold RoutingRunStore's stores in a keyed shard map (#4752)
## What `RoutingRunStore` held two named store fields, `#new` and `#legacy`, and took its routing policy from the order the statements happened to run in. It now holds a `Map<ShardKey, RunStore>`, and the three policies that were implicit are readable data: - **`#probeOrder`** (`new` → `legacy`) — the sequential probe for a lookup with no routable id. The first non-null result wins, and the *last* entry owns the canonical not-found throw. - **`#precedence`** (`legacy` → `new`) — ascending authority for a merge, so the highest-authority shard wins a duplicate id. - **`#idlessRouteShard`** (`new`) and **`#idlessWaitpointShard`** (`legacy`) — the two id-less defaults, which differ by role and were previously two unrelated literals in unrelated methods. The two orders are the **reverse of each other**, which is why they are separate fields rather than one ordering. Nine sites observe the result-array order and must iterate `#probeOrder`; five decide a value by which shard wins a duplicate and must iterate `#precedence`. Five more sum counts and are order-independent, because addition commutes. Four helpers absorb the twenty-six hand-written fan-outs — `#probeFirst`, `#fanOut(order, fn)`, `#fanOutPartitioned`, `#shardsExcept` — and `#shardKeyOf` replaces the inline residency-to-store ternaries. `#fanOut` takes its order as an argument so every call site states which policy it uses. The constructor keeps its exact options type. No union arm, no `shards` member: that would loosen the excess-property check and silently retire the `@ts-expect-error onLegacyRead` lock in the test corpus. N-way construction is a later change. ## One behaviour change `findManyTaskRunWaitpoints` merged its edge rows NEW-first into a last-wins dedupe, so a duplicate edge id resolved to the **legacy** row — the opposite of the rule the other four merges follow, and the opposite of what `dedupeEdgesById`'s own comment claimed. No test pinned it in either direction. It now resolves NEW-wins, consistent with every sibling merge, and a new test pins the winner so it cannot drift back silently. Reaching this case needs one edge id present on both stores at the same time, with no routable `taskRunId`. That only arises from drain mirroring. The drain seam is removed (`runOpsStore.test.ts`, "fan-out spans NEW+LEGACY with no drain seam"), so **no new duplicates can be created** — but removing the code does not delete rows it previously wrote, and this class still carries comments treating mirrored rows as a live data condition. Whether any historical duplicate edge rows persist is an empirical question about production data, not something this diff settles. If such a row is hit, the two copies either agree — in which case the winner is immaterial — or they have diverged, in which case NEW is the authoritative copy by the router's own precedence rule. So the corrected behaviour is at least as correct as the old one in every reachable case. Everything else is behaviour-preserving. ## How it was verified - **`internal-packages/run-store`: 69 files, 379 tests pass.** The corpus is the regression gate for this refactor. 67 of the 68 pre-existing test files are byte-identical; the one that differs (`runOpsStore.mixedResidency.test.ts`) changes only `//` comments. - **`internal-packages/run-engine`: 12 files, 69 tests pass** — every file that constructs the router, exercised at runtime. - **The `@ts-expect-error onLegacyRead` lock still fires.** `tsconfig.build.json` excludes `*.test.ts`, so a green typecheck does not cover it. A scratch probe confirmed `tsc` still reports `TS2353` for `onLegacyRead` and no error for the three real options. - **All 48 construction sites outside the package compile unchanged.** `tsconfig.check.json` also excludes `*.test.ts`, so the 25 webapp test files were checked with the test exclusion dropped and compared against the same check on the base commit: 614 errors before, 614 after, zero present in one and not the other. Those 614 are pre-existing in never-typechecked test files. - `typecheck` passes for `run-store`, `run-engine` and `webapp`. `knip` reports nothing in `run-store`. ## Also Refreshes the sixteen stale `runOpsStore.ts` line references in `runOpsStore.mixedResidency.test.ts`, each verified against the symbol it names. ## Notes for the reviewer - The riskiest possible mistake in this diff is a fan-out passing the wrong order — the compiler cannot catch it, because both orders are `readonly ShardKey[]`. The five `#precedence` sites are `#findRunsOpen`, `findRunsByIdempotencyKeys`, `#collectManyWaitpoints`, `findManyTaskRunWaitpoints` and `findManyWaitpointTags`. Those are the lines worth the closest read. - Four sites previously derived "the other store" by object identity (`home === this.#new ? ...`). They now compare keys. The two are equivalent: in single-database mode both keys map to the same store object, and when the stores are distinct, identity and key comparison agree. - No changeset and no `.server-changes` note: the package is internal and the one behaviour change is unreachable in production, so a release note would tell a user nothing. - Two CI checks fail for reasons that predate this branch and reproduce on the base commit: `lint` (~16 unknown `react/*` rules make `.oxlintrc.json` fail to parse, which disables oxlint entirely — including the two `trigger-runops` fences) and `knip` (`unrun`, an unused devDependency on the default branch). Both want their own fix.
1 parent c5c2ea9 commit aa9b888

3 files changed

Lines changed: 505 additions & 303 deletions

File tree

‎internal-packages/run-store/src/runOpsStore.mixedResidency.test.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,10 @@ async function seedSharedEnv(prisma14: PrismaClient, suffix: string) {
195195
}
196196

197197
describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id #new coexisting)", () => {
198-
// ── Case 1: findRuns by a MIXED bounded id-set (#findRunsByIdSet, runOpsStore.ts:294) ──
198+
// ── Case 1: findRuns by a MIXED bounded id-set (#findRunsByIdSet) ──
199199
// A list-hydrate id set spans cuid (legacy) + run-ops id (new) ids plus a run-ops id absent from legacy.
200200
// Both resident runs returned; take/skip applied GLOBALLY post-merge; orderBy honored; the absent
201-
// run-ops id short-circuits (never probed on LEGACY, :309).
201+
// run-ops id short-circuits (never probed on LEGACY, #fanOutPartitioned).
202202
heteroRunOpsPostgresTest(
203203
"case 1: findRuns by a mixed id-set returns both DBs' runs, ordered, take/skip global",
204204
async ({ prisma14, prisma17 }) => {
@@ -306,7 +306,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
306306
}
307307
);
308308

309-
// ── Case 2: findRuns by an OPEN predicate (#findRunsOpen, runOpsStore.ts:319) ──
309+
// ── Case 2: findRuns by an OPEN predicate (#findRunsOpen) ──
310310
// No id set → query BOTH stores, union, dedup by id (NEW wins). Filter by a shared scalar
311311
// (runtimeEnvironmentId + status) that matches rows on both DBs.
312312
heteroRunOpsPostgresTest(
@@ -355,7 +355,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
355355
}
356356
);
357357

358-
// ── Case 3: expireRunsBatch with a MIXED id list (runOpsStore.ts:474) ──
358+
// ── Case 3: expireRunsBatch with a MIXED id list ──
359359
// Partitions run-ops id→NEW / cuid→LEGACY; each leg called only when non-empty; counts summed; each row
360360
// updated on its OWN DB only.
361361
heteroRunOpsPostgresTest(
@@ -388,7 +388,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
388388
}
389389
);
390390

391-
// ── Case 4: clearIdempotencyKey fan-out arm (byFriendlyIds, runOpsStore.ts:358) ──
391+
// ── Case 4: clearIdempotencyKey fan-out arm (byFriendlyIds) ──
392392
// byFriendlyIds spans mixed residency → fan out to both, sum the count, each row cleared on its home.
393393
heteroRunOpsPostgresTest(
394394
"case 4: clearIdempotencyKey byFriendlyIds clears across both DBs and sums the count",
@@ -429,7 +429,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
429429
}
430430
);
431431

432-
// ── Case 5: countPendingWaitpoints scattered across both DBs (runOpsStore.ts:731) ──
432+
// ── Case 5: countPendingWaitpoints scattered across both DBs ──
433433
// A run's pending waitpoints can be split across both stores mid-drain → count on each and sum.
434434
heteroRunOpsPostgresTest(
435435
"case 5: countPendingWaitpoints sums PENDING waitpoints scattered across both DBs",
@@ -465,7 +465,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
465465
}
466466
);
467467

468-
// ── Case 6: findManyWaitpoints { id: { in: [...mixed...] } } (runOpsStore.ts:793) ──
468+
// ── Case 6: findManyWaitpoints { id: { in: [...mixed...] } } ──
469469
// Merge waitpoints from both DBs for a mixed id set.
470470
heteroRunOpsPostgresTest(
471471
"case 6: findManyWaitpoints merges a mixed id set from both DBs",
@@ -495,7 +495,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
495495

496496
// ── Case 8: findExecutionSnapshot / findManyExecutionSnapshots OPEN (no runId) where ──
497497
// A by-snapshot-id-only lookup (snapshot ids are non-classifiable cuids) must fan out NEW→LEGACY
498-
// (findExecutionSnapshot, :675) / merge both (findManyExecutionSnapshots, :688). Seed a snapshot on
498+
// (findExecutionSnapshot) / merge both (findManyExecutionSnapshots). Seed a snapshot on
499499
// EACH DB (one run-ops run on #new, one cuid run on #legacy) and read with a no-runId where.
500500
heteroRunOpsPostgresTest(
501501
"case 8: findExecutionSnapshot/findManyExecutionSnapshots with an open where reach both DBs",
@@ -544,7 +544,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
544544
}
545545
);
546546

547-
// ── Case 9a: findRun with an UNCLASSIFIABLE where (spanId) on a #legacy run (#findRunUnrouted, :213) ──
547+
// ── Case 9a: findRun with an UNCLASSIFIABLE where (spanId) on a #legacy run (#findRunUnrouted) ──
548548
// A run-ops run on #new and a cuid run on #legacy each carry a distinct spanId. A spanId where can't
549549
// be id-classified → fan out NEW-first then LEGACY. The legacy-resident run must be found.
550550
heteroRunOpsPostgresTest(
@@ -586,7 +586,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
586586
}
587587
);
588588

589-
// ── Case 9b: findRunOrThrow with an UNCLASSIFIABLE where (spanId) on a #legacy run (:593) ──
589+
// ── Case 9b: findRunOrThrow with an UNCLASSIFIABLE where (spanId) on a #legacy run (#findRunOrThrowUnrouted) ──
590590
// The throwing twin must match findRun's fan-out: an unclassifiable where whose only matching run
591591
// lives on #legacy must NOT throw. A NEW-only fallback would miss the legacy run and throw.
592592
heteroRunOpsPostgresTest(
@@ -628,7 +628,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
628628
}
629629
);
630630

631-
// ── Case 7: findManyTaskRunWaitpoints with edges whose relations STRADDLE DBs (runOpsStore.ts:876) ──
631+
// ── Case 7: findManyTaskRunWaitpoints with edges whose relations STRADDLE DBs ──
632632
// An edge co-locates with its RUN, but its `waitpoint`/`taskRun` relations can live on the OTHER DB
633633
// (a cuid token blocking a run-ops run, and vice versa). The per-leg scalar query is stripped of the
634634
// relation keys; the router re-hydrates `waitpoint`/`taskRun` across BOTH DBs. Exercises BOTH
@@ -721,7 +721,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
721721
}
722722
);
723723

724-
// ── Case 7b: the "blocking waitpoint not found on either DB" HARD ERROR (runOpsStore.ts:917) ──
724+
// ── Case 7b: the "blocking waitpoint not found on either DB" HARD ERROR (#hydrateEdgeWaitpointsCrossDb) ──
725725
// An edge whose `waitpointId` resolves on NEITHER DB must throw rather than leave a null status that
726726
// would strand (hang) or wrongly unblock the run.
727727
heteroRunOpsPostgresTest(
@@ -748,7 +748,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
748748
}
749749
);
750750

751-
// ── Case 10: findBatchTaskRunById / findBatchTaskRunByFriendlyId NEW-then-LEGACY probe (:1124,:1137) ──
751+
// ── Case 10: findBatchTaskRunById / findBatchTaskRunByFriendlyId NEW-then-LEGACY probe ──
752752
// A batch resident on #legacy AND a run-ops-id batch landed on #new (the control-plane window mints
753753
// cuid ids, but a run-ops batch resides on #new) are BOTH found via the probe, regardless of id-shape.
754754
heteroRunOpsPostgresTest(
@@ -792,7 +792,7 @@ describe("RoutingRunStore — mixed-residency matrix (cuid #legacy + run-ops id
792792
}
793793
);
794794

795-
// ── Case 11a: updateManyWaitpoints with a NO-ID (batch) where fans out to both and sums (:822) ──
795+
// ── Case 11a: updateManyWaitpoints with a NO-ID (batch) where fans out to both and sums ──
796796
// A batch where (no single routable id, e.g. completedByTaskRunId IS NULL + status PENDING) must
797797
// apply on BOTH DBs and sum the count.
798798
heteroRunOpsPostgresTest(
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
import { describe, expect, it } from "vitest";
2+
import { RoutingRunStore } from "./runOpsStore.js";
3+
import type { ReadClient, RunStore } from "./types.js";
4+
5+
// Pins the routing ALGEBRA: probe order, merge precedence, and the two id-less fallbacks that
6+
// differ by role. No DB — each slot is a fake RunStore recording into ONE shared ordered log, so a
7+
// sequential probe's leg order and a merge's winner are both observable. Real two-DB topology stays
8+
// with the heteroRunOpsPostgresTest suites.
9+
//
10+
// MUST NOT assert invocation order for a PARALLEL fan-out: both legs are issued before either
11+
// resolves, so the order they are created in is not a behaviour.
12+
13+
type Slot = "new" | "legacy";
14+
15+
type Call = { slot: Slot; method: string };
16+
17+
type FakeConfig = {
18+
// Rows this store returns from findRun / findRuns / findRunOrThrow, regardless of filter.
19+
runs?: Array<Record<string, unknown>>;
20+
// Edge rows this store returns from findManyTaskRunWaitpoints, regardless of filter.
21+
edges?: Array<Record<string, unknown>>;
22+
// Waitpoint rows this store returns from findWaitpoint, regardless of filter.
23+
waitpoint?: Record<string, unknown> | null;
24+
};
25+
26+
type FakeStore = RunStore & {
27+
slot: Slot;
28+
primaryReadClient: { __primary: Slot };
29+
};
30+
31+
function fakeStore(slot: Slot, log: Call[], config: FakeConfig = {}): FakeStore {
32+
const record = (method: string) => log.push({ slot, method });
33+
const runs = config.runs ?? [];
34+
35+
const store: Partial<FakeStore> = {
36+
slot,
37+
primaryReadClient: { __primary: slot },
38+
39+
findRun: ((_where: unknown, _args?: unknown) => {
40+
record("findRun");
41+
return Promise.resolve((runs[0] ?? null) as never);
42+
}) as FakeStore["findRun"],
43+
44+
findRunOnPrimary: ((_where: unknown, _args?: unknown) => {
45+
record("findRunOnPrimary");
46+
return Promise.resolve((runs[0] ?? null) as never);
47+
}) as FakeStore["findRunOnPrimary"],
48+
49+
findRunOrThrow: ((_where: unknown, _args?: unknown) => {
50+
record("findRunOrThrow");
51+
if (runs[0] === undefined) {
52+
return Promise.reject(new Error(`no run on ${slot}`)) as never;
53+
}
54+
return Promise.resolve(runs[0] as never);
55+
}) as FakeStore["findRunOrThrow"],
56+
57+
findRuns: ((_args: unknown, _client?: ReadClient) => {
58+
record("findRuns");
59+
return Promise.resolve(runs as never);
60+
}) as FakeStore["findRuns"],
61+
62+
createRun: ((_params: unknown) => {
63+
record("createRun");
64+
return Promise.resolve({ slot } as never);
65+
}) as FakeStore["createRun"],
66+
67+
createTaskRunCheckpoint: ((_args: unknown) => {
68+
record("createTaskRunCheckpoint");
69+
return Promise.resolve({ slot } as never);
70+
}) as FakeStore["createTaskRunCheckpoint"],
71+
72+
findWaitpoint: ((_args: unknown, _client?: ReadClient) => {
73+
record("findWaitpoint");
74+
return Promise.resolve((config.waitpoint ?? null) as never);
75+
}) as FakeStore["findWaitpoint"],
76+
77+
updateWaitpoint: ((_args: unknown) => {
78+
record("updateWaitpoint");
79+
return Promise.resolve({ slot } as never);
80+
}) as FakeStore["updateWaitpoint"],
81+
82+
findManyTaskRunWaitpoints: ((_args: unknown, _client?: ReadClient) => {
83+
record("findManyTaskRunWaitpoints");
84+
return Promise.resolve((config.edges ?? []) as never);
85+
}) as FakeStore["findManyTaskRunWaitpoints"],
86+
};
87+
88+
return store as unknown as FakeStore;
89+
}
90+
91+
// Deterministic residency by id prefix via the classify seam (no dependence on id-shape rules).
92+
function buildRouter(newConfig: FakeConfig = {}, legacyConfig: FakeConfig = {}) {
93+
const log: Call[] = [];
94+
const newStore = fakeStore("new", log, newConfig);
95+
const legacyStore = fakeStore("legacy", log, legacyConfig);
96+
const router = new RoutingRunStore({
97+
new: newStore,
98+
legacy: legacyStore,
99+
classify: (id: string) => (id.startsWith("new") ? "NEW" : "LEGACY"),
100+
});
101+
return { router, newStore, legacyStore, log };
102+
}
103+
104+
const trace = (log: Call[]) => log.map((c) => `${c.slot}:${c.method}`);
105+
106+
describe("RoutingRunStore #probeOrder — new then legacy, sequential", () => {
107+
it("probes new BEFORE legacy for an unrouted findRun", async () => {
108+
const { router, log } = buildRouter();
109+
await router.findRun({ spanId: "span_x" });
110+
expect(trace(log)).toEqual(["new:findRun", "legacy:findRun"]);
111+
});
112+
113+
it("stops at the first non-null leg and never consults legacy", async () => {
114+
const { router, log } = buildRouter({ runs: [{ id: "r1" }] });
115+
await router.findRun({ spanId: "span_x" });
116+
expect(trace(log)).toEqual(["new:findRun"]);
117+
});
118+
119+
it("gives the LAST probe leg the canonical not-found throw", async () => {
120+
const { router, log } = buildRouter();
121+
await expect(router.findRunOrThrow({ spanId: "span_x" })).rejects.toThrow("no run on legacy");
122+
// new is probed with the nullable read; only legacy is asked to throw.
123+
expect(trace(log)).toEqual(["new:findRun", "legacy:findRunOrThrow"]);
124+
});
125+
126+
it("probes each store's own primary for a read-your-writes unrouted findRun", async () => {
127+
const { router, log } = buildRouter();
128+
await router.findRunOnPrimary({ spanId: "span_x" });
129+
expect(trace(log)).toEqual(["new:findRunOnPrimary", "legacy:findRunOnPrimary"]);
130+
});
131+
});
132+
133+
describe("RoutingRunStore #precedence — NEW wins a merge", () => {
134+
it("keeps the NEW row for a duplicate run id on an open predicate", async () => {
135+
const { router } = buildRouter(
136+
{ runs: [{ id: "dup", from: "new" }] },
137+
{ runs: [{ id: "dup", from: "legacy" }] }
138+
);
139+
const rows = (await router.findRuns({
140+
where: { runtimeEnvironmentId: "env_1" },
141+
select: { id: true, from: true },
142+
})) as Array<{ id: string; from: string }>;
143+
expect(rows).toHaveLength(1);
144+
expect(rows[0]?.from).toBe("new");
145+
});
146+
147+
// Every merge in the router MUST resolve a duplicate id NEW-wins, edges included.
148+
it("keeps the NEW row for a duplicate edge id on a waitpoint-keyed edge read", async () => {
149+
const { router } = buildRouter(
150+
{ edges: [{ id: "edge_dup", taskRunId: "new_run" }] },
151+
{ edges: [{ id: "edge_dup", taskRunId: "legacy_run" }] }
152+
);
153+
const edges = (await router.findManyTaskRunWaitpoints({
154+
where: { waitpointId: "waitpoint_x" },
155+
select: { id: true, taskRunId: true },
156+
})) as Array<{ id: string; taskRunId: string }>;
157+
expect(edges).toHaveLength(1);
158+
expect(edges[0]?.taskRunId).toBe("new_run");
159+
});
160+
});
161+
162+
describe("RoutingRunStore id-less fallbacks — the two defaults differ by role", () => {
163+
it("routes an id-less create to new (#idlessRouteShard)", async () => {
164+
const { router, log } = buildRouter();
165+
await router.createRun({ data: {} } as never);
166+
expect(trace(log)).toEqual(["new:createRun"]);
167+
});
168+
169+
it("routes an id-less checkpoint create to new (#idlessRouteShard)", async () => {
170+
const { router, log } = buildRouter();
171+
await router.createTaskRunCheckpoint({ data: {} } as never);
172+
expect(trace(log)).toEqual(["new:createTaskRunCheckpoint"]);
173+
});
174+
175+
it("routes an id-less waitpoint update to legacy (#idlessWaitpointShard)", async () => {
176+
const { router, log } = buildRouter();
177+
await router.updateWaitpoint({ where: { idempotencyKey: "k" }, data: {} } as never);
178+
expect(trace(log)).toEqual(["legacy:updateWaitpoint"]);
179+
});
180+
});

0 commit comments

Comments
 (0)