fix(observability-map): widen the corpus helper to every export form

mutations.ts entryBodies collected exported function declarations and
exported const identifiers only, so it missed the object binding pattern
(export const { action, loader } = createActionApiRoute(...)), the export
clause (const { action } = builder(...); export { action }), and
export const action = route.action. That is 36 of the tree's 427 entry
points, all of them API routes: every whole-body corpus entry skipped them
while the file count suggested otherwise. The scanner has read all four forms
since early on, so this was the harness lagging it.

No assertion could have noticed. A mutation that reaches fewer routes lowers
the score rather than raising it, which is exactly how the suppress-every-check
omission hid, so the answer is the same: assert the population. admin.tsx is
the one exclusion, named rather than counted, because its handler is a concise
arrow with no block for a block wrapper to wrap.

wrap-body-in-rethrow goes from 391 files with 36 entry points missed to 426
files, 1020 sites, 1 missed. Widening changes no entry's verdict: the full
corpus is 55 passed and 1 expected fail either way, and with the narrow
population the only failure is the new population assertion itself.

readTree now calls routeModuleFiles rather than keeping its own copy of the
directory walk. isScannableFile had already replaced the file half of that
copy; the directory half survived.

Author: 1stvamp

internal-packages/observability-map/src/mutationCorpus.test.ts

@@ -1,7 +1,7 @@
-import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { dirname, join, resolve } from "node:path";
-import { isScannableFile, scanDirectory } from "./scan.js";
+import { routeModuleFiles, scanDirectory } from "./scan.js";
 import { buildReport } from "./score.js";
 import { ADDITIVE_IDS, MUTATIONS, type Mutation } from "./mutations.js";
 import { CHECKS } from "./checks/index.js";
@@ -62,27 +62,20 @@ const KNOWN_GAPS = new Set<string>(["dead-classifying-try-with-call"]);
 
 type SourceFile = { relativeName: string; source: string };
 
-/** Route modules exactly as `scanDirectory` enumerates them: flat files, plus one `route.ts(x)` per
- * directory. Read once; every mutation rewrites this list rather than the tree on disk. */
+/**
+ * Route modules exactly as `scanDirectory` enumerates them, because it is the same enumeration and
+ * no longer a copy of it. `isScannableFile` had already replaced the file half of the copy; the
+ * directory half survived, so "one `route.ts(x)` per immediate subdirectory" was still written
+ * twice. A harness that reads a different tree from the scanner reports files and sites the scan
+ * never saw, and those counts are what the thresholds below rest on.
+ *
+ * Read once; every mutation rewrites this list rather than the tree on disk.
+ */
 function readTree(dir: string): SourceFile[] {
-  const files: SourceFile[] = [];
-  const take = (absolutePath: string, relativeName: string) => {
-    files.push({ relativeName, source: readFileSync(absolutePath, "utf8") });
-  };
-  for (const entry of readdirSync(dir, { withFileTypes: true })) {
-    if (entry.isDirectory()) {
-      for (const child of readdirSync(join(dir, entry.name), { withFileTypes: true })) {
-        if (!child.isFile() || (child.name !== "route.ts" && child.name !== "route.tsx")) continue;
-        take(join(dir, entry.name, child.name), `${entry.name}/${child.name}`);
-      }
-      continue;
-    }
-    // `scanDirectory`'s own predicate rather than a copy of it. A copy that drifts lets a mutation
-    // report files and sites the scanner never read, which is what the thresholds below are for.
-    if (!entry.isFile() || !isScannableFile(entry.name)) continue;
-    take(join(dir, entry.name), entry.name);
-  }
-  return files;
+  return routeModuleFiles(dir).map((file) => ({
+    relativeName: file.relativeName,
+    source: readFileSync(file.absolutePath, "utf8"),
+  }));
 }
 
 function materialize(files: SourceFile[]): string {
@@ -318,6 +311,43 @@ describeCorpus("mutation corpus over the real route tree", { timeout: ENTRY_TIME
     baseline = measure(files);
   }, ENTRY_TIMEOUT_MS);
 
+  /**
+   * The corpus's own population, against the scanner's.
+   *
+   * The whole-body entries wrap what `entryBodies` finds, and that helper read two of the four
+   * export forms `scan.ts` reads. It missed `export const { action, loader } = builder(...)`,
+   * `const { action } = builder(...); export { action };` and `export const action = route.action`,
+   * which is 36 of the tree's entry points: the corpus was testing less than its entry count
+   * implied, and no assertion could notice, because a mutation that reaches fewer routes lowers the
+   * score rather than raising it. Same failure mode as the `suppress-every-check` omission above,
+   * so the answer is the same: assert the population rather than wait for a verdict to move.
+   *
+   * `admin.tsx` is the one documented exclusion. Its handler is a concise arrow
+   * (`async ({ user }) => typedjson({ user })`) with no block for a block wrapper to wrap, which is
+   * a limit of the rewrite rather than a gap in the enumeration. It is named rather than counted so
+   * a second one cannot appear silently.
+   */
+  const CONCISE_ARROW_BODIES = new Set(["admin.tsx"]);
+
+  it("wraps a body in every non-delegating entry point the scanner finds", () => {
+    const wrap = MUTATIONS.find((m) => m.id === "wrap-body-in-rethrow")!;
+    const root = materialize(files);
+    let entryPoints;
+    try {
+      ({ entryPoints } = scanDirectory(root));
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+
+    const byName = new Map(files.map((f) => [f.relativeName, f.source]));
+    const untouched = entryPoints
+      .filter((ep) => !ep.delegating && !CONCISE_ARROW_BODIES.has(ep.fileName))
+      .filter((ep) => wrap.apply(ep.fileName, byName.get(ep.fileName)!) === null)
+      .map((ep) => ep.fileName);
+
+    expect(untouched).toEqual([]);
+  });
+
   it("has a baseline worth mutating", () => {
     expect(baseline).not.toBeNull();
     expect(baseline!.entryPoints).toBeGreaterThan(300);

internal-packages/observability-map/src/mutations.ts

@@ -179,18 +179,44 @@ function rootCall(call: ts.CallExpression): ts.CallExpression {
   }
 }
 
-function fromInitializer(expr: ts.Expression, out: EntryFunction[]): void {
+/**
+ * The handler functions an export's initializer resolves to.
+ *
+ * `locals` is consulted for the two indirect spellings, both of which `scan.ts` resolves and
+ * neither of which this reached: `export const action = route.action` beside
+ * `const route = createActionApiRoute(...)`, which is 7 of the tree's API routes, and
+ * `export const action = handleThing` naming a local. `seen` stops `const a = b; const b = a`.
+ */
+function fromInitializer(
+  expr: ts.Expression,
+  out: EntryFunction[],
+  locals: LocalDeclarations,
+  seen: Set<string> = new Set()
+): void {
   const target = unwrap(expr);
   if (isEntryFunction(target)) {
     out.push(target);
     return;
   }
-  if (!ts.isCallExpression(target)) return;
-  for (const arg of rootCall(target).arguments) {
-    const unwrapped = unwrap(arg);
-    if (isEntryFunction(unwrapped)) out.push(unwrapped);
-    else if (ts.isObjectLiteralExpression(unwrapped)) collectNamedHandlers(unwrapped, out);
+  if (ts.isCallExpression(target)) {
+    for (const arg of rootCall(target).arguments) {
+      const unwrapped = unwrap(arg);
+      if (isEntryFunction(unwrapped)) out.push(unwrapped);
+      else if (ts.isObjectLiteralExpression(unwrapped)) collectNamedHandlers(unwrapped, out);
+    }
+    return;
   }
+  // `route.action`, and any longer chain, is resolved from whatever declared its root identifier.
+  let root: ts.Expression = target;
+  while (ts.isPropertyAccessExpression(root) || ts.isElementAccessExpression(root)) {
+    root = unwrap(root.expression);
+  }
+  if (!ts.isIdentifier(root) || seen.has(root.text)) return;
+  const declaration = locals.get(root.text);
+  if (declaration === undefined) return;
+  seen.add(root.text);
+  if (ts.isFunctionDeclaration(declaration)) out.push(declaration);
+  else fromInitializer(declaration, out, locals, seen);
 }
 
 const ENTRY_NAMES = new Set(["loader", "action"]);
@@ -202,23 +228,109 @@ function isExported(node: ts.Node): boolean {
   );
 }
 
-/** Block bodies of the exported `loader`/`action` handlers, the region a whole-body wrapper wraps. */
-function entryBodies(sf: ts.SourceFile): ts.Block[] {
-  const functions: EntryFunction[] = [];
+/**
+ * Top-level declarations by binding name, so a named export clause (`export { action }`) resolves
+ * back to the initializer it came from. Object binding patterns are read element by element, which
+ * is what makes `const { action, loader } = createActionApiRoute(...)` resolvable.
+ */
+type LocalDeclarations = Map<string, ts.Expression | ts.FunctionDeclaration>;
+
+function localDeclarations(sf: ts.SourceFile): LocalDeclarations {
+  const locals: LocalDeclarations = new Map();
   for (const statement of sf.statements) {
-    if (!isExported(statement)) continue;
     if (ts.isFunctionDeclaration(statement) && statement.name) {
-      if (ENTRY_NAMES.has(statement.name.text)) functions.push(statement);
+      locals.set(statement.name.text, statement);
       continue;
     }
     if (!ts.isVariableStatement(statement)) continue;
     for (const decl of statement.declarationList.declarations) {
-      if (!decl.initializer || !ts.isIdentifier(decl.name)) continue;
-      if (ENTRY_NAMES.has(decl.name.text)) fromInitializer(decl.initializer, functions);
+      if (!decl.initializer) continue;
+      if (ts.isIdentifier(decl.name)) {
+        locals.set(decl.name.text, decl.initializer);
+        continue;
+      }
+      if (ts.isObjectBindingPattern(decl.name)) {
+        for (const element of decl.name.elements) {
+          if (ts.isIdentifier(element.name)) locals.set(element.name.text, decl.initializer);
+        }
+      }
     }
   }
+  return locals;
+}
+
+/**
+ * Block bodies of the exported `loader`/`action` handlers, the region a whole-body wrapper wraps.
+ *
+ * Reads the same four export forms `scan.ts` reads: an exported function declaration, an exported
+ * `const`, an exported object binding pattern, and a named export clause resolved back through a
+ * local. It read only the first two, which is the shape of every API route in the tree
+ * (`const { action, loader } = createActionApiRoute(...); export { action, loader };` and the
+ * direct `export const { action } = ...`), so `wrapEveryBody` and the other whole-body entries
+ * silently skipped 36 of the 427 entry points while reporting a file count that suggested
+ * otherwise. `mutationCorpus.test.ts` pins the population now ("wraps a body in every
+ * non-delegating entry point the scanner finds"), so the harness cannot lag the scanner here again
+ * without going red.
+ *
+ * This is NOT a retreat from the deliberate independence `collectNamedHandlers` documents. That
+ * independence is about disagreeing over where a HANDLER sits inside a builder's argument, which is
+ * a judgement the corpus has to be able to make for itself. Which exports exist is not a judgement,
+ * and the harness was simply behind.
+ */
+function entryBodies(sf: ts.SourceFile): ts.Block[] {
+  const functions: EntryFunction[] = [];
+  const locals = localDeclarations(sf);
+  const fromLocal = (name: string) => {
+    const decl = locals.get(name);
+    if (decl === undefined) return;
+    if (ts.isFunctionDeclaration(decl)) functions.push(decl);
+    else fromInitializer(decl, functions, locals);
+  };
+
+  for (const statement of sf.statements) {
+    if (ts.isFunctionDeclaration(statement) && statement.name && isExported(statement)) {
+      if (ENTRY_NAMES.has(statement.name.text)) functions.push(statement);
+      continue;
+    }
+
+    if (ts.isVariableStatement(statement) && isExported(statement)) {
+      for (const decl of statement.declarationList.declarations) {
+        if (!decl.initializer) continue;
+        if (ts.isIdentifier(decl.name)) {
+          if (ENTRY_NAMES.has(decl.name.text)) fromInitializer(decl.initializer, functions, locals);
+          continue;
+        }
+        if (!ts.isObjectBindingPattern(decl.name)) continue;
+        for (const element of decl.name.elements) {
+          if (ts.isIdentifier(element.name) && ENTRY_NAMES.has(element.name.text)) {
+            fromInitializer(decl.initializer, functions, locals);
+          }
+        }
+      }
+      continue;
+    }
+
+    // A re-export (`export { loader } from "./x"`) has no local binding to resolve, and a namespace
+    // clause cannot name a loader or an action.
+    if (
+      ts.isExportDeclaration(statement) &&
+      statement.exportClause &&
+      !statement.moduleSpecifier &&
+      ts.isNamedExports(statement.exportClause)
+    ) {
+      for (const element of statement.exportClause.elements) {
+        if (!ENTRY_NAMES.has(element.name.text)) continue;
+        fromLocal(element.propertyName?.text ?? element.name.text);
+      }
+    }
+  }
+
+  // One handler can serve both exports, and both reach it by their own road: the loader and the
+  // action of `const { loader, action } = createActionApiRoute({ handler })` resolve to the same
+  // node. Wrapping it twice would splice the same text in twice at the same offset, because
+  // `applyEdits` treats two zero-width inserts at one position as non-overlapping.
   const bodies: ts.Block[] = [];
-  for (const fn of functions) {
+  for (const fn of new Set(functions)) {
     if (fn.body && ts.isBlock(fn.body)) bodies.push(fn.body);
   }
   return bodies;