diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts new file mode 100644 index 000000000..efcefbc0e --- /dev/null +++ b/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); + }); +}); diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 5db3b23cb..8da8272f7 100644 --- a/src/plugins/loader.ts +++ b/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(); 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); diff --git a/src/plugins/register.ts b/src/plugins/register.ts index 2171a3f0c..f1474532a 100644 --- a/src/plugins/register.ts +++ b/src/plugins/register.ts @@ -9,7 +9,10 @@ export function isPluginEnabled(config: Record, 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, @@ -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.