Warn on malformed plugin manifests and log swallowed load errors (#924)

* Warn on malformed plugin manifests and log swallowed load errors

* Warn on malformed data-only manifests and accept Claude metadata

A broken native manifest was treated as missing, so data-only plugins
inferred kind with no warning. Claude marketplace metadata is name
and description, not a corbits schema, so validating it produced
false id and kind warnings on untrusted loads.

Author: TheGreatAxios

src/plugins/data-only.ts

@@ -1,6 +1,7 @@
 import { readFile } from "node:fs/promises";
 import { basename, join } from "node:path";
-import { parsePluginManifest, type PluginManifest } from "./manifest.js";
+import { type } from "arktype";
+import { PluginManifestSchema, type PluginManifest } from "./manifest.js";
 import type {
   CommandDefinition,
   CommandPlugin,
@@ -25,13 +26,45 @@ export interface DataOnlyPlugin {
   commandPlugin?: CommandPlugin;
 }
 
-async function readManifestJson(dir: string): Promise<PluginManifest | null> {
+function isENOENT(err: unknown): boolean {
+  return (
+    typeof err === "object" &&
+    err !== null &&
+    "code" in err &&
+    (err as { code?: unknown }).code === "ENOENT"
+  );
+}
+
+function errorText(err: unknown): string {
+  return err instanceof Error ? err.message : String(err);
+}
+
+async function readManifestJson(
+  dir: string,
+  onWarning: (msg: string) => void,
+): Promise<PluginManifest | null> {
+  const manifestPath = join(dir, "manifest.json");
+  let raw: string;
   try {
-    const raw = await readFile(join(dir, "manifest.json"), "utf8");
-    return parsePluginManifest(JSON.parse(raw));
-  } catch {
+    raw = await readFile(manifestPath, "utf8");
+  } catch (err) {
+    if (isENOENT(err)) return null;
+    onWarning(`failed to read ${manifestPath}: ${errorText(err)}`);
+    return null;
+  }
+  let parsed: unknown;
+  try {
+    parsed = JSON.parse(raw);
+  } catch (err) {
+    onWarning(`failed to parse ${manifestPath}: ${errorText(err)}`);
+    return null;
+  }
+  const result = PluginManifestSchema(parsed);
+  if (result instanceof type.errors) {
+    onWarning(`invalid plugin manifest at ${manifestPath}: ${result.summary}`);
     return null;
   }
+  return result as PluginManifest;
 }
 
 // Claude Code marketplace plugins self-describe via `.claude-plugin/plugin.json`
@@ -106,7 +139,7 @@ export async function loadDataOnlyPlugin(
 
   const [nativeManifest, claudeManifest, agents, commands, skillCmds] =
     await Promise.all([
-      readManifestJson(pluginDir),
+      readManifestJson(pluginDir, onWarning),
       readClaudePluginManifest(pluginDir),
       loadDataOnlyAgentPlugin(pluginDir, { cwd, onWarning }),
       loadDataOnlyCommands(pluginDir, { onWarning }),

src/plugins/loader.test.ts

@@ -1,6 +1,15 @@
 import { defined } from "../../tests/helpers/defined.js";
 import { describe, test, expect } from "bun:test";
-import { dedupePluginModules, type PluginModule } from "./loader.js";
+import { mkdir, mkdtemp, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { createPluginLoadDiagnostics } from "./diagnostics.js";
+import {
+  dedupePluginModules,
+  loadPluginEntry,
+  loadPluginsFromPaths,
+  type PluginModule,
+} from "./loader.js";
 import { isPluginModuleEnabled } from "./register.js";
 import { disablePluginSettings } from "./uninstall.js";
 
@@ -101,3 +110,135 @@ describe("isPluginModuleEnabled with dedupe shadowing", () => {
     expect(isPluginModuleEnabled(user, {})).toBe(false);
   });
 });
+
+async function makeJsPlugin(files: Record<string, string>): Promise<string> {
+  const dir = await mkdtemp(join(tmpdir(), "manifest-plugin-"));
+  for (const [rel, body] of Object.entries(files)) {
+    const abs = join(dir, rel);
+    await mkdir(join(abs, ".."), { recursive: true });
+    await writeFile(abs, body);
+  }
+  return dir;
+}
+
+describe("readManifestJson malformed vs missing", () => {
+  test("malformed manifest.json warns with path and parse error", async () => {
+    const dir = await makeJsPlugin({
+      "index.js": "export {};\n",
+      "manifest.json": "{not-json",
+    });
+    const warnings: string[] = [];
+    await loadPluginEntry(dir, { onWarning: (msg) => warnings.push(msg) });
+    const manifestPath = join(dir, "manifest.json");
+    expect(warnings.length).toBeGreaterThan(0);
+    expect(warnings.some((w) => w.includes(manifestPath))).toBe(true);
+    expect(
+      warnings.some(
+        (w) => w.includes(manifestPath) && w.includes("failed to parse"),
+      ),
+    ).toBe(true);
+  });
+
+  test("invalid manifest.json schema warns with path and validation error", async () => {
+    const dir = await makeJsPlugin({
+      "index.js": "export {};\n",
+      "manifest.json": JSON.stringify({ id: "x", name: "X" }),
+    });
+    const warnings: string[] = [];
+    await loadPluginEntry(dir, { onWarning: (msg) => warnings.push(msg) });
+    const manifestPath = join(dir, "manifest.json");
+    expect(warnings.some((w) => w.includes(manifestPath))).toBe(true);
+    expect(
+      warnings.some((w) => w.includes(manifestPath) && w.includes("kind")),
+    ).toBe(true);
+  });
+
+  test("missing manifest.json stays silent", async () => {
+    const dir = await makeJsPlugin({
+      "index.js": "export {};\n",
+    });
+    const warnings: string[] = [];
+    await loadPluginEntry(dir, { onWarning: (msg) => warnings.push(msg) });
+    expect(warnings).toEqual([]);
+  });
+
+  test("malformed .claude-plugin/manifest.json warns on metadata-only load", async () => {
+    const dir = await makeJsPlugin({
+      ".claude-plugin/manifest.json": "{not-json",
+    });
+    const diag = createPluginLoadDiagnostics();
+    const cwd = await mkdtemp(join(tmpdir(), "manifest-cwd-"));
+    const mods = await loadPluginsFromPaths([dir], cwd, {
+      isPluginTrusted: () => false,
+      diagnostics: diag,
+    });
+    expect(mods).toEqual([]);
+    const manifestPath = join(dir, ".claude-plugin", "manifest.json");
+    expect(diag.warnings.some((w) => w.includes(manifestPath))).toBe(true);
+    expect(
+      diag.warnings.some(
+        (w) => w.includes(manifestPath) && w.includes("failed to parse"),
+      ),
+    ).toBe(true);
+  });
+
+  test("missing manifest on metadata-only load stays silent", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "manifest-empty-"));
+    const diag = createPluginLoadDiagnostics();
+    const cwd = await mkdtemp(join(tmpdir(), "manifest-cwd-"));
+    const mods = await loadPluginsFromPaths([dir], cwd, {
+      isPluginTrusted: () => false,
+      diagnostics: diag,
+    });
+    expect(mods).toEqual([]);
+    expect(diag.warnings).toEqual([]);
+  });
+
+  test("malformed native manifest.json on data-only plugin warns and does not silently infer kind", async () => {
+    const dir = await makeJsPlugin({
+      "agents/a.md": "---\nname: a\n---\nbody\n",
+      "manifest.json": "{not-json",
+    });
+    const warnings: string[] = [];
+    const mod = await loadPluginEntry(dir, {
+      onWarning: (msg) => warnings.push(msg),
+    });
+    const manifestPath = join(dir, "manifest.json");
+    expect(
+      warnings.some(
+        (w) => w.includes(manifestPath) && w.includes("failed to parse"),
+      ),
+    ).toBe(true);
+    expect(mod).not.toBeNull();
+    expect(mod?.agentPlugin).toBeDefined();
+  });
+
+  test("Claude-format .claude-plugin/manifest.json does not warn missing id/kind on metadata-only load", async () => {
+    const dir = await makeJsPlugin({
+      ".claude-plugin/manifest.json": JSON.stringify({
+        name: "cmo",
+        description: "Marketing ops",
+      }),
+    });
+    const diag = createPluginLoadDiagnostics();
+    const cwd = await mkdtemp(join(tmpdir(), "manifest-cwd-"));
+    const mods = await loadPluginsFromPaths([dir], cwd, {
+      isPluginTrusted: () => false,
+      diagnostics: diag,
+    });
+    expect(
+      diag.warnings.some((w) => w.includes("invalid plugin manifest")),
+    ).toBe(false);
+    expect(
+      diag.warnings.some(
+        (w) =>
+          w.includes(join(dir, ".claude-plugin", "manifest.json")) &&
+          (w.includes("id") || w.includes("kind")),
+      ),
+    ).toBe(false);
+    const mod = mods.find((m) => m.manifest?.id === "cmo");
+    expect(mod?.metadataOnly).toBe(true);
+    expect(mod?.manifest?.name).toBe("cmo");
+    expect(mod?.manifest?.description).toBe("Marketing ops");
+  });
+});