fix(showcase/shell-dashboard): proxy contract hardening — build-arg, normalization, empty-env, tests

CR R1 follow-ups on top of the /api/ops proxy fix.

Bucket (a) — must-fix:

- Dockerfile: declare ARG/ENV OPS_BASE_URL in the builder stage. next.config.ts
  evaluates rewrites() at build time and throws if OPS_BASE_URL is unset, which
  aborted `next build` in CI. Mirrors the existing NEXT_PUBLIC_SHELL_URL /
  NEXT_PUBLIC_POCKETBASE_URL pattern.

- showcase_deploy.yml: pipe OPS_BASE_URL through to docker build for the
  shell-dashboard matrix entry, defaulting to the production
  showcase-ops-production.up.railway.app URL.

- next.config.ts: strip trailing slashes from OPS_BASE_URL before constructing
  the rewrite destination, matching the same normalization in
  src/lib/ops-api.ts:resolveBaseUrl so server-side rewrite and client-side
  fetch agree on the URL shape.

- src/lib/ops-api.ts: treat empty / whitespace NEXT_PUBLIC_OPS_BASE_URL as
  "no override". `??` only short-circuits on null/undefined, so an env var set
  to "" silently produced baseUrl="" and URLs of the form "/probes" with no
  /api/ops prefix.

- use-probes.integration.test.tsx: snapshot+restore process.env.NEXT_PUBLIC_OPS_BASE_URL
  in beforeEach/afterEach so tests never leak env state. Strengthen the proxy
  contract assertions: lock toHaveBeenCalledTimes(1), assert method=GET,
  cache=no-store, accept JSON header, and signal is an AbortSignal. Tighten
  the 404 regression to assert the canonical ensureOk message shape so a
  refactor that changes the format trips the test.

Bucket (b) — applied since the diff stayed focused:

- triggerProbe: add cache:"no-store" for parity with the GET fetches.
- fetchProbeDetail / triggerProbe: throw early when id is empty so callers
  get a clean error instead of a request to /probes//... .
- ensureOk: bump body-truncation cap from 200 to 500 chars and append a
  `[truncated, N bytes total]` marker so operators can see they're missing
  tail bytes when the server returns a long HTML/stack-trace body.
- ops-api.ts: drop dev-loop review-cycle tag prefixes (R2-C.3, R3-C, R3-D.1)
  from comments. Keep the actual rationale.
- ops-api.ts header docstring: clarify that NEXT_PUBLIC_OPS_BASE_URL is read
  live at runtime in this codebase (SSR + tests), not just statically inlined
  into the client bundle.

Verified:

- Tests: vitest run — 26 files, 293 passed, 1 skipped (no test count change).
- Typecheck: tsc --noEmit clean.
- Lint + format: oxlint + oxfmt clean on changed files.
- Local Docker build: `docker build --build-arg OPS_BASE_URL=https://...` succeeds.
  Without --build-arg the build fails with "OPS_BASE_URL must be set" as
  expected, confirming the fix is load-bearing.

Author: jpr5

.github/workflows/showcase_deploy.yml

@@ -230,7 +230,7 @@ jobs:
             {"dispatch_name":"starter-spring-ai","filter_key":"starter_spring_ai","context":"showcase/starters/spring-ai","image":"showcase-starter-spring-ai","railway_id":"3559ece3-7ba3-41ac-b24c-1f780133ec58","timeout":15,"lfs":false,"build_args":"","dockerfile":"","health_path":"/api/health"},
             {"dispatch_name":"starter-strands","filter_key":"starter_strands","context":"showcase/starters/strands","image":"showcase-starter-strands","railway_id":"06db2bb8-e15d-4c6a-97ad-e14777c92d9f","timeout":15,"lfs":false,"build_args":"","dockerfile":"","health_path":"/api/health"},
             {"dispatch_name":"shell-dojo","filter_key":"shell_dojo","context":".","image":"showcase-shell-dojo","railway_id":"7ad1ece7-2228-49cd-8a78-bddf30322907","timeout":10,"lfs":false,"build_args":"","dockerfile":"showcase/shell-dojo/Dockerfile","health_path":"/"},
-            {"dispatch_name":"shell-dashboard","filter_key":"shell_dashboard","context":".","image":"showcase-shell-dashboard","railway_id":"4d5dfd74-be61-40b2-8564-b53b7dd4c15b","timeout":10,"lfs":true,"build_args_sha":"${{ github.sha }}","build_args_branch":"${{ github.ref_name }}","build_args_pb_url":"https://showcase-pocketbase-production.up.railway.app","build_args_shell_url":"https://showcase.copilotkit.ai","dockerfile":"showcase/shell-dashboard/Dockerfile","health_path":"/"},
+            {"dispatch_name":"shell-dashboard","filter_key":"shell_dashboard","context":".","image":"showcase-shell-dashboard","railway_id":"4d5dfd74-be61-40b2-8564-b53b7dd4c15b","timeout":10,"lfs":true,"build_args_sha":"${{ github.sha }}","build_args_branch":"${{ github.ref_name }}","build_args_pb_url":"https://showcase-pocketbase-production.up.railway.app","build_args_shell_url":"https://showcase.copilotkit.ai","build_args_ops_url":"https://showcase-ops-production.up.railway.app","dockerfile":"showcase/shell-dashboard/Dockerfile","health_path":"/"},
             {"dispatch_name":"shell-docs","filter_key":"shell_docs","context":".","image":"showcase-shell-docs","railway_id":"7badfb8d-4228-414c-9145-b4026803714f","timeout":10,"lfs":true,"build_args_sha":"${{ github.sha }}","build_args_branch":"${{ github.ref_name }}","dockerfile":"showcase/shell-docs/Dockerfile","health_path":"/"},
             {"dispatch_name":"showcase-ops","filter_key":"showcase_ops","context":".","image":"showcase-ops","railway_id":"3a14bfed-0537-4d71-897b-7c593dca161d","timeout":20,"lfs":false,"build_args":"","dockerfile":"showcase/ops/Dockerfile","health_path":"/health"}
           ]'
@@ -352,6 +352,11 @@ jobs:
           if [ -n "${{ matrix.service.build_args_shell_url }}" ]; then
             ARGS="${ARGS:+${ARGS}$'\n'}NEXT_PUBLIC_SHELL_URL=${{ matrix.service.build_args_shell_url }}"
           fi
+          if [ -n "${{ matrix.service.build_args_ops_url }}" ]; then
+            # OPS_BASE_URL is required at build time by shell-dashboard's
+            # next.config.ts rewrites() — without it `next build` aborts.
+            ARGS="${ARGS:+${ARGS}$'\n'}OPS_BASE_URL=${{ matrix.service.build_args_ops_url }}"
+          fi
           # Use delimiter to safely pass multiline value
           echo "args<<BUILDARGS_EOF" >> $GITHUB_OUTPUT
           echo "$ARGS" >> $GITHUB_OUTPUT

showcase/shell-dashboard/Dockerfile

@@ -34,6 +34,13 @@ ARG NEXT_PUBLIC_SHELL_URL
 ENV NEXT_PUBLIC_SHELL_URL=${NEXT_PUBLIC_SHELL_URL}
 ARG NEXT_PUBLIC_POCKETBASE_URL
 ENV NEXT_PUBLIC_POCKETBASE_URL=${NEXT_PUBLIC_POCKETBASE_URL}
+# `next.config.ts` reads OPS_BASE_URL inside `rewrites()` and throws if it
+# is unset. `next build` evaluates rewrites at build time to generate the
+# routing manifest, so the value must be in the BUILDER environment — not
+# just runtime. Without this, `npx next build` aborts with the
+# "OPS_BASE_URL must be set" error from next.config.ts.
+ARG OPS_BASE_URL
+ENV OPS_BASE_URL=${OPS_BASE_URL}
 
 # Generate registry + docs status, then build Next.js
 RUN cd scripts && node node_modules/tsx/dist/cli.mjs generate-registry.ts \

showcase/shell-dashboard/next.config.ts

@@ -25,8 +25,13 @@ const nextConfig: NextConfig = {
           "(without it, /api/ops/* requests cannot proxy to showcase-ops)",
       );
     }
+    // Strip trailing slashes so we never produce `https://host//api/...`
+    // (some servers reject the double slash). Mirrors the same
+    // normalization in `src/lib/ops-api.ts:resolveBaseUrl` so the
+    // server-side rewrite and client-side fetch agree on the URL shape.
+    const normalized = opsBase.replace(/\/+$/, "");
     return [
-      { source: "/api/ops/:path*", destination: `${opsBase}/api/:path*` },
+      { source: "/api/ops/:path*", destination: `${normalized}/api/:path*` },
     ];
   },
 };

showcase/shell-dashboard/src/hooks/use-probes.integration.test.tsx

@@ -24,6 +24,7 @@ import { useProbes } from "./use-probes";
 import type { ProbesResponse } from "../lib/ops-api";
 
 let fetchSpy: ReturnType<typeof vi.fn>;
+let savedOpsBaseUrl: string | undefined;
 
 function jsonResponse(body: unknown): Response {
   return new Response(JSON.stringify(body), {
@@ -35,12 +36,21 @@ function jsonResponse(body: unknown): Response {
 beforeEach(() => {
   fetchSpy = vi.fn();
   vi.stubGlobal("fetch", fetchSpy);
+  // Snapshot + clear NEXT_PUBLIC_OPS_BASE_URL so the resolveBaseUrl()
+  // fallback path is exercised. Restored in afterEach so test ordering
+  // never leaks env state to the next file.
+  savedOpsBaseUrl = process.env.NEXT_PUBLIC_OPS_BASE_URL;
   delete process.env.NEXT_PUBLIC_OPS_BASE_URL;
 });
 
 afterEach(() => {
   vi.unstubAllGlobals();
   vi.restoreAllMocks();
+  if (savedOpsBaseUrl !== undefined) {
+    process.env.NEXT_PUBLIC_OPS_BASE_URL = savedOpsBaseUrl;
+  } else {
+    delete process.env.NEXT_PUBLIC_OPS_BASE_URL;
+  }
 });
 
 describe("useProbes → ops-api → fetch wiring", () => {
@@ -66,9 +76,21 @@ describe("useProbes → ops-api → fetch wiring", () => {
     // The hook must call fetch through ops-api with the same-origin proxy
     // path. Anything else (the dashboard origin + /api/ops, or an inlined
     // ops base URL) means the rewrite contract is broken.
-    expect(fetchSpy).toHaveBeenCalled();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
     const url = String(fetchSpy.mock.calls[0]![0]);
     expect(url).toBe("/api/ops/probes");
+
+    // Lock in the request shape the proxy contract depends on: GET (no
+    // method override), no-store cache (live polling, never serve a stale
+    // response), JSON accept header, and an AbortSignal for hook-level
+    // cancellation. A regression in any of these silently breaks polling.
+    const callArg = fetchSpy.mock.calls[0]![1] as RequestInit | undefined;
+    expect(callArg?.method).toBe("GET");
+    expect(callArg?.cache).toBe("no-store");
+    expect(callArg?.headers).toMatchObject({
+      accept: expect.stringMatching(/json/i),
+    });
+    expect(callArg?.signal).toBeInstanceOf(AbortSignal);
   });
 
   it("parses probes returned by the proxy and exposes them on data", async () => {
@@ -120,8 +142,12 @@ describe("useProbes → ops-api → fetch wiring", () => {
     const { result } = renderHook(() => useProbes());
     await waitFor(() => expect(result.current.error).not.toBeNull());
 
-    expect(result.current.error?.message).toMatch(/404/);
-    expect(result.current.error?.message).toMatch(/\/api\/ops\/probes/);
+    // Tighten to the canonical `ensureOk` shape so a future refactor that
+    // changes the error format (status/statusText/url ordering) trips this
+    // test instead of silently regressing the operator-facing message.
+    expect(result.current.error?.message).toMatch(
+      /^ops-api request failed: 404 Not Found at \/api\/ops\/probes/,
+    );
     expect(result.current.data).toBeNull();
   });
 });

showcase/shell-dashboard/src/lib/ops-api.ts

@@ -10,9 +10,13 @@
  *
  * `baseUrl` resolution order (per call):
  *   1. explicit `baseUrl` param (overrides everything; used in tests + SSR)
- *   2. `process.env.NEXT_PUBLIC_OPS_BASE_URL` (inlined at build — opt-in
- *      escape hatch for direct cross-origin calls; production does NOT use
- *      this because showcase-ops has no CORS allowlist)
+ *   2. `process.env.NEXT_PUBLIC_OPS_BASE_URL` — opt-in escape hatch for
+ *      direct cross-origin calls; production does NOT use this because
+ *      showcase-ops has no CORS allowlist. Note: Next inlines `NEXT_PUBLIC_*`
+ *      into the client bundle at build time, but this module ALSO runs in
+ *      SSR + tests where `process.env` is read live at call time — so
+ *      `delete process.env.NEXT_PUBLIC_OPS_BASE_URL` in a test does take
+ *      effect on subsequent `resolveBaseUrl()` calls.
  *   3. `/api/ops` — same-origin path served by the Next.js rewrite in
  *      `next.config.ts`. This is the production contract, not a guess: the
  *      rewrite forwards `/api/ops/:path*` to `${OPS_BASE_URL}/api/:path*`
@@ -129,10 +133,16 @@ const FALLBACK_BASE_URL = "/api/ops";
  * Resolve the API base URL with the precedence documented at the top of
  * this module. Trailing slashes are stripped so callers don't end up with
  * a double-slash like `http://host//probes` that some servers reject.
+ *
+ * `NEXT_PUBLIC_OPS_BASE_URL` is treated as missing when it is undefined,
+ * empty, or whitespace-only. `??` only falls through on `null`/`undefined`,
+ * so without this an env var set to `""` would silently produce
+ * `baseUrl === ""` and URLs like `"/probes"` (no `/api/ops` prefix) — a
+ * silent failure mode where the dashboard hits its own origin and 404s.
  */
 function resolveBaseUrl(explicit?: string): string {
-  const raw =
-    explicit ?? process.env.NEXT_PUBLIC_OPS_BASE_URL ?? FALLBACK_BASE_URL;
+  const envBase = process.env.NEXT_PUBLIC_OPS_BASE_URL?.trim();
+  const raw = explicit ?? (envBase || undefined) ?? FALLBACK_BASE_URL;
   return raw.replace(/\/+$/, "");
 }
 
@@ -147,11 +157,21 @@ async function ensureOk(response: Response, url: string): Promise<void> {
   let detail = "";
   try {
     const text = await response.text();
-    if (text && text.length <= 200) detail = ` — ${text}`;
+    if (text) {
+      if (text.length <= 500) {
+        detail = ` — ${text}`;
+      } else {
+        // Bodies larger than 500B (often HTML 5xx pages or stack traces)
+        // would balloon the error message and trash log readability;
+        // truncate but keep the marker + total size so an operator can see
+        // they're missing tail data and re-fetch from the source if needed.
+        detail = ` — ${text.slice(0, 500)} [truncated, ${text.length} bytes total]`;
+      }
+    }
   } catch (bodyErr) {
-    // R2-C.3: propagate AbortError as-is so hook-layer cancellation
-    // filters (`err.name === "AbortError"`) keep working. Wrapping it in a
-    // generic Error would surface "AbortError" as user-facing noise.
+    // Propagate AbortError as-is so hook-layer cancellation filters
+    // (`err.name === "AbortError"`) keep working. Wrapping it in a generic
+    // Error would surface "AbortError" as user-facing noise.
     if ((bodyErr as { name?: string })?.name === "AbortError") {
       throw bodyErr;
     }
@@ -174,14 +194,13 @@ async function parseJson<T>(response: Response, url: string): Promise<T> {
   try {
     return (await response.json()) as T;
   } catch (parseErr) {
-    // R2-C.3: propagate AbortError as-is — same rationale as ensureOk.
+    // Propagate AbortError as-is — same rationale as ensureOk.
     if ((parseErr as { name?: string })?.name === "AbortError") {
       throw parseErr;
     }
     const msg = (parseErr as Error)?.message ?? "unknown";
-    // R3-C bonus: preserve the original error as `cause` so debuggers can
-    // walk back to the SyntaxError name/stack without re-parsing the
-    // wrapped message string.
+    // Preserve the original error as `cause` so debuggers can walk back to
+    // the SyntaxError name/stack without re-parsing the wrapped message.
     throw new Error(`ops-api JSON parse failed at ${url}: ${msg}`, {
       cause: parseErr,
     });
@@ -199,9 +218,9 @@ export async function fetchProbes(
   } = {},
 ): Promise<ProbesResponse> {
   const url = `${resolveBaseUrl(opts.baseUrl)}/probes`;
-  // R3-D.1: opt out of browser + Next.js fetch caching. Status data is
-  // polled every ~10s and must reflect the current backend state, not a
-  // cached response from a prior poll.
+  // Opt out of browser + Next.js fetch caching. Status data is polled
+  // every ~10s and must reflect the current backend state, not a cached
+  // response from a prior poll.
   const response = await fetch(url, {
     method: "GET",
     signal: opts.signal,
@@ -216,8 +235,9 @@ export async function fetchProbeDetail(
   id: string,
   opts: { signal?: AbortSignal; baseUrl?: string } = {},
 ): Promise<ProbeDetailResponse> {
+  if (!id) throw new Error("probe id is required");
   const url = `${resolveBaseUrl(opts.baseUrl)}/probes/${encodeURIComponent(id)}`;
-  // R3-D.1: see fetchProbes — same no-store rationale for live detail data.
+  // See fetchProbes — same no-store rationale for live detail data.
   const response = await fetch(url, {
     method: "GET",
     signal: opts.signal,
@@ -239,6 +259,7 @@ export async function triggerProbe(
   id: string,
   opts: TriggerProbeOptions,
 ): Promise<TriggerResponse> {
+  if (!id) throw new Error("probe id is required");
   const url = `${resolveBaseUrl(opts.baseUrl)}/probes/${encodeURIComponent(
     id,
   )}/trigger`;
@@ -255,6 +276,10 @@ export async function triggerProbe(
       authorization: `Bearer ${opts.token}`,
     },
     body: JSON.stringify(body),
+    // No-store for parity with the GET fetches — POST responses can be
+    // cached by intermediaries when CDN-fronted; explicit no-store
+    // guarantees the trigger response goes end-to-end live.
+    cache: "no-store",
     signal: opts.signal,
   });
   await ensureOk(response, url);