refactor: tighten final boundary guardrails

2026-04-26 07:57:40 +00:00 · 2026-04-05 21:13:22 +01:00
parent 714ba48a6f
commit 8206328a94
10 changed files with 280 additions and 170 deletions
--- a/src/plugins/bundled-plugin-metadata.ts
+++ b/src/plugins/bundled-plugin-metadata.ts
@@ -4,6 +4,12 @@ import { fileURLToPath } from "node:url";
 import { createJiti } from "jiti";
 import { buildChannelConfigSchema } from "../channels/plugins/config-schema.js";
 import type { ChannelConfigRuntimeSchema } from "../channels/plugins/types.plugin.js";
+import {
+  collectBundledPluginPublicSurfaceArtifacts,
+  collectBundledPluginRuntimeSidecarArtifacts,
+  deriveBundledPluginIdHint,
+  resolveBundledPluginScanDir,
+} from "./bundled-plugin-scan.js";
 import {
  getPackageManifestMetadata,
  loadPluginManifest,
@@ -30,12 +36,6 @@ const RUNNING_FROM_BUILT_ARTIFACT =
  CURRENT_MODULE_PATH.includes(`${path.sep}dist${path.sep}`) ||
  CURRENT_MODULE_PATH.includes(`${path.sep}dist-runtime${path.sep}`);
 const PUBLIC_SURFACE_SOURCE_EXTENSIONS = [".ts", ".mts", ".js", ".mjs", ".cts", ".cjs"] as const;
-const RUNTIME_SIDECAR_ARTIFACTS = new Set([
-  "helper-api.js",
-  "light-runtime-api.js",
-  "runtime-api.js",
-  "thread-bindings-runtime.js",
-]);
 const SOURCE_CONFIG_SCHEMA_CANDIDATES = [
  path.join("src", "config-schema.ts"),
  path.join("src", "config-schema.js"),
@@ -109,110 +109,6 @@ function readPackageManifest(pluginDir: string): PackageManifest | undefined {
  }
 }

-function deriveIdHint(params: {
-  entryPath: string;
-  manifestId: string;
-  packageName?: string;
-  hasMultipleExtensions: boolean;
-}): string {
-  const base = path.basename(params.entryPath, path.extname(params.entryPath));
-  if (!params.hasMultipleExtensions) {
-    return params.manifestId;
-  }
-  const packageName = trimString(params.packageName);
-  if (!packageName) {
-    return `${params.manifestId}/${base}`;
-  }
-  const unscoped = packageName.includes("/")
-    ? (packageName.split("/").pop() ?? packageName)
-    : packageName;
-  return `${unscoped}/${base}`;
-}
-
-function isTopLevelPublicSurfaceSource(name: string): boolean {
-  if (
-    !PUBLIC_SURFACE_SOURCE_EXTENSIONS.includes(
-      path.extname(name) as (typeof PUBLIC_SURFACE_SOURCE_EXTENSIONS)[number],
-    )
-  ) {
-    return false;
-  }
-  if (name.startsWith(".")) {
-    return false;
-  }
-  if (name.startsWith("test-")) {
-    return false;
-  }
-  if (name.includes(".test-")) {
-    return false;
-  }
-  if (name.endsWith(".d.ts")) {
-    return false;
-  }
-  if (/^config-api(\.[cm]?[jt]s)$/u.test(name)) {
-    return false;
-  }
-  return !/(\.test|\.spec)(\.[cm]?[jt]s)$/u.test(name);
-}
-
-function collectTopLevelPublicSurfaceArtifacts(params: {
-  pluginDir: string;
-  sourceEntry: string;
-  setupEntry?: string;
-}): readonly string[] | undefined {
-  const excluded = new Set(
-    [params.sourceEntry, params.setupEntry]
-      .filter((entry): entry is string => typeof entry === "string" && entry.trim().length > 0)
-      .map((entry) => path.basename(entry)),
-  );
-  const artifacts = fs
-    .readdirSync(params.pluginDir, { withFileTypes: true })
-    .filter((entry) => entry.isFile())
-    .map((entry) => entry.name)
-    .filter(isTopLevelPublicSurfaceSource)
-    .filter((entry) => !excluded.has(entry))
-    .map((entry) => rewriteEntryToBuiltPath(entry))
-    .filter((entry): entry is string => typeof entry === "string" && entry.length > 0)
-    .toSorted((left, right) => left.localeCompare(right));
-  return artifacts.length > 0 ? artifacts : undefined;
-}
-
-function collectRuntimeSidecarArtifacts(
-  publicSurfaceArtifacts: readonly string[] | undefined,
-): readonly string[] | undefined {
-  if (!publicSurfaceArtifacts) {
-    return undefined;
-  }
-  const artifacts = publicSurfaceArtifacts.filter((artifact) =>
-    RUNTIME_SIDECAR_ARTIFACTS.has(artifact),
-  );
-  return artifacts.length > 0 ? artifacts : undefined;
-}
-
-function resolveBundledPluginScanDir(packageRoot: string): string | undefined {
-  const sourceDir = path.join(packageRoot, "extensions");
-  const runtimeDir = path.join(packageRoot, "dist-runtime", "extensions");
-  const builtDir = path.join(packageRoot, "dist", "extensions");
-  if (RUNNING_FROM_BUILT_ARTIFACT) {
-    if (fs.existsSync(builtDir)) {
-      return builtDir;
-    }
-    if (fs.existsSync(runtimeDir)) {
-      return runtimeDir;
-    }
-  }
-  if (fs.existsSync(sourceDir)) {
-    return sourceDir;
-  }
-  if (fs.existsSync(runtimeDir) && fs.existsSync(builtDir)) {
-    return runtimeDir;
-  }
-  if (fs.existsSync(builtDir)) {
-    return builtDir;
-  }
-  return undefined;
-}
-
 function isBuiltChannelConfigSchema(value: unknown): value is ChannelConfigSurface {
  if (!value || typeof value !== "object") {
    return false;
@@ -369,7 +265,10 @@ function collectBundledPluginMetadataForPackageRoot(
  includeChannelConfigs: boolean,
  includeSyntheticChannelConfigs: boolean,
 ): readonly BundledPluginMetadata[] {
-  const scanDir = resolveBundledPluginScanDir(packageRoot);
+  const scanDir = resolveBundledPluginScanDir({
+    packageRoot,
+    runningFromBuiltArtifact: RUNNING_FROM_BUILT_ARTIFACT,
+  });
  if (!scanDir || !fs.existsSync(scanDir)) {
    return [];
  }
@@ -406,12 +305,13 @@ function collectBundledPluginMetadataForPackageRoot(
            built: rewriteEntryToBuiltPath(setupSourcePath)!,
          }
        : undefined;
-    const publicSurfaceArtifacts = collectTopLevelPublicSurfaceArtifacts({
+    const publicSurfaceArtifacts = collectBundledPluginPublicSurfaceArtifacts({
      pluginDir,
      sourceEntry,
      ...(setupSourcePath ? { setupEntry: setupSourcePath } : {}),
    });
-    const runtimeSidecarArtifacts = collectRuntimeSidecarArtifacts(publicSurfaceArtifacts);
+    const runtimeSidecarArtifacts =
+      collectBundledPluginRuntimeSidecarArtifacts(publicSurfaceArtifacts);
    const channelConfigs =
      includeChannelConfigs && includeSyntheticChannelConfigs
        ? collectBundledChannelConfigs({
@@ -423,7 +323,7 @@ function collectBundledPluginMetadataForPackageRoot(

    entries.push({
      dirName,
-      idHint: deriveIdHint({
+      idHint: deriveBundledPluginIdHint({
        entryPath: sourceEntry,
        manifestId: manifestResult.manifest.id,
        packageName: trimString(packageJson?.name),
--- a/src/plugins/bundled-plugin-scan.ts
+++ b/src/plugins/bundled-plugin-scan.ts
@@ -0,0 +1,123 @@
+import fs from "node:fs";
+import path from "node:path";
+
+const PUBLIC_SURFACE_SOURCE_EXTENSIONS = [".ts", ".mts", ".js", ".mjs", ".cts", ".cjs"] as const;
+const RUNTIME_SIDECAR_ARTIFACTS = new Set([
+  "helper-api.js",
+  "light-runtime-api.js",
+  "runtime-api.js",
+  "thread-bindings-runtime.js",
+]);
+
+function trimString(value: unknown): string | undefined {
+  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
+}
+
+function rewriteEntryToBuiltPath(entry: string | undefined): string | undefined {
+  if (!entry) {
+    return undefined;
+  }
+  const normalized = entry.replace(/^\.\//u, "");
+  return normalized.replace(/\.[^.]+$/u, ".js");
+}
+
+function isTopLevelPublicSurfaceSource(name: string): boolean {
+  if (
+    !PUBLIC_SURFACE_SOURCE_EXTENSIONS.includes(
+      path.extname(name) as (typeof PUBLIC_SURFACE_SOURCE_EXTENSIONS)[number],
+    )
+  ) {
+    return false;
+  }
+  if (name.startsWith(".") || name.startsWith("test-") || name.includes(".test-")) {
+    return false;
+  }
+  if (name.endsWith(".d.ts")) {
+    return false;
+  }
+  if (/^config-api(\.[cm]?[jt]s)$/u.test(name)) {
+    return false;
+  }
+  return !/(\.test|\.spec)(\.[cm]?[jt]s)$/u.test(name);
+}
+
+export function deriveBundledPluginIdHint(params: {
+  entryPath: string;
+  manifestId: string;
+  packageName?: string;
+  hasMultipleExtensions: boolean;
+}): string {
+  const base = path.basename(params.entryPath, path.extname(params.entryPath));
+  if (!params.hasMultipleExtensions) {
+    return params.manifestId;
+  }
+  const packageName = trimString(params.packageName);
+  if (!packageName) {
+    return `${params.manifestId}/${base}`;
+  }
+  const unscoped = packageName.includes("/")
+    ? (packageName.split("/").pop() ?? packageName)
+    : packageName;
+  return `${unscoped}/${base}`;
+}
+
+export function collectBundledPluginPublicSurfaceArtifacts(params: {
+  pluginDir: string;
+  sourceEntry: string;
+  setupEntry?: string;
+}): readonly string[] | undefined {
+  const excluded = new Set(
+    [params.sourceEntry, params.setupEntry]
+      .filter((entry): entry is string => typeof entry === "string" && entry.trim().length > 0)
+      .map((entry) => path.basename(entry)),
+  );
+  const artifacts = fs
+    .readdirSync(params.pluginDir, { withFileTypes: true })
+    .filter((entry) => entry.isFile())
+    .map((entry) => entry.name)
+    .filter(isTopLevelPublicSurfaceSource)
+    .filter((entry) => !excluded.has(entry))
+    .map((entry) => rewriteEntryToBuiltPath(entry))
+    .filter((entry): entry is string => typeof entry === "string" && entry.length > 0)
+    .toSorted((left, right) => left.localeCompare(right));
+  return artifacts.length > 0 ? artifacts : undefined;
+}
+
+export function collectBundledPluginRuntimeSidecarArtifacts(
+  publicSurfaceArtifacts: readonly string[] | undefined,
+): readonly string[] | undefined {
+  if (!publicSurfaceArtifacts) {
+    return undefined;
+  }
+  const artifacts = publicSurfaceArtifacts.filter((artifact) =>
+    RUNTIME_SIDECAR_ARTIFACTS.has(artifact),
+  );
+  return artifacts.length > 0 ? artifacts : undefined;
+}
+
+export function resolveBundledPluginScanDir(params: {
+  packageRoot: string;
+  runningFromBuiltArtifact: boolean;
+}): string | undefined {
+  const sourceDir = path.join(params.packageRoot, "extensions");
+  const runtimeDir = path.join(params.packageRoot, "dist-runtime", "extensions");
+  const builtDir = path.join(params.packageRoot, "dist", "extensions");
+  if (params.runningFromBuiltArtifact) {
+    if (fs.existsSync(builtDir)) {
+      return builtDir;
+    }
+    if (fs.existsSync(runtimeDir)) {
+      return runtimeDir;
+    }
+  }
+  if (fs.existsSync(sourceDir)) {
+    return sourceDir;
+  }
+  if (fs.existsSync(runtimeDir) && fs.existsSync(builtDir)) {
+    return runtimeDir;
+  }
+  if (fs.existsSync(builtDir)) {
+    return builtDir;
+  }
+  return undefined;
+}
--- a/src/plugins/contracts/boundary-invariants.test.ts
+++ b/src/plugins/contracts/boundary-invariants.test.ts
@@ -23,6 +23,10 @@ const ALLOWED_CONTRACT_BUNDLED_PATH_HELPERS = new Set([
  "src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts",
 ]);

+const ALLOWED_CHANNEL_BUNDLED_METADATA_CONSUMERS = new Set([
+  "src/channels/plugins/session-conversation.bundled-fallback.test.ts",
+]);
+
 describe("plugin contract boundary invariants", () => {
  it("keeps bundled-capability-metadata confined to contract/test inventory", async () => {
    const { globSync } = await import("glob");
@@ -89,4 +93,35 @@ describe("plugin contract boundary invariants", () => {
    });
    expect(offenders).toEqual([]);
  });
+
+  it("keeps channel production code off bundled-plugin-metadata helpers", async () => {
+    const { globSync } = await import("glob");
+    const files = globSync("src/channels/**/*.ts", {
+      cwd: REPO_ROOT,
+      nodir: true,
+      ignore: ["src/channels/**/*.test.ts"],
+    });
+    const offenders = files.filter((file) => {
+      if (ALLOWED_CHANNEL_BUNDLED_METADATA_CONSUMERS.has(file)) {
+        return false;
+      }
+      const source = readFileSync(resolve(REPO_ROOT, file), "utf8");
+      return source.includes("plugins/bundled-plugin-metadata");
+    });
+    expect(offenders).toEqual([]);
+  });
+
+  it("keeps contract loaders off hand-built bundled extension paths", async () => {
+    const { globSync } = await import("glob");
+    const files = globSync("src/{plugins,channels}/**/*.ts", {
+      cwd: REPO_ROOT,
+      nodir: true,
+      ignore: ["src/**/*.test.ts"],
+    });
+    const offenders = files.filter((file) => {
+      const source = readFileSync(resolve(REPO_ROOT, file), "utf8");
+      return /extensions\/\$\{|\.\.\/\.\.\/\.\.\/\.\.\/extensions\//u.test(source);
+    });
+    expect(offenders).toEqual([]);
+  });
 });
--- a/src/plugins/manifest-registry.ts
+++ b/src/plugins/manifest-registry.ts
@@ -30,6 +30,12 @@ import type {
 } from "./types.js";

 type PluginManifestContractListKey =
+  | "speechProviders"
+  | "mediaUnderstandingProviders"
+  | "realtimeVoiceProviders"
+  | "realtimeTranscriptionProviders"
+  | "imageGenerationProviders"
+  | "videoGenerationProviders"
  | "memoryEmbeddingProviders"
  | "webFetchProviders"
  | "webSearchProviders";