mcp: prove the guardrails fire, test the misconfigured entry path, single-target fake config

Whole-pass audit of the scaffold found three gaps and closes them:

- Guardrail tests only ever passed. The wrangler.toml mapping and the
  MCP_ENVIRONMENT isolation checks are now pure functions
  (test/helpers/) tested against mutated input: preview→production,
  production→fake, local→production, mislabelled env, shared KV id,
  extra "staging" env, changed entry point, a file reading the binding,
  a file branching on the environment name.
- src/index.ts's misconfiguration path had no test. Added: 500 with the
  exact mismatch message, memoized failure across requests, missing
  bindings reported, sound bindings still serve.
- fake-upstream had an [env.preview] block it did not need (one deploy
  target) which made wrangler warn; now top-level only, script `deploy`.

Also verified out-of-band and recorded in CLAUDE.md as rules 4–6 of the
working method: `pnpm install --frozen-lockfile`, `wrangler deploy
--dry-run` for both Workers (production bundle contains no fake code),
the eslint boundary rejects a server→fake import, and both Workers boot
under `wrangler dev` and answer over HTTP.

pnpm check: typecheck, eslint, prettier, 37 tests passing.

Claude-Session: https://claude.ai/code/session_01H3gfN2NCx3hBXSrZXYHUQ3

mcp/CLAUDE.md

@@ -107,11 +107,19 @@ must never arrive in bulk.
    more than one paragraph to describe, split it.
 4. **Do a broader debug pass after the feature is assembled** — run the full suite, the e2e
    client test, and MCP Inspector against `wrangler dev`. This pass is *in addition to*
-   step 1, never a replacement for it.
-5. **Verify claims about the upstream API against the spec or the Rust source, not memory.**
+   step 1, never a replacement for it. It must include things unit tests cannot show: boot
+   the Worker for real (`wrangler dev` + curl), `wrangler deploy --dry-run` for every target,
+   and `pnpm install --frozen-lockfile` (what CI runs).
+5. **A guardrail test must prove it fires.** Any test that exists to catch a bad edit (config
+   invariants, import boundaries, environment-name isolation) needs a case that feeds it the
+   bad edit and asserts the failure. A guardrail that has only ever passed is unproven —
+   express the check as a pure function and test it against mutated input.
+6. **"Small unit" means one thing, then run.** One module and its test, then the checks; not
+   five files and then the checks. If a batch passes first time, that was luck, not method.
+7. **Verify claims about the upstream API against the spec or the Rust source, not memory.**
    The published spec is https://storyteller-docs.netlify.app/api.json; the handlers are under
    `crates/service/web/storyteller_web/src/http_server/`. Read-only reference — see constraints.
-6. **When something is uncertain, say so in the PR description** and pin it with a test that
+8. **When something is uncertain, say so in the PR description** and pin it with a test that
    documents the assumed behaviour, so a wrong assumption fails loudly later instead of silently.
 
 ## Architecture

mcp/fake-upstream/package.json

@@ -6,7 +6,7 @@
   "type": "module",
   "scripts": {
     "dev": "wrangler dev --port 12345",
-    "deploy:preview": "wrangler deploy --env preview"
+    "deploy": "wrangler deploy"
   },
   "dependencies": {
     "hono": "4.13.4"

mcp/fake-upstream/wrangler.toml

@@ -1,10 +1,9 @@
 # Fake of the allowlisted Artcraft API routes. A separate Worker that the MCP server never
 # imports; it exists so local and preview environments never touch api.storyteller.ai.
 # See mcp/CLAUDE.md → "fake-upstream — a fake of the API, not of this server".
+#
+# One deploy target only (workers.dev, used by preview), so no [env.*] blocks.
 
 name = "artcraft-api-fake"
 main = "src/index.ts"
 compatibility_date = "2026-08-01"
-
-[env.preview]
-name = "artcraft-api-fake"

mcp/test/env-isolation.test.ts

@@ -1,5 +1,7 @@
 import { describe, expect, it } from "vitest";
 
+import { environmentNameOffenders } from "./helpers/environment-name-offenders";
+
 /**
  * `MCP_ENVIRONMENT` exists so the Worker can assert its configuration at startup. It must not
  * become a behaviour switch. The only file allowed to mention it is src/config.ts; anything
@@ -20,21 +22,36 @@ describe("environment-name isolation", () => {
     expect(Object.keys(sources)).toContain("../src/config.ts");
   });
 
-  it("references MCP_ENVIRONMENT only in src/config.ts", () => {
-    const offenders = Object.entries(sources)
-      .filter(([path]) => !ALLOWED_FILES.includes(path))
-      .filter(([, source]) => source.includes("MCP_ENVIRONMENT"))
-      .map(([path]) => path);
-    expect(offenders).toEqual([]);
+  it("finds no offenders in the committed tree", () => {
+    expect(environmentNameOffenders(sources, ALLOWED_FILES)).toEqual({
+      readsBinding: [],
+      branches: [],
+    });
   });
+});
 
-  it("never branches on the environment name outside src/config.ts", () => {
-    const offenders = Object.entries(sources)
-      .filter(([path]) => !ALLOWED_FILES.includes(path))
-      .filter(([, source]) =>
-        /environment\s*(===|!==|==|!=)\s*["'](local|preview|production)["']/.test(source),
-      )
-      .map(([path]) => path);
-    expect(offenders).toEqual([]);
+describe("environment-name checks fire on an offender", () => {
+  it("catches a file that reads the binding", () => {
+    const result = environmentNameOffenders(
+      { "../src/leak.ts": 'const isProd = env.MCP_ENVIRONMENT === "production";' },
+      ALLOWED_FILES,
+    );
+    expect(result.readsBinding).toEqual(["../src/leak.ts"]);
+  });
+
+  it("catches a file that branches on the environment name", () => {
+    const result = environmentNameOffenders(
+      { "../src/leak.ts": 'if (config.environment !== "production") { grantFreeCredits(); }' },
+      ALLOWED_FILES,
+    );
+    expect(result.branches).toEqual(["../src/leak.ts"]);
+  });
+
+  it("still allows config.ts itself", () => {
+    const result = environmentNameOffenders(
+      { "../src/config.ts": "MCP_ENVIRONMENT: z.enum([...])" },
+      ALLOWED_FILES,
+    );
+    expect(result).toEqual({ readsBinding: [], branches: [] });
   });
 });

mcp/test/helpers/environment-mapping.ts

@@ -0,0 +1,79 @@
+import { PRODUCTION_UPSTREAM_API_HOST } from "../../src/config";
+
+/**
+ * The wrangler.toml invariants, as a pure function over the parsed file so the tests can prove
+ * they fire against a broken configuration, not only that they pass against the real one.
+ */
+
+export interface EnvBlock {
+  vars?: Record<string, unknown>;
+  kv_namespaces?: { binding: string; id: string }[];
+}
+
+export interface WranglerConfig extends EnvBlock {
+  name: string;
+  main: string;
+  env?: Record<string, EnvBlock>;
+}
+
+const PREVIEW_HOST_PATTERN = /^https:\/\/[a-z0-9.-]+\.workers\.dev$/;
+const LOCAL_HOST_PATTERN = /^http:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/;
+
+/** Returns human-readable violations; an empty array means the mapping is sound. */
+export function environmentMappingViolations(config: WranglerConfig): string[] {
+  const violations: string[] = [];
+  const envNames = Object.keys(config.env ?? {}).sort();
+  if (envNames.join(",") !== "preview,production") {
+    violations.push(
+      `expected exactly the environments preview,production; got ${envNames.join(",")}`,
+    );
+  }
+
+  const blocks: Record<string, EnvBlock | undefined> = {
+    local: config,
+    preview: config.env?.preview,
+    production: config.env?.production,
+  };
+
+  for (const [name, block] of Object.entries(blocks)) {
+    if (!block) {
+      violations.push(`${name}: block missing`);
+      continue;
+    }
+    if (block.vars?.MCP_ENVIRONMENT !== name) {
+      violations.push(`${name}: MCP_ENVIRONMENT must be "${name}"`);
+    }
+    const host = block.vars?.UPSTREAM_API_HOST;
+    if (typeof host !== "string") {
+      violations.push(`${name}: UPSTREAM_API_HOST missing`);
+      continue;
+    }
+    if (name === "production" && host !== PRODUCTION_UPSTREAM_API_HOST) {
+      violations.push(`production: must use ${PRODUCTION_UPSTREAM_API_HOST}, got ${host}`);
+    }
+    if (name !== "production" && host === PRODUCTION_UPSTREAM_API_HOST) {
+      violations.push(`${name}: must never point at the production API`);
+    }
+    if (name === "preview" && !PREVIEW_HOST_PATTERN.test(host)) {
+      violations.push(`preview: must use a workers.dev fake upstream, got ${host}`);
+    }
+    if (name === "local" && !LOCAL_HOST_PATTERN.test(host)) {
+      violations.push(`local: must use a localhost upstream, got ${host}`);
+    }
+  }
+
+  const kvIds = Object.entries(blocks).map(([name, block]) => {
+    const kv = block?.kv_namespaces?.find((namespace) => namespace.binding === "OAUTH_KV");
+    if (!kv) violations.push(`${name}: OAUTH_KV binding missing`);
+    return kv?.id;
+  });
+  if (new Set(kvIds).size !== kvIds.length) {
+    violations.push("OAUTH_KV namespace ids must differ between environments");
+  }
+
+  if (config.main !== "src/index.ts") {
+    violations.push(`main must be src/index.ts, got ${config.main}`);
+  }
+
+  return violations;
+}

mcp/test/helpers/environment-name-offenders.ts

@@ -0,0 +1,22 @@
+/**
+ * Finds server source files that read the environment name or branch on it. Pure so the test
+ * can prove it catches an offender, not only that the current tree has none.
+ */
+
+const ENVIRONMENT_BRANCH_PATTERN =
+  /environment\s*(===|!==|==|!=)\s*["'](local|preview|production)["']/;
+
+export function environmentNameOffenders(
+  sources: Record<string, string>,
+  allowedPaths: readonly string[],
+): { readsBinding: string[]; branches: string[] } {
+  const candidates = Object.entries(sources).filter(([path]) => !allowedPaths.includes(path));
+  return {
+    readsBinding: candidates
+      .filter(([, source]) => source.includes("MCP_ENVIRONMENT"))
+      .map(([path]) => path),
+    branches: candidates
+      .filter(([, source]) => ENVIRONMENT_BRANCH_PATTERN.test(source))
+      .map(([path]) => path),
+  };
+}

mcp/test/index-misconfiguration.test.ts

@@ -0,0 +1,68 @@
+import { createExecutionContext, waitOnExecutionContext } from "cloudflare:test";
+import { env } from "cloudflare:workers";
+import { describe, expect, it, vi } from "vitest";
+
+/**
+ * The Worker entry memoizes its app per isolate. These tests re-import the module for a fresh
+ * isolate-equivalent so each scenario starts from an empty cache.
+ */
+async function freshWorker() {
+  vi.resetModules();
+  const module = await import("../src/index");
+  return module.default;
+}
+
+function bindings(overrides: Partial<Record<keyof Cloudflare.Env, unknown>>): Cloudflare.Env {
+  return { ...env, ...overrides } as unknown as Cloudflare.Env;
+}
+
+async function fetchHealth(
+  worker: Awaited<ReturnType<typeof freshWorker>>,
+  workerEnv: Cloudflare.Env,
+) {
+  const ctx = createExecutionContext();
+  const response = await worker.fetch(new Request("https://mcp.test/healthz"), workerEnv, ctx);
+  await waitOnExecutionContext(ctx);
+  return response;
+}
+
+describe("worker entry with misconfigured bindings", () => {
+  it("fails with a 500 that names the mismatch", async () => {
+    const worker = await freshWorker();
+    const response = await fetchHealth(
+      worker,
+      bindings({ MCP_ENVIRONMENT: "production", UPSTREAM_API_HOST: "http://localhost:12345" }),
+    );
+    expect(response.status).toBe(500);
+    expect(response.headers.get("content-type")).toMatch(/^text\/plain/);
+    expect(await response.text()).toBe(
+      "Worker misconfigured: production must use https://api.storyteller.ai, got http://localhost:12345",
+    );
+  });
+
+  it("keeps failing on subsequent requests (the failure is memoized, not retried)", async () => {
+    const worker = await freshWorker();
+    const bad = bindings({
+      MCP_ENVIRONMENT: "preview",
+      UPSTREAM_API_HOST: "https://api.storyteller.ai",
+    });
+    const first = await fetchHealth(worker, bad);
+    const second = await fetchHealth(worker, bad);
+    expect(first.status).toBe(500);
+    expect(second.status).toBe(500);
+    expect(await second.text()).toMatch(/preview must never point at the production API/);
+  });
+
+  it("reports missing bindings rather than crashing", async () => {
+    const worker = await freshWorker();
+    const response = await fetchHealth(worker, bindings({ UPSTREAM_API_HOST: undefined }));
+    expect(response.status).toBe(500);
+    expect(await response.text()).toMatch(/invalid Worker bindings: UPSTREAM_API_HOST/);
+  });
+
+  it("serves normally when the bindings are sound", async () => {
+    const worker = await freshWorker();
+    const response = await fetchHealth(worker, env);
+    expect(response.status).toBe(200);
+  });
+});

mcp/test/wrangler-config.test.ts

@@ -3,67 +3,114 @@ import { describe, expect, it } from "vitest";
 
 import { PRODUCTION_UPSTREAM_API_HOST } from "../src/config";
 import wranglerToml from "../wrangler.toml?raw";
+import {
+  environmentMappingViolations,
+  type EnvBlock,
+  type WranglerConfig,
+} from "./helpers/environment-mapping";
 
 /**
  * The deployment configuration is part of the security boundary: it decides which upstream
- * each environment may reach. These tests pin the mapping so a wrangler.toml edit that points
- * preview at production (or production anywhere else) fails CI before it can deploy.
+ * each environment may reach. The first block pins the real file; the second proves the
+ * checks actually fire, by mutating a copy the way a careless edit would.
  */
 
-interface EnvBlock {
-  vars?: Record<string, unknown>;
-  kv_namespaces?: { binding: string; id: string }[];
+const REAL_CONFIG = parse(wranglerToml) as unknown as WranglerConfig;
+
+function mutated(mutate: (config: WranglerConfig) => void): WranglerConfig {
+  const copy = structuredClone(REAL_CONFIG);
+  mutate(copy);
+  return copy;
 }
 
-interface WranglerConfig extends EnvBlock {
-  name: string;
-  main: string;
-  env?: Record<string, EnvBlock>;
+function setVar(block: EnvBlock, key: string, value: string): void {
+  block.vars = { ...block.vars, [key]: value };
 }
 
-const config = parse(wranglerToml) as unknown as WranglerConfig;
-const envs = {
-  local: config,
-  preview: config.env?.preview,
-  production: config.env?.production,
-};
+function envBlock(config: WranglerConfig, name: "preview" | "production"): EnvBlock {
+  const block = config.env?.[name];
+  if (!block) throw new Error(`wrangler.toml has no [env.${name}] block`);
+  return block;
+}
 
 describe("wrangler.toml environment mapping", () => {
-  it("declares exactly the three environments", () => {
-    expect(Object.keys(config.env ?? {}).sort()).toEqual(["preview", "production"]);
-    expect(envs.preview).toBeDefined();
-    expect(envs.production).toBeDefined();
+  it("is sound as committed", () => {
+    expect(environmentMappingViolations(REAL_CONFIG)).toEqual([]);
   });
 
-  it("names each environment in MCP_ENVIRONMENT", () => {
-    expect(envs.local.vars?.MCP_ENVIRONMENT).toBe("local");
-    expect(envs.preview?.vars?.MCP_ENVIRONMENT).toBe("preview");
-    expect(envs.production?.vars?.MCP_ENVIRONMENT).toBe("production");
+  it("commits the expected literal hosts", () => {
+    expect(REAL_CONFIG.env?.production?.vars?.UPSTREAM_API_HOST).toBe(PRODUCTION_UPSTREAM_API_HOST);
+    expect(REAL_CONFIG.env?.preview?.vars?.UPSTREAM_API_HOST).toMatch(/\.workers\.dev$/);
+    expect(REAL_CONFIG.vars?.UPSTREAM_API_HOST).toMatch(/^http:\/\/localhost/);
+  });
+});
+
+describe("wrangler.toml checks fire on a broken mapping", () => {
+  it("catches preview pointed at production", () => {
+    const config = mutated((c) => {
+      setVar(envBlock(c, "preview"), "UPSTREAM_API_HOST", PRODUCTION_UPSTREAM_API_HOST);
+    });
+    expect(environmentMappingViolations(config)).toEqual([
+      "preview: must never point at the production API",
+      `preview: must use a workers.dev fake upstream, got ${PRODUCTION_UPSTREAM_API_HOST}`,
+    ]);
   });
 
-  it("points production at the real API, and nothing else at it", () => {
-    expect(envs.production?.vars?.UPSTREAM_API_HOST).toBe(PRODUCTION_UPSTREAM_API_HOST);
-    expect(envs.preview?.vars?.UPSTREAM_API_HOST).not.toBe(PRODUCTION_UPSTREAM_API_HOST);
-    expect(envs.local.vars?.UPSTREAM_API_HOST).not.toBe(PRODUCTION_UPSTREAM_API_HOST);
+  it("catches production pointed at a fake", () => {
+    const config = mutated((c) => {
+      setVar(
+        envBlock(c, "production"),
+        "UPSTREAM_API_HOST",
+        "https://artcraft-api-fake.workers.dev",
+      );
+    });
+    expect(environmentMappingViolations(config)).toEqual([
+      "production: must use https://api.storyteller.ai, got https://artcraft-api-fake.workers.dev",
+    ]);
   });
 
-  it("points preview at a workers.dev fake and local at localhost", () => {
-    expect(envs.preview?.vars?.UPSTREAM_API_HOST).toMatch(/^https:\/\/[a-z0-9.-]+\.workers\.dev$/);
-    expect(envs.local.vars?.UPSTREAM_API_HOST).toMatch(
-      /^http:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/,
+  it("catches local pointed at production", () => {
+    const config = mutated((c) => {
+      setVar(c, "UPSTREAM_API_HOST", PRODUCTION_UPSTREAM_API_HOST);
+    });
+    expect(environmentMappingViolations(config)).toContain(
+      "local: must never point at the production API",
     );
   });
 
-  it("gives every environment its own OAUTH_KV namespace", () => {
-    const ids = [envs.local, envs.preview, envs.production].map((block) => {
-      const kv = block?.kv_namespaces?.find((namespace) => namespace.binding === "OAUTH_KV");
-      expect(kv).toBeDefined();
-      return kv?.id;
+  it("catches a mislabelled environment", () => {
+    const config = mutated((c) => {
+      setVar(envBlock(c, "preview"), "MCP_ENVIRONMENT", "production");
     });
-    expect(new Set(ids).size).toBe(ids.length);
+    expect(environmentMappingViolations(config)).toEqual([
+      'preview: MCP_ENVIRONMENT must be "preview"',
+    ]);
   });
 
-  it("uses the same entry point for every environment", () => {
-    expect(config.main).toBe("src/index.ts");
+  it("catches a shared KV namespace", () => {
+    const config = mutated((c) => {
+      envBlock(c, "preview").kv_namespaces = structuredClone(
+        envBlock(c, "production").kv_namespaces ?? [],
+      );
+    });
+    expect(environmentMappingViolations(config)).toEqual([
+      "OAUTH_KV namespace ids must differ between environments",
+    ]);
+  });
+
+  it("catches an extra environment such as staging", () => {
+    const config = mutated((c) => {
+      c.env = { ...c.env, staging: structuredClone(envBlock(c, "preview")) };
+    });
+    expect(environmentMappingViolations(config)[0]).toMatch(/expected exactly the environments/);
+  });
+
+  it("catches a changed entry point", () => {
+    const config = mutated((c) => {
+      c.main = "src/other.ts";
+    });
+    expect(environmentMappingViolations(config)).toEqual([
+      "main must be src/index.ts, got src/other.ts",
+    ]);
   });
 });