DM-56095: Redirect /v/{edition} without trailing slash

cloudflare-worker/src/resolver.ts

@@ -7,7 +7,9 @@
  *   1. Look up `{project}/{edition}` in KV to get the build mapping.
  *   2. Construct the R2 object key as `{r2_prefix}{file_path}`.
  *   3. Try exact path, then `{path}/index.html` for directory index
- *      resolution.
+ *      resolution. When the index file is found but the request pathname
+ *      lacks a trailing slash (e.g. `/v/weekly` or `/api/core`), return a
+ *      301 to the slash-terminated URL so relative asset links resolve.
  *   4. Return the R2 object with `Content-Type` inferred from the file
  *      extension and `Cache-Control` derived from the KV value's
  *      `cache_profile` field: `CACHE_CONTROL_LONG` for `"long"`,
@@ -39,7 +41,10 @@
  *   responses.
  *
  * - `redirect` routes return a 301 with `Location` set to `route.to` on the
- *   request's origin, preserving the original query string.
+ *   request's origin, preserving the original query string. Every 301 this
+ *   module emits (redirect routes and the directory-index trailing-slash
+ *   canonicalisation) carries `CACHE_CONTROL_LONG`, since the target is a
+ *   pure function of the request URL.
  *
  * 404 handling: every 404-producing branch in this module routes through
  * `notFoundResponse(project, dashboardStore, ctx)`, which serves the
@@ -161,10 +166,24 @@ export async function resolve(
   }
 }
 
+/**
+ * Build a 301 to `to` on the request's origin, preserving the query string.
+ *
+ * Carries an explicit `Cache-Control` so the edge can cache the redirect
+ * and browsers don't fall back to heuristic (effectively permanent) caching
+ * of a header-less 301. The long profile is safe here: the target depends
+ * only on the request URL's shape, never on edition content.
+ */
 function resolveRedirect(to: string, request: Request): Response {
   const url = new URL(request.url);
   url.pathname = to;
-  return Response.redirect(url.toString(), 301);
+  return new Response(null, {
+    status: 301,
+    headers: {
+      "Location": url.toString(),
+      "Cache-Control": CACHE_CONTROL_LONG,
+    },
+  });
 }
 
 /**
@@ -252,11 +271,13 @@ async function resolveEdition(
       : `${r2Key}/index.html`;
     object = await r2.get(indexKey);
     if (object !== null) {
-      // Redirect to trailing slash so relative links resolve correctly
-      if (route.path !== "" && !route.path.endsWith("/")) {
-        const url = new URL(request.url);
-        url.pathname = `${url.pathname}/`;
-        return Response.redirect(url.toString(), 301);
+      // Redirect to trailing slash so relative links resolve correctly.
+      // Decide from the request pathname rather than `route.path`: the
+      // edition root has an empty `route.path` whether the request was
+      // `/v/weekly/` or `/v/weekly`, and only the latter needs the redirect.
+      const { pathname } = new URL(request.url);
+      if (!pathname.endsWith("/")) {
+        return resolveRedirect(`${pathname}/`, request);
       }
       resolvedPath = indexKey.slice(prefix.length);
     }

cloudflare-worker/test/index.test.ts

@@ -132,6 +132,21 @@ describe("Worker integration — subdomain routing", () => {
     );
   });
 
+  it("redirects /v/{edition} without trailing slash to /v/{edition}/", async () => {
+    const response = await SELF.fetch(
+      "https://pipelines.lsst.io/v/v1.0",
+      { redirect: "manual" },
+    );
+
+    expect(response.status).toBe(301);
+    expect(response.headers.get("Location")).toBe(
+      "https://pipelines.lsst.io/v/v1.0/",
+    );
+    expect(response.headers.get("Cache-Control")).toBe(
+      "public, max-age=300, s-maxage=86400",
+    );
+  });
+
   it("infers correct Content-Type for CSS", async () => {
     const response = await SELF.fetch(
       "https://pipelines.lsst.io/style.css",
@@ -429,6 +444,33 @@ describe("Worker integration — path-prefix routing", () => {
     expect(await response.text()).toBe("<html>root</html>");
   });
 
+  it("redirects /docs/{project} without trailing slash", async () => {
+    const response = await fetchPathPrefix(
+      "https://docs.example.com/docs/pipelines",
+      { redirect: "manual" },
+    );
+
+    expect(response.status).toBe(301);
+    expect(response.headers.get("Location")).toBe(
+      "https://docs.example.com/docs/pipelines/",
+    );
+  });
+
+  it("redirects /docs/{project}/v/{edition} without trailing slash", async () => {
+    const response = await fetchPathPrefix(
+      "https://docs.example.com/docs/pipelines/v/main",
+      { redirect: "manual" },
+    );
+
+    expect(response.status).toBe(301);
+    expect(response.headers.get("Location")).toBe(
+      "https://docs.example.com/docs/pipelines/v/main/",
+    );
+    expect(response.headers.get("Cache-Control")).toBe(
+      "public, max-age=300, s-maxage=86400",
+    );
+  });
+
   it("serves a page under __main", async () => {
     const response = await fetchPathPrefix(
       "https://docs.example.com/docs/pipelines/getting-started.html",

cloudflare-worker/test/resolver.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
-import { resolve, notFoundCacheKey } from "../src/resolver";
+import { resolve, notFoundCacheKey, CACHE_CONTROL_LONG } from "../src/resolver";
 import type { DashboardStore } from "../src/dashboardStore";
 import type { Route } from "../src/router";
 
@@ -351,6 +351,68 @@ describe("resolve — edition routes", () => {
     expect(response.headers.get("Location")).toBe(
       "https://example.com/pipelines/api/core/",
     );
+    expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG);
+  });
+
+  it("redirects edition root without trailing slash", async () => {
+    // `/v/weekly` parses to an edition route with an empty path, exactly
+    // like `/v/weekly/`; only the request pathname tells them apart.
+    const rootRoute: Route = {
+      kind: "edition",
+      project: "pipelines",
+      edition: "weekly",
+      path: "",
+    };
+    const kv = createMockKV({
+      "pipelines/weekly": JSON.stringify({
+        build_id: "b123",
+        r2_prefix: "pipelines/__builds/b123/",
+      }),
+    });
+    const r2 = createMockR2({
+      "pipelines/__builds/b123/index.html": {
+        body: streamFromString("<html>weekly</html>"),
+        size: 19,
+      },
+    });
+    const dashboardStore = createMockDashboardStore();
+    const request = new Request("https://pipelines.lsst.io/v/weekly?x=1");
+
+    const response = await resolve(rootRoute, request, kv, r2, dashboardStore, ctx);
+
+    expect(response.status).toBe(301);
+    expect(response.headers.get("Location")).toBe(
+      "https://pipelines.lsst.io/v/weekly/?x=1",
+    );
+    expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG);
+  });
+
+  it("serves the edition root index.html when the request has a trailing slash", async () => {
+    const rootRoute: Route = {
+      kind: "edition",
+      project: "pipelines",
+      edition: "weekly",
+      path: "",
+    };
+    const kv = createMockKV({
+      "pipelines/weekly": JSON.stringify({
+        build_id: "b123",
+        r2_prefix: "pipelines/__builds/b123/",
+      }),
+    });
+    const r2 = createMockR2({
+      "pipelines/__builds/b123/index.html": {
+        body: streamFromString("<html>weekly</html>"),
+        size: 19,
+      },
+    });
+    const dashboardStore = createMockDashboardStore();
+    const request = new Request("https://pipelines.lsst.io/v/weekly/");
+
+    const response = await resolve(rootRoute, request, kv, r2, dashboardStore, ctx);
+
+    expect(response.status).toBe(200);
+    expect(await response.text()).toBe("<html>weekly</html>");
   });
 
   it("serves index.html when path has trailing slash", async () => {
@@ -961,6 +1023,7 @@ describe("resolve — redirect routes", () => {
     expect(new URL(response.headers.get("Location") ?? "").pathname).toBe(
       "/v/",
     );
+    expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG);
     // Edition / dashboard paths must not be touched for redirect dispatch.
     expect(kv.get).not.toHaveBeenCalled();
     expect(r2.get).not.toHaveBeenCalled();