Send deeplinks whose own segment has no page to a page that exists

tasks, waitpoints and metrics only exist as the parent of param routes, so
a bare /deeplink/tasks redirected to a URL matching no route and rendered
a 404 — worse than the environment root it used to fall back to.

Replace the segment allowlist with a map from deeplink name to target
path. tasks points at the environment root, which is the task list;
waitpoints points at waitpoints/tokens; metrics is dropped, being only a
legacy redirect shim with no page of its own. Deeper segments are still
kept as given, so task and run detail links keep working.

The test now checks that every target resolves to a real route and that
no environment page is missing, instead of comparing bare segment names.

Author: claude

apps/webapp/app/routes/deeplink.$.ts

@@ -2,36 +2,33 @@ import { redirect, type LoaderFunctionArgs } from "@remix-run/server-runtime";
 import { prisma } from "~/db.server";
 import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server";
 import { requireUser } from "~/services/session.server";
-import { ENV_PAGE_SEGMENTS } from "~/utils/deeplinkPages";
+import { resolveDeeplinkPage } from "~/utils/deeplinkPages";
 import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/pathBuilder";
 
 /**
  * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to
- * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the environment
- * pages in ENV_PAGE_SEGMENTS are followed, so an unrecognised path can never become the redirect
- * target — it lands on the resolved environment instead.
+ * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the pages in
+ * ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target —
+ * it lands on the resolved environment instead.
  */
 export const loader = async ({ request, params }: LoaderFunctionArgs) => {
   const user = await requireUser(request);
 
-  //traversal segments are dropped so a crafted suffix can't climb out of the environment path
-  const segments = (params["*"] ?? "")
-    .split("/")
-    .filter((segment) => segment.length > 0 && segment !== "." && segment !== "..");
-  //deeper segments are kept, so /deeplink/runs/run_123 reaches the run. They arrive decoded, so
-  //they're re-encoded: a "?" or "#" in a segment must not become the target's query or hash.
-  const page = ENV_PAGE_SEGMENTS.has(segments[0] ?? "")
-    ? segments.map(encodeURIComponent).join("/")
-    : undefined;
-
+  const page = resolveDeeplinkPage(params["*"] ?? "");
   const { search } = new URL(request.url);
 
   const presenter = new SelectBestEnvironmentPresenter();
   try {
     const { project, organization, environment } = await presenter.call({ user });
     const environmentPath = v3EnvironmentPath(organization, project, environment);
 
-    return redirect(page ? `${environmentPath}/${page}${search}` : environmentPath);
+    //an unrecognised path keeps nothing: it lands on the environment as if no suffix was given
+    if (page === undefined) {
+      return redirect(environmentPath);
+    }
+
+    //`tasks` targets the environment root, so there is no suffix to append
+    return redirect(page ? `${environmentPath}/${page}${search}` : `${environmentPath}${search}`);
   } catch (_e) {
     //the presenter throws when the user has no projects, same as the dashboard index
     const organization = await prisma.organization.findFirst({

apps/webapp/app/utils/deeplinkPages.test.ts

@@ -1,61 +1,125 @@
-import { readdirSync } from "node:fs";
+import { existsSync, readdirSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { describe, expect, it } from "vitest";
-import { ENV_PAGE_SEGMENTS } from "./deeplinkPages";
+import { ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages";
+
+const ROUTES_DIR = join(__dirname, "../routes");
 
 // Flat-route prefix for every page that renders inside an environment. The trailing dot matters:
 // it excludes the layout route itself (`…env.$envParam`), which has no segment of its own.
 const ENV_ROUTE_PREFIX = "_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.";
 
 /**
- * Segments that are route files but not deeplink targets:
- * - `_index` is the environment root, which is already where an unrecognised deeplink lands.
+ * Segments that are route files but are not deeplink names:
+ * - `_index` is the environment root. It is where an unrecognised deeplink already lands, and
+ *   `tasks` is the name that points at it.
  * - `queues_` is Remix's "opt out of the parent layout" spelling of `queues`, not a distinct URL.
  */
-const NOT_DEEPLINKABLE = new Set(["_index", "queues_"]);
+const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]);
+
+const routeEntries = readdirSync(ROUTES_DIR);
+
+/** A route directory only contributes a route if it actually holds a `route` module. */
+function isRouteModule(entry: string): boolean {
+  const path = join(ROUTES_DIR, entry);
+  if (!statSync(path).isDirectory()) return true;
+  return existsSync(join(path, "route.tsx")) || existsSync(join(path, "route.ts"));
+}
+
+/**
+ * The route file that a bare `/env/{env}/{target}` URL matches, or undefined when nothing does.
+ * `target` may span segments ("waitpoints/tokens"); "" is the environment root.
+ *
+ * Only literal route names are considered — a param route (`metrics.$dashboardKey`) is not a page
+ * you can land on without supplying the param, which is exactly what this needs to reject.
+ */
+function routeForTarget(target: string): string | undefined {
+  if (target === "") {
+    return isRouteModule(`${ENV_ROUTE_PREFIX}_index`) ? `${ENV_ROUTE_PREFIX}_index` : undefined;
+  }
+
+  const base = ENV_ROUTE_PREFIX + target.split("/").join(".");
+  // A leaf route, or a layout whose index child supplies the bare URL.
+  return [base, `${base}.tsx`, `${base}.ts`, `${base}._index`].find(
+    (candidate) => routeEntries.includes(candidate) && isRouteModule(candidate)
+  );
+}
 
-/** The first path segment of every environment page, read off the route filenames. */
+/** Every first segment appearing under the environment layout. */
 function envRouteSegments(): Set<string> {
-  const entries = readdirSync(join(__dirname, "../routes"));
   const segments = new Set<string>();
-
-  for (const entry of entries) {
+  for (const entry of routeEntries) {
     if (!entry.startsWith(ENV_ROUTE_PREFIX)) continue;
     // `metrics.$dashboardKey.ts` -> `metrics`, `agents` -> `agents`, `errors._index` -> `errors`
     const segment = entry.slice(ENV_ROUTE_PREFIX.length).split(/[./]/)[0];
     // Guards against a future `…env.$envParam.tsx` contributing its extension as a segment.
     if (!segment || segment === "ts" || segment === "tsx") continue;
-    if (NOT_DEEPLINKABLE.has(segment)) continue;
     segments.add(segment);
   }
-
   return segments;
 }
 
-describe("deeplink allowlist", () => {
-  it("matches the environment layout's route segments", () => {
-    // Sorted arrays rather than sets so a mismatch names the segment that drifted.
-    expect([...ENV_PAGE_SEGMENTS].sort()).toEqual([...envRouteSegments()].sort());
-  });
-
+describe("deeplink targets", () => {
   it("found the routes directory", () => {
-    // Guards the test itself: an empty derived set would make the assertion above vacuous
-    // if the allowlist were ever emptied too.
+    // Without this, every assertion below would pass vacuously if the glob ever broke.
     expect(envRouteSegments().size).toBeGreaterThan(20);
   });
 
-  it("excludes the environment root and the layout-opt-out spelling", () => {
-    expect(ENV_PAGE_SEGMENTS.has("_index")).toBe(false);
-    expect(ENV_PAGE_SEGMENTS.has("queues_")).toBe(false);
-    // `queues` itself is still reachable — it is the real URL segment.
-    expect(ENV_PAGE_SEGMENTS.has("queues")).toBe(true);
+  it("every target resolves to a real environment route", () => {
+    const unresolved = [...ENV_PAGE_TARGETS.entries()]
+      .filter(([, target]) => !routeForTarget(target))
+      .map(([name, target]) => `${name} -> ${target || "(environment root)"}`);
+
+    expect(unresolved).toEqual([]);
+  });
+
+  it("every environment page has a deeplink name", () => {
+    // A segment that resolves bare is a page someone could reasonably want to link to.
+    const missing = [...envRouteSegments()]
+      .filter((segment) => !NOT_DEEPLINK_NAMES.has(segment))
+      .filter((segment) => routeForTarget(segment) && !ENV_PAGE_TARGETS.has(segment))
+      .sort();
+
+    expect(missing).toEqual([]);
   });
 
-  it("includes the pages that ENV_PAGE_META omits", () => {
-    // These have no entry in ENV_PAGE_META (their icon/label is special-cased when resolving
-    // page metadata), which is why the allowlist is derived from routes and not from that map.
-    for (const segment of ["tasks", "agents", "settings"]) {
-      expect(ENV_PAGE_SEGMENTS.has(segment)).toBe(true);
+  it("names whose own segment 404s are redirected, not mapped to themselves", () => {
+    // These exist only as the parent of param/child routes, so a bare URL matches no route.
+    for (const segment of ["tasks", "waitpoints", "metrics"]) {
+      expect(routeForTarget(segment)).toBeUndefined();
     }
+
+    // `tasks` and `waitpoints` therefore point somewhere else; `metrics` is only a legacy redirect
+    // shim with no page of its own, so it is deliberately not a deeplink name at all.
+    expect(ENV_PAGE_TARGETS.get("tasks")).toBe("");
+    expect(ENV_PAGE_TARGETS.get("waitpoints")).toBe("waitpoints/tokens");
+    expect(ENV_PAGE_TARGETS.has("metrics")).toBe(false);
+  });
+});
+
+describe("resolveDeeplinkPage", () => {
+  it("maps a bare name to its landing page", () => {
+    expect(resolveDeeplinkPage("apikeys")).toBe("apikeys");
+    expect(resolveDeeplinkPage("waitpoints")).toBe("waitpoints/tokens");
+    expect(resolveDeeplinkPage("tasks")).toBe("");
+  });
+
+  it("keeps deeper segments, which address a real sub-route", () => {
+    expect(resolveDeeplinkPage("runs/run_123")).toBe("runs/run_123");
+    expect(resolveDeeplinkPage("tasks/standard/my-task")).toBe("tasks/standard/my-task");
+    expect(resolveDeeplinkPage("waitpoints/tokens")).toBe("waitpoints/tokens");
+  });
+
+  it("rejects a name that is not a page", () => {
+    expect(resolveDeeplinkPage("")).toBeUndefined();
+    expect(resolveDeeplinkPage("nonsense")).toBeUndefined();
+    expect(resolveDeeplinkPage("metrics")).toBeUndefined();
+  });
+
+  it("drops traversal segments and encodes the rest", () => {
+    expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd");
+    expect(resolveDeeplinkPage("../runs")).toBe("runs");
+    expect(resolveDeeplinkPage("runs/a?b#c")).toBe("runs/a%3Fb%23c");
+    expect(resolveDeeplinkPage("runs//run_1")).toBe("runs/run_1");
   });
 });

apps/webapp/app/utils/deeplinkPages.ts

@@ -1,35 +1,63 @@
 /**
- * Pages that /deeplink/* is allowed to redirect to. This mirrors the first path segment of the
- * environment-layout routes (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`),
- * so add an entry here when a new page is added under that layout.
+ * Where each /deeplink/<name> lands, relative to the resolved environment. Most names are a page
+ * in their own right and map to themselves. A few exist only as the parent of param routes
+ * (`tasks.standard.$taskParam`, `waitpoints.tokens`) — a bare `/tasks` matches no route and would
+ * 404 — so those map to the page a user actually wants instead.
+ *
+ * This mirrors the environment-layout routes
+ * (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`): add a page here when one
+ * is added there. `deeplinkPages.test.ts` checks every target against the route files and fails if
+ * a page is missing or a target stops resolving.
  */
-export const ENV_PAGE_SEGMENTS: ReadonlySet<string> = new Set([
-  "agents",
-  "alerts",
-  "apikeys",
-  "batches",
-  "branches",
-  "bulk-actions",
-  "concurrency",
-  "dashboards",
-  "deployments",
-  "dev-branches",
-  "environment-variables",
-  "errors",
-  "limits",
-  "logs",
-  "metrics",
-  "models",
-  "playground",
-  "prompts",
-  "query",
-  "queues",
-  "regions",
-  "runs",
-  "schedules",
-  "sessions",
-  "settings",
-  "tasks",
-  "test",
-  "waitpoints",
+export const ENV_PAGE_TARGETS: ReadonlyMap<string, string> = new Map([
+  ["agents", "agents"],
+  ["alerts", "alerts"],
+  ["apikeys", "apikeys"],
+  ["batches", "batches"],
+  ["branches", "branches"],
+  ["bulk-actions", "bulk-actions"],
+  ["concurrency", "concurrency"],
+  ["dashboards", "dashboards"],
+  ["deployments", "deployments"],
+  ["dev-branches", "dev-branches"],
+  ["environment-variables", "environment-variables"],
+  ["errors", "errors"],
+  ["limits", "limits"],
+  ["logs", "logs"],
+  ["models", "models"],
+  ["playground", "playground"],
+  ["prompts", "prompts"],
+  ["query", "query"],
+  ["queues", "queues"],
+  ["regions", "regions"],
+  ["runs", "runs"],
+  ["schedules", "schedules"],
+  ["sessions", "sessions"],
+  ["settings", "settings"],
+  // The environment root is the task list (its route is the env `_index`, titled "Tasks"), so a
+  // bare /deeplink/tasks belongs there rather than at the secondary /tasks/dashboard view.
+  ["tasks", ""],
+  ["test", "test"],
+  ["waitpoints", "waitpoints/tokens"],
 ]);
+
+/**
+ * The path a deeplink suffix should redirect to, relative to the environment, or undefined when the
+ * first segment names no page. Returns "" for a target that is the environment root itself.
+ *
+ * Segments beyond the first are kept as given, because they address a real sub-route
+ * (`/deeplink/runs/run_123`, `/deeplink/tasks/standard/my-task`); only a bare name uses the mapped
+ * landing page. They arrive decoded, so they are re-encoded: a "?" or "#" in a segment must not
+ * become the target's query or hash.
+ */
+export function resolveDeeplinkPage(splat: string): string | undefined {
+  //traversal segments are dropped so a crafted suffix can't climb out of the environment path
+  const segments = splat
+    .split("/")
+    .filter((segment) => segment.length > 0 && segment !== "." && segment !== "..");
+
+  const target = ENV_PAGE_TARGETS.get(segments[0] ?? "");
+  if (target === undefined) return undefined;
+
+  return segments.length > 1 ? segments.map(encodeURIComponent).join("/") : target;
+}