fix: name the handler contract when a non-Response is returned in dev

.changeset/olive-pianos-warn.md

@@ -0,0 +1,7 @@
+---
+"@marko/run": patch
+---
+
+Report a clear error in dev when a handler returns something other than a `Response`. Returning data directly (the `return { items }` habit) previously reached the node adapter, which failed with `headers is not iterable` from its own internals and named neither the route nor the contract. The dev-mode guard now names the verb, the route, and what to return instead.
+
+Also register the `NotHandled`/`NotMatched` sentinels with `Symbol.for`. A build can load more than one copy of the runtime module, and a sentinel returned from user code was not recognized by another copy's comparison — flowing into the adapter as if it were a response.

agent-feedback/bugs.md

@@ -8,12 +8,6 @@ Out-of-scope defects noticed while working on something else. Format and rules:
 
 The Netlify edge entry hardcodes `export const config = { pattern: "^[^.]*$" }` (`default-edge-entry.ts:12-14`). Netlify runs the edge function only when the request pathname matches this regex, and `^[^.]*$` matches only paths with zero dots, so any dotted URL never reaches the function; Netlify then serves it from the static publish dir and, with no matching asset, 404s. This silently drops every legitimate dynamic/catch-all route whose URL contains a dot: a `$$rest` catch-all serving `report.2024.pdf`, a `$handle` segment holding a username/slug/email (`jane.doe`), version params, or a handler emitting `.xml`/`.json`/`.txt`. The generated router matches those paths fine, and the same routes serve 200 in dev and on a workerd/Cloudflare build. The functions adapter gets this right with `{ path: "/*", preferStatic: true }` (`default-functions-entry.ts:4-6`), delegating static-vs-dynamic to Netlify's real file check instead of a path regex; the edge dot-heuristic was meant to skip asset paths but over-excludes all dotted paths, which also makes the `|| context.next()` fallback (`default-edge-entry.ts:8`) effectively dead for any app with routes. Fix: use Netlify's `excludedPath`/`excludedPattern` for real asset prefixes, or narrow the pattern to known static extensions, rather than a global dot exclusion. Undocumented: the Netlify section of `website/docs/marko-run/adapters.md` shows `edge: true` with no mention of the limitation. (@marko/run-adapter-netlify 3.0.6, @netlify/edge-functions 3.x.)
 
-## Assert a `+handler` returned a `Response` (not a plain object) in dev, instead of failing later with `headers is not iterable` from an internal adapter helper
-
-`packages/run/src/runtime/internal.ts` › `call` | 2026-07-19 | impact:med | effort:low
-
-A handler that returns a plain object (`return { items: [...] }`, the Next.js/Remix auto-JSON habit) or uses an Express `(req, res)` signature produces a 500 whose message names framework internals and never states the contract. In `call`, the dev branch captures `response = await handler(...)` and already emits helpful `console.warn`s naming `handler.name` for `next()` misuse (`internal.ts:337-349`), but the terminal guard at `internal.ts:364-366` only rejects `null`/`NotMatched`/`NotHandled`, so a truthy plain object flows through `return response` and into `middleware.ts:161-163`, where `copyResponseHeaders(res, response.headers)` runs `for (const [key, value] of headers)` on `undefined` (`middleware.ts:88`) and throws `headers is not iterable`; the stack top is `copyResponseHeaders` in a `dist` file with no user file/line. The Express habit fails differently — `res.json is not a function`, because `res` is bound to the wrapped `next` function — but likewise never reveals the real `(context, next) => Response` contract. For agentic workflows the error string steers a fixing agent to edit adapter internals or flail, never to `return Response.json(obj)`, because it names neither the route/verb nor the return-type rule. Add a dev-mode `response instanceof Response` (or sentinel) assertion mirroring the existing `next()`-misuse guards, naming the route/verb and the contract. The `HandlerReturnValue` type (`types.ts:715-722`) forbids a plain-object return so type-checked projects are protected, but run officially supports plain-JS handlers. Distinct from bugs.md:41/:47, which concern the export SHAPE (a non-function or mis-factoried verb export), not the RETURN value.
-
 ## Configure the HMR websocket in the middleware-mode dev server so concurrent instances don't collide on port 24678
 
 `packages/run/src/adapter/dev-server.ts` › `createViteDevServer` | 2026-07-18 | impact:high | effort:med

cspell.json

@@ -9,11 +9,13 @@
     "cheatsheet",
     "CLAUDECODE",
     "codegen",
+    "coderabbit",
     "colgroup",
     "colocating",
     "dedup",
     "destructures",
     "desyncs",
+    "docstrings",
     "domcontentloaded",
     "draftlog",
     "EADDRINUSE",

packages/run/src/__tests__/call-return-value.test.ts

@@ -0,0 +1,63 @@
+import assert from "assert";
+
+import { call, NotHandled, NotMatched } from "../runtime/internal";
+
+const context = { method: "GET", route: "/api", data: {} } as any;
+const next = (() => new Response("from next")) as any;
+
+function callWith(returned: unknown) {
+  return call((() => returned) as any, next, context);
+}
+
+function withNodeEnv(value: string) {
+  const previous = process.env.NODE_ENV;
+  beforeEach(() => {
+    process.env.NODE_ENV = value;
+  });
+  afterEach(() => {
+    process.env.NODE_ENV = previous;
+  });
+}
+
+describe("call return value", () => {
+  withNodeEnv("development");
+
+  it("should pass a Response through", async () => {
+    const response = new Response("ok");
+    assert.equal(await callWith(response), response);
+  });
+
+  it("should continue when a handler returns nothing", async () => {
+    assert.equal(await (await callWith(undefined)).text(), "from next");
+  });
+
+  it("should name the route and the contract for a non-Response", async () => {
+    await assert.rejects(
+      () => callWith({ items: [] }),
+      /GET \/api returned a value of type "object" instead of a Response/,
+    );
+    await assert.rejects(() => callWith("body"), /type "string"/);
+  });
+
+  // A build can load more than one copy of the runtime module, so a returned
+  // sentinel is not always the instance this copy created.
+  it("should recognize a sentinel from another copy of the runtime", async () => {
+    await assert.rejects(
+      () => callWith(Symbol.for("Run.Response.NotHandled")),
+      (error) => error === NotHandled,
+    );
+    await assert.rejects(
+      () => callWith(Symbol.for("Run.Response.NotMatched")),
+      (error) => error === NotMatched,
+    );
+  });
+
+  describe("in production", () => {
+    withNodeEnv("production");
+
+    it("should leave the return value unchecked", async () => {
+      const items = { items: [] };
+      assert.equal(await callWith(items), items as never);
+    });
+  });
+});

packages/run/src/__tests__/fixtures/handler-returns-object/.marko-run/routes.d.ts

@@ -0,0 +1,61 @@
+/*
+  WARNING: This file is automatically generated and any changes made to it will be overwritten without warning.
+  Do NOT manually edit this file or your changes will be lost.
+*/
+
+import { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform } from "@marko/run/namespace";
+import type * as $ from "@marko/run";
+
+
+declare module "@marko/run" {
+	interface App extends $.DefineRoutes<{
+		"/": [P1];
+		"/api": [H1];
+	}> {}
+}
+
+type H1 = $.Handler<"H1", typeof import("../src/routes/api/+handler")>;
+declare module "../src/routes/api/+handler" {
+  const Run: $.Namespace<H1>;
+  namespace Run {
+    type Context = $.ContextForFile<H1>;
+  }
+
+  /** @deprecated use `Run` namespace instead */
+  namespace MarkoRun {
+    export { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform };
+    export type Route = $.Routes["/api"];
+    export type Context = $.MultiRouteContext<Route>;
+    export type Handler = $.HandlerLike<Route>;
+    export type GET = $.HandlerLike<Route, "GET">;
+    export type HEAD = $.HandlerLike<Route, "HEAD">;
+    export type POST = $.HandlerLike<Route, "POST">;
+    export type PUT = $.HandlerLike<Route, "PUT">;
+    export type DELETE = $.HandlerLike<Route, "DELETE">;
+    export type PATCH = $.HandlerLike<Route, "PATCH">;
+    export type OPTIONS = $.HandlerLike<Route, "OPTIONS">;
+  }
+}
+
+type P1 = $.Template<"P1", typeof import("../src/routes/+page.marko")>;
+declare module "../src/routes/+page.marko" {
+  const Run: $.Namespace<P1>;
+  namespace Run {
+    type Context = $.ContextForFile<P1> & Marko.Global;
+  }
+
+  /** @deprecated use `Run` namespace instead */
+  namespace MarkoRun {
+    export { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform };
+    export type Route = $.Routes["/"];
+    export type Context = Run.Context;
+    export type Handler = $.HandlerLike<Route>;
+    export type GET = $.HandlerLike<Route, "GET">;
+    export type HEAD = $.HandlerLike<Route, "HEAD">;
+    export type POST = $.HandlerLike<Route, "POST">;
+    export type PUT = $.HandlerLike<Route, "PUT">;
+    export type DELETE = $.HandlerLike<Route, "DELETE">;
+    export type PATCH = $.HandlerLike<Route, "PATCH">;
+    export type OPTIONS = $.HandlerLike<Route, "OPTIONS">;
+  }
+}

packages/run/src/__tests__/fixtures/handler-returns-object/__snapshots__/dev.expected.md

@@ -0,0 +1,9 @@
+# Loading
+
+```html
+home
+```
+
+# Step 0
+ctx=>namesTheHandlerAndTheContract(ctx)
+

packages/run/src/__tests__/fixtures/handler-returns-object/src/routes/+page.marko

@@ -0,0 +1,10 @@
+<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8">
+    <title>@marko/run Test Fixture</title>
+  </head>
+  <body>
+    <div#app>home</div>
+  </body>
+</html>

packages/run/src/__tests__/fixtures/handler-returns-object/src/routes/api/+handler.ts

@@ -0,0 +1,4 @@
+export const GET = Run.GET(() => {
+  // The Next.js/Remix habit: return data and expect the framework to serialize.
+  return { items: ["a", "b"] } as never;
+});

packages/run/src/__tests__/fixtures/handler-returns-object/test.config.ts

@@ -0,0 +1,21 @@
+import assert from "assert";
+
+import type { Step, StepContext } from "../../main.test";
+
+// The guard is dev-only, so a production build keeps the unchecked path.
+export const skip_preview = true;
+
+// Returning data instead of a Response used to reach the adapter, which
+// failed reading `headers` off it and named only its own internals.
+async function namesTheHandlerAndTheContract({ page }: StepContext) {
+  const res = await page.fetch(new URL("/api", page.url()).href);
+  assert.equal(res.status, 500);
+
+  const body = await res.text();
+  assert.doesNotMatch(body, /headers is not iterable/);
+  assert.match(body, /GET \/api/);
+  assert.match(body, /instead of a Response/);
+  assert.match(body, /Response\.json/);
+}
+
+export const steps: Step[] = [(ctx) => namesTheHandlerAndTheContract(ctx)];

packages/run/src/__tests__/fixtures/handler-returns-object/tsconfig.json

@@ -0,0 +1,4 @@
+{
+  "extends": "../../tsconfig-base.json",
+  "include": ["src/**/*", ".marko-run/*"],
+}

packages/run/src/runtime/internal.ts

@@ -28,11 +28,13 @@ import { href } from "./url-builder";
 
 export { getMetaDataLookup as normalizeMeta } from "../vite/utils/meta-data";
 
-export const NotHandled: typeof MarkoRun.NotHandled = Symbol(
-  "marko-run not handled",
+// Registered rather than unique: a build can load more than one copy of this
+// module, and the sentinels must compare equal across all of them.
+export const NotHandled: typeof MarkoRun.NotHandled = Symbol.for(
+  "Run.Response.NotHandled",
 ) as any;
-export const NotMatched: typeof MarkoRun.NotMatched = Symbol(
-  "marko-run not matched",
+export const NotMatched: typeof MarkoRun.NotMatched = Symbol.for(
+  "Run.Response.NotMatched",
 ) as any;
 
 const parentContextLookup = new WeakMap<Request, Context>();
@@ -287,6 +289,20 @@ export async function call(
         );
       }
     }
+
+    if (
+      response &&
+      response !== NotHandled &&
+      response !== NotMatched &&
+      !(response instanceof Response)
+    ) {
+      // Left alone this reaches the adapter, which fails reading `headers`
+      // off it and names its own internals rather than the handler.
+      throw new Error(
+        `${handler.name ? `Handler '${handler.name}'` : "A handler"} for ${context.method} ${context.route} returned a value of type "${typeof response}" instead of a Response. ` +
+          "Return `Response.json(value)` to send JSON, return a `new Response(...)`, or call `next()` to continue.",
+      );
+    }
   } else {
     try {
       response = await handler(context, next);