Merge pull request #510 from corbitsdev/cl-6716-prevent-later-discovery-origins-from-shadowing-repo

Preserve repo defaultEnabled through plugin dedupe shadowing

Author: TheGreatAxios

src/plugins/loader.test.ts

@@ -0,0 +1,80 @@
+import { describe, test, expect } from "bun:test";
+import { dedupePluginModules, type PluginModule } from "./loader.js";
+import { isPluginModuleEnabled } from "./register.js";
+
+function repoDefaultEnabled(id: string): PluginModule {
+  return {
+    manifest: { id, name: id, kind: "agent", defaultEnabled: true },
+    origin: "repo",
+  };
+}
+
+function userInstall(id: string): PluginModule {
+  return {
+    manifest: { id, name: id, kind: "agent" },
+    origin: "user",
+    source: "claude",
+  };
+}
+
+describe("dedupePluginModules", () => {
+  test("last occurrence wins for content", () => {
+    const repo = repoDefaultEnabled("scout");
+    const user = userInstall("scout");
+    const [result] = dedupePluginModules([repo, user]);
+    expect(result).toMatchObject({ origin: "user", source: "claude" });
+  });
+
+  // CL-6716: a later non-repo install with the same id as a repo
+  // defaultEnabled plugin must not silently turn the bundled default off.
+  test("stamps shadowedRepoDefaultEnabled when a non-repo module shadows a repo defaultEnabled id", () => {
+    const repo = repoDefaultEnabled("scout");
+    const user = userInstall("scout");
+    const [result] = dedupePluginModules([repo, user]);
+    expect(result!.shadowedRepoDefaultEnabled).toBe(true);
+  });
+
+  test("does not stamp shadowedRepoDefaultEnabled when the repo module wasn't defaultEnabled", () => {
+    const repo: PluginModule = { manifest: { id: "scout", name: "scout", kind: "agent" }, origin: "repo" };
+    const user = userInstall("scout");
+    const [result] = dedupePluginModules([repo, user]);
+    expect(result!.shadowedRepoDefaultEnabled).toBeUndefined();
+  });
+
+  test("does not stamp unrelated ids", () => {
+    const repo = repoDefaultEnabled("scout");
+    const other = userInstall("other");
+    const result = dedupePluginModules([repo, other]);
+    expect(result.find((m) => m.manifest?.id === "other")!.shadowedRepoDefaultEnabled).toBeUndefined();
+  });
+
+  test("propagates the shadow stamp through a chain of later installs", () => {
+    const repo = repoDefaultEnabled("scout");
+    const user = userInstall("scout");
+    const path: PluginModule = { manifest: { id: "scout", name: "scout", kind: "agent" }, origin: "path" };
+    const [result] = dedupePluginModules([repo, user, path]);
+    expect(result).toMatchObject({ origin: "path" });
+    expect(result!.shadowedRepoDefaultEnabled).toBe(true);
+  });
+});
+
+describe("isPluginModuleEnabled with dedupe shadowing", () => {
+  test("a same-id later install stays enabled by default after shadowing a repo defaultEnabled plugin", () => {
+    const repo = repoDefaultEnabled("scout");
+    const user = userInstall("scout");
+    const [survivor] = dedupePluginModules([repo, user]);
+    expect(isPluginModuleEnabled(survivor!, {})).toBe(true);
+  });
+
+  test("an explicit disable in settings still wins over the preserved default-on", () => {
+    const repo = repoDefaultEnabled("scout");
+    const user = userInstall("scout");
+    const [survivor] = dedupePluginModules([repo, user]);
+    expect(isPluginModuleEnabled(survivor!, { scout: { enabled: false } })).toBe(false);
+  });
+
+  test("without dedupe shadowing, a plain user-origin module needs an explicit enable", () => {
+    const user = userInstall("scout");
+    expect(isPluginModuleEnabled(user, {})).toBe(false);
+  });
+});

src/plugins/loader.ts

@@ -58,6 +58,14 @@ export type PluginModule = {
    * `origin`, which drives trust gating.
    */
   source?: string;
+  /**
+   * Set by dedupePluginModules when this module's id shadowed an earlier
+   * repo module that had manifest.defaultEnabled === true. Lets
+   * isPluginModuleEnabled keep the id default-on after a same-id later
+   * install replaces the bundled module, without requiring an explicit
+   * settings flag (CL-6716).
+   */
+  shadowedRepoDefaultEnabled?: boolean;
 };
 
 // Read and validate a manifest.json beside the module. Plugins may declare
@@ -563,6 +571,13 @@ export async function discoverUserPlugins(
 // paths), so "last wins" means an explicit path overrides a user plugin, which
 // overrides a bundled one. Modules without a manifest carry no id and are kept
 // as-is.
+//
+// A later non-repo module with the same id as a repo defaultEnabled plugin
+// would otherwise silently turn the bundled default off — the survivor is
+// non-repo, so isPluginModuleEnabled's origin==="repo" check fails and
+// enablement then requires an explicit settings flag (CL-6716). Carry the
+// repo default-on forward via shadowedRepoDefaultEnabled so the id stays
+// enabled by default unless the user explicitly disables it in settings.
 export function dedupePluginModules(modules: PluginModule[]): PluginModule[] {
   const indexById = new Map<string, number>();
   const result: PluginModule[] = [];
@@ -574,7 +589,13 @@ export function dedupePluginModules(modules: PluginModule[]): PluginModule[] {
     }
     const existing = indexById.get(id);
     if (existing !== undefined) {
-      result[existing] = mod;
+      const prev = result[existing]!;
+      const wasRepoDefaultEnabled =
+        prev.shadowedRepoDefaultEnabled === true
+        || (prev.origin === "repo" && prev.manifest?.defaultEnabled === true);
+      result[existing] = wasRepoDefaultEnabled
+        ? { ...mod, shadowedRepoDefaultEnabled: true }
+        : mod;
     } else {
       indexById.set(id, result.length);
       result.push(mod);

src/plugins/register.ts

@@ -9,7 +9,10 @@ export function isPluginEnabled(config: Record<string, PluginConfig>, id: string
 
 // Enablement for a loaded module: explicit settings win; otherwise only a
 // first-party repo plugin with manifest.defaultEnabled turns on. Marketplace
-// (user), path, and project plugins cannot self-enable via the flag.
+// (user), path, and project plugins cannot self-enable via the flag — except
+// when dedupePluginModules stamped shadowedRepoDefaultEnabled, meaning this
+// module's id shadowed a repo defaultEnabled plugin during discovery dedupe;
+// the bundled default-on survives the shadowing (CL-6716).
 export function isPluginModuleEnabled(
   mod: PluginModule,
   config: Record<string, PluginConfig | undefined>,
@@ -19,7 +22,10 @@ export function isPluginModuleEnabled(
   const enabled = config[id]?.enabled;
   if (enabled === true) return true;
   if (enabled === false) return false;
-  return mod.origin === "repo" && mod.manifest?.defaultEnabled === true;
+  return (
+    (mod.origin === "repo" && mod.manifest?.defaultEnabled === true)
+    || mod.shadowedRepoDefaultEnabled === true
+  );
 }
 
 // Mark a plugin enabled while preserving credentials/consented and other fields.