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 <noreply@anthropic.com>

Author: d-cs

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() {

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);
+}

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" }
+  );
+}

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,

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);
   });
 });

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", () => {