From 7b997d2e108bb6df4bc5d3d2af37dfdc3f19ce41 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 14:36:45 -0700 Subject: [PATCH 1/3] fix(mcp): tighten command checks, carry Codex MCP auth, cite dialect facts A command such as `${CLAUDE_PLUGIN_ROOT}/bin/server --stdio` passed build and validate for Agent Plugins output, because the root variable was rewritten to `./bin/server --stdio` and only bare names were checked for spaces. Clients run the whole value as one file name, so the server never starts. A `./` command that contains whitespace is now a build error that says to move the arguments into `args`. Codex reads per-server MCP auth from `extensions["com.openai"].auth` in mcp.json, but the Agent Plugins 1.0 server shape is closed, so pluginpack rejected the field and Codex auth could not be expressed. The codex target now renders MCP config in a `codex` dialect that passes a per-server `extensions` object through, limited to the `com.openai` namespace, and codex validation accepts the same shape. The agent-plugins target keeps the strict shape, with or without `clients.codex`, because other conforming clients skip a server with an unknown field. CONFORMANCE.md records the conflict and the overrides workaround. The native MCP dialect rules (variable names per client, Cursor inferring the transport, Cursor having no plugin-data variable) had no citations, although every other target fact does. Each rule now carries a documentation URL and a verification date, taken from the Claude Code, Cursor and Copilot CLI plugin references, and the existing citation test covers them. CONFORMANCE.md lists the facts and two places where current behavior is not backed by the docs. A `./` or root-variable command that names a file the plugin does not ship built cleanly and validated cleanly. The build now warns, naming the target, plugin, server and missing file. It does not fail, because a file such as a compiled binary can be added to the output after the build. Validation of an Agent Plugins package reports the missing file as an error. --- CONFORMANCE.md | 65 ++++++++++ README.md | 18 ++- src/agent-plugins.ts | 36 +++++- src/mcp.ts | 220 +++++++++++++++++++++++++++++++--- src/targets/codex.ts | 5 +- src/targets/engine.ts | 11 +- tests/agent-plugins.test.ts | 115 +++++++++++++++++- tests/install-snippet.test.ts | 25 +++- tests/mcp.test.ts | 118 ++++++++++++++++++ 9 files changed, 584 insertions(+), 29 deletions(-) diff --git a/CONFORMANCE.md b/CONFORMANCE.md index 06375bf..04308e5 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -86,6 +86,71 @@ against a temp fixture via [`bintastic`](https://github.com/scalvert/bintastic). `agent-plugins` target with `clients.codex` writes the same `.agents/plugins/marketplace.json`, so the two need separate output roots. +## MCP dialect facts + +`src/mcp.ts` renders one authored MCP config into each client's dialect. Each +dialect rule carries a citation (`getMcpDialectCitations`), and the citation +test in `tests/install-snippet.test.ts` fails if a rule has no documentation +URL or date. All were verified on 2026-10-21: + +| Dialect | Rule | Source | +| --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | +| `claude` | `${CLAUDE_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_DATA}` expand in a stdio server's `command`, `args`, and `env` | | +| `claude` | no `type` means stdio; Streamable HTTP is `http` (`streamable-http` is an alias) | | +| `cursor` | `${CURSOR_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_ROOT}` expand in `command`, `args`, `env`, and `cwd`; `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` don't | | +| `cursor` | no plugin-data variable is documented, so a data variable is a build error | | +| `cursor` | Cursor plugins can infer the transport from `command` or `url`, so `type` is dropped | | +| `copilot` | `${PLUGIN_ROOT}`, with `${CLAUDE_PLUGIN_ROOT}` as an alias | | +| `copilot` | `${PLUGIN_DATA}` is documented only for Agent Plugins packages, so legacy-format data variables stay as authored | | +| `copilot` | remote servers may be labelled `http`, `sse`, or `streamable-http` | | +| `agent-plugins` | closed server variants; one-token `command`; `args`, `env`, `cwd` expand `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` | (§7.2.1, §9) | +| `codex` | the `agent-plugins` rules, plus per-server auth in `extensions["com.openai"].auth` | | + +The docs leave two points open, and pluginpack's current behavior is not +backed by them. Claude Code's list of fields where `${...}` resolves for a +stdio server does not include `cwd`, but pluginpack writes a `./` `cwd` as +`${CLAUDE_PLUGIN_ROOT}/...` for `claude`. Copilot CLI documents expansion in +`args`, `env` and `cwd` (not `command`) for packages, and documents nothing +for the legacy `.mcp.json`. pluginpack still writes a `./` `command` as +`${PLUGIN_ROOT}/...` for `copilot`. + +### Conflict: Codex per-server `extensions` + +The Agent Plugins 1.0 specification (§7.2.1) closes each MCP server variant: +an unknown field makes the server entry invalid, and the client skips it. The +vendored `tests/fixtures/agent-plugins/mcp.schema.json` agrees +(`additionalProperties: false`). OpenAI's +[plugin packaging docs](https://developers.openai.com/codex/plugins/build) +(verified 2026-10-21) put per-server MCP auth in the same file, under +`extensions["com.openai"].auth` on each server. A server written that way is +not valid Agent Plugins 1.0. + +pluginpack resolves the conflict per target: + +- `codex` writes Codex-only output, so it passes a per-server `extensions` + object through. That object may hold only `com.openai`, whose value must be + an object. `validate` accepts the same shape. A server with this object + does not validate against the vendored schema, so the schema oracle covers + only codex servers without it. +- `agent-plugins` keeps the strict 1.0 shape, with or without + `clients.codex`. Its package is meant for every conforming client, and + Copilot CLI skips an invalid server entry. The error says this and names + the workaround. + +Workaround for a project that builds both targets: keep the portable config +in `shared//mcp/config.json`. Set `overrides: "overrides/codex/"` +on the codex plugin, and put a full copy of `config.json` with +`extensions["com.openai"]` in `overrides/codex//mcp/config.json`. An +override replaces the whole file; it is not merged. + +### Command files + +A `./` or `${}/` `command` names a file inside the plugin. If +the artifact has no such file, the build prints a warning on every target. +The build does not fail, because a file such as a compiled binary can be +added after the build. `validate` reports an error for an Agent Plugins +package (`codex`, `agent-plugins`) whose command file is missing on disk. + ## Update-check hook facts The generated update-check hook (`updateCheck` config, claude/cursor targets) diff --git a/README.md b/README.md index 626eded..7e935d2 100644 --- a/README.md +++ b/README.md @@ -466,19 +466,33 @@ For Agent Plugins output, a `${}/bin/server` command becomes `./bin/server`. Anything a conforming client would silently skip fails the build instead: -- a shell string as `command`; +- a shell string as `command`, including a `./` or `${}/` + path followed by arguments (`./bin/server --stdio`): clients run the whole + value as one file name, so arguments go in `args`; - a `${VAR}` other than `${PLUGIN_ROOT}`/`${PLUGIN_DATA}` (left unexpanded), or any placeholder in `url` or `headers`; - a `cwd` that isn't `./`, `${PLUGIN_ROOT}`, `${PLUGIN_DATA}`, or a path under one of them that stays inside it (`.` and bare relative paths fail; omit `cwd` to use the plugin root); - a field outside `type`, `command`, `args`, `env`, `cwd` (stdio) or `type`, - `url`, `headers` (remote); + `url`, `headers` (remote). The one exception is the `codex` target, which + also passes through a per-server `extensions["com.openai"]` object, where + Codex reads MCP auth. The `agent-plugins` target rejects it, even with + `clients.codex`, because other clients skip a server that has it. To build + both targets, set `overrides: "overrides/codex/"` on the codex + plugin and put a copy of `config.json` with the extension in + `overrides/codex//mcp/config.json`. See + [CONFORMANCE.md](CONFORMANCE.md#mcp-dialect-facts); - an `env` entry named `PLUGIN_ROOT` or `PLUGIN_DATA`, or a header set twice; - plain `http` to a non-loopback host. With `format: "legacy"`, `codex` writes `.mcp.json` as authored. +On every target, a `./bin/server` or `${}/bin/server` command +must name a file the plugin ships. If the build output has no such file, the +build prints a warning, because you can still add the file after the build. +`validate` fails an Agent Plugins package whose command file is missing. + A `./bin/server` command or `./server` `cwd` becomes `${}/bin/server` for targets whose clients don't resolve plugin-relative paths. Config already written in diff --git a/src/agent-plugins.ts b/src/agent-plugins.ts index e8be67c..cc4ecd9 100644 --- a/src/agent-plugins.ts +++ b/src/agent-plugins.ts @@ -2,7 +2,12 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { isDeepStrictEqual } from "node:util"; import { exists } from "./fs.js"; -import { AGENT_PLUGINS_MCP_SCHEMA, renderMcpConfig } from "./mcp.js"; +import { + AGENT_PLUGINS_MCP_SCHEMA, + pluginCommandFiles, + renderMcpConfig, + type McpDialect, +} from "./mcp.js"; import { deepMerge, stripUndefined } from "./targets/shared.js"; import { error, parseMatter, readJson } from "./targets/validation-shared.js"; import type { ManifestBuildContext } from "./targets/types.js"; @@ -171,11 +176,14 @@ export async function hasPackageManifest(pluginDir: string): Promise { * manifest, the `mcp.json` shape, and each discovered skill — every check * here is something a conforming client would reject or silently skip. * Returns the parsed manifest, or `undefined` when it couldn't be read. + * `mcpDialect` is `"codex"` only for Codex-only output, which may carry + * Codex's per-server `extensions`. */ export async function validatePackage( pluginDir: string, pluginName: string, issues: ValidationIssue[], + mcpDialect: PackageMcpDialect = "agent-plugins", ): Promise | undefined> { const manifest = await readJson( path.join(pluginDir, packageLayout.manifest), @@ -186,11 +194,14 @@ export async function validatePackage( return undefined; } validateManifest(manifest, pluginName, issues); - await validateMcp(pluginDir, pluginName, issues); + await validateMcp(pluginDir, pluginName, mcpDialect, issues); await validateSkills(pluginDir, pluginName, issues); return manifest; } +/** The MCP dialects a package's `mcp.json` can be written in. */ +export type PackageMcpDialect = Extract; + function validateManifest( manifest: Record, pluginName: string, @@ -275,6 +286,7 @@ function validateManifest( async function validateMcp( pluginDir: string, pluginName: string, + dialect: PackageMcpDialect, issues: ValidationIssue[], ): Promise { const file = path.join(pluginDir, packageLayout.mcp); @@ -308,7 +320,7 @@ async function validateMcp( let rendered: unknown; try { rendered = ( - renderMcpConfig({ [name]: server }, "agent-plugins", pluginName) + renderMcpConfig({ [name]: server }, dialect, pluginName) .mcpServers as Record )[name]; } catch (err) { @@ -334,6 +346,24 @@ async function validateMcp( }; expected ${JSON.stringify(rendered)}.`, ); } + // A ./ command is a file inside the package (spec §7.2.1); if it isn't + // there, the client can't start the server. + for (const { command, file } of pluginCommandFiles({ [name]: rendered })) { + if (!(await isFile(path.join(pluginDir, file)))) { + error( + issues, + `${pluginName}: mcp.json server "${name}" "command" is ${JSON.stringify(command)}, but the package has no file "${file}" — the client can't start the server.`, + ); + } + } + } +} + +async function isFile(file: string): Promise { + try { + return (await fs.stat(file)).isFile(); + } catch { + return false; } } diff --git a/src/mcp.ts b/src/mcp.ts index edaa976..5b811cb 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -1,4 +1,6 @@ +import path from "node:path"; import { isSafeRelativePath } from "./fs.js"; +import type { Citation } from "./targets/types.js"; /** * MCP configuration rendering. @@ -23,16 +25,63 @@ import { isSafeRelativePath } from "./fs.js"; * - `agent-plugins` — the portable Agent Plugins 1.0 `mcp.json`: `$schema`, * an explicit `type` per server, and full validation against the spec, * because a non-conforming server is silently skipped by clients. + * - `codex` — the Agent Plugins dialect plus the per-server + * `extensions["com.openai"]` object Codex reads MCP auth from. The spec's + * closed server shape has no such field, so only Codex-only output may + * carry it (see CONFORMANCE.md, "MCP dialect facts"). * - `claude`, `cursor`, `copilot` — the client's native dialect. * - `verbatim` — emitted exactly as authored, for a client whose dialect * isn't documented. */ -export type McpDialect = - "agent-plugins" | "claude" | "cursor" | "copilot" | "verbatim"; +export const MCP_DIALECTS = [ + "agent-plugins", + "codex", + "claude", + "cursor", + "copilot", + "verbatim", +] as const; + +export type McpDialect = (typeof MCP_DIALECTS)[number]; export const AGENT_PLUGINS_MCP_SCHEMA = "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json"; +const AGENT_PLUGINS_SPEC_CITATION: Citation = { + claim: + 'mcp.json is closed: each server declares "type" and matches exactly one variant, and an unknown field makes the entry invalid; "command" is one bare or ./ executable token with no placeholder expansion; "args", "env", and "cwd" expand ${PLUGIN_ROOT} and ${PLUGIN_DATA}', + documentationUrl: "https://agent-plugins.org/specification", + verifiedAt: "2026-10-21", +}; + +/** The namespace Codex reads per-server MCP settings from. */ +const CODEX_SERVER_NAMESPACE = "com.openai"; + +/** + * The documentation each dialect's rules are based on. `verbatim` has none: + * it exists for clients whose dialect isn't documented. + */ +export function getMcpDialectCitations(dialect: McpDialect): Citation[] { + switch (dialect) { + case "agent-plugins": + return [AGENT_PLUGINS_SPEC_CITATION]; + case "codex": + return [ + AGENT_PLUGINS_SPEC_CITATION, + { + claim: + 'Codex reads per-server MCP authentication from extensions["com.openai"].auth on each server in mcp.json, not from plugin.json', + documentationUrl: "https://developers.openai.com/codex/plugins/build", + verifiedAt: "2026-10-21", + }, + ]; + case "verbatim": + return []; + default: + return Object.values(nativeDialects[dialect].citations); + } +} + /** * Renders authored MCP servers into one dialect's complete config file * object (ready for `json()`). `context` prefixes error messages, e.g. @@ -47,12 +96,13 @@ export function renderMcpConfig( if (dialect === "verbatim") { return { mcpServers: servers }; } - if (dialect === "agent-plugins") { + if (dialect === "agent-plugins" || dialect === "codex") { const mcpServers: Record = {}; for (const [name, server] of Object.entries(servers)) { mcpServers[name] = toAgentPluginsServer( server, `${context}: MCP server "${name}"`, + dialect === "codex", ); } return { $schema: AGENT_PLUGINS_MCP_SCHEMA, mcpServers }; @@ -69,6 +119,33 @@ export function renderMcpConfig( return { mcpServers }; } +/** + * The plugin file each server's `command` names, for commands written as a + * `./` path or rooted at a plugin-root variable. `file` is relative to the + * plugin root. Commands that aren't a plain path inside the plugin (a bare + * name, a data-variable path, an escape) are left to `renderMcpConfig`. + */ +export function pluginCommandFiles( + servers: Record, +): { server: string; command: string; file: string }[] { + const result: { server: string; command: string; file: string }[] = []; + for (const [server, config] of Object.entries(servers)) { + const command = isObject(config) ? config.command : undefined; + if (typeof command !== "string") { + continue; + } + const prefix = ["./", ...ROOT_VARIABLES.map((name) => `\${${name}}/`)].find( + (candidate) => command.startsWith(candidate), + ); + const rest = prefix === undefined ? "" : command.slice(prefix.length); + if (!rest || /\s|\$\{/.test(rest) || !isSafeRelativePath(rest)) { + continue; + } + result.push({ server, command, file: path.posix.normalize(rest) }); + } + return result; +} + const ROOT_VARIABLES = [ "PLUGIN_ROOT", "CLAUDE_PLUGIN_ROOT", @@ -96,23 +173,47 @@ type NativeDialectRules = { * pass through unchanged, and a server without a `type` never gains one. */ types: Record; + /** The documentation behind each of the three rules above. */ + citations: Record<"root" | "data" | "types", Citation>; }; +const CLAUDE_PLUGINS_REFERENCE = + "https://code.claude.com/docs/en/plugins-reference#environment-variables"; +const CURSOR_PLUGINS_REFERENCE = + "https://cursor.com/docs/reference/plugins#mcp-servers"; +const COPILOT_PLUGINS_REFERENCE = + "https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-plugin-reference"; + const nativeDialects: Record< - Exclude, + Exclude, NativeDialectRules > = { - // Claude Code: ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}; a missing - // `type` means stdio; remote Streamable HTTP is labelled "http". claude: { client: "Claude Code", root: { canonical: "CLAUDE_PLUGIN_ROOT", native: ["CLAUDE_PLUGIN_ROOT"] }, data: { canonical: "CLAUDE_PLUGIN_DATA", native: ["CLAUDE_PLUGIN_DATA"] }, types: { "streamable-http": "http" }, + citations: { + root: { + claim: + "Claude Code expands ${CLAUDE_PLUGIN_ROOT} to the plugin's installed path in an MCP stdio server's command, args, and env", + documentationUrl: CLAUDE_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + data: { + claim: + "Claude Code expands ${CLAUDE_PLUGIN_DATA} to a persistent per-plugin directory in the same fields", + documentationUrl: CLAUDE_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + types: { + claim: + 'an entry with no "type" is a stdio server; Streamable HTTP is "http", with "streamable-http" accepted as an alias', + documentationUrl: "https://code.claude.com/docs/en/mcp", + verifiedAt: "2026-10-21", + }, + }, }, - // Cursor expands ${CURSOR_PLUGIN_ROOT} and ${CLAUDE_PLUGIN_ROOT}, not the - // standard ${PLUGIN_ROOT}; it has no plugin-data variable; it infers the - // transport from `command`/`url`. cursor: { client: "Cursor", root: { @@ -121,9 +222,27 @@ const nativeDialects: Record< }, data: "unsupported", types: { stdio: null, http: null, "streamable-http": null }, + citations: { + root: { + claim: + "Cursor expands ${CURSOR_PLUGIN_ROOT} and ${CLAUDE_PLUGIN_ROOT} in command, args, env values, and cwd, and does not expand the standard ${PLUGIN_ROOT}", + documentationUrl: CURSOR_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + data: { + claim: + "Cursor does not expand ${PLUGIN_DATA} and documents no plugin-data variable of its own", + documentationUrl: CURSOR_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + types: { + claim: + "Cursor plugins can infer the transport from command or url instead of declaring it", + documentationUrl: CURSOR_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + }, }, - // Copilot-format plugins expand ${PLUGIN_ROOT} and ${CLAUDE_PLUGIN_ROOT}; - // its plugin-data behaviour isn't documented for this format. copilot: { client: "Copilot", root: { @@ -132,6 +251,26 @@ const nativeDialects: Record< }, data: "passthrough", types: { "streamable-http": "http" }, + citations: { + root: { + claim: + "Copilot CLI names the plugin root ${PLUGIN_ROOT}, with ${CLAUDE_PLUGIN_ROOT} as an alias (documented for plugin agents' MCP servers and LSP config; the legacy .mcp.json format has no separate list)", + documentationUrl: COPILOT_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + data: { + claim: + "${PLUGIN_DATA} (aliases ${COPILOT_PLUGIN_DATA}, ${CLAUDE_PLUGIN_DATA}) is documented only for Agent Plugins 1.0 MCP servers, so legacy-format config keeps data variables as authored", + documentationUrl: COPILOT_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + types: { + claim: + 'Copilot CLI reads remote servers labelled "http", "sse", or "streamable-http"', + documentationUrl: COPILOT_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, + }, }, }; @@ -247,11 +386,13 @@ const REMOTE_FIELDS = new Set(["type", "url", "headers"]); /** * Converts one authored server to the Agent Plugins 1.0 shape and enforces * spec §7.2.1 and §9 — each violation would make a conforming client skip - * the server, so it fails the build here instead. + * the server, so it fails the build here instead. With `codexExtensions`, + * the server may also carry Codex's `extensions["com.openai"]` object. */ function toAgentPluginsServer( server: unknown, context: string, + codexExtensions: boolean, ): Record { if (!isObject(server)) { throw new Error(`${context} must be an object.`); @@ -259,15 +400,55 @@ function toAgentPluginsServer( const type = agentPluginsTransport(server, context); const allowed = type === "stdio" ? STDIO_FIELDS : REMOTE_FIELDS; for (const key of Object.keys(server)) { + if (key === "extensions" && codexExtensions) { + continue; + } if (!allowed.has(key)) { + const hint = + key === "extensions" + ? ` Only the codex target emits per-server "extensions" (Codex reads MCP auth from extensions["${CODEX_SERVER_NAMESPACE}"].auth); other Agent Plugins clients skip a server that has them. Remove it here, and put a copy of config.json with it in the codex plugin's overrides directory (mcp/config.json).` + : ""; throw new Error( - `${context} has field "${key}", which the Agent Plugins ${type} server shape does not allow (allowed: ${[...allowed].join(", ")}).`, + `${context} has field "${key}", which the Agent Plugins ${type} server shape does not allow (allowed: ${[...allowed].join(", ")}).${hint}`, ); } } - return type === "stdio" - ? toAgentPluginsStdio(server, context) - : toAgentPluginsRemote(type, server, context); + const result = + type === "stdio" + ? toAgentPluginsStdio(server, context) + : toAgentPluginsRemote(type, server, context); + if (codexExtensions && server.extensions !== undefined) { + result.extensions = codexServerExtensions(server.extensions, context); + } + return result; +} + +/** + * Codex documents only `extensions["com.openai"]` on an MCP server; any + * other namespace means nothing in Codex-only output, so it is an error. + */ +function codexServerExtensions( + value: unknown, + context: string, +): Record { + if (!isObject(value)) { + throw new Error( + `${context} "extensions" must be an object keyed by extension namespace, such as { "${CODEX_SERVER_NAMESPACE}": { "auth": { ... } } }.`, + ); + } + for (const [namespace, data] of Object.entries(value)) { + if (namespace !== CODEX_SERVER_NAMESPACE) { + throw new Error( + `${context} has extensions["${namespace}"]; Codex reads only extensions["${CODEX_SERVER_NAMESPACE}"] on an MCP server. Remove it.`, + ); + } + if (!isObject(data)) { + throw new Error( + `${context} extensions["${CODEX_SERVER_NAMESPACE}"] must be an object.`, + ); + } + } + return value; } function agentPluginsTransport( @@ -347,6 +528,13 @@ function agentPluginsCommand(value: unknown, context: string): string { ); } if (command.startsWith("./")) { + // A space means the author wrote a shell string: clients run the whole + // value as one file name. + if (/\s/.test(command)) { + throw new Error( + `${context} "command" is ${JSON.stringify(value)}; Agent Plugins requires a single executable token and runs the whole value as one file name. Move arguments into "args".`, + ); + } if (!isSafeRelativePath(command.slice(2)) || command === "./") { throw new Error( `${context} "command" ${JSON.stringify(value)} must stay inside the plugin root.`, diff --git a/src/targets/codex.ts b/src/targets/codex.ts index ddba33a..308898e 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -139,7 +139,7 @@ async function validateCodexPlugin( ((await exists(path.join(pluginDir, packageLayout.manifest))) && !(await exists(path.join(pluginDir, ".codex-plugin", "plugin.json")))) ) { - return validatePackage(pluginDir, pluginName, issues); + return validatePackage(pluginDir, pluginName, issues, "codex"); } const manifest = await readJson( path.join(pluginDir, ".codex-plugin", "plugin.json"), @@ -400,5 +400,6 @@ export const codex: PluginTargetDefinition = { ], mcpConfigPath: (pluginPath) => path.join(pluginPath, packageLayout.mcp), - mcpDialect: "agent-plugins", + // Agent Plugins plus Codex's per-server extensions["com.openai"] (MCP auth). + mcpDialect: "codex", }; diff --git a/src/targets/engine.ts b/src/targets/engine.ts index 93c2d5c..f2172f8 100644 --- a/src/targets/engine.ts +++ b/src/targets/engine.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { collectPluginFiles, resolveMcpServers } from "../render.js"; import { readAuthoredPlugin } from "../source.js"; import { isSafeRelativePath, json, toPosix } from "../fs.js"; -import { renderMcpConfig } from "../mcp.js"; +import { pluginCommandFiles, renderMcpConfig } from "../mcp.js"; import { resolvePartials } from "../partials.js"; import { validateNoSurvivingPartialTags } from "./validation-shared.js"; import { deepMerge, stripUndefined } from "./shared.js"; @@ -217,6 +217,15 @@ export async function emitFromDefinition( ), ), ); + // A warning, not an error: the file may be added to the output after + // the build (e.g. a compiled binary). `validate` checks the result. + for (const { server, command, file } of pluginCommandFiles(mcpServers)) { + if (!files.has(toPosix(path.join(pluginPath, file)))) { + console.warn( + `Target "${target}" plugin "${pluginName}": MCP server "${server}" "command" is ${JSON.stringify(command)}, but the plugin does not ship "${file}". Add the file to the plugin source (an MCP server file goes in mcp/pluginpack.json "files"), or fix the command.`, + ); + } + } } const metadata = emittedPluginMetadata( diff --git a/tests/agent-plugins.test.ts b/tests/agent-plugins.test.ts index 2103d39..bc5270b 100644 --- a/tests/agent-plugins.test.ts +++ b/tests/agent-plugins.test.ts @@ -1,7 +1,7 @@ import { readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import { Project, type ProjectArgs } from "fixturify-project"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { build } from "../src/build.js"; import { validateOutput } from "../src/adapters.js"; import { loadConfig } from "../src/config.js"; @@ -1058,3 +1058,116 @@ describe("agent-plugins target with clients", () => { ]); }); }); + +describe("MCP config in packages", () => { + const openaiAuth = { + "com.openai": { + auth: { type: "oauth", client: { mode: "dcr" } }, + }, + }; + const remoteWithAuth = { + "config.json": `${JSON.stringify({ + mcpServers: { + docs: { + type: "streamable-http", + url: "https://mcp.acme.example/mcp", + extensions: openaiAuth, + }, + }, + })}\n`, + }; + + it("carries Codex MCP auth under extensions on the codex target and validates it", async () => { + const root = await setup( + ` + codex: { + outDir: "out", + plugins: { acme: { source: "shared/acme", ${CODEX_ENTRY} } } + }`, + { mcp: remoteWithAuth }, + ); + + await build({ cwd: root }); + + expect(await readJsonFile(root, "out/plugins/acme/mcp.json")).toEqual({ + $schema: "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json", + mcpServers: { + docs: { + type: "streamable-http", + url: "https://mcp.acme.example/mcp", + extensions: openaiAuth, + }, + }, + }); + await expect( + validateOutput("codex", path.join(root, "out")), + ).resolves.toMatchObject({ ok: true, issues: [] }); + }); + + it("keeps the strict server shape on the agent-plugins target, even with clients.codex", async () => { + for (const clients of ["", "clients: { codex: {} },"]) { + const root = await setup( + ` + "agent-plugins": { + outDir: "out", + ${clients} + plugins: { acme: { source: "shared/acme" } } + }`, + { mcp: remoteWithAuth }, + ); + await expect(build({ cwd: root })).rejects.toThrow( + 'MCP server "docs" has field "extensions"', + ); + await project?.dispose(); + project = undefined; + } + }); + + it("warns at build time when a ./ command names a file the plugin doesn't ship, and validate fails", async () => { + const root = await setup( + ` + codex: { + outDir: "codex", + plugins: { acme: { source: "shared/acme", ${CODEX_ENTRY} } } + }, + claude: { + outDir: "claude", + plugins: { acme: { source: "shared/acme" } } + }`, + { + scripts: { "server.sh": "#!/bin/sh\n" }, + mcp: { + "config.json": `${JSON.stringify({ + mcpServers: { + shipped: { command: "./scripts/server.sh" }, + missing: { command: "./bin/missing" }, + rooted: { command: "${CLAUDE_PLUGIN_ROOT}/bin/also-missing" }, + }, + })}\n`, + }, + }, + ); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await build({ cwd: root }); + const warnings = warn.mock.calls.map((call) => String(call[0])); + const fix = + 'Add the file to the plugin source (an MCP server file goes in mcp/pluginpack.json "files"), or fix the command.'; + expect(warnings).toEqual( + ["claude", "codex"].flatMap((target) => [ + `Target "${target}" plugin "acme": MCP server "missing" "command" is "./bin/missing", but the plugin does not ship "bin/missing". ${fix}`, + `Target "${target}" plugin "acme": MCP server "rooted" "command" is "\${CLAUDE_PLUGIN_ROOT}/bin/also-missing", but the plugin does not ship "bin/also-missing". ${fix}`, + ]), + ); + } finally { + warn.mockRestore(); + } + + const result = await validateOutput("codex", path.join(root, "codex")); + expect(result.ok).toBe(false); + expect(result.issues.map((issue) => issue.message)).toEqual([ + 'acme: mcp.json server "missing" "command" is "./bin/missing", but the package has no file "bin/missing" — the client can\'t start the server.', + 'acme: mcp.json server "rooted" "command" is "./bin/also-missing", but the package has no file "bin/also-missing" — the client can\'t start the server.', + ]); + }); +}); diff --git a/tests/install-snippet.test.ts b/tests/install-snippet.test.ts index 2f78fba..dac9e93 100644 --- a/tests/install-snippet.test.ts +++ b/tests/install-snippet.test.ts @@ -6,6 +6,7 @@ import { getUnsupportedInstallTargets, } from "../src/install-snippet.js"; import type { InstallParams } from "../src/install-snippet.js"; +import { getMcpDialectCitations, MCP_DIALECTS } from "../src/mcp.js"; const params: InstallParams = { repository: "https://github.com/gleanwork/plugins", @@ -81,12 +82,28 @@ describe("install snippet", () => { }); }); - it("every target carries a dated documentation citation", () => { - for (const target of [ + it("every target and every MCP dialect rule carries a dated documentation citation", () => { + const citations = [ ...getSupportedInstallTargets(), ...getUnsupportedInstallTargets(), - ]) { - const citation = getInstallSnippetCitation(target); + ].map((target) => getInstallSnippetCitation(target)); + for (const dialect of MCP_DIALECTS) { + const dialectCitations = getMcpDialectCitations(dialect); + // verbatim is, by definition, the dialect with nothing documented. + if (dialect === "verbatim") { + expect(dialectCitations).toEqual([]); + continue; + } + expect(dialectCitations.length, dialect).toBeGreaterThan(0); + citations.push(...dialectCitations); + } + // Each native dialect cites its root-variable, data-variable, and + // transport-label rules separately. + for (const dialect of ["claude", "cursor", "copilot"] as const) { + expect(getMcpDialectCitations(dialect), dialect).toHaveLength(3); + } + for (const citation of citations) { + expect(citation.claim).not.toBe(""); expect(citation.documentationUrl).toMatch(/^https:\/\//); expect(citation.verifiedAt).toMatch(/^\d{4}-\d{2}-\d{2}$/); } diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index 33e9796..9c76a39 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { AGENT_PLUGINS_MCP_SCHEMA, + pluginCommandFiles, renderMcpConfig, type McpDialect, } from "../src/mcp.js"; @@ -224,6 +225,16 @@ describe("renderMcpConfig: agent-plugins dialect", () => { { command: "./../outside" }, "must stay inside the plugin root", ], + [ + "a root-variable command with arguments", + { command: "${CLAUDE_PLUGIN_ROOT}/bin/server --stdio" }, + 'Target "t" plugin "p": MCP server "s" "command" is "${CLAUDE_PLUGIN_ROOT}/bin/server --stdio"; Agent Plugins requires a single executable token', + ], + [ + "a ./ command with arguments", + { command: "./scripts/server.sh --stdio" }, + 'Move arguments into "args"', + ], [ "a field outside the variant", { command: "x", url: "https://example.com" }, @@ -293,4 +304,111 @@ describe("renderMcpConfig: agent-plugins dialect", () => { render({ broken: { command: "a b" } }, "agent-plugins"), ).toThrow('Target "t" plugin "p": MCP server "broken" "command"'); }); + + it("rejects per-server extensions and says only the codex target emits them", () => { + expect(() => + render( + { + s: { + type: "streamable-http", + url: "https://example.com/mcp", + extensions: { "com.openai": { auth: { type: "none" } } }, + }, + }, + "agent-plugins", + ), + ).toThrow( + 'has field "extensions", which the Agent Plugins streamable-http server shape does not allow (allowed: type, url, headers). Only the codex target emits per-server "extensions"', + ); + }); +}); + +describe("renderMcpConfig: codex dialect", () => { + const auth = { + type: "oauth", + client: { mode: "provided", clientId: "docs-client" }, + baseScopes: ["docs:read"], + }; + + it('passes extensions["com.openai"] through on remote and stdio servers', () => { + const config = render( + { + docs: { + extensions: { "com.openai": { auth } }, + type: "http", + url: "https://example.com/mcp", + }, + local: { + command: "${CLAUDE_PLUGIN_ROOT}/bin/server", + extensions: { "com.openai": { auth: { type: "none" } } }, + }, + }, + "codex", + ); + expect(JSON.stringify(config)).toBe( + JSON.stringify({ + $schema: AGENT_PLUGINS_MCP_SCHEMA, + mcpServers: { + docs: { + type: "streamable-http", + url: "https://example.com/mcp", + extensions: { "com.openai": { auth } }, + }, + local: { + type: "stdio", + command: "./bin/server", + extensions: { "com.openai": { auth: { type: "none" } } }, + }, + }, + }), + ); + }); + + it("applies every Agent Plugins rule to the rest of the server", () => { + expect(() => render({ s: { command: "node server.js" } }, "codex")).toThrow( + "requires a single executable token", + ); + }); + + it.each([ + [ + "a non-object extensions field", + "oauth", + '"extensions" must be an object', + ], + [ + "a namespace Codex doesn't read", + { "com.example": {} }, + 'has extensions["com.example"]; Codex reads only extensions["com.openai"]', + ], + [ + "a non-object com.openai value", + { "com.openai": "oauth" }, + 'extensions["com.openai"] must be an object', + ], + ])("rejects %s", (_label, extensions, message) => { + expect(() => + render({ s: { url: "https://example.com/mcp", extensions } }, "codex"), + ).toThrow(message); + }); +}); + +describe("pluginCommandFiles", () => { + it("lists the plugin file each ./ or root-variable command names", () => { + expect( + pluginCommandFiles({ + a: { command: "./bin/a" }, + b: { command: "${CLAUDE_PLUGIN_ROOT}/bin/b" }, + c: { command: "${PLUGIN_ROOT}/bin/../bin/c" }, + d: { command: "node" }, + e: { command: "${CLAUDE_PLUGIN_DATA}/bin/e" }, + f: { url: "https://example.com" }, + g: { command: "./../outside" }, + }), + ).toEqual([ + { server: "a", command: "./bin/a", file: "bin/a" }, + { server: "b", command: "${CLAUDE_PLUGIN_ROOT}/bin/b", file: "bin/b" }, + { server: "c", command: "${PLUGIN_ROOT}/bin/../bin/c", file: "bin/c" }, + ]); + }); }); From 04eb49ed309a8f68f223b008f0bab5aef023a4e4 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 15:28:38 -0700 Subject: [PATCH 2/3] fix(mcp): reject cwd on claude MCP servers Claude Code resolves plugin variables only in a stdio server's command, args and env, and it starts plugin MCP servers in the session's working directory whatever cwd says. Writing cwd as ${CLAUDE_PLUGIN_ROOT}/... looked right but left the server in the wrong directory, so the claude dialect now fails the build for any server with cwd and says how to fix it: resolve paths in args with ${CLAUDE_PLUGIN_ROOT}, or use a claude-only MCP config override. The dialect cites the Claude Code plugins reference for the rule, and CONFORMANCE.md now states exactly what the Copilot docs cover: they document expansion for Agent Plugins packages and for agent frontmatter, not for the legacy .mcp.json the copilot target emits, so that rewriting stays unverified rather than proven. --- CONFORMANCE.md | 40 +++++++++++++++++++++++++++++------ README.md | 11 ++++++++++ src/mcp.ts | 26 +++++++++++++++++++++-- tests/install-snippet.test.ts | 11 ++++++++-- tests/mcp.test.ts | 23 +++++++++++++------- 5 files changed, 92 insertions(+), 19 deletions(-) diff --git a/CONFORMANCE.md b/CONFORMANCE.md index 04308e5..8e037da 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -97,6 +97,7 @@ URL or date. All were verified on 2026-10-21: | --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | | `claude` | `${CLAUDE_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_DATA}` expand in a stdio server's `command`, `args`, and `env` | | | `claude` | no `type` means stdio; Streamable HTTP is `http` (`streamable-http` is an alias) | | +| `claude` | `cwd` is not a field Claude Code reads for a plugin stdio server, so a server with `cwd` is a build error | | | `cursor` | `${CURSOR_PLUGIN_ROOT}` and `${CLAUDE_PLUGIN_ROOT}` expand in `command`, `args`, `env`, and `cwd`; `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` don't | | | `cursor` | no plugin-data variable is documented, so a data variable is a build error | | | `cursor` | Cursor plugins can infer the transport from `command` or `url`, so `type` is dropped | | @@ -106,13 +107,38 @@ URL or date. All were verified on 2026-10-21: | `agent-plugins` | closed server variants; one-token `command`; `args`, `env`, `cwd` expand `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` | (§7.2.1, §9) | | `codex` | the `agent-plugins` rules, plus per-server auth in `extensions["com.openai"].auth` | | -The docs leave two points open, and pluginpack's current behavior is not -backed by them. Claude Code's list of fields where `${...}` resolves for a -stdio server does not include `cwd`, but pluginpack writes a `./` `cwd` as -`${CLAUDE_PLUGIN_ROOT}/...` for `claude`. Copilot CLI documents expansion in -`args`, `env` and `cwd` (not `command`) for packages, and documents nothing -for the legacy `.mcp.json`. pluginpack still writes a `./` `command` as -`${PLUGIN_ROOT}/...` for `copilot`. +### Claude Code ignores `cwd` + +Claude Code's plugins reference lists the fields where `${...}` resolves for +an MCP stdio server as `command`, `args`, and `env`; its MCP page lists the +same three. Neither documents `cwd`. In a check with Claude Code 2.1.284, a +plugin server with `"cwd": "${CLAUDE_PLUGIN_ROOT}/scripts"` connected, but its +process ran in the session's working directory, not `scripts/`. pluginpack +0.11 copied `cwd` to `claude` output as authored, so a server that relied on +it started in the wrong directory. The `claude` dialect now fails the build for +any server with `cwd`, and the error says to resolve paths in `args` with +`${CLAUDE_PLUGIN_ROOT}` or to use a claude-only `mcp/config.json` override. + +### Open point: Copilot and the legacy `.mcp.json` + +The `copilot` target emits a legacy plugin: a manifest plus `.mcp.json`. The +cited Copilot CLI reference documents placeholder expansion for MCP servers +in two other places, and neither covers the legacy shared `.mcp.json`: + +- Agent Plugins packages (`mcp.json`): `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` + expand in `args`, `env` values, and `cwd`, not in `command`. +- MCP servers in a plugin-shipped agent's own frontmatter: `${PLUGIN_ROOT}` + (with the `${CLAUDE_PLUGIN_ROOT}` and `${COPILOT_PLUGIN_ROOT}` aliases) + expands in `command` and `args`, and the reference says this substitution + applies only there. + +So the `copilot` rows above are the closest documented behavior, not a +documented rule for `.mcp.json`. pluginpack writes a `./` `command` or `cwd` +as `${PLUGIN_ROOT}/...` for `copilot`. The docs neither confirm nor rule out +that Copilot CLI expands it there. Copilot CLI 1.0.102 loads such a plugin +and lists the command as written; whether it expands the variable at launch +is unverified. pluginpack keeps this rewriting until Copilot documents the +legacy format or a launch check shows it is wrong. ### Conflict: Codex per-server `extensions` diff --git a/README.md b/README.md index 7e935d2..7e73c28 100644 --- a/README.md +++ b/README.md @@ -493,6 +493,17 @@ must name a file the plugin ships. If the build output has no such file, the build prints a warning, because you can still add the file after the build. `validate` fails an Agent Plugins package whose command file is missing. +The `claude` target rejects a stdio server that sets `cwd`. Claude Code +ignores `cwd` for plugin MCP servers and starts them in the user's working +directory; it resolves `${CLAUDE_PLUGIN_ROOT}` only in `command`, `args`, and +`env` ([plugins reference](https://code.claude.com/docs/en/plugins-reference#environment-variables), +[MCP docs](https://code.claude.com/docs/en/mcp)). Remove `cwd` and write +plugin paths in `args` with the root variable, for example +`"args": ["${PLUGIN_ROOT}/server/index.js"]`. If other targets need the +`cwd`, keep it in the shared `config.json`, set +`overrides: "overrides/claude/"` on the claude plugin, and put a copy +without `cwd` in `overrides/claude//mcp/config.json`. + A `./bin/server` command or `./server` `cwd` becomes `${}/bin/server` for targets whose clients don't resolve plugin-relative paths. Config already written in diff --git a/src/mcp.ts b/src/mcp.ts index 5b811cb..285a97c 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -77,8 +77,10 @@ export function getMcpDialectCitations(dialect: McpDialect): Citation[] { ]; case "verbatim": return []; - default: - return Object.values(nativeDialects[dialect].citations); + default: { + const { citations, ignoresCwd } = nativeDialects[dialect]; + return [...Object.values(citations), ...(ignoresCwd ? [ignoresCwd] : [])]; + } } } @@ -112,6 +114,7 @@ export function renderMcpConfig( for (const [name, server] of Object.entries(servers)) { mcpServers[name] = toNativeServer( server, + dialect, rules, `${context}: MCP server "${name}"`, ); @@ -175,6 +178,13 @@ type NativeDialectRules = { types: Record; /** The documentation behind each of the three rules above. */ citations: Record<"root" | "data" | "types", Citation>; + /** + * Set when the client ignores a stdio server's `cwd`, with the + * documentation behind it: a server with `cwd` is then a build error, + * because the server would start in the wrong directory. Unset, `cwd` is + * kept, with a `./` path spelled out from the plugin root. + */ + ignoresCwd?: Citation; }; const CLAUDE_PLUGINS_REFERENCE = @@ -213,6 +223,12 @@ const nativeDialects: Record< verifiedAt: "2026-10-21", }, }, + ignoresCwd: { + claim: + 'Claude Code resolves ${...} in a plugin MCP stdio server\'s command, args, and env only; "cwd" is not a documented field, and Claude Code 2.1.284 starts the server in the session\'s working directory whatever "cwd" says', + documentationUrl: CLAUDE_PLUGINS_REFERENCE, + verifiedAt: "2026-10-21", + }, }, cursor: { client: "Cursor", @@ -276,12 +292,18 @@ const nativeDialects: Record< function toNativeServer( server: unknown, + dialect: string, rules: NativeDialectRules, context: string, ): unknown { if (!isObject(server)) { return server; } + if (rules.ignoresCwd && server.cwd !== undefined) { + throw new Error( + `${context} has "cwd", but ${rules.client} ignores "cwd" for plugin MCP servers and starts them in the user's working directory. Remove "cwd" and resolve paths in "args" with \${${rules.root.canonical}} instead (for example "args": ["\${${rules.root.canonical}}/server/index.js"]), or put a ${dialect}-only config.json without "cwd" in the ${dialect} plugin's overrides directory (mcp/config.json).`, + ); + } const rewrite = (value: string) => rewriteNative(value, rules, context); const result: Record = {}; for (const [key, value] of Object.entries(server)) { diff --git a/tests/install-snippet.test.ts b/tests/install-snippet.test.ts index dac9e93..d8eaa11 100644 --- a/tests/install-snippet.test.ts +++ b/tests/install-snippet.test.ts @@ -98,10 +98,17 @@ describe("install snippet", () => { citations.push(...dialectCitations); } // Each native dialect cites its root-variable, data-variable, and - // transport-label rules separately. - for (const dialect of ["claude", "cursor", "copilot"] as const) { + // transport-label rules separately; claude also cites why it rejects + // "cwd". + for (const dialect of ["cursor", "copilot"] as const) { expect(getMcpDialectCitations(dialect), dialect).toHaveLength(3); } + expect(getMcpDialectCitations("claude")).toHaveLength(4); + expect(getMcpDialectCitations("claude")).toContainEqual( + expect.objectContaining({ + claim: expect.stringContaining('"cwd" is not a documented field'), + }), + ); for (const citation of citations) { expect(citation.claim).not.toBe(""); expect(citation.documentationUrl).toMatch(/^https:\/\//); diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index 9c76a39..efe803a 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -105,19 +105,26 @@ describe("renderMcpConfig: native dialects", () => { }); }); + it("rejects any cwd for claude, because Claude Code ignores it for plugin servers", () => { + for (const cwd of [ + "./server", + "./", + "${CLAUDE_PLUGIN_ROOT}/server", + "/srv", + ]) { + expect(() => + render({ s: { type: "stdio", command: "node", cwd } }, "claude"), + ).toThrow( + 'Target "t" plugin "p": MCP server "s" has "cwd", but Claude Code ignores "cwd" for plugin MCP servers and starts them in the user\'s working directory. Remove "cwd" and resolve paths in "args" with ${CLAUDE_PLUGIN_ROOT} instead (for example "args": ["${CLAUDE_PLUGIN_ROOT}/server/index.js"]), or put a claude-only config.json without "cwd" in the claude plugin\'s overrides directory (mcp/config.json).', + ); + } + }); + it("spells out the plugin root for a ./ cwd", () => { const servers = { s: { type: "stdio", command: "node", cwd: "./server" }, root: { type: "stdio", command: "node", cwd: "./" }, }; - expect(render(servers, "claude").mcpServers).toEqual({ - s: { - type: "stdio", - command: "node", - cwd: "${CLAUDE_PLUGIN_ROOT}/server", - }, - root: { type: "stdio", command: "node", cwd: "${CLAUDE_PLUGIN_ROOT}" }, - }); expect(render(servers, "cursor").mcpServers).toEqual({ s: { command: "node", cwd: "${CURSOR_PLUGIN_ROOT}/server" }, root: { command: "node", cwd: "${CURSOR_PLUGIN_ROOT}" }, From 8e0dde04107c37c74fd0e2d48efe46c75dab3d05 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 15:29:08 -0700 Subject: [PATCH 3/3] fix(mcp): warn when an MCP argument names a file the plugin does not ship The build already warned when a ./ or root-variable command named a file missing from the output, but the common form, command "node" with args ["${PLUGIN_ROOT}/server.js"], went unchecked. The build now warns for any args entry that starts with a plugin-root variable and names neither a shipped file nor a shipped directory. It stays a warning, like the command check, because the file can be added after the build. --- README.md | 9 ++++--- src/mcp.ts | 43 +++++++++++++++++++++++++++++++ src/targets/engine.ts | 14 ++++++++++- tests/agent-plugins.test.ts | 9 +++++++ tests/mcp.test.ts | 50 +++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 7e73c28..02a946f 100644 --- a/README.md +++ b/README.md @@ -489,9 +489,12 @@ build instead: With `format: "legacy"`, `codex` writes `.mcp.json` as authored. On every target, a `./bin/server` or `${}/bin/server` command -must name a file the plugin ships. If the build output has no such file, the -build prints a warning, because you can still add the file after the build. -`validate` fails an Agent Plugins package whose command file is missing. +must name a file the plugin ships, and so must an `args` entry that starts +with `${}/`, such as `${PLUGIN_ROOT}/dist/server.js` (a +directory the plugin ships also counts). If the build output has no such +file, the build prints a warning, because you can still add the file after +the build. `validate` fails an Agent Plugins package whose command file is +missing. The `claude` target rejects a stdio server that sets `cwd`. Claude Code ignores `cwd` for plugin MCP servers and starts them in the user's working diff --git a/src/mcp.ts b/src/mcp.ts index 285a97c..ef6843f 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -149,6 +149,49 @@ export function pluginCommandFiles( return result; } +/** + * The plugin file each server's `args` entry names, for entries rooted at a + * plugin-root variable (`${PLUGIN_ROOT}/dist/server.js`). `field` is + * `args[]`; `file` is relative to the plugin root. A `./` argument is + * not listed: clients pass it through, so it resolves against the server's + * working directory, not the plugin. + */ +export function pluginArgFiles( + servers: Record, +): { server: string; field: string; value: string; file: string }[] { + const result: { + server: string; + field: string; + value: string; + file: string; + }[] = []; + for (const [server, config] of Object.entries(servers)) { + const args = isObject(config) ? config.args : undefined; + if (!Array.isArray(args)) { + continue; + } + args.forEach((value: unknown, index) => { + if (typeof value !== "string") { + return; + } + const prefix = ROOT_VARIABLES.map((name) => `\${${name}}/`).find( + (candidate) => value.startsWith(candidate), + ); + const rest = prefix === undefined ? "" : value.slice(prefix.length); + if (!rest || rest.includes("${") || !isSafeRelativePath(rest)) { + return; + } + result.push({ + server, + field: `args[${index}]`, + value, + file: path.posix.normalize(rest), + }); + }); + } + return result; +} + const ROOT_VARIABLES = [ "PLUGIN_ROOT", "CLAUDE_PLUGIN_ROOT", diff --git a/src/targets/engine.ts b/src/targets/engine.ts index f2172f8..ca8ae86 100644 --- a/src/targets/engine.ts +++ b/src/targets/engine.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { collectPluginFiles, resolveMcpServers } from "../render.js"; import { readAuthoredPlugin } from "../source.js"; import { isSafeRelativePath, json, toPosix } from "../fs.js"; -import { pluginCommandFiles, renderMcpConfig } from "../mcp.js"; +import { pluginArgFiles, pluginCommandFiles, renderMcpConfig } from "../mcp.js"; import { resolvePartials } from "../partials.js"; import { validateNoSurvivingPartialTags } from "./validation-shared.js"; import { deepMerge, stripUndefined } from "./shared.js"; @@ -226,6 +226,18 @@ export async function emitFromDefinition( ); } } + // An argument may name a directory the plugin ships, too. + for (const { server, field, value, file } of pluginArgFiles(mcpServers)) { + const shipped = toPosix(path.join(pluginPath, file)); + if ( + !files.has(shipped) && + ![...files.keys()].some((key) => key.startsWith(`${shipped}/`)) + ) { + console.warn( + `Target "${target}" plugin "${pluginName}": MCP server "${server}" "${field}" is ${JSON.stringify(value)}, but the plugin does not ship "${file}". Add the file to the plugin source (an MCP server file goes in mcp/pluginpack.json "files"), or fix the argument.`, + ); + } + } } const metadata = emittedPluginMetadata( diff --git a/tests/agent-plugins.test.ts b/tests/agent-plugins.test.ts index bc5270b..099d40c 100644 --- a/tests/agent-plugins.test.ts +++ b/tests/agent-plugins.test.ts @@ -1142,6 +1142,14 @@ describe("MCP config in packages", () => { shipped: { command: "./scripts/server.sh" }, missing: { command: "./bin/missing" }, rooted: { command: "${CLAUDE_PLUGIN_ROOT}/bin/also-missing" }, + shippedArg: { + command: "node", + args: ["${PLUGIN_ROOT}/scripts/server.sh"], + }, + missingArg: { + command: "node", + args: ["--stdio", "${CLAUDE_PLUGIN_ROOT}/dist/missing.js"], + }, }, })}\n`, }, @@ -1157,6 +1165,7 @@ describe("MCP config in packages", () => { ["claude", "codex"].flatMap((target) => [ `Target "${target}" plugin "acme": MCP server "missing" "command" is "./bin/missing", but the plugin does not ship "bin/missing". ${fix}`, `Target "${target}" plugin "acme": MCP server "rooted" "command" is "\${CLAUDE_PLUGIN_ROOT}/bin/also-missing", but the plugin does not ship "bin/also-missing". ${fix}`, + `Target "${target}" plugin "acme": MCP server "missingArg" "args[1]" is "\${CLAUDE_PLUGIN_ROOT}/dist/missing.js", but the plugin does not ship "dist/missing.js". Add the file to the plugin source (an MCP server file goes in mcp/pluginpack.json "files"), or fix the argument.`, ]), ); } finally { diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index efe803a..2f8983e 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { AGENT_PLUGINS_MCP_SCHEMA, + pluginArgFiles, pluginCommandFiles, renderMcpConfig, type McpDialect, @@ -400,6 +401,55 @@ describe("renderMcpConfig: codex dialect", () => { }); }); +describe("pluginArgFiles", () => { + it("lists the plugin file each root-variable argument names", () => { + expect( + pluginArgFiles({ + a: { command: "node", args: ["${PLUGIN_ROOT}/dist/a.js", "--stdio"] }, + b: { + command: "node", + args: ["--inspect", "${CLAUDE_PLUGIN_ROOT}/lib/../dist/b.js"], + }, + c: { command: "node", args: ["${CURSOR_PLUGIN_ROOT}/my dir/c.js"] }, + // Not plugin files: a ./ argument resolves against the server's + // working directory, and the rest aren't plain paths in the plugin. + d: { + command: "node", + args: [ + "./relative.js", + "${PLUGIN_ROOT}", + "${PLUGIN_DATA}/state.json", + "--config=${PLUGIN_ROOT}/config.json", + "${PLUGIN_ROOT}/../outside.js", + "${PLUGIN_ROOT}/${NAME}.js", + 42, + ], + }, + e: { url: "https://example.com" }, + }), + ).toEqual([ + { + server: "a", + field: "args[0]", + value: "${PLUGIN_ROOT}/dist/a.js", + file: "dist/a.js", + }, + { + server: "b", + field: "args[1]", + value: "${CLAUDE_PLUGIN_ROOT}/lib/../dist/b.js", + file: "dist/b.js", + }, + { + server: "c", + field: "args[0]", + value: "${CURSOR_PLUGIN_ROOT}/my dir/c.js", + file: "my dir/c.js", + }, + ]); + }); +}); + describe("pluginCommandFiles", () => { it("lists the plugin file each ./ or root-variable command names", () => { expect(