diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bf592a..bd573e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog — ChannelGate +- Fix MCP configuration selection when a plugin package supplies both engine manifests. Each + engine uses its own inline declarations or referenced files, avoiding wrong endpoints and + duplicate server errors. + - Reply in Slack when conflicting shared-folder skill or memory settings prevent a request from starting, with admin repair instructions instead of silently logging the error. diff --git a/FEATURES.md b/FEATURES.md index 3712910..15619b3 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -8,7 +8,9 @@ source sync govern the package. Catalog and assignment pickers show Plugin badge component summaries. Claude loads approved native skills/commands/agents and admin-gated hooks; compiled inline hooks use Claude's event map so approved hooks execute instead of being ignored. Codex receives approved skill catalogs. Both receive supported MCP transports through explicit -namespaced grants. Unsupported components/authentication produce actionable failures. Compiled +namespaced grants. When both manifests exist, each engine uses its own inline or file-based MCP +declarations; single-manifest portable packages remain supported. Unsupported components and +authentication produce actionable failures. Compiled files stay inside the channel container's artifact mounts; updates rotate warm snapshots and revocation removes future runtime grants. Source path/symlink/size validation retains last-good revisions on failed sync. Details and compatibility limits: `docs/SKILLS.md`. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index 5f10f92..b7913e5 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -39,6 +39,22 @@ credential in argv or listing summaries. Restore the fixture. Grant it personall author, remove shared grants, and verify another author receives neither its skill catalog nor MCP tools. Verify personal artifact cleanup after the first author's run completes. +Dual-manifest regression: `test/plugin-grant-integration.test.js` imports one package with distinct +inline `mcpServers.lookup.url` values in its Claude and Codex manifests, then repeats with separate +referenced JSON files. Each engine's compiled runtime must select its own endpoint. Codex must +still emit no native plugin directory. +Live follow-up (Claude and Codex separately; UNEXECUTED): copy the portable fixture and replace +its manifests' `mcpServers` paths with inline `fixture` definitions. Both invoke the bundled echo +server, with Claude's args containing `--fixture-engine=claude` and Codex's args containing +`--fixture-engine=codex` after the server path. Import, approve and grant the package to the two +disposable Worker conversations. Ask each "Run plugin echo with the value DUAL-42." Require the +actual namespaced tool result `PLUGIN-ECHO:DUAL-42` and captured server argv showing only that +engine's marker argument. +Repeat after moving each engine's inline configuration into its own `config/.json` and +updating each manifest's `mcpServers` path: neither engine may report duplicate server names or +launch the other engine's command. +Remove the fixture grants/source after capturing candidate and evidence. + **PLUG-03 — native components and compatibility (both engines, with different expectations).** Import and approve `test/fixtures/plugins/native/`. Grant it only to disposable test conversations. As an admin in a Full-access Claude conversation, start a fresh thread and ask “Use the native diff --git a/src/engines/adapters.js b/src/engines/adapters.js index fbdfe6e..84dee86 100644 --- a/src/engines/adapters.js +++ b/src/engines/adapters.js @@ -186,7 +186,7 @@ const claude = validateEngineAdapter({ }); const codex = validateEngineAdapter({ - pluginCapabilities: { manifest: "", components: ["skills", "mcpServers"] }, + pluginCapabilities: { sourceManifest: "codex", manifest: "", components: ["skills", "mcpServers"] }, async resolveOptionalMcpConfig(allowed) { if (!Array.isArray(allowed) || !allowed.length) return {}; const policy = codexMcpPolicyFor(await listEngineMcps("codex"), allowed); diff --git a/src/gateway/plugin-runtime.js b/src/gateway/plugin-runtime.js index ade7973..ec22457 100644 --- a/src/gateway/plugin-runtime.js +++ b/src/gateway/plugin-runtime.js @@ -29,7 +29,8 @@ function readJson(descriptor, file) { function componentConfigs(descriptor, key, manifestEngine) { const out = []; - for (const file of descriptor.components[key] || []) { + const components = descriptor.componentsByEngine?.[manifestEngine] || descriptor.components; + for (const file of components[key] || []) { const owner = Object.entries(descriptor.manifests).find(([engine]) => file === `.${engine}-plugin/plugin.json`); if (owner) { if (owner[0] === manifestEngine) out.push(owner[1][key]); @@ -79,7 +80,10 @@ export function compilePluginPackage(pkg, { capabilities, allowBypass = false, w } } if (d.components.hooks.length && !allowBypass) throw new Error(`Plugin ${slug}: hooks require an authorized live admin turn in a Full-access conversation`); - const manifestEngine = d.manifests[capabilities.manifest] ? capabilities.manifest : d.engines[0]; + // Source selection is independent of native output: Codex uses its own declarations even + // though it receives a skill catalog and explicit MCP settings instead of a native manifest. + const preferredManifest = capabilities.sourceManifest || capabilities.manifest; + const manifestEngine = d.manifests[preferredManifest] ? preferredManifest : d.engines[0]; const manifest = d.manifests[manifestEngine]; // Only known component paths are forwarded. In particular settings, MCP, apps, and LSP // declarations cannot silently expand permissions or activate an unselected service. diff --git a/src/gateway/skills/plugin-package.js b/src/gateway/skills/plugin-package.js index 9e064a5..e814514 100644 --- a/src/gateway/skills/plugin-package.js +++ b/src/gateway/skills/plugin-package.js @@ -16,6 +16,7 @@ function componentPath(value, files) { function describe(files) { const manifests = {}; const components = Object.fromEntries(Object.keys(componentDefaults).map((k) => [k, []])); + const componentsByEngine = {}; let name = "", description = "", version = "", manifestPath = ""; for (const [engine, p] of Object.entries(PLUGIN_MANIFESTS)) { const file = files.find((f) => f.path === p); @@ -30,22 +31,31 @@ function describe(files) { version ||= manifest.version || ""; manifestPath ||= p; manifests[engine] = manifest; + const engineComponents = Object.fromEntries(Object.keys(componentDefaults).map((k) => [k, []])); + componentsByEngine[engine] = engineComponents; for (const [key, defaultPath] of Object.entries(componentDefaults)) { const value = manifest[key]; if (value != null) { - if ((["hooks", "mcpServers", "lspServers", "apps"].includes(key)) && typeof value === "object" && !Array.isArray(value)) components[key].push(p); - else for (const entry of Array.isArray(value) ? value : [value]) components[key].push(componentPath(entry, files)); + if ((["hooks", "mcpServers", "lspServers", "apps"].includes(key)) && typeof value === "object" && !Array.isArray(value)) engineComponents[key].push(p); + else for (const entry of Array.isArray(value) ? value : [value]) engineComponents[key].push(componentPath(entry, files)); } // Claude's conventional component directories remain enabled alongside custom paths. - if (files.some((f) => f.path === defaultPath || f.path.startsWith(`${defaultPath}/`))) components[key].push(defaultPath); + if (files.some((f) => f.path === defaultPath || f.path.startsWith(`${defaultPath}/`))) engineComponents[key].push(defaultPath); + engineComponents[key] = [...new Set(engineComponents[key])]; + components[key].push(...engineComponents[key]); } } if (!name) throw new Error("plugin package needs a .claude-plugin/plugin.json or .codex-plugin/plugin.json manifest"); for (const [key, extras] of Object.entries({ apps: [".app.json"], hooks: ["hooks.json"] })) { - for (const p of extras) if (files.some((f) => f.path === p)) components[key].push(p); + for (const p of extras) if (files.some((f) => f.path === p)) { + components[key].push(p); + for (const engineComponents of Object.values(componentsByEngine)) { + if (!engineComponents[key].includes(p)) engineComponents[key].push(p); + } + } } for (const key of Object.keys(components)) components[key] = [...new Set(components[key])]; - return { kind: "plugin", name, description, version, engines: Object.keys(manifests), manifestPath, manifests, components, files }; + return { kind: "plugin", name, description, version, engines: Object.keys(manifests), manifestPath, manifests, components, componentsByEngine, files }; } export function buildPluginSkill(input) { diff --git a/test/plugin-grant-integration.test.js b/test/plugin-grant-integration.test.js index 7dd044f..307b116 100644 --- a/test/plugin-grant-integration.test.js +++ b/test/plugin-grant-integration.test.js @@ -94,3 +94,25 @@ test("two catalog packages cannot collide in the native plugin namespace", async assert.match(run.pluginRuntime.claude.error, /same plugin name/); assert.deepEqual(run.pluginRuntime.claude.dirs, []); }); + +for (const mode of ["inline", "file"]) test(`dual-manifest packages select each engine's ${mode} MCP configuration`, async (t) => { + const slug = `pkg-dual-manifest-${mode}`; + putSkillRevision({ slug, files: buildPluginSkill(["claude", "codex"].flatMap((engine) => { + const config = { lookup: { url: `https://${engine}.example.com/mcp` } }; + return [{ + path: `.${engine}-plugin/plugin.json`, + content: JSON.stringify({ name: slug, mcpServers: mode === "inline" ? config : `./config/${engine}.json` }), + }, ...(mode === "file" ? [{ path: `config/${engine}.json`, content: JSON.stringify({ mcpServers: config }) }] : [])]; + })) }); + const run = await artifacts("pkg-dual-channel", [slug]); + t.after(() => run.cleanup()); + for (const engine of ["claude", "codex"]) { + assert.equal(run.pluginRuntime[engine].error, undefined); + assert.equal(run.pluginRuntime[engine].servers.length, 1); + assert.equal(run.pluginRuntime[engine].servers[0].definition.url, `https://${engine}.example.com/mcp`); + } + assert.deepEqual(run.pluginRuntime.codex.dirs, [], "Codex still receives no native plugin directory"); + if (mode === "file") for (const engine of ["claude", "codex"]) { + await assert.rejects(access(path.join(run.pluginRuntime.claude.dirs[0], "config", `${engine}.json`)), { code: "ENOENT" }); + } +});