From abde82c033b12b0c1193817f7151750501c85db3 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 16:41:39 +0100 Subject: [PATCH 01/31] feat(core): export isValidShardChar for shard-descriptor validation Co-Authored-By: Claude Opus 4.8 --- .../core/src/v3/isomorphic/friendlyId.test.ts | 16 ++++++++++++++++ packages/core/src/v3/isomorphic/friendlyId.ts | 5 +++++ 2 files changed, 21 insertions(+) diff --git a/packages/core/src/v3/isomorphic/friendlyId.test.ts b/packages/core/src/v3/isomorphic/friendlyId.test.ts index 2e3ba4d83a5..f788b5940da 100644 --- a/packages/core/src/v3/isomorphic/friendlyId.test.ts +++ b/packages/core/src/v3/isomorphic/friendlyId.test.ts @@ -15,6 +15,7 @@ import { base32hexEncode, generateRunOpsId, generateRunOpsIdV2, + isValidShardChar, parseRunId, parseRunOpsIdBody, parseRunOpsIdV2Body, @@ -410,3 +411,18 @@ describe("parseRunId — v2 arm", () => { expect(parseRunId(`waitpoint_${generateRunOpsIdV2("a")}`).format).toBe("legacy"); }); }); + +describe("isValidShardChar", () => { + it("accepts a single [a-z0-9] char", () => { + expect(isValidShardChar("a")).toBe(true); + expect(isValidShardChar("0")).toBe(true); + expect(isValidShardChar("w")).toBe(true); + }); + it("rejects multi-char, empty, uppercase, and punctuation", () => { + expect(isValidShardChar("")).toBe(false); + expect(isValidShardChar("ab")).toBe(false); + expect(isValidShardChar("A")).toBe(false); + expect(isValidShardChar("-")).toBe(false); + expect(isValidShardChar("legacy")).toBe(false); + }); +}); diff --git a/packages/core/src/v3/isomorphic/friendlyId.ts b/packages/core/src/v3/isomorphic/friendlyId.ts index c468de65319..958d189d598 100644 --- a/packages/core/src/v3/isomorphic/friendlyId.ts +++ b/packages/core/src/v3/isomorphic/friendlyId.ts @@ -40,6 +40,11 @@ export const DEFAULT_REGION_CHAR = "0"; const REGION_CHAR_PATTERN = /^[a-z0-9]$/; // Same slot, same range: the gen-2 shard key is a region char's positional twin. const SHARD_CHAR_PATTERN = REGION_CHAR_PATTERN; +/** True iff `value` is a single valid gen-2 shard char. The descriptor validator and + * `resolveShard` share this so a configured key and a decoded key cannot drift. */ +export function isValidShardChar(value: string): boolean { + return SHARD_CHAR_PATTERN.test(value); +} /** One lowercase [a-z0-9] char per supported region, at RUN_OPS_ID_REGION_INDEX. */ export const REGION_CODES: Readonly> = { "us-east-1": "e", From f9fb0c01950165466ddbf2992dc129e9d523da76 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 16:59:03 +0100 Subject: [PATCH 02/31] feat(run-store): add UnknownShardKey and RoutingRunStore.fromShards with an injected shard resolver Co-Authored-By: Claude Opus 4.8 --- .../src/runOpsStore.fromShards.test.ts | 65 ++++++++++++++++ .../run-store/src/runOpsStore.ts | 74 +++++++++++++++++-- 2 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 internal-packages/run-store/src/runOpsStore.fromShards.test.ts diff --git a/internal-packages/run-store/src/runOpsStore.fromShards.test.ts b/internal-packages/run-store/src/runOpsStore.fromShards.test.ts new file mode 100644 index 00000000000..e9f55332ad7 --- /dev/null +++ b/internal-packages/run-store/src/runOpsStore.fromShards.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { + generateRunOpsId, + generateRunOpsIdV2, + resolveShard, + type ShardKey, +} from "@trigger.dev/core/v3/isomorphic"; +import { RoutingRunStore, UnknownShardKey } from "./runOpsStore.js"; +import type { ReadClient, RunStore } from "./types.js"; + +// Pure routing unit test for the N-way fromShards factory. Each shard is a fake RunStore whose +// findRun records which slot answered, so the assertions are purely about WHICH store the router +// selects. No database. +type FakeStore = RunStore & { slot: ShardKey }; + +function fakeStore(slot: ShardKey): FakeStore { + const store: Partial = { + slot, + primaryReadClient: { __primary: slot } as unknown as ReadClient, + findRun: ((_where: unknown, _argsOrClient?: unknown, _client?: unknown) => + Promise.resolve({ slot } as never)) as FakeStore["findRun"], + }; + return store as FakeStore; +} + +function build(shardKeys: ShardKey[]) { + const shards = new Map(); + shards.set("legacy", fakeStore("legacy")); + shards.set("new", fakeStore("new")); + for (const k of shardKeys) shards.set(k, fakeStore(k)); + return RoutingRunStore.fromShards({ + shards, + probeOrder: ["new", ...shardKeys, "legacy"], + precedence: ["legacy", "new", ...shardKeys], + idlessRouteShard: "new", + idlessWaitpointShard: "legacy", + resolveShardKey: resolveShard, + }); +} + +describe("RoutingRunStore.fromShards", () => { + it("routes a gen-2 id to its own shard, not to new", async () => { + const store = build(["a"]); + const found = await store.findRun({ friendlyId: generateRunOpsIdV2("a") }); + expect(found).toMatchObject({ slot: "a" }); + }); + + it("routes a gen-1 v1 id to new", async () => { + const store = build(["a"]); + const found = await store.findRun({ friendlyId: generateRunOpsId() }); + expect(found).toMatchObject({ slot: "new" }); + }); + + it("routes a cuid id to legacy", async () => { + const store = build(["a"]); + const found = await store.findRun({ friendlyId: "clabc123def456ghi789jkl01" }); + expect(found).toMatchObject({ slot: "legacy" }); + }); + + it("raises UnknownShardKey for an unconfigured shard and does not fall back", () => { + const store = build(["a"]); // "b" is not configured + // The route resolves synchronously, so the throw is synchronous (before the promise is built). + expect(() => store.findRun({ friendlyId: generateRunOpsIdV2("b") })).toThrow(UnknownShardKey); + }); +}); diff --git a/internal-packages/run-store/src/runOpsStore.ts b/internal-packages/run-store/src/runOpsStore.ts index 27bd78866d4..8d1c7d5aba4 100644 --- a/internal-packages/run-store/src/runOpsStore.ts +++ b/internal-packages/run-store/src/runOpsStore.ts @@ -40,6 +40,33 @@ import { boundedIn } from "@trigger.dev/database"; const NEW_SHARD: ShardKey = "new"; const LEGACY_SHARD: ShardKey = "legacy"; +/** + * Raised when an id resolves to a shard key that is not configured. NEVER falls back to another + * store — a misconfiguration must fail loud, not misroute silently. Alarmed by ops. + */ +export class UnknownShardKey extends Error { + readonly key: string; + readonly configuredKeys: readonly string[]; + constructor(key: string, configuredKeys: readonly string[]) { + super( + `Unknown run-ops shard key ${JSON.stringify(key)}; configured: [${configuredKeys.join(", ")}]` + ); + this.name = "UnknownShardKey"; + this.key = key; + this.configuredKeys = configuredKeys; + } +} + +type ShardTopology = { + shards: ReadonlyMap; + probeOrder: readonly ShardKey[]; + precedence: readonly ShardKey[]; + idlessRouteShard: ShardKey; + idlessWaitpointShard: ShardKey; + resolveShardKey: (id: string) => ShardKey; + classify?: (id: string) => Residency; +}; + /** * Run-ops routing substrate for the TaskRun-core method group. Implements {@link RunStore} over a * map from shard key to store, selecting one by the residency classifier (`ownerEngine`: run-ops @@ -58,18 +85,24 @@ const LEGACY_SHARD: ShardKey = "legacy"; * each other, so swapping them changes behaviour. */ export class RoutingRunStore implements RunStore { - readonly #shards: ReadonlyMap; + // Not readonly: the compat constructor sets gen-1 defaults, and fromShards() overwrites these + // once (before the instance escapes) via #applyShardTopology. + #shards: ReadonlyMap; // 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. - readonly #probeOrder: readonly ShardKey[]; + #probeOrder: readonly ShardKey[]; // Ascending authority for a merge. The last write wins, so the highest-authority shard wins a // duplicate id. Every merge in this class MUST use this order. - readonly #precedence: readonly ShardKey[]; + #precedence: readonly ShardKey[]; // The two id-less defaults. They differ by role on purpose: a route with no id lands on the // steady-state home, a waitpoint read with no id lands on the legacy store. - readonly #idlessRouteShard: ShardKey; - readonly #idlessWaitpointShard: ShardKey; + #idlessRouteShard: ShardKey; + #idlessWaitpointShard: ShardKey; readonly #classify: (id: string) => Residency; + // The shard that owns an id. Compat: binary over #classify. fromShards: resolveShard, which names + // a gen-2 id's own shard. NEVER throws (resolveShard is total) — an unconfigured key is caught at + // #shardStore, so #routeKeyOrDefault's catch cannot swallow it into a silent legacy read. + #resolveShardKey: (id: string) => ShardKey; // Compat constructor: the two gen-1 stores, keyed by their reserved shard keys. The options type // MUST stay closed — a union arm loosens the excess-property check and retires the @@ -84,6 +117,28 @@ export class RoutingRunStore implements RunStore { this.#idlessRouteShard = NEW_SHARD; this.#idlessWaitpointShard = LEGACY_SHARD; this.#classify = options.classify ?? ownerEngine; + this.#resolveShardKey = (id) => (this.#classify(id) === "NEW" ? NEW_SHARD : LEGACY_SHARD); + } + + // The N-way factory. Builds via the compat constructor (so the closed options type and its + // test-corpus lock are untouched), then installs the shard topology over the gen-1 defaults. + static fromShards(topology: ShardTopology): RoutingRunStore { + const store = new RoutingRunStore({ + new: topology.shards.get(NEW_SHARD)!, + legacy: topology.shards.get(LEGACY_SHARD)!, + classify: topology.classify, + }); + store.#applyShardTopology(topology); + return store; + } + + #applyShardTopology(topology: ShardTopology): void { + this.#shards = topology.shards; + this.#probeOrder = topology.probeOrder; + this.#precedence = topology.precedence; + this.#idlessRouteShard = topology.idlessRouteShard; + this.#idlessWaitpointShard = topology.idlessWaitpointShard; + this.#resolveShardKey = topology.resolveShardKey; } // A routing store spans two databases and has no single primary — routed reads resolve the @@ -108,14 +163,17 @@ export class RoutingRunStore implements RunStore { #shardStore(key: ShardKey): RunStore { const store = this.#shards.get(key); if (store === undefined) { - throw new Error(`RoutingRunStore: no store is configured for shard key "${key}"`); + // The ONLY place an unconfigured key fails. Keep it here, never in #resolveShardKey: + // #routeKeyOrDefault catches resolver throws and downgrades to legacy, which would turn a + // misconfiguration into a silent legacy read. This throw is outside that catch. + throw new UnknownShardKey(key, [...this.#shards.keys()]); } return store; } - // The shard that owns an existing id. Throws only when an injected classifier throws. + // The shard that owns an existing id. Delegates to the installed resolver (see #resolveShardKey). #shardKeyOf(id: string): ShardKey { - return this.#classify(id) === "NEW" ? NEW_SHARD : LEGACY_SHARD; + return this.#resolveShardKey(id); } // An unclassifiable id is treated as LEGACY (probe the control-plane DB rather than drop a From 36a2bf907f306bfefe012851fc7f5416d9289bd2 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:04:18 +0100 Subject: [PATCH 03/31] feat(webapp): add the RUN_OPS_SHARDS zod descriptor validated at boot Includes the cross-field boot refinement requiring RUN_OPS_DATABASE_URL when the shard list is non-empty, since gen-1 v1 ids resolve to the new store permanently. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/env.server.ts | 11 ++ apps/webapp/app/v3/runOpsShards.server.ts | 124 ++++++++++++++++++++++ apps/webapp/test/runOpsShards.test.ts | 71 +++++++++++++ 3 files changed, 206 insertions(+) create mode 100644 apps/webapp/app/v3/runOpsShards.server.ts create mode 100644 apps/webapp/test/runOpsShards.test.ts diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index c9179306124..dcbb751ec15 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { MachinePresetName } from "@trigger.dev/core/v3"; import { BoolEnv } from "./utils/boolEnv"; import { isValidDatabaseUrl } from "./utils/db"; +import { parseRunOpsShards, validateShardListAgainstNewUrl } from "~/v3/runOpsShards.server"; import { isValidRegex } from "./utils/regex"; import { isValidDuration } from "./services/realtime/duration.server"; @@ -310,6 +311,8 @@ const EnvironmentSchema = z RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER: z.string().default("0"), RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER: z.string().default("0"), RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER: z.string().default("0"), + // Gen-2 shard descriptors as a JSON array. Unset/"" -> [] (today). See runOpsShards.server.ts. + RUN_OPS_SHARDS: z.string().optional().transform(parseRunOpsShards), // Control-plane cache relax knobs. Unset -> defaults (DEFAULT_CP_CACHE_TTL_MS / _MAX_ENTRIES). CONTROL_PLANE_CACHE_TTL_MS: z.coerce.number().int().optional(), CONTROL_PLANE_CACHE_MAX_ENTRIES: z.coerce.number().int().optional(), @@ -2467,6 +2470,14 @@ const EnvironmentSchema = z }); } } + if (!validateShardListAgainstNewUrl(env.RUN_OPS_SHARDS, env.RUN_OPS_DATABASE_URL)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["RUN_OPS_SHARDS"], + message: + "RUN_OPS_SHARDS is non-empty but RUN_OPS_DATABASE_URL is unset; a shard requires the gen-1 new store", + }); + } }); export type Environment = z.infer; diff --git a/apps/webapp/app/v3/runOpsShards.server.ts b/apps/webapp/app/v3/runOpsShards.server.ts new file mode 100644 index 00000000000..23c45efa404 --- /dev/null +++ b/apps/webapp/app/v3/runOpsShards.server.ts @@ -0,0 +1,124 @@ +import { z } from "zod"; +import { isValidShardChar } from "@trigger.dev/core/v3/isomorphic"; +import { isValidDatabaseUrl } from "~/utils/db"; + +const KnobsSchema = z + .object({ + writerPoolTimeout: z.number().int().optional(), + writerConnectionTimeout: z.number().int().optional(), + writerDriverAdapter: z.boolean().optional(), + connectionLimit: z.number().int().optional(), + replicaConnectionLimit: z.number().int().optional(), + replicaPoolTimeout: z.number().int().optional(), + replicaConnectionTimeout: z.number().int().optional(), + replicaDriverAdapter: z.boolean().optional(), + transactionMaxWaitMs: z.number().int().optional(), + transactionStartRetryEnabled: z.boolean().optional(), + transactionStartRetryMaxAttempts: z.number().int().optional(), + transactionStartRetryBackoffMinMs: z.number().int().optional(), + transactionStartRetryBackoffMaxMs: z.number().int().optional(), + transactionStartRetryBudgetPerSec: z.number().int().optional(), + transactionStartRetryBudgetBurst: z.number().int().optional(), + }) + .strict(); +export type RunOpsShardKnobs = z.infer; + +const ReplicationSchema = z.object({ + slotName: z.string().min(1), + publicationName: z.string().min(1), + originGeneration: z.number().int().min(2).max(255), +}); + +const DescriptorSchema = z + .object({ + key: z.string().refine(isValidShardChar, "shard key must be a single [a-z0-9] char"), + region: z.string().min(1), + url: z.string().refine(isValidDatabaseUrl, "url is invalid").optional(), + replicaUrl: z.string().refine(isValidDatabaseUrl, "replicaUrl is invalid").optional(), + directUrl: z.string().refine(isValidDatabaseUrl, "directUrl is invalid").optional(), + replication: ReplicationSchema.optional(), + knobs: KnobsSchema.optional(), + aliasOf: z.literal("new").optional(), + }) + .strict() + .superRefine((d, ctx) => { + const hasUrl = d.url !== undefined; + const hasAlias = d.aliasOf !== undefined; + if (hasUrl === hasAlias) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "exactly one of url or aliasOf is required", + }); + } + if (!hasAlias && d.replication === undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "replication is required unless aliasOf is set", + }); + } + }); + +export type RunOpsShardDescriptor = z.infer; + +// Boot-validated transform, in the style of parseMachinePresetCsv. Undefined and "" both mean the +// off state and resolve to []. The undefined guard is load-bearing: an unguarded JSON.parse would +// kill every single-DB boot, which never sets this variable. +export function parseRunOpsShards( + raw: string | undefined, + ctx: z.RefinementCtx +): RunOpsShardDescriptor[] { + if (raw === undefined || raw.trim() === "") return []; + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: "RUN_OPS_SHARDS is not valid JSON" }); + return z.NEVER; + } + + const result = z.array(DescriptorSchema).safeParse(parsed); + if (!result.success) { + for (const issue of result.error.issues) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `RUN_OPS_SHARDS[${issue.path.join(".")}]: ${issue.message}`, + }); + } + return z.NEVER; + } + + const keys = new Set(); + const gens = new Set(); + for (const d of result.data) { + if (keys.has(d.key)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `RUN_OPS_SHARDS: duplicate key ${d.key}`, + }); + return z.NEVER; + } + keys.add(d.key); + if (d.replication) { + if (gens.has(d.replication.originGeneration)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `RUN_OPS_SHARDS: duplicate originGeneration ${d.replication.originGeneration}`, + }); + return z.NEVER; + } + gens.add(d.replication.originGeneration); + } + } + + return result.data; +} + +// A non-empty shard list requires the gen-1 new store, because gen-1 v1 ids resolve to "new" +// forever (append-only). Pure so the boot refinement and its test share one rule. +export function validateShardListAgainstNewUrl( + shards: RunOpsShardDescriptor[], + newUrl: string | undefined +): boolean { + return shards.length === 0 || !!newUrl; +} diff --git a/apps/webapp/test/runOpsShards.test.ts b/apps/webapp/test/runOpsShards.test.ts new file mode 100644 index 00000000000..868ee8fa010 --- /dev/null +++ b/apps/webapp/test/runOpsShards.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { z } from "zod"; +import { parseRunOpsShards, validateShardListAgainstNewUrl } from "~/v3/runOpsShards.server"; + +function run(raw: string | undefined) { + const schema = z.string().optional().transform(parseRunOpsShards); + return schema.safeParse(raw); +} + +const valid = { + key: "a", + region: "us-east-1", + url: "postgres://h/db", + replication: { slotName: "s", publicationName: "p", originGeneration: 2 }, +}; + +describe("parseRunOpsShards", () => { + it("returns [] for undefined", () => { + const r = run(undefined); + expect(r.success && r.data).toEqual([]); + }); + it("returns [] for an empty array literal", () => { + const r = run("[]"); + expect(r.success && r.data).toEqual([]); + }); + it("parses a valid single descriptor", () => { + const r = run(JSON.stringify([valid])); + expect(r.success).toBe(true); + if (r.success) expect(r.data[0].key).toBe("a"); + }); + it("fails on malformed JSON", () => { + expect(run("{not json").success).toBe(false); + }); + it("fails on a multi-char key", () => { + expect(run(JSON.stringify([{ ...valid, key: "ab" }])).success).toBe(false); + }); + it("fails on duplicate keys", () => { + const b = { ...valid, replication: { slotName: "s2", publicationName: "p2", originGeneration: 3 } }; + expect(run(JSON.stringify([valid, b])).success).toBe(false); + }); + it("fails on duplicate origin generations", () => { + const b = { ...valid, key: "b", url: "postgres://h/b" }; + expect(run(JSON.stringify([valid, b])).success).toBe(false); + }); + it("fails when both url and aliasOf are set", () => { + expect(run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db", aliasOf: "new" }])).success).toBe(false); + }); + it("accepts aliasOf without url or replication", () => { + expect(run(JSON.stringify([{ key: "a", region: "x", aliasOf: "new" }])).success).toBe(true); + }); + it("fails on an origin generation below 2 or above 255", () => { + const mk = (g: number) => run(JSON.stringify([{ ...valid, replication: { slotName: "s", publicationName: "p", originGeneration: g } }])); + expect(mk(1).success).toBe(false); + expect(mk(256).success).toBe(false); + }); + it("fails when a non-aliased descriptor omits replication", () => { + expect(run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db" }])).success).toBe(false); + }); +}); + +describe("validateShardListAgainstNewUrl", () => { + it("passes when the list is empty and no new url", () => { + expect(validateShardListAgainstNewUrl([], undefined)).toBe(true); + }); + it("passes when the list is non-empty and new url is set", () => { + expect(validateShardListAgainstNewUrl([valid as never], "postgres://h/new")).toBe(true); + }); + it("fails when the list is non-empty and new url is unset", () => { + expect(validateShardListAgainstNewUrl([valid as never], undefined)).toBe(false); + }); +}); From ae745440ed06da506fb71eca942755c6147e15c2 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:06:12 +0100 Subject: [PATCH 04/31] feat(webapp): resolve per-role run-ops pool knobs in one module Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/v3/runOpsPoolKnobs.server.ts | 91 ++++++++++++++++++++ apps/webapp/test/runOpsPoolKnobs.test.ts | 49 +++++++++++ 2 files changed, 140 insertions(+) create mode 100644 apps/webapp/app/v3/runOpsPoolKnobs.server.ts create mode 100644 apps/webapp/test/runOpsPoolKnobs.test.ts diff --git a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts new file mode 100644 index 00000000000..f5da3b7f2b2 --- /dev/null +++ b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts @@ -0,0 +1,91 @@ +import { env } from "~/env.server"; +import type { RunOpsShardKnobs } from "~/v3/runOpsShards.server"; + +// Pool configuration for one run-ops client, resolved at the app boundary (IoC). Every value is a +// number/boolean/string the generic buildWriterClient/buildReplicaClient consumes directly. Kept +// separate from db.server (which ~156 tests mock wholesale) so a new export breaks no mock. +export type ResolvedPoolKnobs = { + writerPoolTimeout: number; + writerConnectionTimeout: number; + writerDriverAdapter: boolean; + connectionLimit: number; + replicaConnectionLimit: number; + replicaPoolTimeout: number; + replicaConnectionTimeout: number; + replicaDriverAdapter: boolean; + // stdoutLogs and label are role constants, never overridable by a descriptor. The run-ops + // builders had no stdout arms and their own log labels; carrying these keeps the merge inert. + stdoutLogs: boolean; + label: string; +}; + +type Role = "new" | "legacy"; + +// Resolve the pool knobs for a run-ops role, reproducing today's builder expressions exactly. +// descriptorKnobs (gen-2 shards only) override the pool fields; stdoutLogs and label stay fixed. +// Transaction resilience is a SEPARATE mechanism (resolveTransactionResilience) and is not here. +export function resolveRunOpsPoolKnobs( + role: Role, + descriptorKnobs?: RunOpsShardKnobs +): ResolvedPoolKnobs { + const k = descriptorKnobs; + + if (role === "legacy") { + return { + writerPoolTimeout: + k?.writerPoolTimeout ?? + env.RUN_OPS_LEGACY_DATABASE_WRITER_POOL_TIMEOUT ?? + env.DATABASE_POOL_TIMEOUT, + writerConnectionTimeout: + k?.writerConnectionTimeout ?? + env.RUN_OPS_LEGACY_DATABASE_WRITER_CONNECTION_TIMEOUT ?? + env.DATABASE_CONNECTION_TIMEOUT, + writerDriverAdapter: + k?.writerDriverAdapter ?? env.RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER === "1", + connectionLimit: k?.connectionLimit ?? env.DATABASE_CONNECTION_LIMIT, + replicaConnectionLimit: k?.replicaConnectionLimit ?? env.DATABASE_CONNECTION_LIMIT, + replicaPoolTimeout: + k?.replicaPoolTimeout ?? + env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? + env.DATABASE_POOL_TIMEOUT, + replicaConnectionTimeout: + k?.replicaConnectionTimeout ?? + env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? + env.DATABASE_CONNECTION_TIMEOUT, + replicaDriverAdapter: + k?.replicaDriverAdapter ?? env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1", + stdoutLogs: true, + label: "legacy run-ops", + }; + } + + return { + writerPoolTimeout: + k?.writerPoolTimeout ?? + env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? + env.DATABASE_POOL_TIMEOUT, + writerConnectionTimeout: + k?.writerConnectionTimeout ?? + env.RUN_OPS_DATABASE_WRITER_CONNECTION_TIMEOUT ?? + env.DATABASE_CONNECTION_TIMEOUT, + writerDriverAdapter: + k?.writerDriverAdapter ?? env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1", + connectionLimit: k?.connectionLimit ?? env.DATABASE_CONNECTION_LIMIT, + replicaConnectionLimit: + k?.replicaConnectionLimit ?? + env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? + env.DATABASE_CONNECTION_LIMIT, + replicaPoolTimeout: + k?.replicaPoolTimeout ?? + env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? + env.DATABASE_POOL_TIMEOUT, + replicaConnectionTimeout: + k?.replicaConnectionTimeout ?? + env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? + env.DATABASE_CONNECTION_TIMEOUT, + replicaDriverAdapter: + k?.replicaDriverAdapter ?? env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1", + stdoutLogs: false, + label: "run-ops", + }; +} diff --git a/apps/webapp/test/runOpsPoolKnobs.test.ts b/apps/webapp/test/runOpsPoolKnobs.test.ts new file mode 100644 index 00000000000..a567390ab5d --- /dev/null +++ b/apps/webapp/test/runOpsPoolKnobs.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { resolveRunOpsPoolKnobs } from "~/v3/runOpsPoolKnobs.server"; +import { env } from "~/env.server"; + +describe("resolveRunOpsPoolKnobs", () => { + it("new role: reproduces the run-ops builder expressions and stdoutLogs is false", () => { + const k = resolveRunOpsPoolKnobs("new"); + expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); + expect(k.replicaConnectionLimit).toBe( + env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT + ); + expect(k.writerPoolTimeout).toBe( + env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT + ); + expect(k.replicaPoolTimeout).toBe( + env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT + ); + expect(k.writerDriverAdapter).toBe(env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1"); + expect(k.replicaDriverAdapter).toBe(env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1"); + expect(k.stdoutLogs).toBe(false); + }); + + it("legacy role: uses RUN_OPS_LEGACY_* timeouts, generic connection limit, and stdoutLogs true", () => { + const k = resolveRunOpsPoolKnobs("legacy"); + expect(k.stdoutLogs).toBe(true); + expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); + expect(k.replicaConnectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); + expect(k.writerPoolTimeout).toBe( + env.RUN_OPS_LEGACY_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT + ); + expect(k.replicaPoolTimeout).toBe( + env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT + ); + expect(k.writerDriverAdapter).toBe(env.RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER === "1"); + expect(k.replicaDriverAdapter).toBe(env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1"); + }); + + it("a descriptor knob overrides its field", () => { + const k = resolveRunOpsPoolKnobs("new", { connectionLimit: 7, writerDriverAdapter: true }); + expect(k.connectionLimit).toBe(7); + expect(k.writerDriverAdapter).toBe(true); + }); + + it("descriptor knobs never override the role's stdoutLogs or label", () => { + const k = resolveRunOpsPoolKnobs("new", { connectionLimit: 7 }); + expect(k.stdoutLogs).toBe(false); + expect(k.label).toContain("run-ops"); + }); +}); From 42d10b0eb971240ec24353eaf8c4559092045619 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:18:38 +0100 Subject: [PATCH 05/31] refactor(webapp): collapse the two run-ops client builders into one factory Dedupes buildRunOpsWriterClient/buildRunOpsReplicaClient into a single buildRunOpsClient parameterized by role and the resolved pool knobs. The control-plane builders (buildWriterClient/buildReplicaClient) are a separate path and stay untouched. Every resolved value matches the former builders, so split-on deployments are byte-identical. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 198 ++++++------------- apps/webapp/app/v3/runOpsPoolKnobs.server.ts | 16 +- apps/webapp/test/runOpsPoolKnobs.test.ts | 12 +- 3 files changed, 62 insertions(+), 164 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index a69c83cd375..b2831de4c1e 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -31,6 +31,7 @@ import { assertSplitRealtimeInterlock, } from "./v3/runOpsMigration/splitMode.server"; import { computeRunOpsSplitReadEnabled } from "./v3/runOpsMigration/runOpsSplitReadGate"; +import { resolveRunOpsPoolKnobs } from "./v3/runOpsPoolKnobs.server"; import { assertControlPlaneCoresidencyAdvisory } from "./v3/runOpsMigration/controlPlaneCoresidencySentinel.server"; import { DATASOURCE_CONTEXT_KEY, startActiveSpan } from "./v3/tracer.server"; import { @@ -376,6 +377,8 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { ); } + const newPoolKnobs = resolveRunOpsPoolKnobs("new"); + return selectRunOpsTopology( { splitEnabled, @@ -392,10 +395,14 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { captureInfraErrorsRunOps( tagDatasourceRunOps( "run-ops-writer", - buildRunOpsWriterClient({ + buildRunOpsClient({ url, clientType, - useDriverAdapter: env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1", + role: "writer", + connectionLimit: newPoolKnobs.connectionLimit, + poolTimeout: newPoolKnobs.writerPoolTimeout, + connectTimeout: newPoolKnobs.writerConnectionTimeout, + useDriverAdapter: newPoolKnobs.writerDriverAdapter, }) ) ), @@ -409,10 +416,14 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { captureInfraErrorsRunOps( tagDatasourceRunOps( "run-ops-replica", - buildRunOpsReplicaClient({ + buildRunOpsClient({ url, clientType, - useDriverAdapter: env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1", + role: "replica", + connectionLimit: newPoolKnobs.replicaConnectionLimit, + poolTimeout: newPoolKnobs.replicaPoolTimeout, + connectTimeout: newPoolKnobs.replicaConnectionTimeout, + useDriverAdapter: newPoolKnobs.replicaDriverAdapter, }) ) ) @@ -924,161 +935,62 @@ export function buildReplicaClient({ return replicaClient; } -function buildRunOpsWriterClient({ +// One factory for the run-ops writer and replica clients, backed by the dedicated RunOpsPrismaClient +// (a separately generated Prisma package). Parameterized by role and the resolved pool knobs, so a +// gen-1 new store and every gen-2 shard share this single builder. The control-plane builders +// (buildWriterClient/buildReplicaClient) are a DIFFERENT path and are untouched — this reuses only +// the shared low-level helpers (buildPrismaConnectionUrl, buildDriverAdapterPool). +function buildRunOpsClient({ url, clientType, + role, + connectionLimit, + poolTimeout, + connectTimeout, useDriverAdapter = false, }: { url: string; clientType: string; + role: "writer" | "replica"; + connectionLimit: number; + poolTimeout: number; + connectTimeout: number; useDriverAdapter?: boolean; }): RunOpsPrismaClient { - const databaseUrl = buildPrismaConnectionUrl(url, { - connectionLimit: env.DATABASE_CONNECTION_LIMIT.toString(), - poolTimeout: (env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT).toString(), - connectTimeout: ( - env.RUN_OPS_DATABASE_WRITER_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT - ).toString(), - applicationName: env.SERVICE_NAME, - }); - - console.log( - `🔌 setting up run-ops prisma client to ${redactUrlSecrets(databaseUrl)}${ - useDriverAdapter ? " (pg driver adapter)" : "" - }` - ); - - const driverPool = useDriverAdapter - ? buildDriverAdapterPool( - url, - clientType, - env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, - env.DATABASE_CONNECTION_LIMIT - ) - : undefined; - - const client = driverPool - ? new RunOpsPrismaClient({ - adapter: driverPool.adapter, - log: [ - { emit: "event", level: "error" }, - { emit: "event", level: "info" }, - { emit: "event", level: "warn" }, - ...((process.env.VERBOSE_PRISMA_LOGS === "1" || - process.env.VERY_SLOW_QUERY_THRESHOLD_MS !== undefined - ? [{ emit: "event", level: "query" }] - : []) as { emit: "event"; level: "query" }[]), - ], - }) - : new RunOpsPrismaClient({ - datasources: { db: { url: databaseUrl.href } }, - log: [ - { emit: "event", level: "error" }, - { emit: "event", level: "info" }, - { emit: "event", level: "warn" }, - ...((process.env.VERBOSE_PRISMA_LOGS === "1" || - process.env.VERY_SLOW_QUERY_THRESHOLD_MS !== undefined - ? [{ emit: "event", level: "query" }] - : []) as { emit: "event"; level: "query" }[]), - ], - }); - - registerDatabaseMetricsSource( - driverPool - ? { - clientType, - usesDriverAdapter: true, - client, - pool: driverPool.pool, - poolCounters: driverPool.poolCounters, - } - : { clientType, usesDriverAdapter: false, client } - ); - - if (process.env.PRISMA_LOG_TO_STDOUT !== "1") { - client.$on("info", (log) => logger.info("RunOpsPrismaClient info", { clientType, event: log })); - client.$on("warn", (log) => logger.warn("RunOpsPrismaClient warn", { clientType, event: log })); - client.$on("error", (log) => - logger.error("RunOpsPrismaClient error", { clientType, event: log, ignoreError: true }) - ); - } - - client.$on("query", (log) => queryPerformanceMonitor.onQuery("writer", log)); + const isWriter = role === "writer"; + const setupLabel = isWriter ? "run-ops prisma client" : "run-ops read replica connection"; + const connectedLabel = isWriter ? "run-ops prisma client connected" : "run-ops read replica connected"; - const connectPromise = client.$connect(); - if (env.NODE_ENV === "test") { - connectPromise.catch((error) => { - logger.warn("Failed to eagerly connect run-ops prisma client (writer)", { error }); - }); - } - - console.log(`🔌 run-ops prisma client connected`); - - return client; -} - -function buildRunOpsReplicaClient({ - url, - clientType, - useDriverAdapter = false, -}: { - url: string; - clientType: string; - useDriverAdapter?: boolean; -}): RunOpsPrismaClient { - const replicaUrl = buildPrismaConnectionUrl(url, { - connectionLimit: ( - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT - ).toString(), - poolTimeout: ( - env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT - ).toString(), - connectTimeout: ( - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT - ).toString(), + const connectionUrl = buildPrismaConnectionUrl(url, { + connectionLimit: connectionLimit.toString(), + poolTimeout: poolTimeout.toString(), + connectTimeout: connectTimeout.toString(), applicationName: env.SERVICE_NAME, }); console.log( - `🔌 setting up run-ops read replica connection to ${redactUrlSecrets(replicaUrl)}${ + `🔌 setting up ${setupLabel} to ${redactUrlSecrets(connectionUrl)}${ useDriverAdapter ? " (pg driver adapter)" : "" }` ); + const log = [ + { emit: "event", level: "error" }, + { emit: "event", level: "info" }, + { emit: "event", level: "warn" }, + ...((process.env.VERBOSE_PRISMA_LOGS === "1" || + process.env.VERY_SLOW_QUERY_THRESHOLD_MS !== undefined + ? [{ emit: "event", level: "query" }] + : []) as { emit: "event"; level: "query" }[]), + ] as const; + const driverPool = useDriverAdapter - ? buildDriverAdapterPool( - url, - clientType, - env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT - ) + ? buildDriverAdapterPool(url, clientType, poolTimeout, connectionLimit) : undefined; const client = driverPool - ? new RunOpsPrismaClient({ - adapter: driverPool.adapter, - log: [ - { emit: "event", level: "error" }, - { emit: "event", level: "info" }, - { emit: "event", level: "warn" }, - ...((process.env.VERBOSE_PRISMA_LOGS === "1" || - process.env.VERY_SLOW_QUERY_THRESHOLD_MS !== undefined - ? [{ emit: "event", level: "query" }] - : []) as { emit: "event"; level: "query" }[]), - ], - }) - : new RunOpsPrismaClient({ - datasources: { db: { url: replicaUrl.href } }, - log: [ - { emit: "event", level: "error" }, - { emit: "event", level: "info" }, - { emit: "event", level: "warn" }, - ...((process.env.VERBOSE_PRISMA_LOGS === "1" || - process.env.VERY_SLOW_QUERY_THRESHOLD_MS !== undefined - ? [{ emit: "event", level: "query" }] - : []) as { emit: "event"; level: "query" }[]), - ], - }); + ? new RunOpsPrismaClient({ adapter: driverPool.adapter, log: [...log] }) + : new RunOpsPrismaClient({ datasources: { db: { url: connectionUrl.href } }, log: [...log] }); registerDatabaseMetricsSource( driverPool @@ -1096,20 +1008,22 @@ function buildRunOpsReplicaClient({ client.$on("info", (log) => logger.info("RunOpsPrismaClient info", { clientType, event: log })); client.$on("warn", (log) => logger.warn("RunOpsPrismaClient warn", { clientType, event: log })); client.$on("error", (log) => - logger.error("RunOpsPrismaClient error", { clientType, event: log }) + // The writer bridges P2002 -> 422 at the store boundary, so its infra errors are logged once + // there (ignoreError). Replica errors are not on that write path, so they log normally. + logger.error("RunOpsPrismaClient error", { clientType, event: log, ...(isWriter ? { ignoreError: true } : {}) }) ); } - client.$on("query", (log) => queryPerformanceMonitor.onQuery("replica", log)); + client.$on("query", (log) => queryPerformanceMonitor.onQuery(role, log)); const connectPromise = client.$connect(); if (env.NODE_ENV === "test") { connectPromise.catch((error) => { - logger.warn("Failed to eagerly connect run-ops prisma client (replica)", { error }); + logger.warn(`Failed to eagerly connect run-ops prisma client (${role})`, { error }); }); } - console.log(`🔌 run-ops read replica connected`); + console.log(`🔌 ${connectedLabel}`); return client; } diff --git a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts index f5da3b7f2b2..f7bb7b72cf2 100644 --- a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts +++ b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts @@ -1,9 +1,9 @@ import { env } from "~/env.server"; import type { RunOpsShardKnobs } from "~/v3/runOpsShards.server"; -// Pool configuration for one run-ops client, resolved at the app boundary (IoC). Every value is a -// number/boolean/string the generic buildWriterClient/buildReplicaClient consumes directly. Kept -// separate from db.server (which ~156 tests mock wholesale) so a new export breaks no mock. +// Pool configuration for one run-ops store (writer + replica), resolved at the app boundary (IoC). +// Every value reproduces today's run-ops builder expressions. Kept separate from db.server (which +// ~156 tests mock wholesale) so a new export breaks no mock. export type ResolvedPoolKnobs = { writerPoolTimeout: number; writerConnectionTimeout: number; @@ -13,16 +13,12 @@ export type ResolvedPoolKnobs = { replicaPoolTimeout: number; replicaConnectionTimeout: number; replicaDriverAdapter: boolean; - // stdoutLogs and label are role constants, never overridable by a descriptor. The run-ops - // builders had no stdout arms and their own log labels; carrying these keeps the merge inert. - stdoutLogs: boolean; - label: string; }; type Role = "new" | "legacy"; // Resolve the pool knobs for a run-ops role, reproducing today's builder expressions exactly. -// descriptorKnobs (gen-2 shards only) override the pool fields; stdoutLogs and label stay fixed. +// descriptorKnobs (gen-2 shards only) override the pool fields. // Transaction resilience is a SEPARATE mechanism (resolveTransactionResilience) and is not here. export function resolveRunOpsPoolKnobs( role: Role, @@ -54,8 +50,6 @@ export function resolveRunOpsPoolKnobs( env.DATABASE_CONNECTION_TIMEOUT, replicaDriverAdapter: k?.replicaDriverAdapter ?? env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1", - stdoutLogs: true, - label: "legacy run-ops", }; } @@ -85,7 +79,5 @@ export function resolveRunOpsPoolKnobs( env.DATABASE_CONNECTION_TIMEOUT, replicaDriverAdapter: k?.replicaDriverAdapter ?? env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1", - stdoutLogs: false, - label: "run-ops", }; } diff --git a/apps/webapp/test/runOpsPoolKnobs.test.ts b/apps/webapp/test/runOpsPoolKnobs.test.ts index a567390ab5d..e281b8ce19e 100644 --- a/apps/webapp/test/runOpsPoolKnobs.test.ts +++ b/apps/webapp/test/runOpsPoolKnobs.test.ts @@ -3,7 +3,7 @@ import { resolveRunOpsPoolKnobs } from "~/v3/runOpsPoolKnobs.server"; import { env } from "~/env.server"; describe("resolveRunOpsPoolKnobs", () => { - it("new role: reproduces the run-ops builder expressions and stdoutLogs is false", () => { + it("new role: reproduces the run-ops builder expressions", () => { const k = resolveRunOpsPoolKnobs("new"); expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); expect(k.replicaConnectionLimit).toBe( @@ -17,12 +17,10 @@ describe("resolveRunOpsPoolKnobs", () => { ); expect(k.writerDriverAdapter).toBe(env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1"); expect(k.replicaDriverAdapter).toBe(env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1"); - expect(k.stdoutLogs).toBe(false); }); - it("legacy role: uses RUN_OPS_LEGACY_* timeouts, generic connection limit, and stdoutLogs true", () => { + it("legacy role: uses RUN_OPS_LEGACY_* timeouts and the generic connection limit", () => { const k = resolveRunOpsPoolKnobs("legacy"); - expect(k.stdoutLogs).toBe(true); expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); expect(k.replicaConnectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); expect(k.writerPoolTimeout).toBe( @@ -40,10 +38,4 @@ describe("resolveRunOpsPoolKnobs", () => { expect(k.connectionLimit).toBe(7); expect(k.writerDriverAdapter).toBe(true); }); - - it("descriptor knobs never override the role's stdoutLogs or label", () => { - const k = resolveRunOpsPoolKnobs("new", { connectionLimit: 7 }); - expect(k.stdoutLogs).toBe(false); - expect(k.label).toContain("run-ops"); - }); }); From 2222f4c3dde0079d90c02124cab78ae5293431c9 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:19:27 +0100 Subject: [PATCH 06/31] feat(webapp): export per-shard transaction resilience with an own budget per pool Co-Authored-By: Claude Opus 4.8 --- .../webapp/app/v3/transactionResilience.server.ts | 7 +++++-- apps/webapp/test/transactionResilience.test.ts | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 apps/webapp/test/transactionResilience.test.ts diff --git a/apps/webapp/app/v3/transactionResilience.server.ts b/apps/webapp/app/v3/transactionResilience.server.ts index ae5678c987c..c3c7eb7d2f3 100644 --- a/apps/webapp/app/v3/transactionResilience.server.ts +++ b/apps/webapp/app/v3/transactionResilience.server.ts @@ -17,8 +17,11 @@ export type TransactionResilienceConfig = { startRetry: TransactionStartRetryConfig; }; -function resolveTransactionResilience( - pool: "control-plane" | "run-ops" | "run-ops-legacy", +// Exported so the topology singleton can build a per-shard config (each call creates its OWN +// TokenBucketRetryBudget, so one shard's retry storm cannot drain another's). `pool` is a free +// string — it only labels a log line, never keys any behaviour. +export function resolveTransactionResilience( + pool: string, overrides: { maxWaitMs?: number; enabled?: boolean; diff --git a/apps/webapp/test/transactionResilience.test.ts b/apps/webapp/test/transactionResilience.test.ts new file mode 100644 index 00000000000..575d059b11c --- /dev/null +++ b/apps/webapp/test/transactionResilience.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from "vitest"; +import { resolveTransactionResilience } from "~/v3/transactionResilience.server"; + +describe("resolveTransactionResilience per-shard", () => { + it("builds a distinct budget per call, so one shard's storm cannot drain another's", () => { + const a = resolveTransactionResilience("run-ops-shard-a", {}); + const b = resolveTransactionResilience("run-ops-shard-b", {}); + expect(a.startRetry.budget).not.toBe(b.startRetry.budget); + }); + + it("accepts an arbitrary pool label and honours a maxWait override", () => { + expect(() => resolveTransactionResilience("run-ops-shard-z", { maxWaitMs: 1234 })).not.toThrow(); + expect(resolveTransactionResilience("run-ops-shard-z", { maxWaitMs: 1234 }).maxWait).toBe(1234); + }); +}); From 432d7f5ff1e6ff12c6e039700e051acd82e3f980 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:24:12 +0100 Subject: [PATCH 07/31] feat(webapp): build one run-ops client pair per shard descriptor selectRunOpsTopology gains a shard loop and returns a keyed shard map. An aliasOf:"new" descriptor reuses the new store's clients by reference and opens no pool. Each real shard gets its own resilience budget and the new-role pool knobs merged with its per-shard overrides. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 96 +++++++++++++++++-- .../app/v3/transactionResilience.server.ts | 38 ++++++++ apps/webapp/test/runOpsDbTopology.test.ts | 51 ++++++++++ 3 files changed, 178 insertions(+), 7 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index b2831de4c1e..52d55f2773a 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -32,6 +32,7 @@ import { } from "./v3/runOpsMigration/splitMode.server"; import { computeRunOpsSplitReadEnabled } from "./v3/runOpsMigration/runOpsSplitReadGate"; import { resolveRunOpsPoolKnobs } from "./v3/runOpsPoolKnobs.server"; +import { resolveShardResilience } from "./v3/transactionResilience.server"; import { assertControlPlaneCoresidencyAdvisory } from "./v3/runOpsMigration/controlPlaneCoresidencySentinel.server"; import { DATASOURCE_CONTEXT_KEY, startActiveSpan } from "./v3/tracer.server"; import { @@ -276,10 +277,19 @@ export const webhookReplica: WebhookReplicaDatabase = singleton("webhookReplica" type RunOpsClients = { writer: PrismaClient; replica: PrismaReplicaClient }; type NewRunOpsClients = { writer: RunOpsPrismaClient; replica: RunOpsPrismaClient }; +export type ShardTopologyDescriptor = { + key: string; + url?: string; + replicaUrl?: string; + aliasOf?: "new"; +}; export type RunOpsTopology = { newRunOps: NewRunOpsClients; legacyRunOps: RunOpsClients; controlPlane: RunOpsClients; + // One client pair per gen-2 shard descriptor. Empty unless RUN_OPS_SHARDS is configured. An + // aliasOf:"new" descriptor maps to the newRunOps pair BY REFERENCE (no new pool). + shards: Map; }; export type SelectRunOpsTopologyConfig = { splitEnabled: boolean; @@ -289,6 +299,7 @@ export type SelectRunOpsTopologyConfig = { newReplicaUrl?: string; // When true, legacy reuses the control-plane client instead of opening its own pool. Defaults to false. legacySharesControlPlane?: boolean; + shards?: ShardTopologyDescriptor[]; }; export type RunOpsClientBuilders = { controlPlane: RunOpsClients; @@ -298,6 +309,10 @@ export type RunOpsClientBuilders = { // RunOpsPrismaClient double-cast needed): the legacy DB carries the full control-plane schema. buildLegacyWriter: (url: string, clientType: string) => PrismaClient; buildLegacyReplica: (url: string, clientType: string) => PrismaReplicaClient; + // Receive the whole descriptor so the singleton can resolve per-shard knobs and resilience by key. + // Optional so the existing test literals (which build no shards) need no change. + buildShardWriter?: (shard: ShardTopologyDescriptor) => RunOpsPrismaClient; + buildShardReplica?: (shard: ShardTopologyDescriptor) => RunOpsPrismaClient; }; // Pure run-ops client selector. No env, no isSplitEnabled() — those @@ -316,11 +331,11 @@ export function selectRunOpsTopology( }; if (!config.splitEnabled) { - return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane }; + return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane, shards: new Map() }; } if (!config.legacyUrl || !config.newUrl) { - return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane }; + return { newRunOps: cpFallback, legacyRunOps: controlPlane, controlPlane, shards: new Map() }; } // Same-DB legacy reuses the control-plane pool; only build a separate pool once the DSNs diverge. @@ -339,12 +354,28 @@ export function selectRunOpsTopology( const newReplica: RunOpsPrismaClient = config.newReplicaUrl ? builders.buildNewReplica(config.newReplicaUrl, "run-ops-replica") : newWriter; + const newRunOps: NewRunOpsClients = { writer: newWriter, replica: newReplica }; + + const shards = new Map(); + for (const shard of config.shards ?? []) { + if (shard.aliasOf === "new") { + // Aliased: share the new store's clients by reference. No builder, no new pool — the soak path. + shards.set(shard.key, newRunOps); + continue; + } + if (!shard.url || !builders.buildShardWriter || !builders.buildShardReplica) { + throw new Error( + `selectRunOpsTopology: shard "${shard.key}" needs a url and shard builders when not aliased` + ); + } + const shardWriter = builders.buildShardWriter(shard); + const shardReplica: RunOpsPrismaClient = shard.replicaUrl + ? builders.buildShardReplica(shard) + : shardWriter; + shards.set(shard.key, { writer: shardWriter, replica: shardReplica }); + } - return { - newRunOps: { writer: newWriter, replica: newReplica }, - legacyRunOps, - controlPlane, - }; + return { newRunOps, legacyRunOps, controlPlane, shards }; } // The env-bound run-ops topology singleton. The split decision uses @@ -378,6 +409,7 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { } const newPoolKnobs = resolveRunOpsPoolKnobs("new"); + const shardDescriptorsByKey = new Map(env.RUN_OPS_SHARDS.map((d) => [d.key, d])); return selectRunOpsTopology( { @@ -387,6 +419,12 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { newUrl, newReplicaUrl: env.RUN_OPS_DATABASE_READ_REPLICA_URL, legacySharesControlPlane, + shards: env.RUN_OPS_SHARDS.map((d) => ({ + key: d.key, + url: d.url, + replicaUrl: d.replicaUrl, + aliasOf: d.aliasOf, + })), }, { controlPlane: { writer: prisma, replica: $replica }, @@ -461,6 +499,50 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { ) ) ), + // A gen-2 shard is a dedicated run-ops DB, so it mirrors buildNewWriter/buildNewReplica: same + // client class, same wrapper stack, its OWN resilience budget, and the "new"-role pool knobs + // merged with the descriptor's per-shard overrides. Shards share the run-ops datasource tag. + buildShardWriter: (shard) => { + const descriptor = shardDescriptorsByKey.get(shard.key); + const knobs = resolveRunOpsPoolKnobs("new", descriptor?.knobs); + return registerTransactionResilience( + captureInfraErrorsRunOps( + tagDatasourceRunOps( + "run-ops-writer", + buildRunOpsClient({ + url: shard.url!, + clientType: `run-ops-shard-${shard.key}-writer`, + role: "writer", + connectionLimit: knobs.connectionLimit, + poolTimeout: knobs.writerPoolTimeout, + connectTimeout: knobs.writerConnectionTimeout, + useDriverAdapter: knobs.writerDriverAdapter, + }) + ) + ), + resolveShardResilience(shard.key, descriptor?.knobs) + ); + }, + buildShardReplica: (shard) => { + const descriptor = shardDescriptorsByKey.get(shard.key); + const knobs = resolveRunOpsPoolKnobs("new", descriptor?.knobs); + return markReadReplicaClient( + captureInfraErrorsRunOps( + tagDatasourceRunOps( + "run-ops-replica", + buildRunOpsClient({ + url: shard.replicaUrl!, + clientType: `run-ops-shard-${shard.key}-replica`, + role: "replica", + connectionLimit: knobs.replicaConnectionLimit, + poolTimeout: knobs.replicaPoolTimeout, + connectTimeout: knobs.replicaConnectionTimeout, + useDriverAdapter: knobs.replicaDriverAdapter, + }) + ) + ) + ); + }, } ); }); diff --git a/apps/webapp/app/v3/transactionResilience.server.ts b/apps/webapp/app/v3/transactionResilience.server.ts index c3c7eb7d2f3..eabde6691d8 100644 --- a/apps/webapp/app/v3/transactionResilience.server.ts +++ b/apps/webapp/app/v3/transactionResilience.server.ts @@ -67,6 +67,44 @@ export const runOpsTransactionResilience = resolveTransactionResilience("run-ops budgetBurst: env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_BUDGET_BURST, }); +// A gen-2 shard's resilience. Defaults to the RUN_OPS_DATABASE_TRANSACTION_* values (so a shard with +// no overrides matches the gen-1 new store), then applies the descriptor's per-shard overrides. Each +// call builds its OWN budget, so a storm on one shard cannot drain another's. +export function resolveShardResilience( + key: string, + overrides?: { + transactionMaxWaitMs?: number; + transactionStartRetryEnabled?: boolean; + transactionStartRetryMaxAttempts?: number; + transactionStartRetryBackoffMinMs?: number; + transactionStartRetryBackoffMaxMs?: number; + transactionStartRetryBudgetPerSec?: number; + transactionStartRetryBudgetBurst?: number; + } +): TransactionResilienceConfig { + return resolveTransactionResilience(`run-ops-shard-${key}`, { + maxWaitMs: overrides?.transactionMaxWaitMs ?? env.RUN_OPS_DATABASE_TRANSACTION_MAX_WAIT_MS, + enabled: + overrides?.transactionStartRetryEnabled ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_ENABLED, + maxAttempts: + overrides?.transactionStartRetryMaxAttempts ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_MAX_ATTEMPTS, + backoffMinMs: + overrides?.transactionStartRetryBackoffMinMs ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_BACKOFF_MIN_MS, + backoffMaxMs: + overrides?.transactionStartRetryBackoffMaxMs ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_BACKOFF_MAX_MS, + budgetPerSec: + overrides?.transactionStartRetryBudgetPerSec ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_BUDGET_PER_SEC, + budgetBurst: + overrides?.transactionStartRetryBudgetBurst ?? + env.RUN_OPS_DATABASE_TRANSACTION_START_RETRY_BUDGET_BURST, + }); +} + export const runOpsLegacyTransactionResilience = resolveTransactionResilience("run-ops-legacy", { maxWaitMs: env.RUN_OPS_LEGACY_DATABASE_TRANSACTION_MAX_WAIT_MS, enabled: env.RUN_OPS_LEGACY_DATABASE_TRANSACTION_START_RETRY_ENABLED, diff --git a/apps/webapp/test/runOpsDbTopology.test.ts b/apps/webapp/test/runOpsDbTopology.test.ts index 8890fdbb662..f6895a03d29 100644 --- a/apps/webapp/test/runOpsDbTopology.test.ts +++ b/apps/webapp/test/runOpsDbTopology.test.ts @@ -142,6 +142,57 @@ describe("selectRunOpsTopology (pure)", () => { expect(topo.legacyRunOps.replica).toBe(legacyWriter); expect(buildLegacyReplica).not.toHaveBeenCalled(); }); + + const baseSplit = { + splitEnabled: true, + legacyUrl: "postgres://legacy", + newUrl: "postgres://new", + }; + const baseBuilders = () => ({ + controlPlane: cp, + buildNewWriter: vi.fn().mockReturnValue({ tag: "nw" } as any), + buildNewReplica: vi.fn().mockReturnValue({ tag: "nr" } as any), + buildLegacyWriter: vi.fn().mockReturnValue({ tag: "lw" } as any), + buildLegacyReplica: vi.fn().mockReturnValue({ tag: "lr" } as any), + }); + + it("no descriptors: the shards map is empty", () => { + const topo = selectRunOpsTopology(baseSplit, baseBuilders()); + expect(topo.shards.size).toBe(0); + }); + + it("two descriptors: two shard client pairs, each built once", () => { + const buildShardWriter = vi.fn((s: any) => ({ tag: `w:${s.key}` }) as any); + const buildShardReplica = vi.fn((s: any) => ({ tag: `r:${s.key}` }) as any); + const topo = selectRunOpsTopology( + { + ...baseSplit, + shards: [ + { key: "a", url: "postgres://a", replicaUrl: "postgres://a-r" }, + { key: "b", url: "postgres://b" }, + ], + }, + { ...baseBuilders(), buildShardWriter, buildShardReplica } + ); + expect(topo.shards.size).toBe(2); + expect(topo.shards.get("a")!.writer).toEqual({ tag: "w:a" }); + // b has no replicaUrl, so its replica falls back to its writer (buildShardReplica not called for b). + expect(topo.shards.get("b")!.replica).toEqual({ tag: "w:b" }); + expect(buildShardWriter).toHaveBeenCalledTimes(2); + expect(buildShardReplica).toHaveBeenCalledTimes(1); + }); + + it("an alias descriptor reuses newRunOps by reference and calls no shard builder", () => { + const buildShardWriter = vi.fn(); + const buildShardReplica = vi.fn(); + const topo = selectRunOpsTopology( + { ...baseSplit, shards: [{ key: "a", aliasOf: "new" }] }, + { ...baseBuilders(), buildShardWriter, buildShardReplica } + ); + expect(topo.shards.get("a")).toBe(topo.newRunOps); + expect(buildShardWriter).not.toHaveBeenCalled(); + expect(buildShardReplica).not.toHaveBeenCalled(); + }); }); describe("sameDatabaseTarget", () => { From f908f1460c2734751cbca717b333a542be7fef1c Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:28:25 +0100 Subject: [PATCH 08/31] feat(webapp): build N dedicated stores and the keyed router, and log the shard table at boot buildRunStore now produces one dedicated store per shard descriptor and the N-way router via RoutingRunStore.fromShards, keeping the two-store compat router when no shards are configured. The topology singleton logs the resolved shard table (key, address fingerprint, role) only when RUN_OPS_SHARDS is non-empty, so the unset case adds no output. The fingerprint is an address, never an identity claim. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 45 +++++++++++++ apps/webapp/app/v3/runStore.server.ts | 67 +++++++++++++++++-- apps/webapp/test/runOpsShardBootTable.test.ts | 34 ++++++++++ apps/webapp/test/runStoreShardWiring.test.ts | 44 ++++++++++++ 4 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 apps/webapp/test/runOpsShardBootTable.test.ts create mode 100644 apps/webapp/test/runStoreShardWiring.test.ts diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 52d55f2773a..b67c3a9b4aa 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -411,6 +411,14 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => { const newPoolKnobs = resolveRunOpsPoolKnobs("new"); const shardDescriptorsByKey = new Map(env.RUN_OPS_SHARDS.map((d) => [d.key, d])); + // Boot table: emit ONLY when shards are configured, so the inert (RUN_OPS_SHARDS unset) merge adds + // no new log output. The fingerprint is an address, not an identity claim (see runOpsAddressFingerprint). + if (env.RUN_OPS_SHARDS.length > 0) { + logger.info("run-ops shard topology (fingerprint is an address, NOT an identity claim)", { + shards: buildRunOpsShardTable(env.RUN_OPS_SHARDS), + }); + } + return selectRunOpsTopology( { splitEnabled, @@ -568,6 +576,17 @@ export const runOpsLegacyPrismaClient: RunOpsPrismaClient = runOpsTopology.legac export const runOpsLegacyReplicaClient: RunOpsPrismaClient = runOpsTopology.legacyRunOps .replica as unknown as RunOpsPrismaClient; +// Gen-2 shard handles for the run-store boundary. Empty unless RUN_OPS_SHARDS is configured. +export const runOpsShardHandles: Array<{ + key: string; + writer: RunOpsPrismaClient; + replica: RunOpsPrismaClient; +}> = [...runOpsTopology.shards.entries()].map(([key, clients]) => ({ + key, + writer: clients.writer, + replica: clients.replica, +})); + export const runOpsSplitReadEnabled: boolean = computeRunOpsSplitReadEnabled({ newReplica: runOpsNewReplicaClient, controlPlaneWriter: prisma, @@ -1135,6 +1154,32 @@ function redactUrlSecrets(hrefOrUrl: string | URL) { return url.href; } +// A host:port/db address, with NO username and NO query params — never a secret, and deliberately +// NOT an identity claim (two DSNs can share an address yet be different databases; that proof is the +// distinctness sentinel's, not this line's). Same tuple sameDatabaseTarget compares, kept in step. +export function runOpsAddressFingerprint(url: string): string { + try { + const u = new URL(url); + return `${u.hostname}:${u.port || "5432"}${u.pathname}`; + } catch { + return "unparseable"; + } +} + +export type RunOpsShardTableRow = { key: string; fingerprint: string; role: string }; + +// The resolved shard table for the boot log: one row per descriptor. An alias reports its role and +// carries no address (it shares the new store's pool). +export function buildRunOpsShardTable( + descriptors: Array<{ key: string; url?: string; aliasOf?: "new" }> +): RunOpsShardTableRow[] { + return descriptors.map((d) => + d.aliasOf + ? { key: d.key, fingerprint: "alias(new)", role: "alias(new)" } + : { key: d.key, fingerprint: runOpsAddressFingerprint(d.url ?? ""), role: "shard" } + ); +} + export type { PrismaClient } from "@trigger.dev/database"; function getDatabaseSchema() { diff --git a/apps/webapp/app/v3/runStore.server.ts b/apps/webapp/app/v3/runStore.server.ts index 9ccf84b5117..3bc21c14097 100644 --- a/apps/webapp/app/v3/runStore.server.ts +++ b/apps/webapp/app/v3/runStore.server.ts @@ -1,5 +1,5 @@ import { PostgresRunStore, RoutingRunStore, type RunStore } from "@internal/run-store"; -import { ownerEngine, type Residency } from "@trigger.dev/core/v3/isomorphic"; +import { ownerEngine, resolveShard, type Residency, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; import type { PrismaClient, PrismaReplicaClient } from "@trigger.dev/database"; import type { RunOpsPrismaClient } from "@internal/run-ops-database"; import { @@ -9,6 +9,7 @@ import { runOpsLegacyReplica, runOpsNewPrismaClient, runOpsNewReplicaClient, + runOpsShardHandles, } from "~/db.server"; import { env } from "~/env.server"; import { singleton } from "~/utils/singleton"; @@ -31,6 +32,16 @@ type BuildRunStoreDeps = { singleReplica: PrismaReplicaClient; /** Residency classifier; defaults to ownerEngine inside RoutingRunStore. */ classify?: (id: string) => Residency; + /** Gen-2 shard handles. When non-empty, buildRunStore produces N dedicated stores + the keyed + * router (fromShards). Empty/absent keeps today's two-store compat router. */ + shards?: Array<{ + key: ShardKey; + writer: RunOpsPrismaClient; + replica: RunOpsPrismaClient; + resilience?: TransactionResilienceConfig; + }>; + /** Shard-key resolver for the fromShards path; defaults to resolveShard. */ + resolveShardKey?: (id: string) => ShardKey; /** Per-pool transaction-resilience configs threaded into the store(s) this builds (IoC). */ singleResilience?: TransactionResilienceConfig; newResilience?: TransactionResilienceConfig; @@ -79,10 +90,45 @@ export function buildRunStore(deps: BuildRunStoreDeps): RunStore { transactionStartRetry: deps.legacyResilience?.startRetry, }); - return new RoutingRunStore({ - new: newStore, - legacy: legacyStore, - classify: deps.classify ?? ownerEngine, + // No gen-2 shards: today's two-store compat router, byte-identical. + if (!deps.shards || deps.shards.length === 0) { + return new RoutingRunStore({ + new: newStore, + legacy: legacyStore, + classify: deps.classify ?? ownerEngine, + }); + } + + // Gen-2 shards: one dedicated store per descriptor, then the keyed N-way router. Every shard is a + // schemaVariant:"dedicated" instance, exactly like the gen-1 new store. + const shardStores = deps.shards.map((shard) => ({ + key: shard.key, + store: new PostgresRunStore({ + prisma: shard.writer, + readOnlyPrisma: shard.replica, + schemaVariant: "dedicated", + maxWait: shard.resilience?.maxWait, + transactionStartRetry: shard.resilience?.startRetry, + }), + })); + + const shardKeys = shardStores.map((s) => s.key); + const shardMap = new Map([ + ["legacy", legacyStore], + ["new", newStore], + ...shardStores.map(({ key, store }) => [key, store] as const), + ]); + + return RoutingRunStore.fromShards({ + shards: shardMap, + // Ascending authority for a merge: legacy -> new -> shards in configured order. + precedence: ["legacy", "new", ...shardKeys], + // Probe order for an id-less lookup: the reverse of precedence. + probeOrder: ["new", ...shardKeys, "legacy"], + idlessRouteShard: "new", + idlessWaitpointShard: "legacy", + resolveShardKey: deps.resolveShardKey ?? resolveShard, + classify: deps.classify, }); } @@ -110,6 +156,8 @@ function tryResolveRunOpsHandles() { newReplica: runOpsNewReplicaClient, legacyWriter: runOpsLegacyPrisma, legacyReplica: runOpsLegacyReplica, + // Absent under a minimal db.server mock; default to no shards so the compat router is built. + shardHandles: runOpsShardHandles ?? [], }; } catch { return null; @@ -127,9 +175,16 @@ export const runStore: RunStore = singleton("RunStore", () => { singleResilience: resilienceForClient(prisma), }); } + const { shardHandles, ...storeHandles } = handles; return buildRunStore({ splitEnabled: true, - ...handles, + ...storeHandles, + shards: shardHandles.map((shard) => ({ + key: shard.key, + writer: shard.writer, + replica: shard.replica, + resilience: resilienceForClient(shard.writer), + })), singleWriter: prisma, singleReplica: $replica, singleResilience: resilienceForClient(prisma), diff --git a/apps/webapp/test/runOpsShardBootTable.test.ts b/apps/webapp/test/runOpsShardBootTable.test.ts new file mode 100644 index 00000000000..9c7706e3cbd --- /dev/null +++ b/apps/webapp/test/runOpsShardBootTable.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { runOpsAddressFingerprint, buildRunOpsShardTable } from "~/db.server"; + +describe("runOpsAddressFingerprint", () => { + it("returns host:port/db with no username or query params", () => { + const fp = runOpsAddressFingerprint( + "postgres://user:pw@host.example:5433/mydb?schema=public&pool_timeout=20" + ); + expect(fp).toBe("host.example:5433/mydb"); + expect(fp).not.toContain("user"); + expect(fp).not.toContain("pool_timeout"); + }); + it("defaults the port to 5432", () => { + expect(runOpsAddressFingerprint("postgres://h/db")).toBe("h:5432/db"); + }); + it("returns a marker on unparseable input rather than throwing", () => { + expect(runOpsAddressFingerprint("not a url")).toBe("unparseable"); + }); +}); + +describe("buildRunOpsShardTable", () => { + it("one row per descriptor, with key, fingerprint, and role", () => { + const rows = buildRunOpsShardTable([ + { key: "a", url: "postgres://user:pw@h/adb?schema=public" }, + { key: "b", aliasOf: "new" }, + ]); + expect(rows).toHaveLength(2); + expect(rows[0]).toEqual({ key: "a", fingerprint: "h:5432/adb", role: "shard" }); + expect(rows[1]).toEqual({ key: "b", fingerprint: "alias(new)", role: "alias(new)" }); + }); + it("is empty for an empty descriptor list", () => { + expect(buildRunOpsShardTable([])).toEqual([]); + }); +}); diff --git a/apps/webapp/test/runStoreShardWiring.test.ts b/apps/webapp/test/runStoreShardWiring.test.ts new file mode 100644 index 00000000000..728f8ee5623 --- /dev/null +++ b/apps/webapp/test/runStoreShardWiring.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import { RoutingRunStore } from "@internal/run-store"; +import { buildRunStore } from "~/v3/runStore.server"; + +// Construction-only: buildRunStore wraps clients but never connects, so stub handles suffice. This +// asserts the wiring shape (compat router vs N-way router), not query behaviour. +const stub = () => ({}) as any; + +const baseSplit = { + splitEnabled: true as const, + newWriter: stub(), + newReplica: stub(), + legacyWriter: stub(), + legacyReplica: stub(), + singleWriter: stub(), + singleReplica: stub(), +}; + +describe("buildRunStore shard wiring", () => { + it("split ON with no shards builds the two-store compat router", () => { + const store = buildRunStore(baseSplit); + expect(store).toBeInstanceOf(RoutingRunStore); + }); + + it("split ON with two shard descriptors builds the N-way router", () => { + const store = buildRunStore({ + ...baseSplit, + shards: [ + { key: "a", writer: stub(), replica: stub() }, + { key: "b", writer: stub(), replica: stub() }, + ], + }); + expect(store).toBeInstanceOf(RoutingRunStore); + }); + + it("split OFF builds the single-store passthrough (not a router)", () => { + const store = buildRunStore({ + splitEnabled: false, + singleWriter: stub(), + singleReplica: stub(), + }); + expect(store).not.toBeInstanceOf(RoutingRunStore); + }); +}); From dbc22fd3fd9cb0dccf7298be7ccb366acde4109a Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:31:03 +0100 Subject: [PATCH 09/31] feat(webapp): bound the active mint list against the configured shard descriptor keys computeMintShard now intersects the active shard set with routableKeys (the RUN_OPS_SHARDS descriptor keys), so a stored key with no descriptor is never minted into and falls back to gen-1. The empty-set check runs first, so an unconfigured deployment is unchanged. Inert until the gen-2 write path wires in resolveMintShard. Co-Authored-By: Claude Opus 4.8 --- .../mintShardAssignment.test.ts | 34 +++++++++++++++++++ .../v3/runOpsMigration/mintShardAssignment.ts | 18 +++++++++- .../runOpsMigration/runOpsMintShard.server.ts | 2 ++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts index d88e64e1d75..e73f526e665 100644 --- a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts @@ -473,3 +473,37 @@ describe("computeMintShard — the global override wins the complete cutover", ( ); }); }); + +describe("routableKeys bound (the shard descriptor keys this deployment can route)", () => { + it("drops an active key that is not routable, so the hash never returns it", () => { + // "z" is in the active list but not configured as a descriptor -> only "a" is selectable. + const ids = envIds(200); + for (const id of ids) { + const shard = computeMintShard({ id }, deps({ set: ["a", "z"] }, { routableKeys: ["a"] })); + expect(shard).toBe("a"); + } + }); + + it("returns new when the active list holds only non-routable keys (fail-safe to gen-1)", () => { + expect( + computeMintShard({ id: "env_1" }, deps({ set: ["z"] }, { routableKeys: ["a"] })) + ).toBe("new"); + }); + + it("rejects a per-org pin to a non-routable key and falls through to the hash", () => { + const shard = computeMintShard( + { id: "env_1" }, + deps({ set: ["a", "z"] }, { ...orgFlags({ runOpsMintShard: "z" }), routableKeys: ["a"] }) + ); + expect(shard).toBe("a"); + }); + + it("with no routableKeys given, behaviour is unchanged", () => { + const ids = envIds(200); + for (const id of ids) { + const withBound = computeMintShard({ id }, deps({ set: ["a", "b"] }, { routableKeys: ["a", "b"] })); + const without = computeMintShard({ id }, deps({ set: ["a", "b"] })); + expect(withBound).toBe(without); + } + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts index a49a1a6a60d..2855f936249 100644 --- a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts +++ b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts @@ -19,6 +19,10 @@ export type MintShardDeps = { nowMs: number; graceMs: number; orgFeatureFlags: unknown; + // The shard keys this deployment can actually route (the RUN_OPS_SHARDS descriptor keys). The + // active set is bounded to these, so a stored key with no descriptor is never minted into. + // Undefined means "no bound" (today's behaviour). + routableKeys?: readonly string[]; onPinRejected?: (info: { environmentId: string; pin: string; activeSet: string[] }) => void; onOverrideRejected?: (info: { override: string; activeSet: string[] }) => void; }; @@ -94,7 +98,17 @@ function hrwSelect(environmentId: string, activeSet: string[]): string { // would leak the drain the active list performs, and throwing would fail customer triggers // whenever a pinned shard drains. export function computeMintShard(environment: { id: string }, deps: MintShardDeps): ShardKey { - const activeSet = effectiveMintShardSet(deps.resolution, deps.nowMs, deps.graceMs); + const rawActiveSet = effectiveMintShardSet(deps.resolution, deps.nowMs, deps.graceMs); + // Empty check BEFORE the bound, so an unconfigured deployment returns "new" exactly as today. + if (rawActiveSet.length === 0) { + return "new"; + } + + // Bound the active set to the keys this deployment can route. A stored key with no descriptor is + // dropped, never minted into. If nothing survives, fall back to gen-1 (fail-safe, never a throw). + const activeSet = deps.routableKeys + ? rawActiveSet.filter((key) => deps.routableKeys!.includes(key)) + : rawActiveSet; if (activeSet.length === 0) { return "new"; } @@ -148,6 +162,7 @@ export type ResolveMintShardDeps = { ttlMs: number; graceMs: number; orgFeatureFlags: unknown; + routableKeys?: readonly string[]; onPinRejected?: (info: { environmentId: string; pin: string; activeSet: string[] }) => void; onOverrideRejected?: (info: { override: string; activeSet: string[] }) => void; onReadFailed?: (error: unknown) => void; @@ -200,6 +215,7 @@ export async function resolveMintShardWith( nowMs: deps.nowMs, graceMs: deps.graceMs, orgFeatureFlags: deps.orgFeatureFlags, + routableKeys: deps.routableKeys, onPinRejected: deps.onPinRejected, onOverrideRejected: deps.onOverrideRejected, }); diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts index 542384e16f8..c1c2b9ddd48 100644 --- a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts @@ -84,6 +84,8 @@ export async function resolveMintShard(environment: { ttlMs: env.RUN_OPS_MINT_FLAG_CACHE_TTL_MS, graceMs: env.RUN_OPS_MINT_FLIP_GRACE_MS, orgFeatureFlags: environment.orgFeatureFlags, + // Bound the active list to the shards this deployment can actually route. + routableKeys: env.RUN_OPS_SHARDS.map((shard) => shard.key), onPinRejected: reportPinRejected, onOverrideRejected: reportOverrideRejected, onReadFailed: (error) => From 46e97cfe6537a68a54fb088225ffbf6efb5d7bf6 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:32:56 +0100 Subject: [PATCH 10/31] chore(webapp): unexport internal shard descriptor type and apply format/lint Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 18 ++++++++++------ .../mintShardAssignment.test.ts | 11 ++++++---- apps/webapp/app/v3/runOpsPoolKnobs.server.ts | 4 +--- apps/webapp/app/v3/runStore.server.ts | 7 ++++++- apps/webapp/test/runOpsShards.test.ts | 21 +++++++++++++++---- .../webapp/test/transactionResilience.test.ts | 4 +++- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index b67c3a9b4aa..1658219a0c3 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -32,16 +32,16 @@ import { } from "./v3/runOpsMigration/splitMode.server"; import { computeRunOpsSplitReadEnabled } from "./v3/runOpsMigration/runOpsSplitReadGate"; import { resolveRunOpsPoolKnobs } from "./v3/runOpsPoolKnobs.server"; -import { resolveShardResilience } from "./v3/transactionResilience.server"; -import { assertControlPlaneCoresidencyAdvisory } from "./v3/runOpsMigration/controlPlaneCoresidencySentinel.server"; -import { DATASOURCE_CONTEXT_KEY, startActiveSpan } from "./v3/tracer.server"; import { + resolveShardResilience, controlPlaneTransactionResilience, registerTransactionResilience, resilienceForClient, runOpsLegacyTransactionResilience, runOpsTransactionResilience, } from "./v3/transactionResilience.server"; +import { assertControlPlaneCoresidencyAdvisory } from "./v3/runOpsMigration/controlPlaneCoresidencySentinel.server"; +import { DATASOURCE_CONTEXT_KEY, startActiveSpan } from "./v3/tracer.server"; import type { Span } from "@opentelemetry/api"; import { context, trace } from "@opentelemetry/api"; import { queryPerformanceMonitor } from "./utils/queryPerformanceMonitor.server"; @@ -277,7 +277,7 @@ export const webhookReplica: WebhookReplicaDatabase = singleton("webhookReplica" type RunOpsClients = { writer: PrismaClient; replica: PrismaReplicaClient }; type NewRunOpsClients = { writer: RunOpsPrismaClient; replica: RunOpsPrismaClient }; -export type ShardTopologyDescriptor = { +type ShardTopologyDescriptor = { key: string; url?: string; replicaUrl?: string; @@ -1060,7 +1060,9 @@ function buildRunOpsClient({ }): RunOpsPrismaClient { const isWriter = role === "writer"; const setupLabel = isWriter ? "run-ops prisma client" : "run-ops read replica connection"; - const connectedLabel = isWriter ? "run-ops prisma client connected" : "run-ops read replica connected"; + const connectedLabel = isWriter + ? "run-ops prisma client connected" + : "run-ops read replica connected"; const connectionUrl = buildPrismaConnectionUrl(url, { connectionLimit: connectionLimit.toString(), @@ -1111,7 +1113,11 @@ function buildRunOpsClient({ client.$on("error", (log) => // The writer bridges P2002 -> 422 at the store boundary, so its infra errors are logged once // there (ignoreError). Replica errors are not on that write path, so they log normally. - logger.error("RunOpsPrismaClient error", { clientType, event: log, ...(isWriter ? { ignoreError: true } : {}) }) + logger.error("RunOpsPrismaClient error", { + clientType, + event: log, + ...(isWriter ? { ignoreError: true } : {}), + }) ); } diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts index e73f526e665..a4e4a64d36d 100644 --- a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts @@ -485,9 +485,9 @@ describe("routableKeys bound (the shard descriptor keys this deployment can rout }); it("returns new when the active list holds only non-routable keys (fail-safe to gen-1)", () => { - expect( - computeMintShard({ id: "env_1" }, deps({ set: ["z"] }, { routableKeys: ["a"] })) - ).toBe("new"); + expect(computeMintShard({ id: "env_1" }, deps({ set: ["z"] }, { routableKeys: ["a"] }))).toBe( + "new" + ); }); it("rejects a per-org pin to a non-routable key and falls through to the hash", () => { @@ -501,7 +501,10 @@ describe("routableKeys bound (the shard descriptor keys this deployment can rout it("with no routableKeys given, behaviour is unchanged", () => { const ids = envIds(200); for (const id of ids) { - const withBound = computeMintShard({ id }, deps({ set: ["a", "b"] }, { routableKeys: ["a", "b"] })); + const withBound = computeMintShard( + { id }, + deps({ set: ["a", "b"] }, { routableKeys: ["a", "b"] }) + ); const without = computeMintShard({ id }, deps({ set: ["a", "b"] })); expect(withBound).toBe(without); } diff --git a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts index f7bb7b72cf2..195f1305f36 100644 --- a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts +++ b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts @@ -55,9 +55,7 @@ export function resolveRunOpsPoolKnobs( return { writerPoolTimeout: - k?.writerPoolTimeout ?? - env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? - env.DATABASE_POOL_TIMEOUT, + k?.writerPoolTimeout ?? env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, writerConnectionTimeout: k?.writerConnectionTimeout ?? env.RUN_OPS_DATABASE_WRITER_CONNECTION_TIMEOUT ?? diff --git a/apps/webapp/app/v3/runStore.server.ts b/apps/webapp/app/v3/runStore.server.ts index 3bc21c14097..c1cd6eafb19 100644 --- a/apps/webapp/app/v3/runStore.server.ts +++ b/apps/webapp/app/v3/runStore.server.ts @@ -1,5 +1,10 @@ import { PostgresRunStore, RoutingRunStore, type RunStore } from "@internal/run-store"; -import { ownerEngine, resolveShard, type Residency, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import { + ownerEngine, + resolveShard, + type Residency, + type ShardKey, +} from "@trigger.dev/core/v3/isomorphic"; import type { PrismaClient, PrismaReplicaClient } from "@trigger.dev/database"; import type { RunOpsPrismaClient } from "@internal/run-ops-database"; import { diff --git a/apps/webapp/test/runOpsShards.test.ts b/apps/webapp/test/runOpsShards.test.ts index 868ee8fa010..fef7e925e75 100644 --- a/apps/webapp/test/runOpsShards.test.ts +++ b/apps/webapp/test/runOpsShards.test.ts @@ -35,7 +35,10 @@ describe("parseRunOpsShards", () => { expect(run(JSON.stringify([{ ...valid, key: "ab" }])).success).toBe(false); }); it("fails on duplicate keys", () => { - const b = { ...valid, replication: { slotName: "s2", publicationName: "p2", originGeneration: 3 } }; + const b = { + ...valid, + replication: { slotName: "s2", publicationName: "p2", originGeneration: 3 }, + }; expect(run(JSON.stringify([valid, b])).success).toBe(false); }); it("fails on duplicate origin generations", () => { @@ -43,18 +46,28 @@ describe("parseRunOpsShards", () => { expect(run(JSON.stringify([valid, b])).success).toBe(false); }); it("fails when both url and aliasOf are set", () => { - expect(run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db", aliasOf: "new" }])).success).toBe(false); + expect( + run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db", aliasOf: "new" }])) + .success + ).toBe(false); }); it("accepts aliasOf without url or replication", () => { expect(run(JSON.stringify([{ key: "a", region: "x", aliasOf: "new" }])).success).toBe(true); }); it("fails on an origin generation below 2 or above 255", () => { - const mk = (g: number) => run(JSON.stringify([{ ...valid, replication: { slotName: "s", publicationName: "p", originGeneration: g } }])); + const mk = (g: number) => + run( + JSON.stringify([ + { ...valid, replication: { slotName: "s", publicationName: "p", originGeneration: g } }, + ]) + ); expect(mk(1).success).toBe(false); expect(mk(256).success).toBe(false); }); it("fails when a non-aliased descriptor omits replication", () => { - expect(run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db" }])).success).toBe(false); + expect(run(JSON.stringify([{ key: "a", region: "x", url: "postgres://h/db" }])).success).toBe( + false + ); }); }); diff --git a/apps/webapp/test/transactionResilience.test.ts b/apps/webapp/test/transactionResilience.test.ts index 575d059b11c..a033f3be985 100644 --- a/apps/webapp/test/transactionResilience.test.ts +++ b/apps/webapp/test/transactionResilience.test.ts @@ -9,7 +9,9 @@ describe("resolveTransactionResilience per-shard", () => { }); it("accepts an arbitrary pool label and honours a maxWait override", () => { - expect(() => resolveTransactionResilience("run-ops-shard-z", { maxWaitMs: 1234 })).not.toThrow(); + expect(() => + resolveTransactionResilience("run-ops-shard-z", { maxWaitMs: 1234 }) + ).not.toThrow(); expect(resolveTransactionResilience("run-ops-shard-z", { maxWaitMs: 1234 }).maxWait).toBe(1234); }); }); From 20f0ee3635bc1ed989c50c3fea99452bb0db4d61 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:57:30 +0100 Subject: [PATCH 11/31] refactor(webapp): address review feedback on shard wiring - Make probeOrder a true reverse of precedence so the merge and probe paths agree on a duplicate id, matching the RoutingRunStore invariant. - Split resolveRunOpsPoolKnobs into a pure applyPoolKnobOverrides (tested with literal defaults, no env import) plus an env-reading defaults function. - Move the pure boot-table helpers to runOpsShardTable.ts so their test does not construct the db.server Prisma topology. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 27 +----- apps/webapp/app/v3/runOpsPoolKnobs.server.ts | 89 +++++++++---------- apps/webapp/app/v3/runOpsShardTable.ts | 28 ++++++ apps/webapp/app/v3/runStore.server.ts | 12 ++- apps/webapp/test/runOpsPoolKnobs.test.ts | 67 +++++++------- apps/webapp/test/runOpsShardBootTable.test.ts | 2 +- 6 files changed, 116 insertions(+), 109 deletions(-) create mode 100644 apps/webapp/app/v3/runOpsShardTable.ts diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 1658219a0c3..f6f1e519d4a 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -32,6 +32,7 @@ import { } from "./v3/runOpsMigration/splitMode.server"; import { computeRunOpsSplitReadEnabled } from "./v3/runOpsMigration/runOpsSplitReadGate"; import { resolveRunOpsPoolKnobs } from "./v3/runOpsPoolKnobs.server"; +import { buildRunOpsShardTable } from "./v3/runOpsShardTable"; import { resolveShardResilience, controlPlaneTransactionResilience, @@ -1160,32 +1161,6 @@ function redactUrlSecrets(hrefOrUrl: string | URL) { return url.href; } -// A host:port/db address, with NO username and NO query params — never a secret, and deliberately -// NOT an identity claim (two DSNs can share an address yet be different databases; that proof is the -// distinctness sentinel's, not this line's). Same tuple sameDatabaseTarget compares, kept in step. -export function runOpsAddressFingerprint(url: string): string { - try { - const u = new URL(url); - return `${u.hostname}:${u.port || "5432"}${u.pathname}`; - } catch { - return "unparseable"; - } -} - -export type RunOpsShardTableRow = { key: string; fingerprint: string; role: string }; - -// The resolved shard table for the boot log: one row per descriptor. An alias reports its role and -// carries no address (it shares the new store's pool). -export function buildRunOpsShardTable( - descriptors: Array<{ key: string; url?: string; aliasOf?: "new" }> -): RunOpsShardTableRow[] { - return descriptors.map((d) => - d.aliasOf - ? { key: d.key, fingerprint: "alias(new)", role: "alias(new)" } - : { key: d.key, fingerprint: runOpsAddressFingerprint(d.url ?? ""), role: "shard" } - ); -} - export type { PrismaClient } from "@trigger.dev/database"; function getDatabaseSchema() { diff --git a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts index 195f1305f36..0396af0f6d2 100644 --- a/apps/webapp/app/v3/runOpsPoolKnobs.server.ts +++ b/apps/webapp/app/v3/runOpsPoolKnobs.server.ts @@ -1,8 +1,7 @@ import { env } from "~/env.server"; import type { RunOpsShardKnobs } from "~/v3/runOpsShards.server"; -// Pool configuration for one run-ops store (writer + replica), resolved at the app boundary (IoC). -// Every value reproduces today's run-ops builder expressions. Kept separate from db.server (which +// Pool configuration for one run-ops store (writer + replica). Kept separate from db.server (which // ~156 tests mock wholesale) so a new export breaks no mock. export type ResolvedPoolKnobs = { writerPoolTimeout: number; @@ -17,65 +16,65 @@ export type ResolvedPoolKnobs = { type Role = "new" | "legacy"; -// Resolve the pool knobs for a run-ops role, reproducing today's builder expressions exactly. -// descriptorKnobs (gen-2 shards only) override the pool fields. -// Transaction resilience is a SEPARATE mechanism (resolveTransactionResilience) and is not here. -export function resolveRunOpsPoolKnobs( - role: Role, - descriptorKnobs?: RunOpsShardKnobs +// PURE: overlay a gen-2 shard's descriptor knobs on a role's resolved defaults. This holds the only +// logic (per-field override), so a test drives it with literal defaults and literal overrides — +// no env import, no circular assertion against the same env expression the impl reads. +export function applyPoolKnobOverrides( + defaults: ResolvedPoolKnobs, + k?: RunOpsShardKnobs ): ResolvedPoolKnobs { - const k = descriptorKnobs; + return { + writerPoolTimeout: k?.writerPoolTimeout ?? defaults.writerPoolTimeout, + writerConnectionTimeout: k?.writerConnectionTimeout ?? defaults.writerConnectionTimeout, + writerDriverAdapter: k?.writerDriverAdapter ?? defaults.writerDriverAdapter, + connectionLimit: k?.connectionLimit ?? defaults.connectionLimit, + replicaConnectionLimit: k?.replicaConnectionLimit ?? defaults.replicaConnectionLimit, + replicaPoolTimeout: k?.replicaPoolTimeout ?? defaults.replicaPoolTimeout, + replicaConnectionTimeout: k?.replicaConnectionTimeout ?? defaults.replicaConnectionTimeout, + replicaDriverAdapter: k?.replicaDriverAdapter ?? defaults.replicaDriverAdapter, + }; +} +// The env-derived defaults for a role, reproducing today's run-ops builder expressions exactly. A +// flat mapping (no logic), verified by inspection against the former builders. Transaction +// resilience is a SEPARATE mechanism (resolveTransactionResilience) and is not here. +function poolKnobDefaults(role: Role): ResolvedPoolKnobs { if (role === "legacy") { return { writerPoolTimeout: - k?.writerPoolTimeout ?? - env.RUN_OPS_LEGACY_DATABASE_WRITER_POOL_TIMEOUT ?? - env.DATABASE_POOL_TIMEOUT, + env.RUN_OPS_LEGACY_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, writerConnectionTimeout: - k?.writerConnectionTimeout ?? - env.RUN_OPS_LEGACY_DATABASE_WRITER_CONNECTION_TIMEOUT ?? - env.DATABASE_CONNECTION_TIMEOUT, - writerDriverAdapter: - k?.writerDriverAdapter ?? env.RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER === "1", - connectionLimit: k?.connectionLimit ?? env.DATABASE_CONNECTION_LIMIT, - replicaConnectionLimit: k?.replicaConnectionLimit ?? env.DATABASE_CONNECTION_LIMIT, + env.RUN_OPS_LEGACY_DATABASE_WRITER_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT, + writerDriverAdapter: env.RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER === "1", + connectionLimit: env.DATABASE_CONNECTION_LIMIT, + replicaConnectionLimit: env.DATABASE_CONNECTION_LIMIT, replicaPoolTimeout: - k?.replicaPoolTimeout ?? - env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? - env.DATABASE_POOL_TIMEOUT, + env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, replicaConnectionTimeout: - k?.replicaConnectionTimeout ?? env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT, - replicaDriverAdapter: - k?.replicaDriverAdapter ?? env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1", + replicaDriverAdapter: env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1", }; } return { - writerPoolTimeout: - k?.writerPoolTimeout ?? env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, + writerPoolTimeout: env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, writerConnectionTimeout: - k?.writerConnectionTimeout ?? - env.RUN_OPS_DATABASE_WRITER_CONNECTION_TIMEOUT ?? - env.DATABASE_CONNECTION_TIMEOUT, - writerDriverAdapter: - k?.writerDriverAdapter ?? env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1", - connectionLimit: k?.connectionLimit ?? env.DATABASE_CONNECTION_LIMIT, + env.RUN_OPS_DATABASE_WRITER_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT, + writerDriverAdapter: env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1", + connectionLimit: env.DATABASE_CONNECTION_LIMIT, replicaConnectionLimit: - k?.replicaConnectionLimit ?? - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? - env.DATABASE_CONNECTION_LIMIT, - replicaPoolTimeout: - k?.replicaPoolTimeout ?? - env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? - env.DATABASE_POOL_TIMEOUT, + env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT, + replicaPoolTimeout: env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT, replicaConnectionTimeout: - k?.replicaConnectionTimeout ?? - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? - env.DATABASE_CONNECTION_TIMEOUT, - replicaDriverAdapter: - k?.replicaDriverAdapter ?? env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1", + env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_TIMEOUT ?? env.DATABASE_CONNECTION_TIMEOUT, + replicaDriverAdapter: env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1", }; } + +export function resolveRunOpsPoolKnobs( + role: Role, + descriptorKnobs?: RunOpsShardKnobs +): ResolvedPoolKnobs { + return applyPoolKnobOverrides(poolKnobDefaults(role), descriptorKnobs); +} diff --git a/apps/webapp/app/v3/runOpsShardTable.ts b/apps/webapp/app/v3/runOpsShardTable.ts new file mode 100644 index 00000000000..6741b3cb5b0 --- /dev/null +++ b/apps/webapp/app/v3/runOpsShardTable.ts @@ -0,0 +1,28 @@ +// Pure boot-table helpers. Dependency-free (no db.server, no env) so a test of these two string +// functions never constructs a Prisma client. db.server imports them for the boot log. + +// A host:port/db address, with NO username and NO query params — never a secret, and deliberately +// NOT an identity claim (two DSNs can share an address yet be different databases; that proof is the +// distinctness sentinel's, not this line's). Same tuple sameDatabaseTarget compares, kept in step. +export function runOpsAddressFingerprint(url: string): string { + try { + const u = new URL(url); + return `${u.hostname}:${u.port || "5432"}${u.pathname}`; + } catch { + return "unparseable"; + } +} + +export type RunOpsShardTableRow = { key: string; fingerprint: string; role: string }; + +// The resolved shard table for the boot log: one row per descriptor. An alias reports its role and +// carries no address (it shares the new store's pool). +export function buildRunOpsShardTable( + descriptors: Array<{ key: string; url?: string; aliasOf?: "new" }> +): RunOpsShardTableRow[] { + return descriptors.map((d) => + d.aliasOf + ? { key: d.key, fingerprint: "alias(new)", role: "alias(new)" } + : { key: d.key, fingerprint: runOpsAddressFingerprint(d.url ?? ""), role: "shard" } + ); +} diff --git a/apps/webapp/app/v3/runStore.server.ts b/apps/webapp/app/v3/runStore.server.ts index c1cd6eafb19..3fd0fcfa3d0 100644 --- a/apps/webapp/app/v3/runStore.server.ts +++ b/apps/webapp/app/v3/runStore.server.ts @@ -124,12 +124,16 @@ export function buildRunStore(deps: BuildRunStoreDeps): RunStore { ...shardStores.map(({ key, store }) => [key, store] as const), ]); + // Ascending authority for a merge: legacy -> new -> shards in configured order. The router + // requires probeOrder to be the exact reverse (see the class invariant in runOpsStore.ts), so a + // duplicate id resolves the same way on the merge path and the probe path. + const precedence: ShardKey[] = ["legacy", "new", ...shardKeys]; + const probeOrder = [...precedence].reverse(); + return RoutingRunStore.fromShards({ shards: shardMap, - // Ascending authority for a merge: legacy -> new -> shards in configured order. - precedence: ["legacy", "new", ...shardKeys], - // Probe order for an id-less lookup: the reverse of precedence. - probeOrder: ["new", ...shardKeys, "legacy"], + precedence, + probeOrder, idlessRouteShard: "new", idlessWaitpointShard: "legacy", resolveShardKey: deps.resolveShardKey ?? resolveShard, diff --git a/apps/webapp/test/runOpsPoolKnobs.test.ts b/apps/webapp/test/runOpsPoolKnobs.test.ts index e281b8ce19e..38353682cb2 100644 --- a/apps/webapp/test/runOpsPoolKnobs.test.ts +++ b/apps/webapp/test/runOpsPoolKnobs.test.ts @@ -1,41 +1,42 @@ import { describe, expect, it } from "vitest"; -import { resolveRunOpsPoolKnobs } from "~/v3/runOpsPoolKnobs.server"; -import { env } from "~/env.server"; +import { applyPoolKnobOverrides, type ResolvedPoolKnobs } from "~/v3/runOpsPoolKnobs.server"; -describe("resolveRunOpsPoolKnobs", () => { - it("new role: reproduces the run-ops builder expressions", () => { - const k = resolveRunOpsPoolKnobs("new"); - expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); - expect(k.replicaConnectionLimit).toBe( - env.RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT ?? env.DATABASE_CONNECTION_LIMIT - ); - expect(k.writerPoolTimeout).toBe( - env.RUN_OPS_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT - ); - expect(k.replicaPoolTimeout).toBe( - env.RUN_OPS_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT - ); - expect(k.writerDriverAdapter).toBe(env.RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER === "1"); - expect(k.replicaDriverAdapter).toBe(env.RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER === "1"); +// Literal defaults, so the assertions lock the override logic against fixed values rather than +// against the same env expression the implementation reads. No env import (webapp test rule). +const DEFAULTS: ResolvedPoolKnobs = { + writerPoolTimeout: 10, + writerConnectionTimeout: 20, + writerDriverAdapter: false, + connectionLimit: 30, + replicaConnectionLimit: 40, + replicaPoolTimeout: 50, + replicaConnectionTimeout: 60, + replicaDriverAdapter: false, +}; + +describe("applyPoolKnobOverrides", () => { + it("returns the defaults verbatim when no descriptor knobs are given", () => { + expect(applyPoolKnobOverrides(DEFAULTS)).toEqual(DEFAULTS); + expect(applyPoolKnobOverrides(DEFAULTS, {})).toEqual(DEFAULTS); }); - it("legacy role: uses RUN_OPS_LEGACY_* timeouts and the generic connection limit", () => { - const k = resolveRunOpsPoolKnobs("legacy"); - expect(k.connectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); - expect(k.replicaConnectionLimit).toBe(env.DATABASE_CONNECTION_LIMIT); - expect(k.writerPoolTimeout).toBe( - env.RUN_OPS_LEGACY_DATABASE_WRITER_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT - ); - expect(k.replicaPoolTimeout).toBe( - env.RUN_OPS_LEGACY_DATABASE_READ_REPLICA_POOL_TIMEOUT ?? env.DATABASE_POOL_TIMEOUT - ); - expect(k.writerDriverAdapter).toBe(env.RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER === "1"); - expect(k.replicaDriverAdapter).toBe(env.RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER === "1"); + it("overrides only the fields the descriptor sets", () => { + const result = applyPoolKnobOverrides(DEFAULTS, { + connectionLimit: 999, + writerDriverAdapter: true, + replicaPoolTimeout: 555, + }); + expect(result.connectionLimit).toBe(999); + expect(result.writerDriverAdapter).toBe(true); + expect(result.replicaPoolTimeout).toBe(555); + // Untouched fields keep the defaults. + expect(result.writerPoolTimeout).toBe(10); + expect(result.replicaConnectionLimit).toBe(40); + expect(result.replicaDriverAdapter).toBe(false); }); - it("a descriptor knob overrides its field", () => { - const k = resolveRunOpsPoolKnobs("new", { connectionLimit: 7, writerDriverAdapter: true }); - expect(k.connectionLimit).toBe(7); - expect(k.writerDriverAdapter).toBe(true); + it("does not read the transaction knobs off the descriptor", () => { + const result = applyPoolKnobOverrides(DEFAULTS, { transactionMaxWaitMs: 1234 }); + expect(result).toEqual(DEFAULTS); }); }); diff --git a/apps/webapp/test/runOpsShardBootTable.test.ts b/apps/webapp/test/runOpsShardBootTable.test.ts index 9c7706e3cbd..3031608d701 100644 --- a/apps/webapp/test/runOpsShardBootTable.test.ts +++ b/apps/webapp/test/runOpsShardBootTable.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { runOpsAddressFingerprint, buildRunOpsShardTable } from "~/db.server"; +import { runOpsAddressFingerprint, buildRunOpsShardTable } from "~/v3/runOpsShardTable"; describe("runOpsAddressFingerprint", () => { it("returns host:port/db with no username or query params", () => { From 908fcb587e7e622faea68339bfc74e0cf0f101a5 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Mon, 24 Aug 2026 17:59:43 +0100 Subject: [PATCH 12/31] test(webapp): cover the dedicated-shard misconfiguration throw in selectRunOpsTopology Co-Authored-By: Claude Opus 4.8 --- apps/webapp/test/runOpsDbTopology.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/apps/webapp/test/runOpsDbTopology.test.ts b/apps/webapp/test/runOpsDbTopology.test.ts index f6895a03d29..f2bcc0bf5ea 100644 --- a/apps/webapp/test/runOpsDbTopology.test.ts +++ b/apps/webapp/test/runOpsDbTopology.test.ts @@ -193,6 +193,24 @@ describe("selectRunOpsTopology (pure)", () => { expect(buildShardWriter).not.toHaveBeenCalled(); expect(buildShardReplica).not.toHaveBeenCalled(); }); + + it("throws when a non-aliased shard has no url (guards the shard.url non-null assertion)", () => { + expect(() => + selectRunOpsTopology( + { ...baseSplit, shards: [{ key: "a" }] }, + { ...baseBuilders(), buildShardWriter: vi.fn(), buildShardReplica: vi.fn() } + ) + ).toThrow(/shard "a" needs a url/); + }); + + it("throws when a non-aliased shard is configured but the shard builders are absent", () => { + expect(() => + selectRunOpsTopology( + { ...baseSplit, shards: [{ key: "a", url: "postgres://a" }] }, + baseBuilders() + ) + ).toThrow(/shard "a" needs a url and shard builders/); + }); }); describe("sameDatabaseTarget", () => { From 67163dd127c096b3fc41619e8ee2d166515fc56b Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 12:51:55 +0100 Subject: [PATCH 13/31] fix(webapp): don't let an unreachable run-ops shard crash webapp startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The run-ops client factory eagerly $connects for warm-up, but only caught the rejection under NODE_ENV=test — outside test an unreachable shard/run-ops DB at boot surfaced as an unhandled promise rejection. Always catch and log instead; Prisma reconnects lazily on first query, so one unreachable shard must not take down startup. Scoped to the run-ops factory only; the control-plane/legacy builders are unchanged, so the RUN_OPS_SHARDS-unset path stays byte-identical. Co-Authored-By: Claude Opus 4.8 --- apps/webapp/app/db.server.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index f6f1e519d4a..11a8aa307e9 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -1124,12 +1124,13 @@ function buildRunOpsClient({ client.$on("query", (log) => queryPerformanceMonitor.onQuery(role, log)); - const connectPromise = client.$connect(); - if (env.NODE_ENV === "test") { - connectPromise.catch((error) => { - logger.warn(`Failed to eagerly connect run-ops prisma client (${role})`, { error }); - }); - } + // Eager connect is a warm-up only — Prisma reconnects lazily on first query. ALWAYS catch the + // rejection (not just under NODE_ENV=test), so a shard/run-ops DB that is unreachable at boot + // logs a warning instead of surfacing as an unhandled promise rejection. One unreachable shard + // must not take down webapp startup. + client.$connect().catch((error) => { + logger.warn(`Failed to eagerly connect run-ops prisma client (${role})`, { error }); + }); console.log(`🔌 ${connectedLabel}`); From dc945f13dc98752015310a68e8b68ca88eeeea5f Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:44:49 +0100 Subject: [PATCH 14/31] feat(webapp): derive the non-aliased shard target list once Every boot check that must not treat two handles over one database as two databases needs the same list. The alias exemption keys on the declared aliasOf field, never on client object identity: two store objects can sit over one database, which identity comparison cannot see. --- apps/webapp/app/v3/runOpsShards.server.ts | 30 ++++++++++++ apps/webapp/test/runOpsShards.test.ts | 56 ++++++++++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/v3/runOpsShards.server.ts b/apps/webapp/app/v3/runOpsShards.server.ts index 23c45efa404..12cc460aa06 100644 --- a/apps/webapp/app/v3/runOpsShards.server.ts +++ b/apps/webapp/app/v3/runOpsShards.server.ts @@ -122,3 +122,33 @@ export function validateShardListAgainstNewUrl( ): boolean { return shards.length === 0 || !!newUrl; } + +// A shard that owns its own physical database. Every boot check that must not treat two handles +// over one database as two databases derives its target list from here: the distinctness sentinel, +// the coresidency loop, the read gate, the replication sources and the migration loop. +export type ShardTarget = { + key: string; + url: string; + replicaUrl?: string; + directUrl?: string; +}; + +// An aliased shard shares its target's client BY REFERENCE, so it is never its own database. The +// exemption keys on the declared `aliasOf` field, never on client object identity: two store objects +// can sit over one database, which identity comparison cannot see. +export function nonAliasedShards(shards: RunOpsShardDescriptor[]): ShardTarget[] { + const targets: ShardTarget[] = []; + for (const shard of shards) { + if (shard.aliasOf !== undefined) continue; + // Unreachable for a valid descriptor (the schema requires exactly one of url/aliasOf); this is + // the type narrowing, not a second policy. + if (shard.url === undefined) continue; + targets.push({ + key: shard.key, + url: shard.url, + ...(shard.replicaUrl !== undefined ? { replicaUrl: shard.replicaUrl } : {}), + ...(shard.directUrl !== undefined ? { directUrl: shard.directUrl } : {}), + }); + } + return targets; +} diff --git a/apps/webapp/test/runOpsShards.test.ts b/apps/webapp/test/runOpsShards.test.ts index fef7e925e75..c7fd0e0defa 100644 --- a/apps/webapp/test/runOpsShards.test.ts +++ b/apps/webapp/test/runOpsShards.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; import { z } from "zod"; -import { parseRunOpsShards, validateShardListAgainstNewUrl } from "~/v3/runOpsShards.server"; +import { + nonAliasedShards, + parseRunOpsShards, + validateShardListAgainstNewUrl, + type RunOpsShardDescriptor, +} from "~/v3/runOpsShards.server"; function run(raw: string | undefined) { const schema = z.string().optional().transform(parseRunOpsShards); @@ -82,3 +87,52 @@ describe("validateShardListAgainstNewUrl", () => { expect(validateShardListAgainstNewUrl([valid as never], undefined)).toBe(false); }); }); + +describe("nonAliasedShards", () => { + const shardA: RunOpsShardDescriptor = { + key: "a", + region: "us-east-1", + url: "postgres://h/a", + replication: { slotName: "sa", publicationName: "pa", originGeneration: 2 }, + }; + const shardB: RunOpsShardDescriptor = { + key: "b", + region: "us-west-2", + url: "postgres://h/b", + replicaUrl: "postgres://h/b-replica", + directUrl: "postgres://h/b-direct", + replication: { slotName: "sb", publicationName: "pb", originGeneration: 3 }, + }; + const aliased: RunOpsShardDescriptor = { + key: "z", + region: "us-east-1", + aliasOf: "new", + }; + + it("returns [] for no descriptors", () => { + expect(nonAliasedShards([])).toEqual([]); + }); + + it("keeps a shard that owns its own database", () => { + expect(nonAliasedShards([shardA])).toEqual([{ key: "a", url: "postgres://h/a" }]); + }); + + it("carries the replica and direct URLs when the descriptor sets them", () => { + expect(nonAliasedShards([shardB])).toEqual([ + { + key: "b", + url: "postgres://h/b", + replicaUrl: "postgres://h/b-replica", + directUrl: "postgres://h/b-direct", + }, + ]); + }); + + it("drops an aliased shard, because it shares its target's database", () => { + expect(nonAliasedShards([aliased])).toEqual([]); + }); + + it("keeps declaration order across a mixed list", () => { + expect(nonAliasedShards([shardA, aliased, shardB]).map((s) => s.key)).toEqual(["a", "b"]); + }); +}); From 4b1085bf965c07ddefb69ed95fa978baf16fe79f Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:46:56 +0100 Subject: [PATCH 15/31] feat(webapp): check distinctness as set uniqueness over every run-ops store probeDistinctStores groups every target by its system identifier and database name, so a duplicate between any two stores blocks the boot, not only a duplicate between the gen-1 pair. Fail-closed is unchanged: a probe that cannot answer returns not-distinct. probeDistinctDatabases stays exported as a delegate over a 2-element list. Its four existing container tests are the proof that set uniqueness over one pair is the pairwise compare of today. --- .../distinctDbSentinel.server.ts | 74 +++++++---- .../distinctDbSentinel.server.test.ts | 115 +++++++++++++++++- 2 files changed, 166 insertions(+), 23 deletions(-) diff --git a/apps/webapp/app/v3/runOpsMigration/distinctDbSentinel.server.ts b/apps/webapp/app/v3/runOpsMigration/distinctDbSentinel.server.ts index 4b2bfd9d986..ed7fb0cb237 100644 --- a/apps/webapp/app/v3/runOpsMigration/distinctDbSentinel.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/distinctDbSentinel.server.ts @@ -62,32 +62,46 @@ export async function probeControlPlaneCoresidency( } } -export async function probeDistinctDatabases( - legacyUrl: string, - newUrl: string, +export type DistinctTarget = { id: string; url: string }; + +/** + * Set uniqueness over every store that owns its own database. Fail-closed: a probe that cannot + * answer returns NOT distinct, because "distinct" is a positive claim a failed probe cannot support. + * + * Same-cluster-different-database policy (unchanged from the pairwise probe): two databases inside + * the SAME cluster (same system identifier, different current_database()) are reported distinct. + * They are genuinely separate Postgres databases with separate WAL-visible state for our purposes. + * + * An ALIASED shard never appears in `targets`. It shares its target's client by reference, so it is + * not its own database and inclusion would guarantee a duplicate. See nonAliasedShards. + */ +export async function probeDistinctStores( + targets: DistinctTarget[], opts?: { logger?: { warn: (msg: string, meta?: Record) => void } } ): Promise<{ distinct: true } | { distinct: false; reason: string }> { + if (targets.length < 2) { + return { distinct: true }; + } + try { - const [legacy, next] = await Promise.all([ - readDatabaseFingerprint(legacyUrl), - readDatabaseFingerprint(newUrl), - ]); - const sameCluster = legacy.systemIdentifier === next.systemIdentifier; - const sameDb = sameCluster && legacy.databaseName === next.databaseName; - // Same-cluster-different-database policy: two databases inside the SAME cluster - // (same system identifier, different current_database()) are reported distinct: true. - // That is acceptable — they are genuinely separate Postgres databases with separate - // WAL-visible state for our purposes, and the Cloud topology always uses separate - // clusters anyway. A stricter "must be a different cluster" policy would gate on - // sameCluster alone; that is flagged as an open question, not decided here. - if (sameDb) { - const reason = - "run-ops legacy and new URLs resolve to the SAME physical database " + - `(systemIdentifier=${legacy.systemIdentifier}, database=${legacy.databaseName}); ` + - "refusing to enable split — pooler/replica likely."; - opts?.logger?.warn(reason); - return { distinct: false, reason }; + const fingerprints = await Promise.all(targets.map((t) => readDatabaseFingerprint(t.url))); + + const seen = new Map(); + for (const [index, target] of targets.entries()) { + const fingerprint = fingerprints[index]; + const key = `${fingerprint.systemIdentifier}/${fingerprint.databaseName}`; + const first = seen.get(key); + if (first !== undefined) { + const reason = + `run-ops stores "${first}" and "${target.id}" resolve to the SAME physical database ` + + `(systemIdentifier=${fingerprint.systemIdentifier}, database=${fingerprint.databaseName}); ` + + "refusing to enable split — pooler/replica likely."; + opts?.logger?.warn(reason); + return { distinct: false, reason }; + } + seen.set(key, target.id); } + return { distinct: true }; } catch (error) { const reason = `distinct-db sentinel probe failed; failing closed (single-DB). ${String(error)}`; @@ -95,3 +109,19 @@ export async function probeDistinctDatabases( return { distinct: false, reason }; } } + +// The gen-1 pairwise entry point, kept as a thin delegate over a 2-element target list. Set +// uniqueness over one pair IS the pairwise compare, and this function's tests are the proof. +export async function probeDistinctDatabases( + legacyUrl: string, + newUrl: string, + opts?: { logger?: { warn: (msg: string, meta?: Record) => void } } +): Promise<{ distinct: true } | { distinct: false; reason: string }> { + return probeDistinctStores( + [ + { id: "legacy", url: legacyUrl }, + { id: "new", url: newUrl }, + ], + opts + ); +} diff --git a/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts b/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts index d2baaa6404a..562d50b63d5 100644 --- a/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts +++ b/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts @@ -1,7 +1,10 @@ import { heteroPostgresTest } from "@internal/testcontainers"; import { PrismaClient } from "@trigger.dev/database"; import { describe, expect, vi } from "vitest"; -import { probeDistinctDatabases } from "~/v3/runOpsMigration/distinctDbSentinel.server"; +import { + probeDistinctDatabases, + probeDistinctStores, +} from "~/v3/runOpsMigration/distinctDbSentinel.server"; // Spinning up two separate postgres clusters and probing each can exceed the 5s default. vi.setConfig({ testTimeout: 60_000 }); @@ -62,3 +65,113 @@ describe("probeDistinctDatabases", () => { } ); }); + +describe("probeDistinctStores (set uniqueness at N)", () => { + heteroPostgresTest( + "reports distinct for two separate physical clusters", + async ({ uri14, uri17 }) => { + const result = await probeDistinctStores([ + { id: "legacy", url: uri14 }, + { id: "new", url: uri17 }, + ]); + expect(result).toEqual({ distinct: true }); + } + ); + + heteroPostgresTest("reports distinct for a single target", async ({ uri14 }) => { + const result = await probeDistinctStores([{ id: "legacy", url: uri14 }]); + expect(result).toEqual({ distinct: true }); + }); + + heteroPostgresTest("reports distinct for an empty target list", async () => { + const result = await probeDistinctStores([]); + expect(result).toEqual({ distinct: true }); + }); + + heteroPostgresTest( + "reports NOT distinct, naming both ids, when two targets resolve to one database", + async ({ uri14, uri17 }) => { + const result = await probeDistinctStores([ + { id: "legacy", url: uri14 }, + { id: "new", url: uri17 }, + { id: "shard-a", url: uri14 }, + ]); + expect(result.distinct).toBe(false); + if (result.distinct === false) { + expect(result.reason).toMatch(/same physical database/i); + expect(result.reason).toMatch(/legacy/); + expect(result.reason).toMatch(/shard-a/); + } + } + ); + + // A pairwise implementation that only ever compares the first two targets passes every other + // case in this file and fails this one: legacy vs new is clean, and the duplicate pair is + // shard against shard on a third database. + heteroPostgresTest( + "catches a duplicate between two SHARDS while the gen-1 pair is clean", + async ({ postgresContainer14, uri14, uri17 }) => { + const shardDb = `sentinel_shard_dupe_${Date.now()}`; + const admin = new PrismaClient({ + datasources: { + db: { url: urlWithDatabase(postgresContainer14.getConnectionUri(), "postgres") }, + }, + }); + try { + await admin.$executeRawUnsafe(`CREATE DATABASE "${shardDb}"`); + } finally { + await admin.$disconnect(); + } + const shardUrl = urlWithDatabase(uri14, shardDb); + + const result = await probeDistinctStores([ + { id: "legacy", url: uri14 }, + { id: "new", url: uri17 }, + { id: "shard-a", url: shardUrl }, + { id: "shard-b", url: shardUrl }, + ]); + expect(result.distinct).toBe(false); + if (result.distinct === false) { + expect(result.reason).toMatch(/shard-a/); + expect(result.reason).toMatch(/shard-b/); + } + } + ); + + heteroPostgresTest( + "reports distinct for two databases in the SAME cluster", + async ({ postgresContainer14, uri14, uri17 }) => { + const otherDb = `sentinel_set_other_${Date.now()}`; + const admin = new PrismaClient({ + datasources: { + db: { url: urlWithDatabase(postgresContainer14.getConnectionUri(), "postgres") }, + }, + }); + try { + await admin.$executeRawUnsafe(`CREATE DATABASE "${otherDb}"`); + } finally { + await admin.$disconnect(); + } + + const result = await probeDistinctStores([ + { id: "legacy", url: uri14 }, + { id: "new", url: uri17 }, + { id: "shard-a", url: urlWithDatabase(uri14, otherDb) }, + ]); + expect(result).toEqual({ distinct: true }); + } + ); + + heteroPostgresTest( + "fails closed to NOT distinct when one target cannot be reached", + async ({ uri14, uri17 }) => { + const unreachable = "postgresql://nobody:nobody@127.0.0.1:1/does_not_exist"; + const result = await probeDistinctStores([ + { id: "legacy", url: uri14 }, + { id: "new", url: uri17 }, + { id: "shard-a", url: unreachable }, + ]); + expect(result.distinct).toBe(false); + } + ); +}); From 8f3aa11376f4f9398ba056584d954e2bbe73aa2f Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:48:05 +0100 Subject: [PATCH 16/31] feat(webapp): give the split gate every store that owns its own database computeSplitEnabled builds one probe target per store and passes them to the set-uniqueness probe. An aliased shard is already absent from the list, so it needs no exception. The flag-off short circuit is unchanged, so a single-database boot still opens no second connection. --- .../v3/runOpsMigration/splitMode.server.ts | 17 ++++- apps/webapp/test/runOpsSplitMode.test.ts | 71 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/apps/webapp/app/v3/runOpsMigration/splitMode.server.ts b/apps/webapp/app/v3/runOpsMigration/splitMode.server.ts index 688f95bac03..b9a4e3dfdf2 100644 --- a/apps/webapp/app/v3/runOpsMigration/splitMode.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/splitMode.server.ts @@ -6,12 +6,15 @@ */ import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; -import { probeDistinctDatabases as defaultProbe } from "./distinctDbSentinel.server"; +import { probeDistinctStores as defaultProbe } from "./distinctDbSentinel.server"; +import { nonAliasedShards, type ShardTarget } from "~/v3/runOpsShards.server"; export type SplitModeConfig = { flagEnabled: boolean; legacyUrl?: string; newUrl?: string; + /** Gen-2 shards that own their own database. Empty (the default) is today's gen-1 pair. */ + shards?: ShardTarget[]; }; export type SplitModeDeps = { @@ -34,9 +37,16 @@ export async function computeSplitEnabled( ); return false; } - // Hard gate #2: runtime sentinel must confirm physically-distinct DBs. + // Hard gate #2: runtime sentinel must confirm physically-distinct DBs. At N stores this is set + // uniqueness over every store that owns its own database, not a compare of the gen-1 pair. An + // aliased shard is already absent from `shards` — it shares its target's client by reference. const probe = deps.probe ?? defaultProbe; - const result = await probe(config.legacyUrl, config.newUrl, { logger: deps.logger }); + const targets = [ + { id: "legacy", url: config.legacyUrl }, + { id: "new", url: config.newUrl }, + ...(config.shards ?? []).map((shard) => ({ id: `shard-${shard.key}`, url: shard.url })), + ]; + const result = await probe(targets, { logger: deps.logger }); return result.distinct === true; } @@ -72,6 +82,7 @@ export function isSplitEnabled(): Promise { flagEnabled: env.RUN_OPS_SPLIT_ENABLED, legacyUrl: env.RUN_OPS_LEGACY_DATABASE_URL, newUrl: env.RUN_OPS_DATABASE_URL, + shards: nonAliasedShards(env.RUN_OPS_SHARDS), }, { logger } ); diff --git a/apps/webapp/test/runOpsSplitMode.test.ts b/apps/webapp/test/runOpsSplitMode.test.ts index 7ce2bec3a5d..fd7da6f356c 100644 --- a/apps/webapp/test/runOpsSplitMode.test.ts +++ b/apps/webapp/test/runOpsSplitMode.test.ts @@ -61,6 +61,77 @@ describe("computeSplitEnabled (pure)", () => { }); }); +describe("computeSplitEnabled shard targets", () => { + const shardA = { key: "a", url: "postgres://shard-a" }; + const shardB = { key: "b", url: "postgres://shard-b" }; + + it("probes the gen-1 pair only when no shard is configured", async () => { + const probe = vi.fn().mockResolvedValue({ distinct: true }); + await computeSplitEnabled( + { flagEnabled: true, legacyUrl: "postgres://a", newUrl: "postgres://b" }, + { probe } + ); + expect(probe).toHaveBeenCalledWith( + [ + { id: "legacy", url: "postgres://a" }, + { id: "new", url: "postgres://b" }, + ], + expect.anything() + ); + }); + + it("appends one target per shard, keyed by shard id", async () => { + const probe = vi.fn().mockResolvedValue({ distinct: true }); + await computeSplitEnabled( + { + flagEnabled: true, + legacyUrl: "postgres://a", + newUrl: "postgres://b", + shards: [shardA, shardB], + }, + { probe } + ); + expect(probe).toHaveBeenCalledWith( + [ + { id: "legacy", url: "postgres://a" }, + { id: "new", url: "postgres://b" }, + { id: "shard-a", url: "postgres://shard-a" }, + { id: "shard-b", url: "postgres://shard-b" }, + ], + expect.anything() + ); + }); + + it("stays single-DB when a shard duplicates another store", async () => { + const probe = vi.fn().mockResolvedValue({ distinct: false, reason: "same DB" }); + expect( + await computeSplitEnabled( + { + flagEnabled: true, + legacyUrl: "postgres://a", + newUrl: "postgres://b", + shards: [shardA], + }, + { probe } + ) + ).toBe(false); + }); + + it("never probes a shard when the flag is off", async () => { + const probe = vi.fn(); + await computeSplitEnabled( + { + flagEnabled: false, + legacyUrl: "postgres://a", + newUrl: "postgres://b", + shards: [shardA], + }, + { probe } + ); + expect(probe).not.toHaveBeenCalled(); + }); +}); + describe("assertSplitRealtimeInterlock (pure)", () => { it("throws when split is on but the native realtime backend is off", () => { expect(() => From 45c44507bb30b299c206ca8e86d3991d4048aea2 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:50:55 +0100 Subject: [PATCH 17/31] feat(webapp): run the co-residency advisory per run-ops store Every store that owns its own database is probed against the control plane. The legacy emission keeps its exact call shape and its untagged metric series, so a deployment with no shard configured reports what it reports today. A shard emission carries its shard key. Every store emits before any enforcement throw, so one offending store never costs another store its metric. A probe that throws degrades that one store to unknown. Two tests read RUN_OPS_LEGACY_DATABASE_URL from the developer's .env, because ?? only guards nullish and they passed undefined. They now pin the value, so they no longer depend on the local environment. --- ...rolPlaneCoresidencySentinel.server.test.ts | 126 +++++++++++++++++- .../controlPlaneCoresidencySentinel.server.ts | 92 +++++++++---- 2 files changed, 192 insertions(+), 26 deletions(-) diff --git a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.test.ts b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.test.ts index 37301f68743..d08f96529c9 100644 --- a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.test.ts @@ -34,6 +34,7 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { const emit = vi.fn(); await assertControlPlaneCoresidencyAdvisory({ ...urls, + shards: [], expectSplit: false, probe: async () => ({ coresident: "true" }), emit, @@ -46,6 +47,7 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { await expect( assertControlPlaneCoresidencyAdvisory({ ...urls, + shards: [], expectSplit: true, probe: async () => ({ coresident: "true" }), emit: vi.fn(), @@ -58,6 +60,7 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { const emit = vi.fn(); await assertControlPlaneCoresidencyAdvisory({ ...urls, + shards: [], expectSplit: true, probe: async () => ({ coresident: "unknown", reason: "denied" }), emit, @@ -71,6 +74,7 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { const warn = vi.fn(); await assertControlPlaneCoresidencyAdvisory({ ...urls, + shards: [], expectSplit: true, probe: async () => { throw new Error("probe blew up"); @@ -86,9 +90,12 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { const emit = vi.fn(); const probe = vi.fn(); await assertControlPlaneCoresidencyAdvisory({ - legacyUrl: undefined, + // "" and not undefined: ?? only guards nullish, so undefined would read the ambient + // RUN_OPS_LEGACY_DATABASE_URL and this test would depend on the developer's .env. + legacyUrl: "", controlPlaneUrl: "postgres://cp", expectSplit: true, + shards: [], probe, emit, log: noopLog, @@ -97,3 +104,120 @@ describe("assertControlPlaneCoresidencyAdvisory", () => { expect(emit).not.toHaveBeenCalled(); }); }); + +describe("assertControlPlaneCoresidencyAdvisory at N shards", () => { + const urls = { legacyUrl: "postgres://legacy", controlPlaneUrl: "postgres://cp" }; + const shardA = { key: "a", url: "postgres://shard-a" }; + const shardB = { key: "b", url: "postgres://shard-b" }; + + it("emits the legacy verdict with NO shard key, so today's series is unchanged", async () => { + const emit = vi.fn(); + await assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: false, + shards: [], + probe: async () => ({ coresident: "false" }), + emit, + log: noopLog, + }); + expect(emit).toHaveBeenCalledTimes(1); + expect(emit).toHaveBeenCalledWith("false"); + }); + + it("emits one tagged verdict per shard, plus the untagged legacy verdict", async () => { + const emit = vi.fn(); + await assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: false, + shards: [shardA, shardB], + probe: async () => ({ coresident: "false" }), + emit, + log: noopLog, + }); + expect(emit).toHaveBeenCalledTimes(3); + expect(emit).toHaveBeenCalledWith("false"); + expect(emit).toHaveBeenCalledWith("false", "a"); + expect(emit).toHaveBeenCalledWith("false", "b"); + }); + + it("probes each shard against the control plane", async () => { + const probe = vi.fn().mockResolvedValue({ coresident: "false" }); + await assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: false, + shards: [shardA], + probe, + emit: vi.fn(), + log: noopLog, + }); + expect(probe).toHaveBeenCalledWith("postgres://legacy", "postgres://cp", expect.anything()); + expect(probe).toHaveBeenCalledWith("postgres://shard-a", "postgres://cp", expect.anything()); + }); + + it("names the offending shard when enforcement is opted in and a shard is co-resident", async () => { + await expect( + assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: true, + shards: [shardA], + probe: async (url: string) => + url === "postgres://shard-a" + ? ({ coresident: "true", reason: "same db" } as const) + : ({ coresident: "false" } as const), + emit: vi.fn(), + log: noopLog, + }) + ).rejects.toThrow(/shard a/i); + }); + + it("emits every store before it throws, so no store loses its metric", async () => { + const emit = vi.fn(); + await expect( + assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: true, + shards: [shardA, shardB], + probe: async (url: string) => + url === "postgres://shard-a" + ? ({ coresident: "true", reason: "same db" } as const) + : ({ coresident: "false" } as const), + emit, + log: noopLog, + }) + ).rejects.toThrow(); + expect(emit).toHaveBeenCalledTimes(3); + expect(emit).toHaveBeenCalledWith("false", "b"); + }); + + it("degrades one shard's throwing probe to unknown and still reports the others", async () => { + const emit = vi.fn(); + await assertControlPlaneCoresidencyAdvisory({ + ...urls, + expectSplit: true, + shards: [shardA, shardB], + probe: async (url: string) => { + if (url === "postgres://shard-a") throw new Error("probe blew up"); + return { coresident: "false" } as const; + }, + emit, + log: { info: () => {}, warn: () => {} }, + }); + expect(emit).toHaveBeenCalledWith("unknown", "a"); + expect(emit).toHaveBeenCalledWith("false", "b"); + }); + + it("still probes the shards when there is no legacy DSN", async () => { + const emit = vi.fn(); + await assertControlPlaneCoresidencyAdvisory({ + legacyUrl: "", + controlPlaneUrl: "postgres://cp", + expectSplit: false, + shards: [shardA], + probe: async () => ({ coresident: "false" }), + emit, + log: noopLog, + }); + expect(emit).toHaveBeenCalledTimes(1); + expect(emit).toHaveBeenCalledWith("false", "a"); + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts index 1fb741b0e67..eab0fe1f034 100644 --- a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts @@ -12,6 +12,7 @@ import type { Counter } from "@opentelemetry/api"; import { getMeter } from "@internal/tracing"; import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; +import { nonAliasedShards, type ShardTarget } from "~/v3/runOpsShards.server"; import { probeControlPlaneCoresidency, type CoresidencyProbeResult, @@ -39,12 +40,15 @@ export type CoresidencyEnforcement = { throw: false } | { throw: true; message: export function resolveCoresidencyEnforcement(args: { coresident: CoresidencyVerdict; expectSplit: boolean; + /** Omitted for the legacy store, so its message stays exactly as it was. */ + shardKey?: string; }): CoresidencyEnforcement { if (args.expectSplit && args.coresident === "true") { + const store = args.shardKey === undefined ? "legacy run-ops DB" : `shard ${args.shardKey}`; return { throw: true, message: - "RUN_OPS_EXPECT_CONTROL_PLANE_SPLIT is on but the legacy run-ops DB is still co-resident with the control-plane DB; refusing to start.", + `RUN_OPS_EXPECT_CONTROL_PLANE_SPLIT is on but the ${store} is still co-resident with the control-plane DB; refusing to start.`, }; } return { throw: false }; @@ -57,46 +61,84 @@ type AdvisoryLogger = { export async function assertControlPlaneCoresidencyAdvisory(deps?: { probe?: typeof probeControlPlaneCoresidency; - emit?: (verdict: CoresidencyVerdict) => void; + /** shardKey is omitted for the legacy store, so its metric series is unchanged at N=0. */ + emit?: (verdict: CoresidencyVerdict, shardKey?: string) => void; log?: AdvisoryLogger; expectSplit?: boolean; legacyUrl?: string; controlPlaneUrl?: string; + shards?: ShardTarget[]; }): Promise { const log = deps?.log ?? logger; const legacyUrl = deps?.legacyUrl ?? env.RUN_OPS_LEGACY_DATABASE_URL; const controlPlaneUrl = deps?.controlPlaneUrl ?? env.CONTROL_PLANE_DATABASE_URL ?? env.DATABASE_URL; - // No legacy DSN (single-DB / self-host) or no control-plane DSN -> nothing to compare. - if (!legacyUrl || !controlPlaneUrl) return; + const shards = deps?.shards ?? nonAliasedShards(env.RUN_OPS_SHARDS); + // No control-plane DSN -> nothing to compare against, for any store. + if (!controlPlaneUrl) return; + + // The legacy store carries NO shard key, so its metric series and its message are unchanged. + // An aliased shard is already absent from `shards`: it shares its target's database on purpose, + // so a co-residency verdict for it would duplicate its target's verdict. + const stores: Array<{ url: string; shardKey?: string }> = [ + ...(legacyUrl ? [{ url: legacyUrl }] : []), + ...shards.map((shard) => ({ url: shard.url, shardKey: shard.key })), + ]; + if (stores.length === 0) return; const probe = deps?.probe ?? probeControlPlaneCoresidency; const emit = deps?.emit ?? - ((verdict: CoresidencyVerdict) => getCoresidentCounter().add(1, { result: verdict })); + ((verdict: CoresidencyVerdict, shardKey?: string) => + getCoresidentCounter().add( + 1, + shardKey === undefined ? { result: verdict } : { result: verdict, shard: shardKey } + )); const expectSplit = deps?.expectSplit ?? env.RUN_OPS_EXPECT_CONTROL_PLANE_SPLIT; - let result: CoresidencyProbeResult; - try { - result = await probe(legacyUrl, controlPlaneUrl, { logger: log }); - } catch (error) { - // Any unexpected throw still degrades to "unknown" — the advisory arm must never crash boot. - log.warn("run-ops control-plane co-residency probe threw; reporting unknown", { error }); - result = { coresident: "unknown", reason: String(error) }; - } + const results = await Promise.all( + stores.map(async (store) => { + let result: CoresidencyProbeResult; + try { + result = await probe(store.url, controlPlaneUrl, { logger: log }); + } catch (error) { + // Any unexpected throw degrades THAT store to "unknown" — the advisory arm must never + // crash boot, and one store's denied probe must not hide another store's verdict. + log.warn("run-ops control-plane co-residency probe threw; reporting unknown", { + error, + shard: store.shardKey, + }); + result = { coresident: "unknown", reason: String(error) }; + } + return { store, result }; + }) + ); - emit(result.coresident); - log.info("run_ops_legacy_control_plane_coresident", { - coresident: result.coresident, - reason: "reason" in result ? result.reason : undefined, - expectSplit, - }); + // Emit and log EVERY store before any enforcement throw, so a failing store never costs + // another store its metric. + for (const { store, result } of results) { + // One argument for the legacy store, so its emission is byte-identical to today's. + if (store.shardKey === undefined) { + emit(result.coresident); + } else { + emit(result.coresident, store.shardKey); + } + log.info("run_ops_legacy_control_plane_coresident", { + coresident: result.coresident, + reason: "reason" in result ? result.reason : undefined, + expectSplit, + shard: store.shardKey, + }); + } - const enforcement = resolveCoresidencyEnforcement({ - coresident: result.coresident, - expectSplit, - }); - if (enforcement.throw) { - throw new Error(enforcement.message); + for (const { store, result } of results) { + const enforcement = resolveCoresidencyEnforcement({ + coresident: result.coresident, + expectSplit, + shardKey: store.shardKey, + }); + if (enforcement.throw) { + throw new Error(enforcement.message); + } } } From f8e9f4f834c59a77e7e29370c31b648c0c26dcc7 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:51:30 +0100 Subject: [PATCH 18/31] feat(webapp): warn per shard when a shard handle is not a distinct instance The gate takes the shard replica handles and warns for any non-aliased shard whose client is not distinct from the control-plane or gen-1 new client. The returned verdict stays the gen-1 verdict: the distinctness sentinel already fail-closes the boot on the same condition, and a gen-2 fault must not disable the proven gen-1 read fan-out on top of that. An aliased shard shares its target's client on purpose, so identity equality is its correct state and never a fault. --- .../v3/runOpsMigration/runOpsSplitReadGate.ts | 23 +++++ apps/webapp/test/runOpsSplitReadGate.test.ts | 87 +++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts b/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts index 0872a256508..89066e26bc6 100644 --- a/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts +++ b/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts @@ -8,6 +8,12 @@ export function computeRunOpsSplitReadEnabled(args: { controlPlaneReplica: unknown; hasNewUrl: boolean; hasLegacyUrl: boolean; + /** + * Gen-2 shard replica handles. Observability only: a non-distinct shard handle WARNS and never + * changes the returned verdict. The distinctness sentinel already fail-closes the boot on the + * same condition, and a gen-2 fault must not disable the proven gen-1 read fan-out as well. + */ + shardHandles?: Array<{ key: string; replica: unknown; aliasOf?: "new" }>; logger?: { warn: (msg: string, meta?: Record) => void }; }): boolean { const newIsDistinctDedicatedClient = @@ -24,5 +30,22 @@ export function computeRunOpsSplitReadEnabled(args: { ); } + // An aliased shard shares its target's client by reference, so identity equality is its correct + // state and never a fault. Keyed on the declared field, not on object identity. + for (const shard of args.shardHandles ?? []) { + if (shard.aliasOf !== undefined) continue; + if ( + shard.replica === args.controlPlaneWriter || + shard.replica === args.controlPlaneReplica || + shard.replica === args.newReplica + ) { + args.logger?.warn( + `run-ops shard ${shard.key} declares its own database but its replica client is not a ` + + "distinct instance from the control-plane or gen-1 new client; reads for that shard " + + "would not reach its database." + ); + } + } + return enabled; } diff --git a/apps/webapp/test/runOpsSplitReadGate.test.ts b/apps/webapp/test/runOpsSplitReadGate.test.ts index 4deb0bb5329..0b87e13b905 100644 --- a/apps/webapp/test/runOpsSplitReadGate.test.ts +++ b/apps/webapp/test/runOpsSplitReadGate.test.ts @@ -165,3 +165,90 @@ describe("computeRunOpsSplitReadEnabled", () => { }); }); }); + +describe("computeRunOpsSplitReadEnabled shard handles", () => { + const shardA = { __tag: "shard-a" }; + const shardB = { __tag: "shard-b" }; + const base = { + newReplica: dedicatedNew, + controlPlaneWriter: cpWriter, + controlPlaneReplica: cpReplica, + hasNewUrl: true, + hasLegacyUrl: true, + }; + + it("does not warn when every shard handle is a distinct instance", () => { + const warn = vi.fn(); + const enabled = computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [ + { key: "a", replica: shardA }, + { key: "b", replica: shardB }, + ], + logger: { warn }, + }); + expect(enabled).toBe(true); + expect(warn).not.toHaveBeenCalled(); + }); + + it("warns, naming the shard, when a shard replica aliases a control-plane handle", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", replica: cpReplica }], + logger: { warn }, + }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toMatch(/shard a/i); + }); + + it("warns when a shard replica aliases the gen-1 new replica", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", replica: dedicatedNew }], + logger: { warn }, + }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it("does NOT warn for an aliased shard, because sharing is its purpose", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "z", replica: dedicatedNew, aliasOf: "new" as const }], + logger: { warn }, + }); + expect(warn).not.toHaveBeenCalled(); + }); + + // The distinctness sentinel already fail-closes the boot on this condition. A gen-2 fault must + // not disable the proven gen-1 read fan-out on top of that. + it("keeps the gen-1 verdict when a shard handle is not distinct", () => { + expect( + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", replica: cpReplica }], + }) + ).toBe(true); + }); + + it("warns once per offending shard", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [ + { key: "a", replica: cpReplica }, + { key: "b", replica: cpWriter }, + ], + logger: { warn }, + }); + expect(warn).toHaveBeenCalledTimes(2); + }); + + it("is unchanged when no shard handle is supplied", () => { + const warn = vi.fn(); + expect(computeRunOpsSplitReadEnabled({ ...base, logger: { warn } })).toBe(true); + expect(warn).not.toHaveBeenCalled(); + }); +}); From ff1f6ca19657e6c026c937a336c71595084ba6da Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:55:08 +0100 Subject: [PATCH 19/31] fix(run-store): fan out waitpoint resolution across all shards, not just the first When a gen-2 shard is configured, RoutingRunStore builds three or more stores (legacy + new + shard) and a waitpoint that is not on its home/run store must be found by probing the others. Three call sites took only the first "other" store (`#shardsExcept(key)[0]`), which was correct with the two-store compat router but silently skips the remaining stores once a shard exists. The effect, observed with a single shard configured: waitpoint lookups return "Waitpoint not found", pending-token counts undercount (which prematurely unblocks a still-waiting run), and many-waitpoint reads miss rows. Fix `#resolveWaitpointStore`, `countPendingWaitpoints` and `#collectManyWaitpoints` to fan out over every other store and merge. Adds a routing unit test that reproduces all three at the production probe order, with the target placed on the store the first-other truncation skipped. Co-Authored-By: Claude Opus 4.8 --- .../run-store/src/runOpsStore.ts | 52 +++++----- .../runOpsStore.waitpointShardFanout.test.ts | 95 +++++++++++++++++++ 2 files changed, 123 insertions(+), 24 deletions(-) create mode 100644 internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts diff --git a/internal-packages/run-store/src/runOpsStore.ts b/internal-packages/run-store/src/runOpsStore.ts index 8d1c7d5aba4..b697182e7e5 100644 --- a/internal-packages/run-store/src/runOpsStore.ts +++ b/internal-packages/run-store/src/runOpsStore.ts @@ -247,8 +247,8 @@ export class RoutingRunStore implements RunStore { } // Every shard other than `key`, in probe order. With the compat constructor this yields exactly - // one entry, which is why each caller may take the first. At more than two shards a caller MUST - // fan out over all of them instead. + // one entry; with three+ stores it yields several, so callers fan out over all of them (a waitpoint + // absent from its home/run store can live on any one of the others). #shardsExcept(key: ShardKey): Array<{ key: ShardKey; store: RunStore }> { return this.#probeOrder .filter((k) => k !== key) @@ -333,16 +333,14 @@ export class RoutingRunStore implements RunStore { ) { return home; } - const [other] = this.#shardsExcept(homeKey); - if (other === undefined) { - return home; + for (const { store } of this.#shardsExcept(homeKey)) { + if ( + await store.findWaitpoint({ where: { id } }, onPrimary ? store.primaryReadClient : undefined) + ) { + return store; + } } - return (await other.store.findWaitpoint( - { where: { id } }, - onPrimary ? other.store.primaryReadClient : undefined - )) - ? other.store - : home; + return home; } static #waitpointId(clause: unknown): string | undefined { @@ -1231,15 +1229,16 @@ export class RoutingRunStore implements RunStore { if (missing.length === 0) { return pendingIds.length; } - const [other] = this.#shardsExcept(runKey); - if (other === undefined) { + const others = this.#shardsExcept(runKey); + if (others.length === 0) { return pendingIds.length; } - const otherPending = await other.store.countPendingWaitpoints( - missing, - RoutingRunStore.#ownPrimary(other.store, client) + const otherPending = await Promise.all( + others.map(({ store }) => + store.countPendingWaitpoints(missing, RoutingRunStore.#ownPrimary(store, client)) + ) ); - return pendingIds.length + otherPending; + return pendingIds.length + otherPending.reduce((sum, n) => sum + n, 0); } // Fan out and union: an id lives on exactly one store in steady state (a drain-mirror can put it on @@ -1433,15 +1432,20 @@ export class RoutingRunStore implements RunStore { if (missing.length === 0) { return fromRun; } - const [other] = this.#shardsExcept(runKey); - if (other === undefined) { + const others = this.#shardsExcept(runKey); + if (others.length === 0) { return fromRun; } - const fromOther = (await other.store.findManyWaitpoints( - narrowArgsToIds(scalarArgs, missing) as Prisma.WaitpointFindManyArgs, - RoutingRunStore.#ownPrimary(other.store, client) - )) as Record[]; - return [...fromRun, ...fromOther]; + const fromOthers = await Promise.all( + others.map( + ({ store }) => + store.findManyWaitpoints( + narrowArgsToIds(scalarArgs, missing) as Prisma.WaitpointFindManyArgs, + RoutingRunStore.#ownPrimary(store, client) + ) as Promise[]> + ) + ); + return [...fromRun, ...fromOthers.flat()]; } // No bounded id set to partition on → fall through to the fan-out path. } diff --git a/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts b/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts new file mode 100644 index 00000000000..13545fa043c --- /dev/null +++ b/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import { generateRunOpsId, resolveShard, type ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import { RoutingRunStore } from "./runOpsStore.js"; +import type { ReadClient, RunStore } from "./types.js"; + +// Regression guard for the N-way waitpoint fan-out. A waitpoint that is not on its home/run store is +// resolved by probing the OTHER shards. With two stores there is exactly one other, so taking the +// first was correct; with three+ stores (legacy + new + a gen-2 shard) taking only the first other +// silently skips the rest, missing a waitpoint that lives on a later shard. Each scenario places the +// target on the store the old first-other truncation skipped (empty shard "a" sorts first). + +type Held = { waitpoints?: string[]; pending?: string[] }; +type FakeStore = RunStore & { slot: ShardKey }; + +function idsFromArgs(args: unknown): string[] { + const where = (args as { where?: { id?: unknown } })?.where ?? {}; + const id = where.id; + if (typeof id === "string") return [id]; + if (id && typeof id === "object" && Array.isArray((id as { in?: unknown[] }).in)) { + return (id as { in: unknown[] }).in.filter((x): x is string => typeof x === "string"); + } + return []; +} + +function fakeStore(slot: ShardKey, held: Held = {}): FakeStore { + const has = new Set(held.waitpoints ?? []); + const pending = new Set(held.pending ?? []); + const findWaitpoint = (args: unknown) => { + const [id] = idsFromArgs(args); + return Promise.resolve(id && has.has(id) ? ({ id, slot } as never) : null); + }; + const store: Partial = { + slot, + primaryReadClient: { __primary: slot } as unknown as ReadClient, + findWaitpoint: findWaitpoint as FakeStore["findWaitpoint"], + findWaitpointOnPrimary: findWaitpoint as FakeStore["findWaitpointOnPrimary"], + countPendingWaitpoints: ((ids: string[]) => + Promise.resolve(ids.filter((id) => pending.has(id)).length)) as FakeStore["countPendingWaitpoints"], + countPendingWaitpointsWithPresence: ((ids: string[]) => + Promise.resolve({ + pendingIds: ids.filter((id) => pending.has(id)), + presentIds: ids.filter((id) => has.has(id)), + })) as FakeStore["countPendingWaitpointsWithPresence"], + findManyWaitpoints: ((args: unknown) => + Promise.resolve( + idsFromArgs(args) + .filter((id) => has.has(id)) + .map((id) => ({ id, slot })) + )) as unknown as FakeStore["findManyWaitpoints"], + }; + return store as FakeStore; +} + +// Production topology: precedence legacy -> new -> shards; probeOrder its exact reverse. +function build(stores: { legacy?: Held; new?: Held; a?: Held }) { + const shards = new Map(); + shards.set("legacy", fakeStore("legacy", stores.legacy)); + shards.set("new", fakeStore("new", stores.new)); + shards.set("a", fakeStore("a", stores.a)); + return RoutingRunStore.fromShards({ + shards, + probeOrder: ["a", "new", "legacy"], + precedence: ["legacy", "new", "a"], + idlessRouteShard: "new", + idlessWaitpointShard: "legacy", + resolveShardKey: resolveShard, + }); +} + +// A cuid waitpoint id resolves home to "legacy"; a gen-1 run id routes the run store to "new". +const CUID_WAITPOINT = "clabc123def456ghi789jkl01"; + +describe("RoutingRunStore N-way waitpoint fan-out", () => { + it("#resolveWaitpointStore finds a waitpoint that lives past the first other shard", async () => { + const store = build({ new: { waitpoints: [CUID_WAITPOINT] } }); + const row = await store.findWaitpoint({ where: { id: CUID_WAITPOINT } }); + expect(row).toMatchObject({ id: CUID_WAITPOINT, slot: "new" }); + }); + + it("countPendingWaitpoints counts a pending token on a later shard (never undercounts)", async () => { + const runId = generateRunOpsId(); // gen-1 -> routes to "new" + const token = "wp_pending_on_legacy"; + const store = build({ legacy: { waitpoints: [token], pending: [token] } }); + const count = await store.countPendingWaitpoints([token], undefined, runId); + expect(count).toBe(1); + }); + + it("#collectManyWaitpoints collects a waitpoint on a later shard", async () => { + const runId = generateRunOpsId(); // gen-1 -> routes to "new" + const token = "wp_on_legacy"; + const store = build({ legacy: { waitpoints: [token] } }); + const rows = await store.findManyWaitpoints({ where: { id: { in: [token] } } }, undefined, runId); + expect(rows).toEqual([{ id: token, slot: "legacy" }]); + }); +}); From 8af907d0afdd11ad281075bb14a3aac5bfb9cc7f Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:56:23 +0100 Subject: [PATCH 20/31] feat(webapp): build one replication source per shard and refuse an uncovered shard buildReplicationSources appends a source per shard that owns its own database, each with the slot, publication and origin generation its descriptor declares. An aliased shard takes no source: its target's slot already carries its WAL. assertReplicationCoversSplit now also requires a source per non-aliased shard. ShardReplicationMisconfiguredError subclasses the split error, so the boot catch site reaches the same process.exit(1) it reaches today. A shard whose runs never arrive in ClickHouse must not serve traffic. RunsReplicationService is untouched. Its own check already rejects a duplicate source id, slot name or origin generation. --- .../runsReplicationInstance.server.ts | 94 +++++++++- .../test/runsReplicationInstance.test.ts | 164 ++++++++++++++++++ 2 files changed, 249 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/services/runsReplicationInstance.server.ts b/apps/webapp/app/services/runsReplicationInstance.server.ts index 164ce07fb92..b732d2c47f4 100644 --- a/apps/webapp/app/services/runsReplicationInstance.server.ts +++ b/apps/webapp/app/services/runsReplicationInstance.server.ts @@ -3,6 +3,7 @@ import { env } from "~/env.server"; import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server"; import { singleton } from "~/utils/singleton"; import { isSplitEnabled } from "~/v3/runOpsMigration/splitMode.server"; +import { nonAliasedShards } from "~/v3/runOpsShards.server"; import { meter, provider } from "~/v3/tracer.server"; import { setRunsReplicationConfiguredSources, @@ -31,6 +32,15 @@ export function buildReplicationSources(args: { newSlotName: string; newPublicationName: string; newOriginGeneration: number; + /** + * Gen-2 shards that own their own database, each with its own slot, publication and origin + * generation. An aliased shard is absent: its target's slot already covers its WAL. + */ + shards?: Array<{ + key: string; + url: string; + replication: { slotName: string; publicationName: string; originGeneration: number }; + }>; }): RunsReplicationSource[] { const legacy: RunsReplicationSource = { id: "legacy", @@ -54,7 +64,25 @@ export function buildReplicationSources(args: { originGeneration: args.newOriginGeneration, }; - return [legacy, next]; + // Shard sources come after the gen-1 pair. Reached only when the new source is on, because + // split is the precondition for a shard to exist at all. The origin generations come from the + // descriptor, which the boot parser already bounds to 2..255 and checks for duplicates; the + // service re-checks uniqueness across every source it is given. + const shardSources: RunsReplicationSource[] = (args.shards ?? []).map((shard) => ({ + id: shardSourceId(shard.key), + pgConnectionUrl: shard.url, + slotName: shard.replication.slotName, + publicationName: shard.replication.publicationName, + originGeneration: shard.replication.originGeneration, + })); + + return [legacy, next, ...shardSources]; +} + +// The replication source id for a shard. It derives the client name and the leader-lock key, so it +// must be stable and unique across sources. +export function shardSourceId(key: string): string { + return `shard-${key}`; } /** @@ -66,24 +94,54 @@ export function buildReplicationSources(args: { * rather than ship a fleet-wide under-count. */ export class SplitReplicationMisconfiguredError extends Error { - constructor() { + constructor(message?: string) { super( - 'RUN_OPS_SPLIT_ENABLED is on but the runs-replication sources[] has no "new" source: ' + - "run-ops runs on the new DB would not replicate to ClickHouse, under-counting every " + - "ClickHouse-fronted aggregate. Enable the new replication source " + - "(RUN_REPLICATION_NEW_ENABLED / RUN_REPLICATION_RUN_OPS_DATABASE_URL) or turn the split off." + message ?? + 'RUN_OPS_SPLIT_ENABLED is on but the runs-replication sources[] has no "new" source: ' + + "run-ops runs on the new DB would not replicate to ClickHouse, under-counting every " + + "ClickHouse-fronted aggregate. Enable the new replication source " + + "(RUN_REPLICATION_NEW_ENABLED / RUN_REPLICATION_RUN_OPS_DATABASE_URL) or turn the split off." ); this.name = "SplitReplicationMisconfiguredError"; } } +/** + * A configured shard with no replication source of its own. Subclasses the split error on purpose: + * the boot catch site tests `instanceof SplitReplicationMisconfiguredError` to reach + * process.exit(1), and a shard whose runs never reach ClickHouse must take that same exit. + */ +export class ShardReplicationMisconfiguredError extends SplitReplicationMisconfiguredError { + constructor(shardKey: string) { + super( + `run-ops shard ${shardKey} is configured but the runs-replication sources[] has no ` + + `"${shardSourceId(shardKey)}" source: runs on that shard would not replicate to ` + + "ClickHouse, under-counting every ClickHouse-fronted aggregate. Give the shard a " + + "replication slot, publication and origin generation, or remove the shard." + ); + this.name = "ShardReplicationMisconfiguredError"; + } +} + export function assertReplicationCoversSplit(args: { splitEnabled: boolean; sources: RunsReplicationSource[]; + /** Every configured shard, aliased ones included. An aliased shard needs no source of its own. */ + shards?: Array<{ key: string; aliasOf?: "new" }>; }): void { - if (args.splitEnabled && !args.sources.some((s) => s.id === "new")) { + if (!args.splitEnabled) { + return; + } + if (!args.sources.some((s) => s.id === "new")) { throw new SplitReplicationMisconfiguredError(); } + for (const shard of args.shards ?? []) { + // An aliased shard shares its target's database, so the target's slot already carries its WAL. + if (shard.aliasOf !== undefined) continue; + if (!args.sources.some((s) => s.id === shardSourceId(shard.key))) { + throw new ShardReplicationMisconfiguredError(shard.key); + } + } } function initializeRunsReplicationInstance() { @@ -171,6 +229,18 @@ function initializeRunsReplicationInstance() { // The legacy-only instance above is never started in the dual path (no slot/lock // taken). runsReplicationService.server.ts is untouched. The create route also calls // setRunsReplicationGlobal — last-writer-wins is the existing contract. + // An aliased shard replicates through its target's slot, so only the shards that own their own + // database take a source. Coverage is then checked against EVERY descriptor, aliased included. + // The schema requires `replication` on every non-aliased descriptor, so the guard below is a + // type narrowing and not a policy. + const shardReplicationByKey = new Map( + env.RUN_OPS_SHARDS.flatMap((d) => (d.replication ? [[d.key, d.replication] as const] : [])) + ); + const shardsWithReplication = nonAliasedShards(env.RUN_OPS_SHARDS).flatMap((shard) => { + const replication = shardReplicationByKey.get(shard.key); + return replication ? [{ key: shard.key, url: shard.url, replication }] : []; + }); + isSplitEnabled() .then(async (splitEnabled) => { const sources = buildReplicationSources({ @@ -184,10 +254,16 @@ function initializeRunsReplicationInstance() { newSlotName: env.RUN_REPLICATION_NEW_SLOT_NAME, newPublicationName: env.RUN_REPLICATION_NEW_PUBLICATION_NAME, newOriginGeneration: env.RUN_REPLICATION_NEW_ORIGIN_GENERATION, + shards: shardsWithReplication, }); - // Refuse to start replication if split is on but `#new` is not a source. - assertReplicationCoversSplit({ splitEnabled, sources }); + // Refuse to start replication if split is on but `#new` is not a source, or if any shard + // that owns its own database has no source of its own. + assertReplicationCoversSplit({ + splitEnabled, + sources, + shards: env.RUN_OPS_SHARDS.map((d) => ({ key: d.key, aliasOf: d.aliasOf })), + }); if (sources.length > 1) { // Release the bootstrap instance's eager replication client (Redis + Redlock) diff --git a/apps/webapp/test/runsReplicationInstance.test.ts b/apps/webapp/test/runsReplicationInstance.test.ts index 67f595c597c..f9f1716028b 100644 --- a/apps/webapp/test/runsReplicationInstance.test.ts +++ b/apps/webapp/test/runsReplicationInstance.test.ts @@ -202,6 +202,170 @@ describe("assertReplicationCoversSplit (boot gate-coupling)", () => { }); }); +describe("replication sources at N shards", () => { + const baseArgs = { + legacyUrl: "postgres://legacy", + legacySlotName: "task_runs_to_clickhouse_v1", + legacyPublicationName: "task_runs_to_clickhouse_v1_publication", + legacyOriginGeneration: 0, + newSlotName: "task_runs_to_clickhouse_v2", + newPublicationName: "task_runs_to_clickhouse_v2_publication", + newOriginGeneration: 1, + splitEnabled: true, + newUrl: "postgres://new", + }; + + const shardA = { + key: "a", + url: "postgres://shard-a", + replication: { + slotName: "task_runs_to_clickhouse_shard_a", + publicationName: "task_runs_to_clickhouse_shard_a_publication", + originGeneration: 2, + }, + }; + const shardB = { + key: "b", + url: "postgres://shard-b", + replication: { + slotName: "task_runs_to_clickhouse_shard_b", + publicationName: "task_runs_to_clickhouse_shard_b_publication", + originGeneration: 3, + }, + }; + + it("appends nothing when no shard is configured", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [] }); + expect(sources.map((s) => s.id)).toEqual(["legacy", "new"]); + }); + + it("appends one source per shard, after legacy and new", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [shardA, shardB] }); + expect(sources.map((s) => s.id)).toEqual(["legacy", "new", "shard-a", "shard-b"]); + expect(sources[2]).toEqual({ + id: "shard-a", + pgConnectionUrl: "postgres://shard-a", + slotName: "task_runs_to_clickhouse_shard_a", + publicationName: "task_runs_to_clickhouse_shard_a_publication", + originGeneration: 2, + }); + }); + + it("gives each shard its own origin generation, between 2 and 255", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [shardA, shardB] }); + const gens = sources.map((s) => s.originGeneration); + expect(new Set(gens).size).toBe(gens.length); + for (const gen of gens.slice(2)) { + expect(gen).toBeGreaterThanOrEqual(2); + expect(gen).toBeLessThanOrEqual(255); + } + }); + + it("appends no shard source when the new source is off, because split is the precondition", () => { + const sources = buildReplicationSources({ + ...baseArgs, + splitEnabled: false, + shards: [shardA], + }); + expect(sources.map((s) => s.id)).toEqual(["legacy"]); + }); + + it("throws when a shard that owns its database has no source", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources, + shards: [{ key: "a" }], + }) + ).toThrow(SplitReplicationMisconfiguredError); + }); + + it("names the uncovered shard in the message", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [shardA] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources, + shards: [{ key: "a" }, { key: "b" }], + }) + ).toThrow(/shard b/i); + }); + + it("does NOT throw when every shard has its own source", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [shardA, shardB] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources, + shards: [{ key: "a" }, { key: "b" }], + }) + ).not.toThrow(); + }); + + it("does NOT require a source for an aliased shard, because its target's slot covers it", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources, + shards: [{ key: "z", aliasOf: "new" }], + }) + ).not.toThrow(); + }); + + it("does NOT check shard coverage when split is off", () => { + const sources = buildReplicationSources({ ...baseArgs, splitEnabled: false, shards: [] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: false, + sources, + shards: [{ key: "a" }], + }) + ).not.toThrow(); + }); + + // The catch site keys on `instanceof SplitReplicationMisconfiguredError` to reach + // process.exit(1). A shard with no replication must reach the same exit. + it("raises an error the existing exit path recognizes", () => { + try { + assertReplicationCoversSplit({ + splitEnabled: true, + sources: buildReplicationSources({ ...baseArgs, shards: [] }), + shards: [{ key: "a" }], + }); + expect.unreachable("expected a throw"); + } catch (error) { + expect(error).toBeInstanceOf(SplitReplicationMisconfiguredError); + } + }); + + // The service validates sources before it builds a single replication client, so this needs no + // container. RunsReplicationService itself is untouched by this change: the check already exists. + it("rejects two shards that share an origin generation, via the service's own check", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [shardA, { ...shardB, replication: { ...shardB.replication, originGeneration: 2 } }], + }); + + expect( + () => + new RunsReplicationService({ + clickhouseFactory: new TestReplicationClickhouseFactory( + new ClickHouse({ url: "http://127.0.0.1:1", name: "unused", logLevel: "warn" }) + ), + serviceName: "runs-replication", + pgConnectionUrl: "postgres://legacy", + slotName: "unused", + publicationName: "unused", + redisOptions: { host: "127.0.0.1", port: 1 }, + sources, + logLevel: "warn", + }) + ).toThrow(/duplicate originGeneration/i); + }); +}); + describe("RunsReplication new-source backfill origin generation (integration)", () => { replicationContainerTest( "backfill via the new source tags the ClickHouse row with the new origin generation (gen=1), not gen=0", From 7afd2a167bcdc7ec6d90d65cb69c3eef54b5b562 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:56:23 +0100 Subject: [PATCH 21/31] feat(docker): list the migration DSN of every shard that owns its own database The runner image has no jq, so the entrypoint cannot parse RUN_OPS_SHARDS on its own. This script prints one DSN per line and takes a unit test, which an inline node -e string could not. An unset or blank variable prints nothing, so a single-database install is unaffected. Invalid JSON exits 1, so a malformed descriptor stops the container before the migrations run rather than after. --- apps/webapp/test/runOpsShardDsns.test.ts | 63 ++++++++++++++++++++++++ docker/scripts/runOpsShardDsns.mjs | 60 ++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 apps/webapp/test/runOpsShardDsns.test.ts create mode 100644 docker/scripts/runOpsShardDsns.mjs diff --git a/apps/webapp/test/runOpsShardDsns.test.ts b/apps/webapp/test/runOpsShardDsns.test.ts new file mode 100644 index 00000000000..02f3be673fa --- /dev/null +++ b/apps/webapp/test/runOpsShardDsns.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +// The entrypoint calls this file with plain `node`, so it takes no path alias and no bundler. +import { shardMigrationDsns } from "../../../docker/scripts/runOpsShardDsns.mjs"; + +const shardA = { + key: "a", + region: "us-east-1", + url: "postgres://h/a", + replication: { slotName: "sa", publicationName: "pa", originGeneration: 2 }, +}; + +describe("shardMigrationDsns", () => { + it("returns nothing when the variable is unset", () => { + expect(shardMigrationDsns(undefined)).toEqual([]); + }); + + it("returns nothing when the variable is blank", () => { + expect(shardMigrationDsns(" ")).toEqual([]); + }); + + it("returns nothing for an empty array", () => { + expect(shardMigrationDsns("[]")).toEqual([]); + }); + + it("throws on invalid JSON, so the entrypoint stops before it migrates", () => { + expect(() => shardMigrationDsns("{not json")).toThrow(/not valid JSON/i); + }); + + it("throws when the value is JSON but not an array", () => { + expect(() => shardMigrationDsns('{"key":"a"}')).toThrow(/not a JSON array/i); + }); + + it("returns the url of a shard that owns its own database", () => { + expect(shardMigrationDsns(JSON.stringify([shardA]))).toEqual(["postgres://h/a"]); + }); + + it("prefers directUrl over url, because migrations must not go through a pooler", () => { + const withDirect = { ...shardA, directUrl: "postgres://h/a-direct" }; + expect(shardMigrationDsns(JSON.stringify([withDirect]))).toEqual(["postgres://h/a-direct"]); + }); + + it("skips an aliased shard, because its target's invocation already migrates it", () => { + const aliased = { key: "z", region: "us-east-1", aliasOf: "new" }; + expect(shardMigrationDsns(JSON.stringify([shardA, aliased]))).toEqual(["postgres://h/a"]); + }); + + it("skips a descriptor with neither url nor directUrl", () => { + const noUrl = { key: "b", region: "us-east-1" }; + expect(shardMigrationDsns(JSON.stringify([shardA, noUrl]))).toEqual(["postgres://h/a"]); + }); + + it("keeps declaration order across several shards", () => { + const shardB = { ...shardA, key: "b", url: "postgres://h/b" }; + expect(shardMigrationDsns(JSON.stringify([shardA, shardB]))).toEqual([ + "postgres://h/a", + "postgres://h/b", + ]); + }); + + it("throws when an entry is not an object", () => { + expect(() => shardMigrationDsns('["postgres://h/a"]')).toThrow(/not an object/i); + }); +}); diff --git a/docker/scripts/runOpsShardDsns.mjs b/docker/scripts/runOpsShardDsns.mjs new file mode 100644 index 00000000000..384e543e996 --- /dev/null +++ b/docker/scripts/runOpsShardDsns.mjs @@ -0,0 +1,60 @@ +// Print the migration DSN of every run-ops shard that owns its own database, one per line, so +// entrypoint.sh can loop over them. The runner image has no `jq`, and this script is unit-tested, +// which an inline `node -e` string could not be. +// +// Contract: +// RUN_OPS_SHARDS unset or blank -> print nothing, exit 0 (single-DB and gen-1-only installs) +// invalid JSON, or not an array -> message on stderr, exit 1 (the app rejects the same value) +// a descriptor with `aliasOf` -> skipped; it shares its target's database +// the DSN -> `directUrl` if set, else `url`; skipped if neither is set +// +// Never print a DSN to stderr or to a log: stdout is consumed by the caller, nothing else. + +export function shardMigrationDsns(raw) { + if (raw === undefined || raw === null || String(raw).trim() === "") { + return []; + } + + let parsed; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error("RUN_OPS_SHARDS is not valid JSON"); + } + + if (!Array.isArray(parsed)) { + throw new Error("RUN_OPS_SHARDS is not a JSON array"); + } + + const dsns = []; + for (const descriptor of parsed) { + if (descriptor === null || typeof descriptor !== "object") { + throw new Error("RUN_OPS_SHARDS holds an entry that is not an object"); + } + // An aliased shard is the same database as its target, which is migrated by its own invocation. + if (descriptor.aliasOf !== undefined && descriptor.aliasOf !== null) { + continue; + } + const dsn = descriptor.directUrl ?? descriptor.url; + if (typeof dsn !== "string" || dsn === "") { + continue; + } + dsns.push(dsn); + } + return dsns; +} + +// `import.meta.main` is not available on every supported node, so compare argv instead. +const invokedDirectly = + process.argv[1] !== undefined && process.argv[1].endsWith("runOpsShardDsns.mjs"); + +if (invokedDirectly) { + try { + for (const dsn of shardMigrationDsns(process.env.RUN_OPS_SHARDS)) { + process.stdout.write(`${dsn}\n`); + } + } catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(1); + } +} From 16165ce18ed0bb2aba351ba370e208217e052098 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 16:57:52 +0100 Subject: [PATCH 22/31] feat(docker): migrate every run-ops shard that owns its own database The two hardcoded run-ops invocations gain a loop over the shard DSNs. Each shard runs the identical schema, so this is the existing migrations against a new DSN, and @internal/run-ops-database needs no change. A for loop and not a while-read pipeline: a pipeline subshell would swallow a failed migration on any iteration but the last, so a broken shard would boot. Tracing stays off across the capture and the loop, because set -x prints an assignment and the DSN carries credentials. Installs that never set RUN_OPS_SHARDS skip the block entirely. --- docker/scripts/entrypoint.sh | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docker/scripts/entrypoint.sh b/docker/scripts/entrypoint.sh index 1e5c7c7cab0..4d9899f2732 100755 --- a/docker/scripts/entrypoint.sh +++ b/docker/scripts/entrypoint.sh @@ -49,6 +49,44 @@ else echo "RUN_OPS_LEGACY_DIRECT_URL not set, skipping legacy run-ops migrations." fi +# Run-ops shards: migrate every gen-2 shard that owns its own database. Each shard runs the +# identical schema, so this is the existing run-ops migrations against a new DSN. An aliased shard is +# skipped by the DSN script: it IS its target's database. Installs that never set RUN_OPS_SHARDS +# skip this entirely. +{ set +x; } 2>/dev/null +if [ -n "$RUN_OPS_SHARDS" ]; then + set -x + if [ "$SKIP_RUN_OPS_SHARD_MIGRATIONS" != "1" ]; then + echo "Running run-ops shard migrations" + # Tracing stays OFF from here to the end of the loop: `set -x` prints an assignment, so + # capturing a DSN under tracing would put the credentials in the logs. + { set +x; } 2>/dev/null + # A malformed descriptor exits 1 here, so the container stops before it migrates anything. + shard_dsns=$(node scripts/runOpsShardDsns.mjs) + # A `for` loop and NOT `... | while read`: a pipeline subshell would swallow a failed migration + # on any iteration but the last. Here `set -e` stops the boot on the first shard that fails. + # IFS is newline-only so a DSN is never split on other whitespace, and `set -f` stops a DSN + # query string (it holds `?`) from being read as a glob pattern. + old_ifs=$IFS + IFS=' +' + set -f + for shard_dsn in $shard_dsns; do + # Subshell with tracing off so `set -x` does not print the DSN (with credentials) to the logs. + (set +x; RUN_OPS_DATABASE_URL="$shard_dsn" DIRECT_URL="$shard_dsn" pnpm --filter @internal/run-ops-database db:migrate:deploy) + done + set +f + IFS=$old_ifs + set -x + echo "Run-ops shard migrations done" + else + echo "SKIP_RUN_OPS_SHARD_MIGRATIONS=1, skipping run-ops shard migrations." + fi +else + set -x + echo "RUN_OPS_SHARDS not set, skipping run-ops shard migrations." +fi + if [ "$SKIP_DASHBOARD_AGENT_MIGRATIONS" != "1" ]; then echo "Running dashboard agent migrations" pnpm --filter @internal/dashboard-agent-db db:migrate:deploy From 04cc56ac8ff93ab4cc87e84a9b5eef525008a7b4 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:00:00 +0100 Subject: [PATCH 23/31] feat(webapp): give the read gate the shard handles and the replication builder the descriptors The read gate receives one handle per shard, with the declared aliasOf, so a shard whose client is not distinct warns. The verdict it returns is unchanged. The replication instance passes the shard descriptors to the source builder and to the coverage check. The distinctness sentinel and the co-residency advisory read the descriptors themselves, so the boot order is unchanged: the probe still runs where it ran, with a wider target list. --- apps/webapp/app/db.server.ts | 9 +++++++++ .../app/services/runsReplicationInstance.server.ts | 4 ++-- .../controlPlaneCoresidencySentinel.server.ts | 3 +-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 11a8aa307e9..40feafa3da5 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -594,6 +594,15 @@ export const runOpsSplitReadEnabled: boolean = computeRunOpsSplitReadEnabled({ controlPlaneReplica: $replica, hasNewUrl: !!env.RUN_OPS_DATABASE_URL, hasLegacyUrl: !!env.RUN_OPS_LEGACY_DATABASE_URL, + // Observability only: a non-distinct shard handle warns and never changes the gen-1 verdict. + // Empty unless RUN_OPS_SHARDS is configured. + shardHandles: runOpsShardHandles.map((handle) => ({ + key: handle.key, + replica: handle.replica, + // The DECLARED field, not client identity: an aliased shard shares its target's client by + // reference, so identity comparison cannot tell the two apart. + aliasOf: env.RUN_OPS_SHARDS.find((d) => d.key === handle.key)?.aliasOf, + })), logger, }); diff --git a/apps/webapp/app/services/runsReplicationInstance.server.ts b/apps/webapp/app/services/runsReplicationInstance.server.ts index b732d2c47f4..6244cccbcdf 100644 --- a/apps/webapp/app/services/runsReplicationInstance.server.ts +++ b/apps/webapp/app/services/runsReplicationInstance.server.ts @@ -81,7 +81,7 @@ export function buildReplicationSources(args: { // The replication source id for a shard. It derives the client name and the leader-lock key, so it // must be stable and unique across sources. -export function shardSourceId(key: string): string { +function shardSourceId(key: string): string { return `shard-${key}`; } @@ -111,7 +111,7 @@ export class SplitReplicationMisconfiguredError extends Error { * the boot catch site tests `instanceof SplitReplicationMisconfiguredError` to reach * process.exit(1), and a shard whose runs never reach ClickHouse must take that same exit. */ -export class ShardReplicationMisconfiguredError extends SplitReplicationMisconfiguredError { +class ShardReplicationMisconfiguredError extends SplitReplicationMisconfiguredError { constructor(shardKey: string) { super( `run-ops shard ${shardKey} is configured but the runs-replication sources[] has no ` + diff --git a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts index eab0fe1f034..870f51c75bc 100644 --- a/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/controlPlaneCoresidencySentinel.server.ts @@ -47,8 +47,7 @@ export function resolveCoresidencyEnforcement(args: { const store = args.shardKey === undefined ? "legacy run-ops DB" : `shard ${args.shardKey}`; return { throw: true, - message: - `RUN_OPS_EXPECT_CONTROL_PLANE_SPLIT is on but the ${store} is still co-resident with the control-plane DB; refusing to start.`, + message: `RUN_OPS_EXPECT_CONTROL_PLANE_SPLIT is on but the ${store} is still co-resident with the control-plane DB; refusing to start.`, }; } return { throw: false }; From a684713ee38158568a760b1b104998db06d65646 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:14:42 +0100 Subject: [PATCH 24/31] fix(webapp): refuse a replication identity collision at the fatal boot gate The descriptor parser checks uniqueness among shards only, so a shard could take the slot name, publication name or origin generation of the legacy or the new source. The service has its own check, but it throws from the constructor, which the caller reaches only after it has shut the bootstrap instance down. That left the process up with NO replication at all, legacy included, behind one console.error. It is the exact silent ClickHouse under-count this family of errors exists to prevent. The check now runs in assertReplicationCoversSplit, before anything is torn down, and raises a subclass the existing catch site already recognizes. A correct deployment satisfies this today, because two consumers on one slot is a data race that cannot work. --- .../runsReplicationInstance.server.ts | 37 ++++++++- .../test/runsReplicationInstance.test.ts | 75 ++++++++++++++++--- 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/apps/webapp/app/services/runsReplicationInstance.server.ts b/apps/webapp/app/services/runsReplicationInstance.server.ts index 6244cccbcdf..750a8fbf789 100644 --- a/apps/webapp/app/services/runsReplicationInstance.server.ts +++ b/apps/webapp/app/services/runsReplicationInstance.server.ts @@ -79,8 +79,9 @@ export function buildReplicationSources(args: { return [legacy, next, ...shardSources]; } -// The replication source id for a shard. It derives the client name and the leader-lock key, so it -// must be stable and unique across sources. +// The replication source id for a shard. It derives the per-source client name and the key the +// status route probes, so it must be stable and unique across sources. The leader lock is keyed on +// the slot name, not on this id. function shardSourceId(key: string): string { return `shard-${key}`; } @@ -106,6 +107,25 @@ export class SplitReplicationMisconfiguredError extends Error { } } +/** + * Two sources that share an identity. The descriptor parser checks uniqueness AMONG shards only, so + * it cannot see the env-configured legacy and new sources: a shard can collide with either. The + * service has its own check, but it throws from the constructor, which the caller reaches only AFTER + * it has shut the bootstrap instance down — leaving the process up with NO replication at all, which + * is the exact silent under-count this family of errors exists to prevent. So the check runs here, + * at the fatal gate, before anything is torn down. + */ +class DuplicateReplicationIdentityError extends SplitReplicationMisconfiguredError { + constructor(field: string, value: unknown) { + super( + `the runs-replication sources[] has two sources with the same ${field} "${String(value)}": ` + + "two consumers on one WAL stream is a data race, and a shared origin generation defeats the " + + "ClickHouse dedup tiebreak. Give every source its own slot, publication and origin generation." + ); + this.name = "DuplicateReplicationIdentityError"; + } +} + /** * A configured shard with no replication source of its own. Subclasses the split error on purpose: * the boot catch site tests `instanceof SplitReplicationMisconfiguredError` to reach @@ -142,6 +162,19 @@ export function assertReplicationCoversSplit(args: { throw new ShardReplicationMisconfiguredError(shard.key); } } + + // Cross-source identity, over EVERY source and not only the shards. A correct two-source + // deployment already satisfies this, because two consumers on one WAL slot is a data race that + // cannot work. So this adds a loud failure for a configuration that was already broken silently. + for (const field of ["id", "slotName", "publicationName", "originGeneration"] as const) { + const seen = new Set(); + for (const source of args.sources) { + if (seen.has(source[field])) { + throw new DuplicateReplicationIdentityError(field, source[field]); + } + seen.add(source[field]); + } + } } function initializeRunsReplicationInstance() { diff --git a/apps/webapp/test/runsReplicationInstance.test.ts b/apps/webapp/test/runsReplicationInstance.test.ts index f9f1716028b..edc2f34260f 100644 --- a/apps/webapp/test/runsReplicationInstance.test.ts +++ b/apps/webapp/test/runsReplicationInstance.test.ts @@ -251,16 +251,6 @@ describe("replication sources at N shards", () => { }); }); - it("gives each shard its own origin generation, between 2 and 255", () => { - const sources = buildReplicationSources({ ...baseArgs, shards: [shardA, shardB] }); - const gens = sources.map((s) => s.originGeneration); - expect(new Set(gens).size).toBe(gens.length); - for (const gen of gens.slice(2)) { - expect(gen).toBeGreaterThanOrEqual(2); - expect(gen).toBeLessThanOrEqual(255); - } - }); - it("appends no shard source when the new source is off, because split is the precondition", () => { const sources = buildReplicationSources({ ...baseArgs, @@ -340,6 +330,71 @@ describe("replication sources at N shards", () => { } }); + // F1 class: the descriptor parser checks uniqueness AMONG shards only. It cannot see the + // env-configured legacy and new sources, so a shard can collide with them. The service's own + // check throws too late: the caller has already shut the bootstrap instance down, so the throw + // leaves the process up with NO replication at all. These must fail at the fatal boot gate. + it("throws when a shard's slot name collides with the gen-1 new slot", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [ + { ...shardA, replication: { ...shardA.replication, slotName: baseArgs.newSlotName } }, + ], + }); + expect(() => + assertReplicationCoversSplit({ splitEnabled: true, sources, shards: [{ key: "a" }] }) + ).toThrow(SplitReplicationMisconfiguredError); + }); + + it("throws when a shard's origin generation collides with the gen-1 new generation", () => { + const sources = buildReplicationSources({ + ...baseArgs, + newOriginGeneration: 2, + shards: [shardA], + }); + expect(() => + assertReplicationCoversSplit({ splitEnabled: true, sources, shards: [{ key: "a" }] }) + ).toThrow(SplitReplicationMisconfiguredError); + }); + + it("throws when a shard's publication name collides with the legacy publication", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [ + { + ...shardA, + replication: { ...shardA.replication, publicationName: baseArgs.legacyPublicationName }, + }, + ], + }); + expect(() => + assertReplicationCoversSplit({ splitEnabled: true, sources, shards: [{ key: "a" }] }) + ).toThrow(SplitReplicationMisconfiguredError); + }); + + it("names the colliding field in the message", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [ + { ...shardA, replication: { ...shardA.replication, slotName: baseArgs.newSlotName } }, + ], + }); + expect(() => + assertReplicationCoversSplit({ splitEnabled: true, sources, shards: [{ key: "a" }] }) + ).toThrow(/slotName/); + }); + + it("does NOT throw when every shard's slot, publication and generation are its own", () => { + const sources = buildReplicationSources({ ...baseArgs, shards: [shardA, shardB] }); + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources, + shards: [{ key: "a" }, { key: "b" }], + }) + ).not.toThrow(); + }); + // The service validates sources before it builds a single replication client, so this needs no // container. RunsReplicationService itself is untouched by this change: the check already exists. it("rejects two shards that share an origin generation, via the service's own check", () => { From ca8e79375a4958cf437e14a5c6248ca9322911cb Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:14:42 +0100 Subject: [PATCH 25/31] fix(webapp): warn about the read-gate case a shard can actually reach A shard with no replicaUrl takes its own writer as its replica handle, so its reads go to its primary. That is the per-shard analogue of the existing legacy warning, and it is reachable today. The control-plane identity check stays as a regression guard, now marked as unreachable by construction: a non-aliased shard always gets a freshly built client. It exists so a future control-plane fallback for shards cannot silently route a shard's reads to another database. --- apps/webapp/app/db.server.ts | 1 + .../v3/runOpsMigration/runOpsSplitReadGate.ts | 22 +++++++-- apps/webapp/test/runOpsSplitReadGate.test.ts | 46 +++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 40feafa3da5..d4f3bd3fa88 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -598,6 +598,7 @@ export const runOpsSplitReadEnabled: boolean = computeRunOpsSplitReadEnabled({ // Empty unless RUN_OPS_SHARDS is configured. shardHandles: runOpsShardHandles.map((handle) => ({ key: handle.key, + writer: handle.writer, replica: handle.replica, // The DECLARED field, not client identity: an aliased shard shares its target's client by // reference, so identity comparison cannot tell the two apart. diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts b/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts index 89066e26bc6..e70dc29f449 100644 --- a/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts +++ b/apps/webapp/app/v3/runOpsMigration/runOpsSplitReadGate.ts @@ -9,11 +9,11 @@ export function computeRunOpsSplitReadEnabled(args: { hasNewUrl: boolean; hasLegacyUrl: boolean; /** - * Gen-2 shard replica handles. Observability only: a non-distinct shard handle WARNS and never - * changes the returned verdict. The distinctness sentinel already fail-closes the boot on the - * same condition, and a gen-2 fault must not disable the proven gen-1 read fan-out as well. + * Gen-2 shard handles. Observability only: a non-distinct shard handle WARNS and never changes the + * returned verdict. A gen-2 fault must not disable the proven gen-1 read fan-out, and the + * distinctness sentinel already fail-closes the boot when two stores share a database. */ - shardHandles?: Array<{ key: string; replica: unknown; aliasOf?: "new" }>; + shardHandles?: Array<{ key: string; writer?: unknown; replica: unknown; aliasOf?: "new" }>; logger?: { warn: (msg: string, meta?: Record) => void }; }): boolean { const newIsDistinctDedicatedClient = @@ -34,6 +34,20 @@ export function computeRunOpsSplitReadEnabled(args: { // state and never a fault. Keyed on the declared field, not on object identity. for (const shard of args.shardHandles ?? []) { if (shard.aliasOf !== undefined) continue; + + // A shard with no replica URL takes its own writer as its replica handle, so its reads go to + // its primary. This is the per-shard analogue of the existing legacy-primary warning. + if (shard.writer !== undefined && shard.replica === shard.writer) { + args.logger?.warn( + `run-ops shard ${shard.key} has no read replica handle; reads for that shard will hit the ` + + "shard primary. Set the shard's replicaUrl to keep replica reads off its primary." + ); + continue; + } + + // Unreachable by construction today: a non-aliased shard always gets a freshly built client. + // Kept as a regression guard, so a future control-plane fallback for shards cannot silently + // route a shard's reads to another database. if ( shard.replica === args.controlPlaneWriter || shard.replica === args.controlPlaneReplica || diff --git a/apps/webapp/test/runOpsSplitReadGate.test.ts b/apps/webapp/test/runOpsSplitReadGate.test.ts index 0b87e13b905..430abbff859 100644 --- a/apps/webapp/test/runOpsSplitReadGate.test.ts +++ b/apps/webapp/test/runOpsSplitReadGate.test.ts @@ -233,6 +233,52 @@ describe("computeRunOpsSplitReadEnabled shard handles", () => { ).toBe(true); }); + // The reachable case. A shard with no replicaUrl gets its own WRITER as its replica handle, so its + // reads go to its primary. selectRunOpsTopology does exactly that (db.server.ts), which makes this + // the per-shard analogue of the existing legacy "reads will hit the legacy primary" warning. + it("warns when a shard has no distinct replica handle, so its reads hit its primary", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", writer: shardA, replica: shardA }], + logger: { warn }, + }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toMatch(/shard a/i); + expect(warn.mock.calls[0][0]).toMatch(/primary/i); + }); + + it("does not warn when a shard has its own distinct replica handle", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", writer: shardA, replica: shardB }], + logger: { warn }, + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it("does NOT warn about primary reads for an aliased shard", () => { + const warn = vi.fn(); + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [ + { key: "z", writer: dedicatedNew, replica: dedicatedNew, aliasOf: "new" as const }, + ], + logger: { warn }, + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it("keeps the gen-1 verdict when a shard reads from its primary", () => { + expect( + computeRunOpsSplitReadEnabled({ + ...base, + shardHandles: [{ key: "a", writer: shardA, replica: shardA }], + }) + ).toBe(true); + }); + it("warns once per offending shard", () => { const warn = vi.fn(); computeRunOpsSplitReadEnabled({ From d84017601f4c66a4e95bd3334b540879e70e1af1 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:14:42 +0100 Subject: [PATCH 26/31] fix(docker): reject a shard DSN holding a line break, and scope the loop's shell options One DSN per line is the protocol between the script and the entrypoint, and the URL parser strips ASCII line breaks, so a DSN holding one would split into two bogus DSNs with nothing upstream to reject it. The loop now runs in a subshell, so its IFS and noglob changes need no restore and cannot leak into the rest of the entrypoint. --- apps/webapp/test/runOpsShardDsns.test.ts | 14 ++++++++++++++ docker/scripts/entrypoint.sh | 22 +++++++++++----------- docker/scripts/runOpsShardDsns.mjs | 5 +++++ 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/apps/webapp/test/runOpsShardDsns.test.ts b/apps/webapp/test/runOpsShardDsns.test.ts index 02f3be673fa..73c6601a6e6 100644 --- a/apps/webapp/test/runOpsShardDsns.test.ts +++ b/apps/webapp/test/runOpsShardDsns.test.ts @@ -61,3 +61,17 @@ describe("shardMigrationDsns", () => { expect(() => shardMigrationDsns('["postgres://h/a"]')).toThrow(/not an object/i); }); }); + +describe("shardMigrationDsns line protocol", () => { + // One DSN per line is the protocol with entrypoint.sh, so a line break would split one DSN into + // two bogus ones. The URL parser strips ASCII line breaks, so nothing upstream rejects this. + it("throws when a DSN holds a line break", () => { + const bad = { key: "a", region: "r", url: "postgres://h/a\npostgres://evil/db" }; + expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i); + }); + + it("throws when a directUrl holds a carriage return", () => { + const bad = { key: "a", region: "r", url: "postgres://h/a", directUrl: "postgres://h/a\rx" }; + expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i); + }); +}); diff --git a/docker/scripts/entrypoint.sh b/docker/scripts/entrypoint.sh index 4d9899f2732..58d17bc6ce9 100755 --- a/docker/scripts/entrypoint.sh +++ b/docker/scripts/entrypoint.sh @@ -65,18 +65,18 @@ if [ -n "$RUN_OPS_SHARDS" ]; then shard_dsns=$(node scripts/runOpsShardDsns.mjs) # A `for` loop and NOT `... | while read`: a pipeline subshell would swallow a failed migration # on any iteration but the last. Here `set -e` stops the boot on the first shard that fails. - # IFS is newline-only so a DSN is never split on other whitespace, and `set -f` stops a DSN - # query string (it holds `?`) from being read as a glob pattern. - old_ifs=$IFS - IFS=' + # The whole loop runs in a subshell, so the IFS and `set -f` changes need no restore and cannot + # leak into the rest of the entrypoint. IFS is newline-only so a DSN is never split on other + # whitespace, and `set -f` stops a DSN query string (it holds `?`) from acting as a glob. + ( + IFS=' ' - set -f - for shard_dsn in $shard_dsns; do - # Subshell with tracing off so `set -x` does not print the DSN (with credentials) to the logs. - (set +x; RUN_OPS_DATABASE_URL="$shard_dsn" DIRECT_URL="$shard_dsn" pnpm --filter @internal/run-ops-database db:migrate:deploy) - done - set +f - IFS=$old_ifs + set -f + for shard_dsn in $shard_dsns; do + # Tracing stays off so `set -x` never prints the DSN (with credentials) to the logs. + RUN_OPS_DATABASE_URL="$shard_dsn" DIRECT_URL="$shard_dsn" pnpm --filter @internal/run-ops-database db:migrate:deploy + done + ) set -x echo "Run-ops shard migrations done" else diff --git a/docker/scripts/runOpsShardDsns.mjs b/docker/scripts/runOpsShardDsns.mjs index 384e543e996..ccb457b4c34 100644 --- a/docker/scripts/runOpsShardDsns.mjs +++ b/docker/scripts/runOpsShardDsns.mjs @@ -39,6 +39,11 @@ export function shardMigrationDsns(raw) { if (typeof dsn !== "string" || dsn === "") { continue; } + // One DSN per line IS the protocol with the caller, so a DSN holding a line break would split + // into two bogus DSNs. The URL parser strips ASCII line breaks, so nothing upstream rejects it. + if (/[\r\n]/.test(dsn)) { + throw new Error("RUN_OPS_SHARDS holds a DSN containing a line break"); + } dsns.push(dsn); } return dsns; From 24eb536301c363cffd709d2160f199ddfc7c0486 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:33:42 +0100 Subject: [PATCH 27/31] test(run-store): drop mock-based waitpoint fan-out test; main's nShardMatrix covers it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's runOpsStore.nShardMatrix.test.ts is a four-store testcontainer matrix that already guards the N-way waitpoint fan-out on real databases — the gen-2-shard union with no double count, the mirrored-cuid case, alias dedup, and cross-tree completion. The removed test used fakeStore() stubs, which both duplicates that coverage and violates the repo's "never mock, use testcontainers" rule (CodeRabbit). Removing it also clears the code-quality oxfmt --check failure the unformatted file caused. --- .../runOpsStore.waitpointShardFanout.test.ts | 96 ------------------- 1 file changed, 96 deletions(-) delete mode 100644 internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts diff --git a/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts b/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts deleted file mode 100644 index f1eb9e3335f..00000000000 --- a/internal-packages/run-store/src/runOpsStore.waitpointShardFanout.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - generateRunOpsId, - generateRunOpsIdV2, - resolveShard, - type ShardKey, -} from "@trigger.dev/core/v3/isomorphic"; -import { RoutingRunStore } from "./runOpsStore.js"; -import type { ReadClient, RunStore } from "./types.js"; - -// Regression guard for the N-way waitpoint fan-out. A waitpoint that is not on its home/run store is -// resolved by probing the other stores: a cuid can only be drain-relocated between the two gen-1 -// stores, and a gen-2 id names exactly one shard. Each scenario places the target on a store other -// than the run's own and asserts the router still finds/counts it. - -type Held = { waitpoints?: string[]; pending?: string[] }; -type FakeStore = RunStore & { slot: ShardKey }; - -function idsFromArgs(args: unknown): string[] { - const where = (args as { where?: { id?: unknown } })?.where ?? {}; - const id = where.id; - if (typeof id === "string") return [id]; - if (id && typeof id === "object" && Array.isArray((id as { in?: unknown[] }).in)) { - return (id as { in: unknown[] }).in.filter((x): x is string => typeof x === "string"); - } - return []; -} - -function fakeStore(slot: ShardKey, held: Held = {}): FakeStore { - const has = new Set(held.waitpoints ?? []); - const pending = new Set(held.pending ?? []); - const findWaitpoint = (args: unknown) => { - const [id] = idsFromArgs(args); - return Promise.resolve(id && has.has(id) ? ({ id, slot } as never) : null); - }; - const store: Partial = { - slot, - primaryReadClient: { __primary: slot } as unknown as ReadClient, - findWaitpoint: findWaitpoint as FakeStore["findWaitpoint"], - findWaitpointOnPrimary: findWaitpoint as FakeStore["findWaitpointOnPrimary"], - countPendingWaitpoints: ((ids: string[]) => - Promise.resolve(ids.filter((id) => pending.has(id)).length)) as FakeStore["countPendingWaitpoints"], - countPendingWaitpointsWithPresence: ((ids: string[]) => - Promise.resolve({ - pendingIds: ids.filter((id) => pending.has(id)), - presentIds: ids.filter((id) => has.has(id)), - })) as FakeStore["countPendingWaitpointsWithPresence"], - findManyWaitpoints: ((args: unknown) => - Promise.resolve( - idsFromArgs(args) - .filter((id) => has.has(id)) - .map((id) => ({ id, slot })) - )) as unknown as FakeStore["findManyWaitpoints"], - }; - return store as FakeStore; -} - -// One gen-2 shard "a" alongside the gen-1 pair. The constructor derives probe/precedence order. -function build(stores: { legacy?: Held; new?: Held; a?: Held }) { - return new RoutingRunStore({ - new: fakeStore("new", stores.new), - legacy: fakeStore("legacy", stores.legacy), - shards: [{ key: "a", store: fakeStore("a", stores.a) }], - resolveShard, - }); -} - -// A cuid waitpoint id resolves home to "legacy"; a gen-1 run id routes the run store to "new". -const CUID_WAITPOINT = "clabc123def456ghi789jkl01"; - -describe("RoutingRunStore N-way waitpoint fan-out", () => { - it("resolves a cuid waitpoint that was drain-relocated onto the other gen-1 store", async () => { - const store = build({ new: { waitpoints: [CUID_WAITPOINT] } }); - const row = await store.findWaitpoint({ where: { id: CUID_WAITPOINT } }); - expect(row).toMatchObject({ id: CUID_WAITPOINT, slot: "new" }); - }); - - it("counts a pending cuid token on the other gen-1 store (never undercounts)", async () => { - const runId = generateRunOpsId(); // gen-1 -> run store is "new" - const store = build({ legacy: { waitpoints: [CUID_WAITPOINT], pending: [CUID_WAITPOINT] } }); - const count = await store.countPendingWaitpoints([CUID_WAITPOINT], undefined, runId); - expect(count).toBe(1); - }); - - it("collects a gen-2 waitpoint that lives on its own shard, not the run's store", async () => { - const runId = generateRunOpsId(); // gen-1 -> run store is "new" - const shardWaitpoint = generateRunOpsIdV2("a"); // resolves to shard "a" - const store = build({ a: { waitpoints: [shardWaitpoint] } }); - const rows = await store.findManyWaitpoints( - { where: { id: { in: [shardWaitpoint] } } }, - undefined, - runId - ); - expect(rows).toEqual([{ id: shardWaitpoint, slot: "a" }]); - }); -}); From aa2bb6835314c3b9769d29a8cdb9a95738fdfa5b Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 17:38:00 +0100 Subject: [PATCH 28/31] test(run-store): remove stale fromShards test after merging main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runOpsStore.fromShards.test.ts imported UnknownShardKey and called RoutingRunStore.fromShards — both removed in main's RoutingRunStore refactor (constructor-based shards). The file is unique to this branch and now references APIs that no longer exist, so it fails the run-store suite. Its routing coverage lives in main's shardMap/runKeyedRouting/nShardMatrix tests. --- .../src/runOpsStore.fromShards.test.ts | 65 ------------------- 1 file changed, 65 deletions(-) delete mode 100644 internal-packages/run-store/src/runOpsStore.fromShards.test.ts diff --git a/internal-packages/run-store/src/runOpsStore.fromShards.test.ts b/internal-packages/run-store/src/runOpsStore.fromShards.test.ts deleted file mode 100644 index e9f55332ad7..00000000000 --- a/internal-packages/run-store/src/runOpsStore.fromShards.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - generateRunOpsId, - generateRunOpsIdV2, - resolveShard, - type ShardKey, -} from "@trigger.dev/core/v3/isomorphic"; -import { RoutingRunStore, UnknownShardKey } from "./runOpsStore.js"; -import type { ReadClient, RunStore } from "./types.js"; - -// Pure routing unit test for the N-way fromShards factory. Each shard is a fake RunStore whose -// findRun records which slot answered, so the assertions are purely about WHICH store the router -// selects. No database. -type FakeStore = RunStore & { slot: ShardKey }; - -function fakeStore(slot: ShardKey): FakeStore { - const store: Partial = { - slot, - primaryReadClient: { __primary: slot } as unknown as ReadClient, - findRun: ((_where: unknown, _argsOrClient?: unknown, _client?: unknown) => - Promise.resolve({ slot } as never)) as FakeStore["findRun"], - }; - return store as FakeStore; -} - -function build(shardKeys: ShardKey[]) { - const shards = new Map(); - shards.set("legacy", fakeStore("legacy")); - shards.set("new", fakeStore("new")); - for (const k of shardKeys) shards.set(k, fakeStore(k)); - return RoutingRunStore.fromShards({ - shards, - probeOrder: ["new", ...shardKeys, "legacy"], - precedence: ["legacy", "new", ...shardKeys], - idlessRouteShard: "new", - idlessWaitpointShard: "legacy", - resolveShardKey: resolveShard, - }); -} - -describe("RoutingRunStore.fromShards", () => { - it("routes a gen-2 id to its own shard, not to new", async () => { - const store = build(["a"]); - const found = await store.findRun({ friendlyId: generateRunOpsIdV2("a") }); - expect(found).toMatchObject({ slot: "a" }); - }); - - it("routes a gen-1 v1 id to new", async () => { - const store = build(["a"]); - const found = await store.findRun({ friendlyId: generateRunOpsId() }); - expect(found).toMatchObject({ slot: "new" }); - }); - - it("routes a cuid id to legacy", async () => { - const store = build(["a"]); - const found = await store.findRun({ friendlyId: "clabc123def456ghi789jkl01" }); - expect(found).toMatchObject({ slot: "legacy" }); - }); - - it("raises UnknownShardKey for an unconfigured shard and does not fall back", () => { - const store = build(["a"]); // "b" is not configured - // The route resolves synchronously, so the throw is synchronous (before the promise is built). - expect(() => store.findRun({ friendlyId: generateRunOpsIdV2("b") })).toThrow(UnknownShardKey); - }); -}); From e6b40d1693025f447700b161e2d472ad6bcd9d5b Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Tue, 25 Aug 2026 18:06:19 +0100 Subject: [PATCH 29/31] fix(docker): hold the shard DSN script to the same contract as the boot schema The script was laxer than the schema that validates the same variable. It treated any aliasOf value as an alias, so a shard with a typo in that field was skipped and its database never migrated. It also accepted a shard that owns its database but declares no replication, which the application rejects at boot: the entrypoint migrated the database first and the boot failed afterwards. The script now rejects an unsupported aliasOf value, a descriptor that sets both url and aliasOf or neither, and a non-aliased descriptor with no replication. So an invalid descriptor stops the container before any migration runs, which is what the block exists to guarantee. --- apps/webapp/test/runOpsShardDsns.test.ts | 42 +++++++++++++++++++++--- docker/scripts/runOpsShardDsns.mjs | 17 +++++++++- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/apps/webapp/test/runOpsShardDsns.test.ts b/apps/webapp/test/runOpsShardDsns.test.ts index 73c6601a6e6..a46f160e3fb 100644 --- a/apps/webapp/test/runOpsShardDsns.test.ts +++ b/apps/webapp/test/runOpsShardDsns.test.ts @@ -44,9 +44,9 @@ describe("shardMigrationDsns", () => { expect(shardMigrationDsns(JSON.stringify([shardA, aliased]))).toEqual(["postgres://h/a"]); }); - it("skips a descriptor with neither url nor directUrl", () => { + it("rejects a descriptor with neither url nor aliasOf", () => { const noUrl = { key: "b", region: "us-east-1" }; - expect(shardMigrationDsns(JSON.stringify([shardA, noUrl]))).toEqual(["postgres://h/a"]); + expect(() => shardMigrationDsns(JSON.stringify([shardA, noUrl]))).toThrow(/exactly one/i); }); it("keeps declaration order across several shards", () => { @@ -66,12 +66,46 @@ describe("shardMigrationDsns line protocol", () => { // One DSN per line is the protocol with entrypoint.sh, so a line break would split one DSN into // two bogus ones. The URL parser strips ASCII line breaks, so nothing upstream rejects this. it("throws when a DSN holds a line break", () => { - const bad = { key: "a", region: "r", url: "postgres://h/a\npostgres://evil/db" }; + const bad = { ...shardA, url: "postgres://h/a\npostgres://evil/db" }; expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i); }); it("throws when a directUrl holds a carriage return", () => { - const bad = { key: "a", region: "r", url: "postgres://h/a", directUrl: "postgres://h/a\rx" }; + const bad = { ...shardA, directUrl: "postgres://h/a\rx" }; expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i); }); }); + +// The script and the boot schema validate the same variable, so they must agree. If the script is +// laxer, the entrypoint migrates a database and the application then refuses to start, which breaks +// the fail-before-migration contract the entrypoint exists to hold. +describe("shardMigrationDsns matches the descriptor contract", () => { + it("rejects an aliasOf value the schema does not allow", () => { + const bad = [{ key: "a", region: "r", url: "postgres://h/a", aliasOf: "other" }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/aliasOf/i); + }); + + it("rejects a shard that owns its database but declares no replication", () => { + const bad = [{ key: "b", region: "r", url: "postgres://h/b" }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/replication/i); + }); + + it("rejects a descriptor that sets both url and aliasOf", () => { + const bad = [{ key: "c", region: "r", url: "postgres://h/c", aliasOf: "new" }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/exactly one/i); + }); + + it("rejects a descriptor that sets neither url nor aliasOf", () => { + const bad = [{ key: "d", region: "r" }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/exactly one/i); + }); + + it("still accepts a valid aliased descriptor and skips it", () => { + const ok = [{ key: "z", region: "r", aliasOf: "new" }]; + expect(shardMigrationDsns(JSON.stringify(ok))).toEqual([]); + }); + + it("still accepts a valid owning descriptor", () => { + expect(shardMigrationDsns(JSON.stringify([shardA]))).toEqual(["postgres://h/a"]); + }); +}); diff --git a/docker/scripts/runOpsShardDsns.mjs b/docker/scripts/runOpsShardDsns.mjs index ccb457b4c34..009359cb219 100644 --- a/docker/scripts/runOpsShardDsns.mjs +++ b/docker/scripts/runOpsShardDsns.mjs @@ -31,8 +31,23 @@ export function shardMigrationDsns(raw) { if (descriptor === null || typeof descriptor !== "object") { throw new Error("RUN_OPS_SHARDS holds an entry that is not an object"); } + // The boot schema (runOpsShards.server.ts) validates the same variable. This script must not be + // laxer: a descriptor it accepts and the application rejects would migrate a database and then + // fail the boot, which breaks the fail-before-migration contract. + const hasAlias = descriptor.aliasOf !== undefined && descriptor.aliasOf !== null; + if (hasAlias && descriptor.aliasOf !== "new") { + throw new Error(`RUN_OPS_SHARDS: aliasOf must be "new", got "${descriptor.aliasOf}"`); + } + const hasUrl = typeof descriptor.url === "string" && descriptor.url !== ""; + if (hasUrl === hasAlias) { + throw new Error("RUN_OPS_SHARDS: exactly one of url or aliasOf is required"); + } + if (!hasAlias && (descriptor.replication === undefined || descriptor.replication === null)) { + throw new Error("RUN_OPS_SHARDS: replication is required unless aliasOf is set"); + } + // An aliased shard is the same database as its target, which is migrated by its own invocation. - if (descriptor.aliasOf !== undefined && descriptor.aliasOf !== null) { + if (hasAlias) { continue; } const dsn = descriptor.directUrl ?? descriptor.url; From 7fa060925d26cb3a7af8177d99c155ee93413e31 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Wed, 26 Aug 2026 12:55:40 +0100 Subject: [PATCH 30/31] fix(docker): validate descriptor values, not just their shape, before emitting DSNs The script checked that url was a non-empty string, while the boot schema requires a parseable URL with no empty schema param. So a descriptor the application rejects could pass the script, and a valid descriptor ahead of an invalid one got its database migrated before the configuration failed. Every descriptor is now validated before any DSN is collected: the shard key shape, the region, the three URLs, the alias value, and the replication slot, publication and origin generation bound. Unknown fields are deliberately still accepted, because rejecting them would fail the entrypoint on a descriptor a newer application accepts. --- apps/webapp/test/runOpsShardDsns.test.ts | 80 ++++++++++++++++++ docker/scripts/runOpsShardDsns.mjs | 100 +++++++++++++++++------ 2 files changed, 156 insertions(+), 24 deletions(-) diff --git a/apps/webapp/test/runOpsShardDsns.test.ts b/apps/webapp/test/runOpsShardDsns.test.ts index a46f160e3fb..a2cef335b1d 100644 --- a/apps/webapp/test/runOpsShardDsns.test.ts +++ b/apps/webapp/test/runOpsShardDsns.test.ts @@ -109,3 +109,83 @@ describe("shardMigrationDsns matches the descriptor contract", () => { expect(shardMigrationDsns(JSON.stringify([shardA]))).toEqual(["postgres://h/a"]); }); }); + +// The boot schema URL-validates url, replicaUrl and directUrl with isValidDatabaseUrl. The script +// must not be laxer, or a valid descriptor ahead of an invalid one gets its database migrated before +// the configuration is rejected. +describe("shardMigrationDsns validates descriptor values", () => { + const rep = { slotName: "s", publicationName: "p", originGeneration: 2 }; + + it("rejects a url that is not a parseable URL", () => { + const bad = [{ key: "a", region: "r", url: "not a URL", replication: rep }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/url/i); + }); + + it("rejects a directUrl that is not a parseable URL", () => { + const bad = [ + { key: "a", region: "r", url: "postgres://h/a", directUrl: "nope", replication: rep }, + ]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/directUrl/i); + }); + + it("rejects a replicaUrl that is not a parseable URL", () => { + const bad = [ + { key: "a", region: "r", url: "postgres://h/a", replicaUrl: "nope", replication: rep }, + ]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/replicaUrl/i); + }); + + it("rejects an empty schema search param, matching the boot schema", () => { + const bad = [{ key: "a", region: "r", url: "postgres://h/a?schema=", replication: rep }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/url/i); + }); + + it("rejects a multi-char shard key", () => { + const bad = [{ key: "ab", region: "r", url: "postgres://h/a", replication: rep }]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/key/i); + }); + + it("rejects an origin generation outside 2..255", () => { + for (const gen of [1, 256]) { + const bad = [ + { + key: "a", + region: "r", + url: "postgres://h/a", + replication: { ...rep, originGeneration: gen }, + }, + ]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/originGeneration/i); + } + }); + + it("rejects a replication block with a blank slot name", () => { + const bad = [ + { key: "a", region: "r", url: "postgres://h/a", replication: { ...rep, slotName: "" } }, + ]; + expect(() => shardMigrationDsns(JSON.stringify(bad))).toThrow(/slotName/i); + }); + + // The failure must come BEFORE any DSN is handed back, so no database is migrated first. + it("emits nothing when a later descriptor is invalid", () => { + const mixed = [ + { key: "a", region: "r", url: "postgres://h/a", replication: rep }, + { key: "b", region: "r", url: "not a URL", replication: { ...rep, originGeneration: 3 } }, + ]; + expect(() => shardMigrationDsns(JSON.stringify(mixed))).toThrow(/url/i); + }); + + it("still accepts a fully valid descriptor with all three URLs", () => { + const ok = [ + { + key: "a", + region: "r", + url: "postgres://h/a?schema=public", + replicaUrl: "postgres://h/a-replica?schema=public", + directUrl: "postgres://h/a-direct?schema=public", + replication: rep, + }, + ]; + expect(shardMigrationDsns(JSON.stringify(ok))).toEqual(["postgres://h/a-direct?schema=public"]); + }); +}); diff --git a/docker/scripts/runOpsShardDsns.mjs b/docker/scripts/runOpsShardDsns.mjs index 009359cb219..dc47d1500f7 100644 --- a/docker/scripts/runOpsShardDsns.mjs +++ b/docker/scripts/runOpsShardDsns.mjs @@ -10,6 +10,71 @@ // // Never print a DSN to stderr or to a log: stdout is consumed by the caller, nothing else. +// Mirrors isValidDatabaseUrl in the webapp: parseable by URL(), and no empty `schema` param. +function assertDatabaseUrl(value, field, key) { + try { + const parsed = new URL(value); + if (parsed.searchParams.get("schema") === "") { + throw new Error("empty schema param"); + } + } catch { + throw new Error(`RUN_OPS_SHARDS[${key}]: ${field} is not a valid database URL`); + } +} + +/** + * Mirrors the boot schema's rules for one descriptor. Kept deliberately narrow: it checks the rules + * that make the application REJECT the value at boot, so an invalid descriptor stops the entrypoint + * before any migration runs. It does NOT reject unknown fields, because doing so would fail the + * entrypoint on a descriptor a newer application accepts, which is drift in the other direction. + */ +function assertDescriptor(d) { + const key = typeof d.key === "string" ? d.key : "?"; + if (typeof d.key !== "string" || !/^[a-z0-9]$/.test(d.key)) { + throw new Error(`RUN_OPS_SHARDS[${key}]: key must be a single [a-z0-9] char`); + } + if (typeof d.region !== "string" || d.region === "") { + throw new Error(`RUN_OPS_SHARDS[${key}]: region is required`); + } + + const hasAlias = d.aliasOf !== undefined && d.aliasOf !== null; + if (hasAlias && d.aliasOf !== "new") { + throw new Error(`RUN_OPS_SHARDS[${key}]: aliasOf must be "new", got "${d.aliasOf}"`); + } + + const hasUrl = d.url !== undefined && d.url !== null; + if (hasUrl === hasAlias) { + throw new Error(`RUN_OPS_SHARDS[${key}]: exactly one of url or aliasOf is required`); + } + + for (const field of ["url", "replicaUrl", "directUrl"]) { + const value = d[field]; + if (value === undefined || value === null) continue; + if (typeof value !== "string" || value === "") { + throw new Error(`RUN_OPS_SHARDS[${key}]: ${field} is not a valid database URL`); + } + assertDatabaseUrl(value, field, key); + } + + if (hasAlias) return; + + const rep = d.replication; + if (rep === undefined || rep === null || typeof rep !== "object") { + throw new Error(`RUN_OPS_SHARDS[${key}]: replication is required unless aliasOf is set`); + } + for (const field of ["slotName", "publicationName"]) { + if (typeof rep[field] !== "string" || rep[field] === "") { + throw new Error(`RUN_OPS_SHARDS[${key}]: replication.${field} must be a non-empty string`); + } + } + const gen = rep.originGeneration; + if (!Number.isInteger(gen) || gen < 2 || gen > 255) { + throw new Error( + `RUN_OPS_SHARDS[${key}]: replication.originGeneration must be an integer 2..255` + ); + } +} + export function shardMigrationDsns(raw) { if (raw === undefined || raw === null || String(raw).trim() === "") { return []; @@ -26,36 +91,23 @@ export function shardMigrationDsns(raw) { throw new Error("RUN_OPS_SHARDS is not a JSON array"); } - const dsns = []; + // Validate EVERY descriptor before collecting any DSN, so a valid descriptor ahead of an invalid + // one never gets its database migrated before the configuration is rejected. for (const descriptor of parsed) { - if (descriptor === null || typeof descriptor !== "object") { + if (descriptor === null || typeof descriptor !== "object" || Array.isArray(descriptor)) { throw new Error("RUN_OPS_SHARDS holds an entry that is not an object"); } - // The boot schema (runOpsShards.server.ts) validates the same variable. This script must not be - // laxer: a descriptor it accepts and the application rejects would migrate a database and then - // fail the boot, which breaks the fail-before-migration contract. - const hasAlias = descriptor.aliasOf !== undefined && descriptor.aliasOf !== null; - if (hasAlias && descriptor.aliasOf !== "new") { - throw new Error(`RUN_OPS_SHARDS: aliasOf must be "new", got "${descriptor.aliasOf}"`); - } - const hasUrl = typeof descriptor.url === "string" && descriptor.url !== ""; - if (hasUrl === hasAlias) { - throw new Error("RUN_OPS_SHARDS: exactly one of url or aliasOf is required"); - } - if (!hasAlias && (descriptor.replication === undefined || descriptor.replication === null)) { - throw new Error("RUN_OPS_SHARDS: replication is required unless aliasOf is set"); - } + assertDescriptor(descriptor); + } + + const dsns = []; + for (const descriptor of parsed) { + // An aliased shard is the same database as its target, which its own invocation migrates. + if (descriptor.aliasOf !== undefined && descriptor.aliasOf !== null) continue; - // An aliased shard is the same database as its target, which is migrated by its own invocation. - if (hasAlias) { - continue; - } const dsn = descriptor.directUrl ?? descriptor.url; - if (typeof dsn !== "string" || dsn === "") { - continue; - } // One DSN per line IS the protocol with the caller, so a DSN holding a line break would split - // into two bogus DSNs. The URL parser strips ASCII line breaks, so nothing upstream rejects it. + // into two bogus DSNs. The URL parser strips ASCII line breaks, so nothing else rejects it. if (/[\r\n]/.test(dsn)) { throw new Error("RUN_OPS_SHARDS holds a DSN containing a line break"); } From a4b599826e12b5f8b42cf8d2f8e3f6950aaca721 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Wed, 26 Aug 2026 13:19:02 +0100 Subject: [PATCH 31/31] fix(webapp): give a shard's replication source its direct connection, not the pooled one A shard source took the shard's writer dsn. Logical replication needs a session-mode connection, and a transaction pooler cannot serve one, so a pooled writer dsn makes the replication client throw inside start(). That throw is not a SplitReplicationMisconfiguredError, so the process stayed up with every source down, legacy included. Gen-1 already keeps this separation through its own RUN_REPLICATION_* variables, and the migration loop already prefers directUrl. A shard source now takes directUrl, and a shard that declares replication without a directUrl refuses the boot rather than falling back to a dsn that may be pooled. --- .../runsReplicationInstance.server.ts | 39 +++++++- .../test/runsReplicationInstance.test.ts | 93 +++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/services/runsReplicationInstance.server.ts b/apps/webapp/app/services/runsReplicationInstance.server.ts index 750a8fbf789..7c074b1586b 100644 --- a/apps/webapp/app/services/runsReplicationInstance.server.ts +++ b/apps/webapp/app/services/runsReplicationInstance.server.ts @@ -39,6 +39,8 @@ export function buildReplicationSources(args: { shards?: Array<{ key: string; url: string; + /** The DIRECT, non-pooled DSN. Logical replication needs a session-mode connection. */ + directUrl?: string; replication: { slotName: string; publicationName: string; originGeneration: number }; }>; }): RunsReplicationSource[] { @@ -68,9 +70,12 @@ export function buildReplicationSources(args: { // split is the precondition for a shard to exist at all. The origin generations come from the // descriptor, which the boot parser already bounds to 2..255 and checks for duplicates; the // service re-checks uniqueness across every source it is given. + // The DIRECT dsn, not the app writer dsn. A transaction pooler cannot serve the replication + // protocol, and the writer dsn is pooled in a real deployment. Gen-1 keeps the same separation + // through its own RUN_REPLICATION_* variables, and the migration loop prefers directUrl too. const shardSources: RunsReplicationSource[] = (args.shards ?? []).map((shard) => ({ id: shardSourceId(shard.key), - pgConnectionUrl: shard.url, + pgConnectionUrl: shard.directUrl ?? shard.url, slotName: shard.replication.slotName, publicationName: shard.replication.publicationName, originGeneration: shard.replication.originGeneration, @@ -143,11 +148,28 @@ class ShardReplicationMisconfiguredError extends SplitReplicationMisconfiguredEr } } +/** + * A shard that replicates but declares no direct dsn. Falling back to its writer dsn is a silent + * trap: if that dsn is pooled, the replication client throws inside start(), which is NOT a + * SplitReplicationMisconfiguredError, so the process stays up with EVERY source down, legacy + * included. Refuse the boot instead. + */ +class ShardDirectUrlMissingError extends SplitReplicationMisconfiguredError { + constructor(shardKey: string) { + super( + `run-ops shard ${shardKey} declares replication but no directUrl: logical replication needs a ` + + "session-mode connection, which a transaction pooler cannot serve. Give the shard a directUrl " + + "pointing at its direct, non-pooled endpoint." + ); + this.name = "ShardDirectUrlMissingError"; + } +} + export function assertReplicationCoversSplit(args: { splitEnabled: boolean; sources: RunsReplicationSource[]; /** Every configured shard, aliased ones included. An aliased shard needs no source of its own. */ - shards?: Array<{ key: string; aliasOf?: "new" }>; + shards?: Array<{ key: string; aliasOf?: "new"; hasDirectUrl?: boolean }>; }): void { if (!args.splitEnabled) { return; @@ -161,6 +183,9 @@ export function assertReplicationCoversSplit(args: { if (!args.sources.some((s) => s.id === shardSourceId(shard.key))) { throw new ShardReplicationMisconfiguredError(shard.key); } + if (shard.hasDirectUrl === false) { + throw new ShardDirectUrlMissingError(shard.key); + } } // Cross-source identity, over EVERY source and not only the shards. A correct two-source @@ -271,7 +296,9 @@ function initializeRunsReplicationInstance() { ); const shardsWithReplication = nonAliasedShards(env.RUN_OPS_SHARDS).flatMap((shard) => { const replication = shardReplicationByKey.get(shard.key); - return replication ? [{ key: shard.key, url: shard.url, replication }] : []; + return replication + ? [{ key: shard.key, url: shard.url, directUrl: shard.directUrl, replication }] + : []; }); isSplitEnabled() @@ -295,7 +322,11 @@ function initializeRunsReplicationInstance() { assertReplicationCoversSplit({ splitEnabled, sources, - shards: env.RUN_OPS_SHARDS.map((d) => ({ key: d.key, aliasOf: d.aliasOf })), + shards: env.RUN_OPS_SHARDS.map((d) => ({ + key: d.key, + aliasOf: d.aliasOf, + hasDirectUrl: d.directUrl !== undefined, + })), }); if (sources.length > 1) { diff --git a/apps/webapp/test/runsReplicationInstance.test.ts b/apps/webapp/test/runsReplicationInstance.test.ts index edc2f34260f..86aeb86116e 100644 --- a/apps/webapp/test/runsReplicationInstance.test.ts +++ b/apps/webapp/test/runsReplicationInstance.test.ts @@ -653,3 +653,96 @@ describe("RunsReplication multi-source wiring (integration)", () => { } ); }); + +// Logical replication needs a session-mode connection, which a transaction pooler cannot serve. +// The app writer DSN is pooled in a real deployment, so a shard replication source must take the +// shard's DIRECT url. Getting this wrong throws inside service.start(), which is not a +// SplitReplicationMisconfiguredError, so the process stays up with every source down. +describe("shard replication uses the direct connection", () => { + const baseArgs = { + legacyUrl: "postgres://legacy", + legacySlotName: "v1", + legacyPublicationName: "v1_pub", + legacyOriginGeneration: 0, + newSlotName: "v2", + newPublicationName: "v2_pub", + newOriginGeneration: 1, + splitEnabled: true, + newUrl: "postgres://new", + }; + const rep = { slotName: "sa", publicationName: "pa", originGeneration: 2 }; + + it("prefers the shard's directUrl over its pooled url", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [ + { + key: "a", + url: "postgres://pooled:6432/shard_a", + directUrl: "postgres://direct:5432/shard_a", + replication: rep, + }, + ], + }); + const shard = sources.find((s) => s.id === "shard-a"); + expect(shard?.pgConnectionUrl).toBe("postgres://direct:5432/shard_a"); + }); + + it("falls back to url when no directUrl is given", () => { + const sources = buildReplicationSources({ + ...baseArgs, + shards: [{ key: "a", url: "postgres://only-url/shard_a", replication: rep }], + }); + const shard = sources.find((s) => s.id === "shard-a"); + expect(shard?.pgConnectionUrl).toBe("postgres://only-url/shard_a"); + }); + + it("refuses the boot when a replicating shard declares no directUrl", () => { + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources: buildReplicationSources({ + ...baseArgs, + shards: [{ key: "a", url: "postgres://u", replication: rep }], + }), + shards: [{ key: "a", hasDirectUrl: false }], + }) + ).toThrow(SplitReplicationMisconfiguredError); + }); + + it("names the shard and the reason in that failure", () => { + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources: buildReplicationSources({ + ...baseArgs, + shards: [{ key: "a", url: "postgres://u", replication: rep }], + }), + shards: [{ key: "a", hasDirectUrl: false }], + }) + ).toThrow(/shard a.*directUrl|directUrl.*shard a/is); + }); + + it("does NOT refuse when the replicating shard declares a directUrl", () => { + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources: buildReplicationSources({ + ...baseArgs, + shards: [{ key: "a", url: "postgres://u", directUrl: "postgres://d", replication: rep }], + }), + shards: [{ key: "a", hasDirectUrl: true }], + }) + ).not.toThrow(); + }); + + it("does NOT require a directUrl for an aliased shard", () => { + expect(() => + assertReplicationCoversSplit({ + splitEnabled: true, + sources: buildReplicationSources({ ...baseArgs, shards: [] }), + shards: [{ key: "z", aliasOf: "new", hasDirectUrl: false }], + }) + ).not.toThrow(); + }); +});