test(guard): widen the client-bundle guard to every "use client" entry point (#10692) (#10700)

The guard shipped with #10695 watched two hand-picked modules. It now walks the static
import graph from all 753 "use client" files in src/ (plus the two originally pinned
entries), so the invariant is verified across the repo instead of where someone
remembered to look. Full sweep runs in ~750ms.

Two exclusions make that practical:

- `import type` is not an edge — TypeScript erases it before the bundler sees it.
  Counting type imports turns 3 real findings into 29; a guard that cries wolf gets
  switched off.
- Dynamic `import()` is still not followed. It does not break a bundle edge (that was
  tried for #10692 and failed) but it does move the module into a chunk the browser
  fetches on demand, which is a legitimate boundary.

The widened sweep immediately found what the narrow one could not: five value-form
imports of `db/batches` / `db/files` across three files under dashboard/batch, each
reaching db/core → the SQLite driver. All five bind only interfaces (BatchRecord,
FileRecord) used in type position, so the compiler was eliding them and the build stayed
green — the same latent shape as #10692 before #10647 removed the toolchain's tolerance.
Marking them `import type` makes the elision explicit instead of incidental.

Refs #10692

Co-authored-by: Xiangzhe <bakryun0718@proton.me>

scripts/ops/deploy-canary.mjs

@@ -28,11 +28,16 @@
  *   OMNIROUTE_SMOKE_API_KEY      sent as Authorization: Bearer when the gateway requires auth
  */
 
-import { execFileSync } from "node:child_process";
+import { execFileSync, spawnSync } from "node:child_process";
 import path from "node:path";
 import process from "node:process";
 
-import { buildRemoteSteps, evaluateSmoke, planCanaryDeploy } from "./deployCanary.ts";
+import {
+  buildRemoteSteps,
+  classifyInstallOutcome,
+  evaluateSmoke,
+  planCanaryDeploy,
+} from "./deployCanary.ts";
 import { makeGitAncestryProbe, readBuildSha } from "../build/buildProvenance.ts";
 
 function parseArgs(argv) {
@@ -61,6 +66,22 @@ function run(step) {
   return execFileSync(command, rest, { encoding: "utf8" }).trim();
 }
 
+/**
+ * Like `run`, but never throws: returns the exit code plus both streams. Used for the
+ * install, whose exit code does not decide the outcome (see classifyInstallOutcome) and
+ * whose stderr must reach the log — it used to be swallowed by execFileSync throwing.
+ */
+function runCapturing(step) {
+  console.log(`\n▶ ${step.name}: ${step.description}`);
+  const [command, ...rest] = step.argv;
+  const result = spawnSync(command, rest, { encoding: "utf8" });
+  return {
+    exitCode: result.status ?? 1,
+    stdout: (result.stdout || "").trim(),
+    stderr: (result.stderr || "").trim(),
+  };
+}
+
 async function probeHealth(baseUrl) {
   try {
     const response = await fetch(new URL("/api/monitoring/health", baseUrl), {
@@ -110,8 +131,9 @@ if (args.models.length === 0) {
 }
 
 const repoRoot = process.cwd();
+const localBuildSha = readBuildSha(repoRoot);
 const plan = planCanaryDeploy({
-  buildSha: readBuildSha(repoRoot),
+  buildSha: localBuildSha,
   isAncestorOfRelease: makeGitAncestryProbe(
     process.env.OMNIROUTE_RELEASE_REF || "origin/main",
     repoRoot
@@ -147,7 +169,23 @@ try {
   console.log(`\n▶ upload: ${args.tarball} → ${args.host}:${remoteTarball}`);
   execFileSync("scp", [args.tarball, `${args.host}:${remoteTarball}`], { stdio: "inherit" });
 
-  run(install);
+  const installResult = runCapturing(install);
+  const outcome = classifyInstallOutcome({
+    exitCode: installResult.exitCode,
+    stderr: installResult.stderr,
+    installedSha: run(verify),
+    expectedSha: localBuildSha,
+  });
+  if (!outcome.installed) {
+    if (installResult.stderr) console.error(installResult.stderr);
+    fail(`install did not land: ${outcome.reason}`);
+  }
+  if (outcome.kind === "installed-with-cleanup-failure") {
+    console.warn(`   ⚠️  ${outcome.reason}`);
+  } else {
+    console.log(`   ${outcome.reason}`);
+  }
+
   run(restart);
 
   const installedSha = run(verify);

scripts/ops/deployCanary.ts

@@ -152,3 +152,80 @@ export function buildRemoteSteps(input: RemoteStepsInput): RemoteStep[] {
     },
   ];
 }
+
+export type InstallOutcomeInput = {
+  exitCode: number;
+  stderr: string;
+  /** BUILD_SHA read back from the installed package AFTER the install ran. */
+  installedSha: string | null | undefined;
+  /** BUILD_SHA of the artifact being shipped. */
+  expectedSha: string;
+};
+
+export type InstallOutcome = {
+  installed: boolean;
+  kind: "installed" | "installed-with-cleanup-failure" | "failed";
+  reason: string;
+};
+
+/**
+ * Decide whether the global install actually landed.
+ *
+ * The exit code alone is not trustworthy in either direction:
+ *
+ *  - `npm install -g` on the .17 gateway writes the whole package and *then* fails renaming
+ *    the old tree into its staging directory (`ENOTEMPTY`, exit 217). Treating that as a
+ *    failure aborts the deploy after the artifact is already on disk — which happened twice
+ *    on 2026-08-18, each time leaving the host with new files and an old running process.
+ *  - The 2026-08-14 outage went the other way: the install exited 0 while shipping a package
+ *    built from the wrong branch.
+ *
+ * So the SHA on disk decides, and it must match exactly. An absent or unreadable SHA fails
+ * closed — an artifact that cannot be identified is never attested (same rule as the
+ * provenance gate).
+ */
+export function classifyInstallOutcome(input: InstallOutcomeInput): InstallOutcome {
+  const { exitCode, stderr, installedSha, expectedSha } = input;
+  const onDisk = (installedSha ?? "").trim();
+
+  if (!onDisk) {
+    return {
+      installed: false,
+      kind: "failed",
+      reason: "no BUILD_SHA could be read from the installed package after the install",
+    };
+  }
+  if (onDisk !== expectedSha) {
+    return {
+      installed: false,
+      kind: "failed",
+      reason: `installed BUILD_SHA is ${onDisk}, expected ${expectedSha}`,
+    };
+  }
+  if (exitCode === 0) {
+    return { installed: true, kind: "installed", reason: `installed ${onDisk}` };
+  }
+
+  const staging = orphanStagingDirFromStderr(stderr);
+  const enotempty = /ENOTEMPTY/.test(stderr);
+  return {
+    installed: true,
+    kind: "installed-with-cleanup-failure",
+    reason:
+      `npm exited ${exitCode} but ${onDisk} is on disk — the package installed and npm failed ` +
+      `during its own cleanup${enotempty ? " (ENOTEMPTY on the staging rename)" : ""}` +
+      (staging ? `; orphaned staging dir left behind: ${staging}` : ""),
+  };
+}
+
+/**
+ * The staging directory npm failed to rename into, if it named one. It blocks the NEXT
+ * install with the same error (npm reuses the name), so the operator has to clear it —
+ * surfacing the exact path is the whole point. Deliberately not removed automatically:
+ * this is a path under /usr/lib and a blind `rm -rf` there is not something a deploy
+ * script should do on its own.
+ */
+export function orphanStagingDirFromStderr(stderr: string): string | null {
+  const match = /npm error dest (\/\S*\/\.\S+)/.exec(stderr || "");
+  return match ? match[1] : null;
+}

src/app/(dashboard)/dashboard/batch/batch-utils.ts

@@ -1,5 +1,5 @@
-import { BatchRecord } from "@/lib/db/batches";
-import { FileRecord } from "@/lib/db/files";
+import type { BatchRecord } from "@/lib/db/batches";
+import type { FileRecord } from "@/lib/db/files";
 
 export function mapBatchApiToRecord(b: any): BatchRecord {
   return {

src/app/(dashboard)/dashboard/batch/files/page.tsx

@@ -6,8 +6,8 @@ import FilesListTab from "../FilesListTab";
 import FilesConceptCard from "../components/FilesConceptCard";
 import UploadFileModal from "../components/UploadFileModal";
 import { mapFileApiToRecord, mapBatchApiToRecord } from "../batch-utils";
-import { FileRecord } from "@/lib/db/files";
-import { BatchRecord } from "@/lib/db/batches";
+import type { FileRecord } from "@/lib/db/files";
+import type { BatchRecord } from "@/lib/db/batches";
 
 export default function BatchFilesPage() {
   const t = useTranslations("common");

src/app/(dashboard)/dashboard/batch/page.tsx

@@ -3,8 +3,8 @@
 import { useState, useEffect, useCallback, useRef } from "react";
 import { useTranslations } from "next-intl";
 import BatchListTab from "./BatchListTab";
-import { FileRecord } from "@/lib/db/files";
-import { BatchRecord } from "@/lib/db/batches";
+import type { FileRecord } from "@/lib/db/files";
+import type { BatchRecord } from "@/lib/db/batches";
 import { mapBatchApiToRecord, mapFileApiToRecord } from "./batch-utils";
 import BatchConceptCard from "./components/BatchConceptCard";
 import NewBatchWizard from "./components/NewBatchWizard";

tests/unit/canary-install-outcome-10429.test.ts

@@ -0,0 +1,106 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import {
+  classifyInstallOutcome,
+  orphanStagingDirFromStderr,
+} from "../../scripts/ops/deployCanary.ts";
+
+/**
+ * `npm install -g <tarball>` on the .17 gateway finishes writing the package and then fails
+ * while renaming the old tree into its staging directory:
+ *
+ *   npm error code ENOTEMPTY
+ *   npm error syscall rename
+ *   npm error path /usr/lib/node_modules/omniroute
+ *   npm error dest /usr/lib/node_modules/.omniroute-h797OOZa
+ *
+ * Exit status is 217, but `dist/BUILD_SHA`, the package version and every dependency are the
+ * new ones. The canary treated the non-zero exit as "install failed", aborted before the
+ * restart, and discarded npm's stderr — so the deploy stopped half-done twice (2026-08-18)
+ * with no clue in the log about why.
+ *
+ * The exit code is not the source of truth here; the SHA on disk is. These cases pin that,
+ * including the inverse trap: a ZERO exit that installed the wrong artifact must still fail.
+ */
+
+const ENOTEMPTY_STDERR = [
+  "npm warn deprecated boolean@3.2.0: Package no longer supported.",
+  "npm error code ENOTEMPTY",
+  "npm error syscall rename",
+  "npm error path /usr/lib/node_modules/omniroute",
+  "npm error dest /usr/lib/node_modules/.omniroute-h797OOZa",
+  "npm error ENOTEMPTY: directory not empty, rename '/usr/lib/node_modules/omniroute' -> " +
+    "'/usr/lib/node_modules/.omniroute-h797OOZa'",
+].join("\n");
+
+test("non-zero exit with the expected SHA on disk is a cleanup failure, not an install failure", () => {
+  const outcome = classifyInstallOutcome({
+    exitCode: 217,
+    stderr: ENOTEMPTY_STDERR,
+    installedSha: "22b89a273b",
+    expectedSha: "22b89a273b",
+  });
+  assert.equal(outcome.installed, true, "the artifact is on disk — the deploy must continue");
+  assert.equal(outcome.kind, "installed-with-cleanup-failure");
+  assert.match(outcome.reason, /ENOTEMPTY|cleanup/i);
+});
+
+test("a clean install is reported as such", () => {
+  const outcome = classifyInstallOutcome({
+    exitCode: 0,
+    stderr: "",
+    installedSha: "22b89a273b",
+    expectedSha: "22b89a273b",
+  });
+  assert.equal(outcome.installed, true);
+  assert.equal(outcome.kind, "installed");
+});
+
+test("non-zero exit with a stale SHA is a real failure", () => {
+  const outcome = classifyInstallOutcome({
+    exitCode: 217,
+    stderr: ENOTEMPTY_STDERR,
+    installedSha: "e05ac345da",
+    expectedSha: "22b89a273b",
+  });
+  assert.equal(outcome.installed, false);
+  assert.equal(outcome.kind, "failed");
+});
+
+test("a ZERO exit that left the wrong artifact still fails", () => {
+  // The 2026-08-14 outage shipped a package built from the wrong branch. An install that
+  // "succeeds" while the SHA does not match must never be waved through.
+  const outcome = classifyInstallOutcome({
+    exitCode: 0,
+    stderr: "",
+    installedSha: "178febc50f",
+    expectedSha: "22b89a273b",
+  });
+  assert.equal(outcome.installed, false);
+  assert.equal(outcome.kind, "failed");
+});
+
+test("an unreadable SHA fails closed", () => {
+  for (const installedSha of ["", null, undefined]) {
+    const outcome = classifyInstallOutcome({
+      exitCode: 0,
+      stderr: "",
+      installedSha: installedSha as string | null,
+      expectedSha: "22b89a273b",
+    });
+    assert.equal(outcome.installed, false, `installedSha=${JSON.stringify(installedSha)}`);
+    assert.equal(outcome.kind, "failed");
+  }
+});
+
+test("the orphaned staging directory is extracted so the operator can clear it", () => {
+  assert.equal(
+    orphanStagingDirFromStderr(ENOTEMPTY_STDERR),
+    "/usr/lib/node_modules/.omniroute-h797OOZa"
+  );
+});
+
+test("no staging directory is invented when npm did not report one", () => {
+  assert.equal(orphanStagingDirFromStderr(""), null);
+  assert.equal(orphanStagingDirFromStderr("npm error code EACCES"), null);
+});

tests/unit/client-bundle-no-server-only-10692.test.ts

@@ -5,38 +5,52 @@ import path from "node:path";
 import { fileURLToPath } from "node:url";
 
 /**
- * #10692: `src/app/(dashboard)/dashboard/providers/page.tsx` is a `"use client"` page.
- * Through `serviceKindIndex → mediaServiceKinds → imageRegistry → aihorde/imageModels →
- * aihordeImageCatalog → safeOutboundFetch → proxyFetch → featureFlags → db/core` it reached
- * the SQLite driver, so the production build tried to bundle `fs`/`net`/`tls` for the browser
- * and failed with 28 `Module not found` errors (`Build App` red for 60 consecutive runs).
+ * #10692: a `"use client"` page reached the SQLite driver through
+ * `serviceKindIndex → mediaServiceKinds → imageRegistry → aihorde/imageModels →
+ * aihordeImageCatalog → safeOutboundFetch → proxyFetch → featureFlags → db/core`, so the
+ * production build tried to bundle `fs`/`net`/`tls` for the browser and failed with 28
+ * `Module not found` errors (`Build App` red for 60 consecutive runs).
  *
- * `serviceKindIndex.ts` already documents the invariant this guard enforces:
- *   "Client-safe: `mediaServiceKinds` only pulls in the pure-data media registries
- *    (no server-only deps)."
+ * `serviceKindIndex.ts` had stated the invariant in a comment — *"Client-safe:
+ * `mediaServiceKinds` only pulls in the pure-data media registries (no server-only deps)"* —
+ * and a comment cannot fail a build, so #10542 broke it unnoticed.
  *
- * That was a comment, so nothing stopped #10542 from breaking it. This walks the real
- * static-import graph instead — the same edges the bundler follows. Dynamic `import()` is
- * deliberately NOT followed: deferring a server-only module behind one is exactly how the
- * leak is fixed, and the bundler splits it into a chunk the browser never loads.
+ * This walks the real static-import graph, the same edges the bundler follows, from EVERY
+ * `"use client"` file in the repo rather than a hand-picked pair.
+ *
+ * Two deliberate exclusions, both load-bearing:
+ *
+ *  - **`import type` is not an edge.** TypeScript erases it before the bundler sees it. A scan
+ *    that counts type imports reports 26 phantom leaks against 2 real ones here — a guard that
+ *    cries wolf gets switched off.
+ *  - **Dynamic `import()` is not followed.** It does not actually break a bundle edge (that was
+ *    tried for #10692 and failed), but it does move the module into a chunk the browser only
+ *    fetches on demand, which is a legitimate boundary for a lazily-used server path.
  */
 const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
 
-/** Entry points that end up in a client bundle and must stay free of server-only code. */
-const CLIENT_SAFE_ENTRIES = [
+/** Modules that pull in Node builtins (fs/net/tls) and must never be statically reachable. */
+const SERVER_ONLY = new Set([
+  "src/lib/db/core.ts",
+  "src/lib/db/adapters/driverFactory.ts",
+  "src/lib/db/adapters/sqljsAdapter.ts",
+  "src/lib/db/migrationRunner.ts",
+  "open-sse/utils/proxyFetch.ts",
+  "open-sse/utils/tlsClient.ts",
+]);
+
+/**
+ * Non-`"use client"` entry points that still end up in a client bundle because client
+ * components import them. Kept explicit so the original #10692 chain stays pinned even if the
+ * page that exposed it is refactored.
+ */
+const EXTRA_ENTRIES = [
   "src/lib/providers/serviceKindIndex.ts",
   "open-sse/config/mediaServiceKinds.ts",
 ];
 
-/** Modules that pull in Node builtins (fs/net/tls) and must never be statically reachable. */
-const SERVER_ONLY = [
-  "src/lib/db/core.ts",
-  "src/lib/db/adapters/driverFactory.ts",
-  "src/lib/db/adapters/sqljsAdapter.ts",
-  "open-sse/utils/proxyFetch.ts",
-];
-
 const EXTENSIONS = [".ts", ".tsx", ".mts", ".js"];
+const SKIP_DIRS = new Set(["node_modules", ".git", ".build", "dist", ".next", ".claude"]);
 
 /** Resolve an import specifier to a repo-relative file, or null when it leaves the repo. */
 function resolveSpecifier(fromFile: string, specifier: string): string | null {
@@ -68,53 +82,105 @@ function resolveSpecifier(fromFile: string, specifier: string): string | null {
   return null;
 }
 
-/** Static import/export specifiers only — `import(...)` expressions are intentionally skipped. */
+/** True when the import clause contributes no runtime binding (pure `import type`). */
+function isTypeOnlyClause(clause: string): boolean {
+  if (/^\s*type\s/.test(clause)) return true;
+  const named = /\{([^}]*)\}/.exec(clause);
+  if (!named) return false;
+  // `import Default, { type A }` still emits an edge for the default binding.
+  const outsideBraces = clause.replace(/\{[^}]*\}/, "").trim();
+  if (/[A-Za-z_$*]/.test(outsideBraces)) return false;
+  const bindings = named[1]
+    .split(",")
+    .map((binding) => binding.trim())
+    .filter(Boolean);
+  return bindings.length > 0 && bindings.every((binding) => /^type\s/.test(binding));
+}
+
+/** Value-carrying static specifiers only. */
 function staticSpecifiers(source: string): string[] {
   const withoutDynamic = source.replace(/\bimport\s*\(/g, "__dynamic_import__(");
   const out: string[] = [];
-  const patterns = [
-    /(?:^|\n)\s*import\s+[^;'"]*from\s*["']([^"']+)["']/g,
-    /(?:^|\n)\s*import\s*["']([^"']+)["']/g,
-    /(?:^|\n)\s*export\s+[^;'"]*from\s*["']([^"']+)["']/g,
-  ];
-  for (const pattern of patterns) {
-    for (const match of withoutDynamic.matchAll(pattern)) out.push(match[1]);
+  for (const pattern of [
+    /(?:^|\n)\s*import\s+([^;'"]*)from\s*["']([^"']+)["']/g,
+    /(?:^|\n)\s*export\s+([^;'"]*)from\s*["']([^"']+)["']/g,
+  ]) {
+    for (const match of withoutDynamic.matchAll(pattern)) {
+      if (isTypeOnlyClause(match[1])) continue;
+      out.push(match[2]);
+    }
+  }
+  // Side-effect imports (`import "./x"`) always emit an edge.
+  for (const match of withoutDynamic.matchAll(/(?:^|\n)\s*import\s*["']([^"']+)["']/g)) {
+    out.push(match[1]);
   }
   return out;
 }
 
+const specifierCache = new Map<string, string[]>();
+function edgesOf(file: string): string[] {
+  const cached = specifierCache.get(file);
+  if (cached) return cached;
+  const absolute = path.join(REPO_ROOT, file);
+  let edges: string[] = [];
+  if (fs.existsSync(absolute)) {
+    edges = staticSpecifiers(fs.readFileSync(absolute, "utf8"))
+      .map((specifier) => resolveSpecifier(file, specifier))
+      .filter((resolved): resolved is string => resolved !== null);
+  }
+  specifierCache.set(file, edges);
+  return edges;
+}
+
 /** BFS over static imports; returns the first path reaching a server-only module. */
 function findServerOnlyPath(entry: string): string[] | null {
   const seen = new Set<string>([entry]);
   const queue: Array<string[]> = [[entry]];
   while (queue.length > 0) {
     const trail = queue.shift()!;
-    const current = trail[trail.length - 1];
-    const absolute = path.join(REPO_ROOT, current);
-    if (!fs.existsSync(absolute)) continue;
-
-    for (const specifier of staticSpecifiers(fs.readFileSync(absolute, "utf8"))) {
-      const resolved = resolveSpecifier(current, specifier);
-      if (!resolved || seen.has(resolved)) continue;
-      const next = [...trail, resolved];
-      if (SERVER_ONLY.includes(resolved)) return next;
+    for (const resolved of edgesOf(trail[trail.length - 1])) {
+      if (seen.has(resolved)) continue;
+      if (SERVER_ONLY.has(resolved)) return [...trail, resolved];
       seen.add(resolved);
-      queue.push(next);
+      queue.push([...trail, resolved]);
     }
   }
   return null;
 }
 
-for (const entry of CLIENT_SAFE_ENTRIES) {
-  test(`${entry} does not statically reach server-only code`, () => {
-    const trail = findServerOnlyPath(entry);
-    assert.equal(
-      trail,
-      null,
-      trail
-        ? `A client bundle would have to include a server-only module. Static import chain:\n  ${trail.join("\n  → ")}\n` +
-            `Break the chain (a dynamic import at the boundary is enough) rather than widening this guard.`
-        : ""
-    );
-  });
+function walk(dir: string, acc: string[] = []): string[] {
+  for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) {
+      if (SKIP_DIRS.has(entry.name)) continue;
+      walk(full, acc);
+    } else if (/\.tsx?$/.test(entry.name)) {
+      acc.push(path.relative(REPO_ROOT, full));
+    }
+  }
+  return acc;
 }
+
+function clientEntryPoints(): string[] {
+  return walk(path.join(REPO_ROOT, "src")).filter((file) =>
+    /^\s*["']use client["']/m.test(fs.readFileSync(path.join(REPO_ROOT, file), "utf8").slice(0, 200))
+  );
+}
+
+test("no client entry point statically reaches server-only code", () => {
+  const entries = [...clientEntryPoints(), ...EXTRA_ENTRIES];
+  assert.ok(entries.length > 100, `expected the repo's client components, found ${entries.length}`);
+
+  const offenders = entries
+    .map((entry) => ({ entry, trail: findServerOnlyPath(entry) }))
+    .filter((row): row is { entry: string; trail: string[] } => row.trail !== null);
+
+  assert.deepEqual(
+    offenders.map((o) => o.entry),
+    [],
+    "A client bundle would have to include server-only modules:\n" +
+      offenders.map((o) => `  ${o.trail.join("\n    → ")}`).join("\n\n") +
+      "\nBreak the chain — or, when the binding is only a type, mark it `import type` so it " +
+      "carries no runtime edge."
+  );
+});