fix(cli,templates): make a scaffolded plugin installable and buildable

apps/cli/src/__tests__/plugin-new.test.ts

@@ -1,5 +1,12 @@
-import { describe, expect, it } from "vitest";
-import { resolveNewPluginTarget } from "../commands/plugin.js";
+import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { delimiter, join } from "node:path";
+import { Command } from "commander";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+  registerPluginCommands,
+  resolveNewPluginTarget,
+} from "../commands/plugin.js";
 
 describe("resolveNewPluginTarget", () => {
   it.each([
@@ -23,3 +30,152 @@ describe("resolveNewPluginTarget", () => {
     expect(resolveNewPluginTarget(name)).toBeNull();
   });
 });
+
+/**
+ * `bb plugin new` runs npm itself, and the packaged CLI runs with
+ * NODE_ENV=production (bb-app's launcher sets it), which npm reads as
+ * `omit=dev`. Issue #1133: npm skipped the packages the scaffold needs, exited
+ * 0, and the CLI reported success for a plugin that could not build.
+ *
+ * The fake npm reproduces npm's actual config rule rather than recording
+ * arguments, so these pin the outcome — the scaffold's declared tree is on
+ * disk, and the CLI only claims success when it is — instead of a flag string
+ * the CLI happens to pass today.
+ */
+const FAKE_NPM = `#!/usr/bin/env node
+const { mkdirSync, readFileSync } = require("node:fs");
+const { join } = require("node:path");
+
+const args = process.argv.slice(2);
+// npm treats NODE_ENV=production as omit=dev; a command-line --include=dev
+// outranks it. BB_TEST_NPM_ALWAYS_OMIT_DEV forces the omission to stand in for
+// an install that silently drops packages.
+const omitDev =
+  process.env.BB_TEST_NPM_ALWAYS_OMIT_DEV === "1" ||
+  (process.env.NODE_ENV === "production" && !args.includes("--include=dev"));
+const manifest = JSON.parse(readFileSync(join(process.cwd(), "package.json"), "utf8"));
+const installed = {
+  ...manifest.dependencies,
+  ...(omitDev ? {} : manifest.devDependencies),
+};
+// npm installs the whole workspace and hoists to its root when the package is
+// a workspace member; BB_TEST_NPM_HOIST_TO stands in for that root.
+const installRoot = process.env.BB_TEST_NPM_HOIST_TO ?? process.cwd();
+for (const name of Object.keys(installed)) {
+  mkdirSync(join(installRoot, "node_modules", ...name.split("/")), {
+    recursive: true,
+  });
+}
+`;
+
+describe.sequential("bb plugin new dependency install", () => {
+  const originalCwd = process.cwd();
+  let workDir: string;
+  let logged: string[];
+  let warned: string[];
+
+  beforeEach(async () => {
+    workDir = await mkdtemp(join(tmpdir(), "bb-plugin-new-"));
+    const binDir = join(workDir, "bin");
+    await mkdir(binDir);
+    await writeFile(join(binDir, "npm"), FAKE_NPM, { mode: 0o755 });
+    process.chdir(workDir);
+    // Only the fake npm is reachable, so a real npm can never service these.
+    vi.stubEnv("PATH", `${binDir}${delimiter}${process.env.PATH ?? ""}`);
+    vi.stubEnv("NODE_ENV", "production");
+    logged = [];
+    warned = [];
+    vi.spyOn(console, "log").mockImplementation((line: unknown) => {
+      logged.push(String(line));
+    });
+    vi.spyOn(console, "warn").mockImplementation((line: unknown) => {
+      warned.push(String(line));
+    });
+  });
+
+  afterEach(async () => {
+    process.chdir(originalCwd);
+    vi.unstubAllEnvs();
+    vi.restoreAllMocks();
+    await rm(workDir, { recursive: true, force: true });
+  });
+
+  async function runPluginNew(args: string[]): Promise<void> {
+    const program = new Command();
+    program.exitOverride();
+    registerPluginCommands(program, () => "http://localhost");
+    await program.parseAsync(["node", "bb", "plugin", "new", ...args]);
+  }
+
+  async function isInstalled(
+    directoryName: string,
+    packageName: string,
+  ): Promise<boolean> {
+    return stat(join(workDir, directoryName, "node_modules", packageName))
+      .then(() => true)
+      .catch(() => false);
+  }
+
+  it("installs the packages the plugin needs to build under NODE_ENV=production", async () => {
+    await runPluginNew(["prod-env", "--app"]);
+
+    // zod is imported by the generated server.ts and inlined by the build;
+    // typescript/@types are what the scaffold typechecks against.
+    expect(await isInstalled("bb-plugin-prod-env", "zod")).toBe(true);
+    expect(await isInstalled("bb-plugin-prod-env", "typescript")).toBe(true);
+    expect(await isInstalled("bb-plugin-prod-env", "clsx")).toBe(true);
+    expect(warned).toEqual([]);
+    expect(logged).toContain("Installed dependencies (npm install).");
+    expect(logged).not.toContain("  npm install --include=dev");
+  });
+
+  it("installs headless scaffolds too, whose server.ts also imports zod", async () => {
+    await runPluginNew(["headless"]);
+
+    expect(await isInstalled("bb-plugin-headless", "zod")).toBe(true);
+    expect(logged).toContain("Installed dependencies (npm install).");
+  });
+
+  it("accepts a tree npm hoisted to a workspace root", async () => {
+    // npm installs the whole workspace and hoists when the scaffold lands
+    // inside one, so the plugin's own node_modules stays empty even though
+    // every package resolves. Warning here would send the author back to an
+    // `npm install` that hoists again.
+    await writeFile(
+      join(workDir, "package.json"),
+      JSON.stringify({ name: "host", private: true, workspaces: ["*"] }),
+    );
+    vi.stubEnv("BB_TEST_NPM_HOIST_TO", workDir);
+
+    await runPluginNew(["hoisted", "--app"]);
+
+    expect(await isInstalled("bb-plugin-hoisted", "zod")).toBe(false);
+    expect(warned).toEqual([]);
+    expect(logged).toContain("Installed dependencies (npm install).");
+  });
+
+  it("does not report success when npm exits 0 without installing the tree", async () => {
+    vi.stubEnv("BB_TEST_NPM_ALWAYS_OMIT_DEV", "1");
+
+    await runPluginNew(["silent-omit", "--app"]);
+
+    expect(await isInstalled("bb-plugin-silent-omit", "typescript")).toBe(
+      false,
+    );
+    expect(logged).not.toContain("Installed dependencies (npm install).");
+    expect(warned.join("\n")).toMatch(
+      /npm install reported success but .*\btypescript\b.* missing from node_modules/,
+    );
+    // The manual step is the only way out, so the next steps must show it.
+    expect(logged).toContain("  npm install --include=dev");
+  });
+
+  it("falls back to the manual step when npm is not on PATH", async () => {
+    vi.stubEnv("PATH", join(workDir, "empty-bin"));
+
+    await runPluginNew(["no-npm"]);
+
+    expect(warned.join("\n")).toContain("Could not run npm install");
+    expect(logged).toContain("  npm install --include=dev");
+  });
+});

apps/cli/src/__tests__/plugin-scaffold-dependencies.test.ts

@@ -0,0 +1,154 @@
+import { mkdtemp, readFile, readdir, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join, relative } from "node:path";
+import {
+  PLUGIN_SERVER_EXTERNALS,
+  RUNTIME_SLOT_BY_SPECIFIER,
+} from "@bb/plugin-build";
+import { scaffoldPlugin } from "@bb/templates/plugin-scaffold";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+
+/**
+ * A scaffold that imports a package it declares as a devDependency is
+ * unbuildable the moment something installs without dev deps — and that is the
+ * common case, not the exotic one: the packaged CLI runs with
+ * NODE_ENV=production (npm reads it as `omit=dev`), and the server installs
+ * git: plugins with an explicit `--omit=dev`. Issue #1133 was exactly this,
+ * with `zod` in devDependencies while `server.ts` imported it.
+ *
+ * The rule is derived from the build's own externals/shim lists rather than
+ * restated here, so adding a shim or an external cannot leave this stale.
+ * Lives in the CLI because `bb plugin new` writes the scaffold and
+ * `bb plugin build` consumes it; @bb/templates cannot depend on
+ * @bb/plugin-build without a workspace cycle.
+ */
+
+const DIRS_WITHOUT_BUNDLED_SOURCE = new Set([
+  "node_modules",
+  "dist",
+  // Bundled SDK declarations, resolved through tsconfig paths — the npm types
+  // they reference are devDependencies by design.
+  "types",
+  "skills",
+]);
+
+async function generatedSourceFiles(rootDir: string): Promise<string[]> {
+  const files: string[] = [];
+  async function walk(dir: string): Promise<void> {
+    for (const entry of await readdir(dir, { withFileTypes: true })) {
+      const entryPath = join(dir, entry.name);
+      if (entry.isDirectory()) {
+        if (!DIRS_WITHOUT_BUNDLED_SOURCE.has(entry.name)) {
+          await walk(entryPath);
+        }
+      } else if (/\.tsx?$/.test(entry.name) && !entry.name.endsWith(".d.ts")) {
+        files.push(entryPath);
+      }
+    }
+  }
+  await walk(rootDir);
+  return files;
+}
+
+/** Bare npm specifiers a generated file imports (not relative, alias, builtin). */
+function importedSpecifiers(source: string): string[] {
+  const specifiers = new Set<string>();
+  for (const match of source.matchAll(/(?:from|import)\s*["']([^"']+)["']/g)) {
+    const specifier = match[1];
+    if (
+      specifier === undefined ||
+      specifier.startsWith(".") ||
+      specifier.startsWith("@/") ||
+      specifier.startsWith("node:")
+    ) {
+      continue;
+    }
+    specifiers.add(specifier);
+  }
+  return [...specifiers];
+}
+
+/** The npm package owning a specifier: `react/jsx-runtime` → `react`. */
+function packageNameOf(specifier: string): string {
+  const segments = specifier.split("/");
+  return specifier.startsWith("@")
+    ? segments.slice(0, 2).join("/")
+    : (segments[0] ?? specifier);
+}
+
+async function scaffoldWithDependencies(args: {
+  workDir: string;
+  app: boolean;
+}): Promise<{ targetDir: string; dependencies: string[] }> {
+  const packageName = `bb-plugin-${args.app ? "app" : "headless"}`;
+  const targetDir = join(args.workDir, packageName);
+  await scaffoldPlugin({
+    targetDir,
+    packageName,
+    bbVersion: "0.9.0",
+    app: args.app,
+  });
+  const manifest: { dependencies?: Record<string, string> } = JSON.parse(
+    await readFile(join(targetDir, "package.json"), "utf8"),
+  );
+  return { targetDir, dependencies: Object.keys(manifest.dependencies ?? {}) };
+}
+
+describe("scaffold dependency classification", () => {
+  let workDir: string;
+
+  beforeEach(async () => {
+    workDir = await mkdtemp(join(tmpdir(), "bb-scaffold-deps-"));
+  });
+
+  afterEach(async () => {
+    await rm(workDir, { recursive: true, force: true });
+  });
+
+  it.each([{ app: false }, { app: true }])(
+    "declares every bundled import as a dependency (app: $app)",
+    async ({ app }) => {
+      const { targetDir, dependencies } = await scaffoldWithDependencies({
+        workDir,
+        app,
+      });
+
+      const misdeclared: string[] = [];
+      for (const file of await generatedSourceFiles(targetDir)) {
+        for (const specifier of importedSpecifiers(
+          await readFile(file, "utf8"),
+        )) {
+          // Swapped for a host runtime shim, or left unresolved for the
+          // loader — either way it is never read from node_modules.
+          if (specifier in RUNTIME_SLOT_BY_SPECIFIER) continue;
+          const packageName = packageNameOf(specifier);
+          if (PLUGIN_SERVER_EXTERNALS.includes(packageName)) continue;
+          if (dependencies.includes(packageName)) continue;
+          misdeclared.push(
+            `${relative(targetDir, file)} imports "${specifier}"`,
+          );
+        }
+      }
+
+      expect(misdeclared).toEqual([]);
+    },
+  );
+
+  it("keeps host-provided packages out of dependencies", async () => {
+    const { dependencies } = await scaffoldWithDependencies({
+      workDir,
+      app: true,
+    });
+
+    // Bundling a shimmed package ships a second copy of a singleton (a second
+    // React means "Invalid hook call"), and bundling an external defeats the
+    // loader alias.
+    expect(
+      dependencies.filter(
+        (name) =>
+          name in RUNTIME_SLOT_BY_SPECIFIER ||
+          PLUGIN_SERVER_EXTERNALS.includes(name),
+      ),
+    ).toEqual([]);
+  });
+});