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..02709f5 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 @@ -377,16 +393,18 @@ The skipped/live cases below remain unverified; this branch is not a release can - [x] Automated: `node --test test/inherited-file-commands.test.js test/slack-attachment-recovery.test.js test/message-normalize.test.js test/message-to-reply-e2e.test.js test/codex-message-to-reply-e2e.test.js`. The actual message pipeline handles current text commands after a preceding bot file, a root - attachment and no attachment. Require retirement guidance and zero engine starts for legacy `/files`, - stop, pending, `/help` and bare `/next`. Canonical text overrides incomplete trigger text; + attachment and no attachment. Require retirement guidance for legacy `/files` and ordinary daemon + responses for stop, pending, `/help` and bare `/next`, with zero engine starts for all of them. + Canonical text overrides incomplete trigger text; current canonical files omitted from the trigger and trigger files retained after a lookup failure must keep attachment semantics. Ordinary/unknown commands, queued tasks and Claude's native `/compact` keep prior-file recovery. - [ ] Live, separately for Claude and Codex in authorized Worker/Auto QA channels: create a disposable two-line text file and share it into a thread. Immediately type a native bot mention followed by `/files`, attaching nothing to that message. Repeat with an older root attachment - separated by a text reply. Require the ephemeral **Open files** button, working native explorer - and no engine run, download or file-content answer for either command. In the same fixtures + separated by a text reply. Require retirement guidance pointing to the retained reply folder + button or **Browse channel files** shortcut, with no **Open files** button, explorer, engine run, + download or file-content answer for either invocation. In the same fixtures test `pending`, `/help` and bare `/next`; require their ordinary daemon responses without a run. While an owned finite task is active, send mentioned `stop`/`cancel` after a file share and require the task to stop; then test `/next ` during another active task and require normal 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" }); + } +});