fix(§12.1): a branch-declared function is routed BEFORE it is collected — closing the client half alone leaked a server fn body into client.js

compiler/src/codegen/collect.ts

@@ -187,6 +187,32 @@ export function collectFunctions(fileAST: FileAST): Node[] {
         result.push(node);
       }
       if (Array.isArray(node.children)) visit(node.children);
+      // §17.1.1 if-chain — THE FUNCTION-DEFINITION LIMB, and the one that
+      // decides whether a function declared inside a branch EXISTS at all.
+      //
+      // `collapseIfChains` (ast-builder.js) rewrites an `if=`/`else-if=`/`else`
+      // chain WITH an else arm into `{kind:"if-chain", branches:[{condition,
+      // element}], elseBranch}`. The `node.children` descent above reaches
+      // neither: `branches` holds `{condition, element}` RECORDS, and
+      // `elseBranch` is a single object. So a `${ function helper() {…} }`
+      // inside a branch was never collected, emit-functions.ts emitted ZERO
+      // definitions, and every call site shipped as a BARE `helper()` —
+      // `ReferenceError` at exit 0 with zero diagnostics. The lone-`if=` twin of
+      // the same source emitted one `_scrml_helper_N` definition and four
+      // mangled calls.
+      //
+      // ⛑ THIS WALK IS DOWNSTREAM OF ROUTE INFERENCE, AND THAT ORDER IS LOAD-
+      // BEARING. `analyze.ts` hands this result to BOTH emit-functions.ts (the
+      // client emitter) and emit-server.ts. The client emitter omits a
+      // `server fn` body only because RI claimed the function as an endpoint —
+      // and RI's own walk (`route-inference.ts` `collectFileFunctions`) was
+      // blind to this same shape. Closing THIS walk while that one stayed blind
+      // shipped the `server fn` BODY into `client.js` with no `server.js` at
+      // all: measured body-in-client 1 with this hunk alone, 0 without it, 0 on
+      // base. Both walks are closed together and
+      // `g-if-chain-branch-cell-never-wired.test.js`'s LEAK GUARD is what keeps
+      // them that way. Child SHAPE via `ifChainChildNodes` (ast-if-chain.js).
+      for (const branchBody of ifChainChildNodes(node)) visit([branchBody]);
     }
   }
   visit(nodes);

compiler/src/route-inference.ts

@@ -89,6 +89,7 @@ import { filePrintBuiltinsShadowed } from "./codegen/log-loc.ts";
 // name-extractor rather than re-hand-rolling it. Cycle-safe: type-system's
 // direct deps do not import route-inference.
 import { isDestructurePattern, iterDestructuredNames } from "./type-system.ts";
+import { ifChainChildNodes } from "./ast-if-chain.js";
 
 // ---------------------------------------------------------------------------
 // RI-internal types
@@ -1107,6 +1108,33 @@ export function collectFileFunctions(fileAST: FileAST): FunctionDeclNode[] {
       if ("children" in node && Array.isArray((node as any).children)) {
         visitNodes((node as any).children);
       }
+      // §17.1.1 if-chain — THE SERVER-BOUNDARY LIMB, and the one that decides
+      // whether a `server fn` declared inside a branch ever becomes a ROUTE.
+      //
+      // `collapseIfChains` (ast-builder.js) rewrites an `if=`/`else-if=`/`else`
+      // chain WITH an else arm into `{kind:"if-chain", branches:[{condition,
+      // element}], elseBranch}`. The `children` descent above reaches neither:
+      // `branches` holds `{condition, element}` RECORDS, and `elseBranch` is a
+      // single object. So RI never saw the declaration and never built a route
+      // for it — which is what leaves `route.boundary` unset, and it is
+      // `route.boundary === "server"` (checked in `codegen/emit-functions.ts`)
+      // that keeps a `server fn` body out of `client.js`. NOTE: this is NOT
+      // `endpointClientSkipIds` — that set is built solely from `<endpoint>`
+      // private-arm reachability and carries no `server fn` placement path.
+      // (An earlier draft of this comment named it; a reader who follows that
+      // pointer finds no `server fn` path there and wrongly concludes this walk
+      // is not the cause.)
+      //
+      // ⛑ THIS WALK MUST STAY AHEAD OF `codegen/collect.ts`'s
+      // `collectFunctions`. The client function emitter skips a `server fn`
+      // body ONLY because RI claimed it here. Teaching the CLIENT collector to
+      // see branch-declared functions while THIS walk stays blind emits the
+      // `server fn` BODY into `client.js` and produces no `server.js` at all —
+      // measured, and the reason the two halves land together
+      // (g-collect-functions-branch-decl-vs-server-boundary-routing, HIGH,
+      // security-gated). Child SHAPE via `ifChainChildNodes`
+      // (ast-if-chain.js), the one module that owns the fact.
+      for (const branchBody of ifChainChildNodes(node)) visitNodes([branchBody as ASTNode]);
     }
   }
 
@@ -1159,6 +1187,18 @@ function collectWorkerBodyFunctionIds(fileAST: FileAST): Set<number> {
       if ("children" in node && Array.isArray((node as any).children)) {
         visitNodes((node as any).children, enteringWorker);
       }
+      // §17.1.1 if-chain — this SUPPRESSION set must stay in lockstep with
+      // `collectFileFunctions` above. That walk now descends branch bodies, so a
+      // `server fn` declared inside an `if=`/`else` branch of a
+      // `<program name="…">` worker body is now COLLECTED; if this set stayed
+      // blind, the same function would be missing from the worker-body
+      // suppression and E-ROUTE-001 would false-fire on it. Closing one descent
+      // without the other converts a silent miss into a spurious error.
+      // `enteringWorker` threads through unchanged — the branch body is
+      // lexically inside whatever `<program>` encloses the chain.
+      for (const branchBody of ifChainChildNodes(node)) {
+        visitNodes([branchBody as ASTNode], enteringWorker);
+      }
     }
   }
 

compiler/tests/unit/g-if-chain-branch-cell-never-wired.test.js

@@ -36,16 +36,34 @@
  * file". The accepting half is right — SPEC §6.1.1 + the §6.1.2 Read bullet say
  * so — but it is only right WITH this.
  *
- * ⚠ THE TRAP THIS TEST ALSO PINS, because it was walked into and backed out of.
- * `collect.ts`'s `collectFunctions` has the SAME blind spot, and closing it is
- * NOT the same edit. It feeds the CLIENT function emitter, and the walk that
- * routes a `server fn` to the server bundle is blind in its own separate way —
- * so collecting branch-declared functions there emits a `server fn`'s BODY into
- * `client.js` and produces no `server.js` at all. MEASURED: with that hunk the
- * body shipped to the client; without it, it did not. A loud `ReferenceError`
- * traded for a silent server-body leak is a strictly worse trade, so
- * `collectFunctions` is deliberately NOT closed here and the leak guard below
- * is what keeps it that way.
+ * ⚠ THE TRAP THIS TEST PINS, because it was walked into and backed out of once
+ * before it was closed properly. `collect.ts`'s `collectFunctions` had the SAME
+ * blind spot, and closing it is NOT the same edit. `analyze.ts` hands its result
+ * to BOTH the client function emitter (`emit-functions.ts`) and the server
+ * emitter (`emit-server.ts`) — but the client emitter omits a `server fn` body
+ * ONLY because route inference claimed the function as an endpoint, and RI's own
+ * walk (`route-inference.ts` `collectFileFunctions`) was blind to this same
+ * shape. So collecting branch-declared functions on the codegen side ALONE
+ * emitted the `server fn`'s BODY into `client.js` and produced no `server.js`
+ * at all. A loud `ReferenceError` traded for a silent server-body leak is a
+ * strictly worse trade.
+ *
+ * BOTH walks are now closed, ROUTING FIRST, and this is the three-way control
+ * that decided the landing order (`41 + 1` = the server fn's body):
+ *
+ *   |                                  | body in client | server.js bytes |
+ *   |----------------------------------|----------------|-----------------|
+ *   | base (neither walk)              |       0        |        0        |
+ *   | codegen half ALONE (the trap)    |       1        |        0        |
+ *   | RI half ALONE                    |       0        |        0        |
+ *   | BOTH (shipped)                   |       0        |      2308       |
+ *   | lone-`if=` oracle                |       0        |      2308       |
+ *
+ * The leak guard below is what keeps the two halves together. It is written to
+ * RED against the codegen-half-alone column — verified by re-applying exactly
+ * that hunk and running this file — and it asserts the POSITIVE limb too,
+ * because an absence-only guard is also satisfied by the base column, where the
+ * function vanished entirely.
  *
  * VALUE-asserting (R26): compiles real .scrml end-to-end and asserts on the
  * EMITTED artifact, because the defect is invisible in the diagnostic stream by
@@ -81,6 +99,14 @@ function compileSource(source) {
 
 const count = (hay, needle) => hay.split(needle).length - 1;
 
+// Strip ALL whitespace before asserting on emitted-code SHAPE.
+//
+// ⚠ WHY: `expect(clientJs).not.toContain("41 + 1")` — this guard's original and
+// only biting assertion — is whitespace-SENSITIVE. An emitter that spaced the
+// same leaked body as `41+1`, or wrapped it across a line, passed it while
+// leaking. A guard that a formatting change can silence is not a guard.
+const squash = (s) => s.replace(/\s+/g, "");
+
 // The declaration under test, plus a read INSIDE the branch and one OUTSIDE it.
 const DECL_BRANCH = `<div if=@open>${D} <n>: number = 7 }<p>in ${D}@n}</p></div>`;
 const ELSE_ARM = `<div else><p>c</p></div>`;
@@ -121,16 +147,115 @@ describe("g-if-chain-branch-cell-never-wired", () => {
     }
   });
 
-  test("LEAK GUARD: a server fn in a branch never ships its body to the client", () => {
-    // This is the trap `collectFunctions` sets — see the header. If someone
-    // closes that walk without first closing the server-boundary routing walk,
-    // THIS is what reds.
-    const src = `<open> = true
+  // A `server fn` DECLARED INSIDE a branch, called from that branch's markup.
+  const SERVER_FN_SRC = `<open> = true
 <div if=@open>${D} server fn zzload() { return 41 + 1 } }<p>v ${D}zzload()}</p></div>
 ${ELSE_ARM}
 `;
-    const r = compileSource(src);
-    expect(r.clientJs).not.toContain("41 + 1");
-    expect(r.clientJs).not.toContain("server fn");
+
+  test("LEAK GUARD: a server fn in a branch never ships its body to the client", () => {
+    // This is the trap `collectFunctions` set — see the header. If someone
+    // re-opens the codegen half without the server-boundary routing walk, THIS
+    // is what reds. Verified to red against exactly that hunk.
+    const r = compileSource(SERVER_FN_SRC);
+    expect(r.errors.length).toBe(0);
+
+    // (a) THE BODY. Whitespace-normalized: the leak shipped
+    // `function _scrml_zzload_7() { return 41 + 1; }` verbatim into client.js,
+    // and `41+1` must not survive any reformatting of that.
+    expect(squash(r.clientJs)).not.toContain("41+1");
+
+    // (b) THE DEFINITION. Every `function …zzload…` the client is allowed to
+    // define is the generated FETCH STUB (`_scrml_fetch_zzload_N`) that POSTs to
+    // the route. A definition WITHOUT `fetch` in its name is the server body
+    // inlined — the leak shape — and stays caught even if the body itself is
+    // constant-folded or rewritten past assertion (a).
+    const clientZzloadDefs = [...r.clientJs.matchAll(/function\s+([A-Za-z0-9_$]*zzload[A-Za-z0-9_$]*)\s*\(/g)]
+      .map((m) => m[1]);
+    expect(clientZzloadDefs.every((n) => n.includes("fetch"))).toBe(true);
+
+    // (c) THE HANDLER PRELUDE. Route-handler machinery is server-only material;
+    // none of it may appear in a client bundle.
+    // (Replaces the ORIGINAL `not.toContain("server fn")` assertion, which was
+    // VACUOUS — emitted JS never contains the scrml source keyword regardless of
+    // where the body lands, so it could not fail for any input.)
+    for (const serverOnlyMarker of ["_scrml_handler_", "_scrml_validate_csrf", "Set-Cookie"]) {
+      expect(r.clientJs).not.toContain(serverOnlyMarker);
+    }
+
+    // (d) THE POSITIVE LIMB — without it, (a)-(c) are ALSO satisfied by the base
+    // column of the header table, where the function vanished from every bundle
+    // and the page threw `ReferenceError`. "Absent from the client" only means
+    // something once the function is PRESENT on the server.
+    expect(r.serverJs.length).toBeGreaterThan(0);
+    expect(squash(r.serverJs)).toContain("41+1");
+    expect(r.serverJs).toContain("zzload");
+  });
+
+  test("ROUTING PARITY: the branch-declared server fn routes exactly like the lone-if= oracle", () => {
+    // Same source in the two wrappers whose only difference is the `<div else>`
+    // sibling — the discriminator that makes `collapseIfChains` fire.
+    const loneSrc = SERVER_FN_SRC.replace(ELSE_ARM + "\n", "");
+    // ⛑ ANTI-TAUTOLOGY GUARD. `String.replace` NO-OPS SILENTLY if `ELSE_ARM +
+    // "\n"` stops being a literal substring of `SERVER_FN_SRC` — a trailing
+    // space, a reflow of the template, an attribute added to ELSE_ARM. Without
+    // this line `loneSrc === SERVER_FN_SRC`, the test compiles the SAME source
+    // twice, and every assertion below passes trivially with the chain-vs-oracle
+    // discriminator gone. A gate that cannot fail is indistinguishable from a
+    // gate that never fails.
+    expect(loneSrc).not.toBe(SERVER_FN_SRC);
+    const lone = compileSource(loneSrc);
+    const chain = compileSource(SERVER_FN_SRC);
+
+    expect(lone.errors.length).toBe(0);
+    expect(chain.errors.length).toBe(0);
+
+    // The oracle emits a server bundle; so must the chain.
+    expect(lone.serverJs.length).toBeGreaterThan(0);
+    expect(chain.serverJs.length).toBeGreaterThan(0);
+    // NB: byte-length EQUALITY between the two bundles is deliberately NOT
+    // asserted. The chain source carries an extra `<div else>` node, which
+    // shifts the node-id counter (this file's own Phase-2 note records
+    // `_scrml_helper_5` vs `_scrml_helper_8`). Equality holds today only because
+    // no node id happens to reach `server.js`; the first change that stamps one
+    // into a server artifact would red this test with no defect behind it. The
+    // per-marker count loop below carries the real signal.
+
+    // Route identity, handler, and body all match.
+    for (const marker of ["_scrml_handler_zzload_1", "/_scrml/__ri_route_zzload_1", "41 + 1"]) {
+      expect(chain.serverJs).toContain(marker);
+      expect(count(chain.serverJs, marker)).toBe(count(lone.serverJs, marker));
+    }
+
+    // And the client side calls it the same way in both: through a fetch stub.
+    expect(chain.clientJs).toContain("/_scrml/__ri_route_zzload_1");
+    expect(squash(chain.clientJs)).not.toContain("41+1");
+    expect(squash(lone.clientJs)).not.toContain("41+1");
+  });
+
+  test("a plain function declared in an if=/else branch is DEFINED and called by name", () => {
+    // The client-side half of the same class: with `collectFunctions` blind, the
+    // chain emitted ZERO definitions and four BARE `helper()` call sites —
+    // `ReferenceError` at exit 0 with zero diagnostics. The lone-`if=` oracle
+    // emitted one mangled definition and four mangled calls.
+    const decl = `<div if=@open>${D} function zzhelper() { return 1 } }<p>a ${D}zzhelper()}</p><p>b ${D}zzhelper()}</p></div>`;
+    const chain = compileSource(`<open> = true\n${decl}\n${ELSE_ARM}\n`);
+    const lone = compileSource(`<open> = true\n${decl}\n`);
+
+    expect(chain.errors.length).toBe(0);
+
+    const defs = (js) => (js.match(/function\s+_scrml_zzhelper_\d+\s*\(/g) ?? []).length;
+    const mangledCalls = (js) => (js.match(/_scrml_zzhelper_\d+\s*\(\s*\)/g) ?? []).length;
+    // A call site the emitter never resolved to a definition ships as the raw
+    // source name — that is the `ReferenceError`.
+    const bareCalls = (js) => (js.match(/(?<![\w$])zzhelper\s*\(/g) ?? []).length;
+
+    expect(defs(chain.clientJs)).toBe(1);
+    expect(bareCalls(chain.clientJs)).toBe(0);
+    // Definition + 4 calls (2 module-init statements + 2 render-value sites),
+    // byte-for-byte the oracle's counts.
+    expect(defs(chain.clientJs)).toBe(defs(lone.clientJs));
+    expect(mangledCalls(chain.clientJs)).toBe(mangledCalls(lone.clientJs));
+    expect(bareCalls(lone.clientJs)).toBe(0);
   });
 });

conformance/cases/control-flow/if-chain-branch-declared-function-pos/case.scrml

@@ -0,0 +1,16 @@
+${
+    <open> = true
+    <seen> : string = "none"
+}
+<div id="wrap" if=@open>
+    ${
+        function branchHelper() {
+            return "from-branch"
+        }
+    }
+    <p id="out">${branchHelper()}</>
+    <button id="go" onclick=@seen = branchHelper()>Go</>
+</>
+<div else>
+    <p id="alt">alt</>
+</>

conformance/cases/control-flow/if-chain-branch-declared-function-pos/expected.json

@@ -0,0 +1,17 @@
+{
+  "id": "control-flow-if-chain-branch-declared-function-pos",
+  "description": "§17.1.1 if-chain — a `function` DECLARED INSIDE a branch is emitted, and a call site bound to it resolves. `collapseIfChains` moves branch bodies under `branches[].element` / `elseBranch`, which no `children`-only walk reaches; codegen's function collector was one of those walks, so this exact source emitted ZERO definitions of `branchHelper` and shipped BARE `branchHelper()` call sites. The `<div else>` sibling is the whole variable: drop it and the chain is never collapsed and the same source always worked. Pre-fix this case does not merely fail — the emitted bundle throws `ReferenceError: branchHelper is not defined` on load, which is exactly the runtime cost the compiler reported nothing about (exit 0, zero diagnostics).",
+  "language-version": "1.0",
+  "spec": "§17.1.1",
+  "rationale": "§17.1.1 defines a chain's branch bodies as ordinary markup, so a declaration inside one is a declaration in the program — not a declaration the compiler may lose. The assertion is driven through the HANDLER position (`onclick` → a state write) rather than the `${branchHelper()}` interpolation on #out, and that choice is deliberate: a call-expression interpolation inside a collapsed branch renders EMPTY on `main` today, for a top-level function just as much as for a branch-declared one, so asserting it here would pin an unrelated open defect instead of this one. #out's mere PRESENCE is asserted (the active branch mounted) and #alt's absence pins §17.1.1's own only-one-branch-exists sentence.",
+  "expect": {
+    "codes": [],
+    "notCodes": ["E-SCOPE-001", "E-STATE-UNDECLARED"],
+    "input": [{ "click": "#go" }],
+    "state": { "seen": "from-branch" },
+    "domAnchored": [
+      { "selector": "#out", "count": 1 },
+      { "selector": "#alt", "count": 0 }
+    ]
+  }
+}

conformance/cases/server-fn/branch-declared-server-fn-routes-to-server/case.scrml

@@ -0,0 +1,19 @@
+${
+    <open> = true
+    <label> : string = "pending"
+}
+<div id="wrap" if=@open>
+    ${
+        server fn branchLoad() : string {
+            return "COMPUTED-IN-THE-BODY"
+        }
+        function pull() {
+            @label = branchLoad()
+        }
+    }
+    <button id="go" onclick=pull()>Go</>
+</>
+<div else>
+    <p id="alt">alt</>
+</>
+<p id="out">${@label}</>