fix(webapp,run-store): answer 404, not 500, for an unroutable or absent id

Two read paths answered 500 for input a caller controls. Found while
validating the gen-2 shard arms against a local instance.

`resolveShard` is pure id-shape, so any base32hex core plus [a-z0-9] plus
"2" parses as gen-2 and names a shard. The routing store throws for a
shard key it has no store for, and that throw reached the API boundary as
a 500 — so any caller holding an API key could induce one by guessing a
shard char, of which there are 36. A 5xx on a read also trips canary
rollbacks on deploy.

The throw stays: failing loud beats reading the wrong database. What
changes is the status the boundary reports. `#shardStore` now throws a
typed `UnknownShardKey` carrying the key and the configured set, and the
API boundary maps it to 404 — the same answer an absent gen-1 or cuid id
already gives. The boundary still logs at error level with the key, so a
shard dropped from a config meant to be append-only still alarms.

Separately, the waitpoint wait route threw `json(404)` from inside its own
try, so the catch-all caught that Response and re-threw it as a 500. Every
not-found waitpoint answered 500, including a plain absent cuid. Re-throw
a Response untouched: it carries a deliberate status, not a failure.

`#shardStore` also claimed its throw was unreachable with the two-store
compat constructor. It is reachable, because that constructor defaults to
the real resolver. Comment corrected and the reachability pinned by a test
that uses the real resolver rather than an injected one.

Measured on a local instance: all four cases that returned 500 now return
404, absent gen-1 and cuid ids still 404, and a real run still 200.

Author: d-cs

apps/webapp/app/services/routeBuilders/apiBuilder.server.ts

@@ -25,13 +25,14 @@ import { getApiVersion } from "~/api/versions";
 import { WORKER_HEADERS } from "@trigger.dev/core/v3/runEngineWorker";
 import { ServiceValidationError } from "~/v3/services/common.server";
 import { EngineServiceValidationError } from "@internal/run-engine";
+import { unroutableIdResponse } from "./unroutableId.server";
 import { tenantContext, tenantContextFromAuthEnvironment } from "~/services/tenantContext.server";
 
 // Client aborts and service-level validation errors aren't bugs — they're
 // expected at API boundaries. Log them at `warn` so they stay in stdout
 // without flowing to Sentry via Logger.onError.
 function logBoundaryError(
-  message: "Error in loader" | "Error in action",
+  message: "Error in loader" | "Error in action" | "Unroutable id",
   error: unknown,
   url: string
 ) {
@@ -451,6 +452,12 @@ export function createLoaderApiRoute<
           return await wrapResponse(request, error, corsStrategy !== "none");
         }
 
+        const unroutable = unroutableIdResponse(error);
+        if (unroutable) {
+          logBoundaryError("Unroutable id", error, request.url);
+          return await wrapResponse(request, unroutable, corsStrategy !== "none");
+        }
+
         logBoundaryError("Error in loader", error, request.url);
 
         return await wrapResponse(
@@ -722,6 +729,12 @@ export function createLoaderPATApiRoute<
         if (error instanceof Response) {
           return await wrapResponse(request, error, corsStrategy !== "none");
         }
+
+        const unroutable = unroutableIdResponse(error);
+        if (unroutable) {
+          logBoundaryError("Unroutable id", error, request.url);
+          return await wrapResponse(request, unroutable, corsStrategy !== "none");
+        }
         return await wrapResponse(
           request,
           json({ error: "Internal Server Error" }, { status: 500 }),
@@ -996,6 +1009,12 @@ export function createActionPATApiRoute<
           return await wrapResponse(request, error, corsStrategy !== "none");
         }
 
+        const unroutable = unroutableIdResponse(error);
+        if (unroutable) {
+          logBoundaryError("Unroutable id", error, request.url);
+          return await wrapResponse(request, unroutable, corsStrategy !== "none");
+        }
+
         logBoundaryError("Error in action", error, request.url);
 
         // Typed validation errors map to their own status (default 400);
@@ -1346,6 +1365,12 @@ export function createActionApiRoute<
           return await wrapResponse(request, error, corsStrategy !== "none");
         }
 
+        const unroutable = unroutableIdResponse(error);
+        if (unroutable) {
+          logBoundaryError("Unroutable id", error, request.url);
+          return await wrapResponse(request, unroutable, corsStrategy !== "none");
+        }
+
         logBoundaryError("Error in action", error, request.url);
 
         return await wrapResponse(
@@ -1612,6 +1637,12 @@ export function createMultiMethodApiRoute<
           return await wrapResponse(request, error, corsStrategy !== "none");
         }
 
+        const unroutable = unroutableIdResponse(error);
+        if (unroutable) {
+          logBoundaryError("Unroutable id", error, request.url);
+          return await wrapResponse(request, unroutable, corsStrategy !== "none");
+        }
+
         logBoundaryError("Error in action", error, request.url);
 
         return await wrapResponse(

apps/webapp/app/services/routeBuilders/unroutableId.server.ts

@@ -0,0 +1,17 @@
+import { json } from "@remix-run/server-runtime";
+import { UnknownShardKey } from "@internal/run-store";
+
+/**
+ * An id naming a shard the topology has no store for cannot be routed, so a read cannot locate
+ * the row: that is a 404, and matches what an absent gen-1 or cuid id already returns. It must
+ * not be a 500 — `resolveShard` is pure id-shape, so any base32hex core plus `[a-z0-9]` plus "2"
+ * parses as gen-2, which lets any caller induce a 5xx, and a 5xx on a read trips canary rollbacks.
+ *
+ * The router still throws. Callers log it before returning this, so a genuine misconfiguration —
+ * a shard key dropped from a config that is meant to be append-only — still alarms.
+ */
+export function unroutableIdResponse(error: unknown): Response | undefined {
+  return error instanceof UnknownShardKey
+    ? json({ error: "Not Found" }, { status: 404 })
+    : undefined;
+}

apps/webapp/test/unroutableIdStatus.test.ts

@@ -0,0 +1,37 @@
+// `resolveShard` is pure id-shape, so any base32hex core plus `[a-z0-9]` plus "2" parses as gen-2
+// and names a shard — including one a caller invents. The routing store throws for a key it has no
+// store for, which is correct and deliberately loud, but a read route that lets it reach the
+// boundary answered 500 for caller-supplied input. These tests pin the boundary status.
+import { describe, expect, it } from "vitest";
+import { json } from "@remix-run/server-runtime";
+import { UnknownShardKey } from "@internal/run-store";
+import { unroutableIdResponse } from "~/services/routeBuilders/unroutableId.server";
+
+describe("unroutableIdResponse", () => {
+  it("answers 404 for an id naming a shard with no configured store", async () => {
+    const response = unroutableIdResponse(new UnknownShardKey("z", ["legacy", "new"]));
+
+    expect(response).toBeDefined();
+    expect(response!.status).toBe(404);
+    await expect(response!.json()).resolves.toEqual({ error: "Not Found" });
+  });
+
+  it("declines an unrelated error so it still reaches the 500 path", () => {
+    expect(unroutableIdResponse(new Error("db down"))).toBeUndefined();
+    expect(unroutableIdResponse(undefined)).toBeUndefined();
+    expect(unroutableIdResponse("a string")).toBeUndefined();
+  });
+
+  it("declines a deliberately thrown Response, which carries its own status", () => {
+    expect(unroutableIdResponse(json({ error: "nope" }, { status: 422 }))).toBeUndefined();
+  });
+
+  it("keeps the key and the configured set on the error for the operator", () => {
+    // A 404 to the caller must not cost the operator what separates a forged id from a shard
+    // key dropped out of a config that is meant to be append-only.
+    const error = new UnknownShardKey("z", ["legacy", "new", "a"]);
+
+    expect(error.shardKey).toBe("z");
+    expect(error.configured).toContain("a");
+  });
+});

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

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { RoutingRunStore } from "./runOpsStore.js";
+import { RoutingRunStore, UnknownShardKey } from "./runOpsStore.js";
 import type { ReadClient, RunStore } from "./types.js";
 
 // Pins the routing ALGEBRA: probe order, merge precedence, and the two id-less fallbacks that
@@ -302,6 +302,29 @@ describe("RoutingRunStore id-to-shard-key seam", () => {
     expect(trace(log)).toEqual([]);
   });
 
+  // Typed, not a bare Error: the API boundary matches on it to answer 404 instead of 500, and
+  // the operator needs the key and the configured set to tell a forged id from a dropped shard.
+  it("throws a typed UnknownShardKey carrying the key and the configured set", () => {
+    const log: Call[] = [];
+    const router = new RoutingRunStore({
+      new: fakeStore("new", log),
+      legacy: fakeStore("legacy", log),
+    });
+
+    let thrown: unknown;
+    try {
+      router.findRun({ id: `${"0".repeat(24)}a2` });
+    } catch (error) {
+      thrown = error;
+    }
+
+    expect(thrown).toBeInstanceOf(UnknownShardKey);
+    const error = thrown as UnknownShardKey;
+    expect(error.name).toBe("UnknownShardKey");
+    expect(error.shardKey).toBe("a");
+    expect([...error.configured].sort()).toEqual(["legacy", "new"]);
+  });
+
   it("still routes gen-1 shapes on the compat pair with the real resolver", () => {
     const log: Call[] = [];
     const router = new RoutingRunStore({

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

@@ -62,6 +62,24 @@ const LEGACY_SHARD: ShardKey = "legacy";
  * and a merge lets a gen-2 shard win. A probe MUST iterate #probeOrder and a merge MUST iterate
  * #precedence.
  */
+/**
+ * An id resolved to a shard key the topology has no store for. Typed so a caller above the
+ * router can answer a 4xx instead of letting a routing failure surface as a 5xx: these ids
+ * arrive as URL parameters, and `resolveShard` is pure id-shape, so any gen-2 shaped id names
+ * a shard char whether or not one is configured.
+ */
+export class UnknownShardKey extends Error {
+  readonly shardKey: string;
+  readonly configured: string[];
+
+  constructor(shardKey: string, configured: string[]) {
+    super(`RoutingRunStore: no store is configured for shard key "${shardKey}"`);
+    this.name = "UnknownShardKey";
+    this.shardKey = shardKey;
+    this.configured = configured;
+  }
+}
+
 export class RoutingRunStore implements RunStore {
   readonly #shards: ReadonlyMap<ShardKey, RunStore>;
   // Sequential probe for a lookup with no routable id. The first non-null result wins, and the LAST
@@ -175,12 +193,12 @@ export class RoutingRunStore implements RunStore {
 
   // The store for a shard key. REACHABLE with the compat constructor: it defaults to the real
   // `resolveShard`, which is pure id-shape, so any gen-2 shaped id names a shard char even when
-  // no shard is configured. Callers above the router must therefore translate this into a 4xx on
-  // a read path — these ids arrive as URL parameters — rather than let it surface as a 5xx.
+  // no shard is configured. Fails loud rather than reading the wrong database; the API boundary
+  // turns `UnknownShardKey` into a 404 so a caller-supplied id cannot induce a 5xx.
   #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}"`);
+      throw new UnknownShardKey(key, [...this.#shards.keys()]);
     }
     return store;
   }