diff --git a/.changeset/plugged-in-keepers.md b/.changeset/plugged-in-keepers.md new file mode 100644 index 00000000..1755f047 --- /dev/null +++ b/.changeset/plugged-in-keepers.md @@ -0,0 +1,39 @@ +--- +"@paddock/server": minor +"@paddock/web": minor +--- + +Inherit the host's Claude Code **plugins**, and stop degrading `sse` / +header-authenticated MCP servers (#700). + +Requires `@herdctl/core` 5.32.0, which adds the two things Paddock had no channel +for. + +**Plugins.** A plugin that provides an MCP server — a Slack plugin installed on +your laptop, say — was invisible in Paddock on every setting, because the SDK +enables a discovered plugin from `enabledPlugins` in the **user** settings source +and Paddock's agents are invoked with `setting_sources: ["project"]`. Paddock now +enumerates the host's installed plugin directories from the CLI's own +`installed_plugins.json` registry and passes them explicitly, which needs no +settings-source grant. Two levers gate it, because a plugin is mostly +instructions and only sometimes MCP servers: + +| `claude.instructions` | `claude.mcpServers` | what a keeper gets | +|---|---|---| +| `host` | `host` | the plugin, including its MCP servers | +| `host` | `own` | the plugin's commands/agents/skills/hooks only | +| `own` | *any* | no plugins (`instructions` is what bridges `plugins/`) | + +Each plugin server's `mcp__plugin____*` pattern is added to the +keeper's allowed tools automatically — without it the server connects and then has +every call auto-denied with no prompt. A plugin whose manifest points `mcpServers` +at a bundle rather than declaring them inline cannot be enumerated that way; it is +still attached, and a boot warning names it and the pattern to add by hand. + +**MCP server fields.** `headers` and an explicit `type` (`sse`) are now carried +through verbatim instead of being stripped. So a bearer-authenticated or `sse` +server inherited under `claude.mcpServers: host` arrives intact and finds its +stored OAuth token (which is keyed on a hash of `{type, url, headers}`), and the +boot warnings v0.62.0 shipped for both are gone. The instance's own `mcpServers:` +block accepts both keys too — `headers` values take `env:VAR_NAME` references like +everything else there, and are never printed. diff --git a/CLAUDE.md b/CLAUDE.md index 9947d1ed..623f4adc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,10 +77,20 @@ servers into each keeper's `mcp_servers` agent config, the one seam both runtime (`claude-mcp.ts`). A **sibling** `mcpServers:` block declares servers to paddock itself rather than borrowing the machine's (`mcp-servers.ts`); it wins a name clash with `host`, is file-only, and takes `env:VAR_NAME` references anywhere a string goes so tokens stay -out of the git-tracked file. Two rules that are load-bearing for anything touching MCP -here: an attached server whose `mcp____*` pattern is not added to the keeper's -`allowed_tools` has every call auto-denied with no prompt, and nothing may ever log or -serialise a declared server's values (`describeServer` is the only renderer). +out of the git-tracked file. A host Claude Code **plugin** is the third contributor and +the one neither of those can see, because a plugin declares its servers inside itself +(#700): `claude-plugins.ts` enumerates the host's installed plugin directories from the +CLI's own `plugins/installed_plugins.json` and passes them as `agent.plugins`, gated by +`claude.instructions` (which is what bridges `plugins/`) with `claude.mcpServers` +deciding only whether the plugins' own servers come too, via `skipMcpDiscovery`. Three +rules that are load-bearing for anything touching MCP here: an attached server whose +`mcp____*` pattern is not added to the keeper's `allowed_tools` has every call +auto-denied with no prompt (a PLUGIN's server is registered as +`plugin::`, so its pattern is `mcp__plugin____*` — +derived, not read); nothing may ever log or serialise a declared server's values +(`describeServer` is the only renderer); and under `driveMode: batch` the CLI runtime +puts the whole `mcp_servers` record in one `--mcp-config` argv element, so an `env` +value or an `Authorization` header is visible in `/proc//cmdline` to the same user. ## Dev conventions diff --git a/package-lock.json b/package-lock.json index a3322082..ade0064c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1531,9 +1531,9 @@ } }, "node_modules/@herdctl/core": { - "version": "5.31.0", - "resolved": "https://registry.npmjs.org/@herdctl/core/-/core-5.31.0.tgz", - "integrity": "sha512-Ln9oH8+JWOVMRFFD9WH3I3p97sdX9TP8pv9tA0VzkyTByT5k3aZLWmh6GU3OIg7UVV1KHqQ22EokyJt/R7ei3w==", + "version": "5.32.0", + "resolved": "https://registry.npmjs.org/@herdctl/core/-/core-5.32.0.tgz", + "integrity": "sha512-PHlWrId/MZQfKqKH0Z2gi90pjYL3tZLfNF39o5m166AuyuVVv4TfCdh0iXoR9hjsFWoFUhrduAT/iI8XiU8zIg==", "license": "MIT", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.3.215", @@ -10784,7 +10784,7 @@ "@fastify/swagger-ui": "^4.2.0", "@fastify/websocket": "10.0.1", "@herdctl/chat": "^0.8.0", - "@herdctl/core": "^5.31.0", + "@herdctl/core": "^5.32.0", "@modelcontextprotocol/sdk": "^1.29.0", "fastify": "4.28.1", "jose": "^6.2.3", diff --git a/packages/server/package.json b/packages/server/package.json index 665184ff..5b4fea66 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -27,7 +27,7 @@ "@fastify/swagger-ui": "^4.2.0", "@fastify/websocket": "10.0.1", "@herdctl/chat": "^0.8.0", - "@herdctl/core": "^5.31.0", + "@herdctl/core": "^5.32.0", "@modelcontextprotocol/sdk": "^1.29.0", "fastify": "4.28.1", "jose": "^6.2.3", diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 0601a3c0..f889ac06 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -25,6 +25,7 @@ import { findPlantedChatsLinks, } from "./claude-home.js"; import { loadHostMcpSource } from "./claude-mcp.js"; +import { loadHostPlugins } from "./claude-plugins.js"; import { declaredMcpNotices } from "./mcp-servers.js"; import { installHerdctlLogBridge } from "./agent-errors.js"; import { ProjectStore, ROOT_KEY } from "./projects.js"; @@ -180,7 +181,18 @@ export async function buildApp(opts: BuildAppOptions = {}): Promise { })) { app.log[notice.level](notice.message); } - const herdctl = new HerdctlService(cfg, { ...hostMcp.source, declared: cfg.mcpServers }); + // …and the host's Claude Code PLUGINS (#700), which no amount of reading + // `.claude.json` can find: a plugin declares its own MCP servers internally. + // Gated by `claude.instructions` (which is what bridges `plugins/`), with + // `claude.mcpServers` deciding only whether the plugins' servers come too — + // see `claude-plugins.ts` for why that is not the single lever #700 assumes. + const hostPlugins = await loadHostPlugins(cfg); + for (const notice of hostPlugins.notices) app.log[notice.level](notice.message); + const herdctl = new HerdctlService( + cfg, + { ...hostMcp.source, declared: cfg.mcpServers }, + hostPlugins.source, + ); const git = new GitService(cfg.projectsRoot, cfg.gitAuthor); const githubAuth = new GithubAuth(path.join(cfg.dataDir, "github-auth.json"), cfg.githubClientId); const archive = new ArchiveStore(cfg.dataDir); diff --git a/packages/server/src/claude-mcp.ts b/packages/server/src/claude-mcp.ts index df36e1d9..ba224556 100644 --- a/packages/server/src/claude-mcp.ts +++ b/packages/server/src/claude-mcp.ts @@ -71,44 +71,73 @@ * only when there are some, so an instance with no host servers stays * byte-identical to before. * - * ## What cannot be carried, honestly - * - * herdctl's `McpServerSchema` is `{command?, args?, env?, url?}` and it is a - * plain `z.object`, so anything else is silently STRIPPED at `addAgent`. Two - * consequences that matter and that #691 does not mention: - * - * - **`headers` is dropped.** A remote MCP server authenticated with a bearer - * header therefore arrives unauthenticated. - * - **`type` is dropped**, and `transformMcpServer` maps any `url` to - * `type: "http"`. An `sse` server is silently downgraded to HTTP. - * - * And those two strippings are worse than they look, because of where MCP OAuth - * tokens turn out to live. #691 listed that as an open question; it is now - * answered, from the bundled CLI binary rather than by inference: - * - * - Tokens are stored under a top-level **`mcpOAuth`** key in the SAME credential - * store as `claudeAiOauth` — `/.credentials.json` on Linux, - * the one `Claude Code-credentials` keychain item on darwin. There is no - * MCP-specific service name and no `mcp-oauth/` directory. The store is - * resolved by `CLAUDE_SECURESTORAGE_CONFIG_DIR ?? CLAUDE_CONFIG_DIR ?? ~/.claude`, - * which is exactly the variable `claude.credentials` drives. **So - * `credentials: host` DOES carry MCP OAuth tokens** — they are not separable - * from the Anthropic login, in either direction. - * - But the per-server key is `` `${serverName}|${sha256({type,url,headers}).slice(0,16)}` ``. - * A dropped `headers`, or an `sse` coerced to `http`, changes that hash and the - * stored token is simply not found. So the combination that works today is - * `credentials: host` + `mcpServers: host` for an OAuth server declared with no - * headers and no `type` disagreement; a header-authenticated or `sse` server - * arrives looking unauthenticated even though its token is right there. - * - * Neither is paddock's to fix here (both need a herdctl schema change). What - * paddock CAN do is refuse to be silent about it: {@link parseHostMcpConfig} - * records every key it had to drop and every `sse` it had to coerce, and the - * boot notice names the server. The server is still passed — this is a - * capability lever, not a security one, and a user who declared a server should - * get it plus a warning rather than nothing plus a warning. The one case that IS - * dropped is a server with neither `command` nor `url`, which cannot be started - * at all. + * ## What could not be carried, and now can (herdctl 5.32.0) + * + * Until `@herdctl/core@5.32.0`, `McpServerSchema` was `{command?, args?, env?, + * url?}`, so `headers` and `type` were silently STRIPPED at `addAgent` and + * `transformMcpServer` rewrote every `url` to `type: "http"`. A + * header-authenticated server arrived unauthenticated and an `sse` server was + * downgraded to HTTP, so #699 shipped a boot warning naming every server that + * lost a field. + * + * herdctl#446 (in 5.32.0) widened the schema to mirror the SDK's own + * `McpServerConfig` — `type`, `headers`, `timeout` and `alwaysLoad` — and an + * explicit `type` now wins over the bare-`url` inference. Verified against the + * installed 5.32.0 rather than assumed: `addAgent` → `getAgents()` → + * `toSDKOptions()` returns `{type: "sse", url, headers}` unchanged for a server + * declared that way, and still infers `type: "http"` for a bare `url`. So the + * two `degraded` caveats are gone and {@link McpServerDef} carries both fields. + * + * Why that mattered more than it looked, and why it is worth not regressing: + * MCP OAuth tokens are stored under a top-level **`mcpOAuth`** key in the SAME + * credential store as `claudeAiOauth` — `/.credentials.json` + * on Linux, the one `Claude Code-credentials` keychain item on darwin. There is + * no MCP-specific service name and no `mcp-oauth/` directory. The store is + * resolved by `CLAUDE_SECURESTORAGE_CONFIG_DIR ?? CLAUDE_CONFIG_DIR ?? ~/.claude`, + * which is exactly the variable `claude.credentials` drives — **so + * `credentials: host` DOES carry MCP OAuth tokens**, in either direction. But + * the per-server key is `` `${serverName}|${sha256({type,url,headers}).slice(0,16)}` ``, + * so a dropped header or a coerced `type` changed the hash and the stored token + * was simply not found. Carrying both fields verbatim is what makes + * `credentials: host` + `mcpServers: host` work for an OAuth server at all. + * + * `z.object` still strips what it has no field for (`tools`, and anything Claude + * Code grows next), so this narrowing is still a narrowing — it is just no longer + * one that breaks authentication. The one case that is still DROPPED is a server + * with neither `command` nor `url`, which cannot be started at all. + * + * ## The cost of carrying `headers`, which is real and is upstream + * + * herdctl's **CLI runtime** serialises the whole `mcp_servers` record into a + * single `--mcp-config '{"mcpServers":…}'` argv element, so everything in it is + * readable from `/proc//cmdline` by any process of the same user. That was + * already true of an `env` value; it is now true of an `Authorization` header as + * well, which is the more likely place a bearer token lives. The SDK runtime + * passes the record in-process and is unaffected. + * + * Paddock's chats default to the SDK runtime (`driveMode: session`), but the + * sweeper and triggers are always one-shot CLI runs and a `driveMode: batch` + * project's turns are too — and neither the sweeper nor a trigger is given these + * servers, so the exposure is `driveMode: batch` only. Not paddock's to fix (the + * argv shape is herdctl's) and not a reason to go back to dropping the header: + * a stripped header is an authentication failure for everyone, while this is a + * same-user disclosure on one non-default drive mode. Worth knowing before + * putting a long-lived token in a `headers` block on a shared box. + * + * #702 found and measured this for a DECLARED server's `env` (it reads the token + * back out of a real spawned process's argv), and `mcp-servers.ts`'s + * `argvExposure` is the boot warning — now widened to `headers` for the same + * reason. There is no equivalent warning for a HOST server, deliberately: this + * module does not look at a value it did not resolve, and a user who ran + * `claude mcp add` on the box is already in the argv-exposure position with their + * own terminal. + * + * ## Plugins are the other half of this lever + * + * A Claude Code **plugin** can provide MCP servers too, and they are invisible to + * everything above: they are declared inside the plugin, not in `.claude.json` + * (#700). `claude-plugins.ts` is that half — it enumerates the host's installed + * plugin directories for `agent.plugins`, the second thing 5.32.0 added. * * ## Scope: keepers only * @@ -172,15 +201,20 @@ export function hostMcpConfigPath(legacyClaudeHome: string): string { * One MCP server, in the only shape herdctl's `McpServerSchema` can carry. * * Deliberately NOT `SDKMcpServerConfig`: that is what herdctl produces on the - * far side of `transformMcpServer`, and pinning the narrower agent-config shape - * here is what makes the lossiness above visible at the type level rather than - * at runtime. + * far side of `transformMcpServer`, and pinning the agent-config shape here is + * what keeps any remaining lossiness visible at the type level rather than at + * runtime. As of herdctl 5.32.0 the two differ only in what the SDK has that + * herdctl still has no field for (`tools`). */ export interface McpServerDef { + /** Explicit transport. Wins over the bare-`url` ⇒ `http` inference (5.32.0). */ + type?: "stdio" | "sse" | "http"; command?: string; args?: string[]; env?: Record; url?: string; + /** Carried verbatim since 5.32.0 — and part of the OAuth token's key. */ + headers?: Record; } /** A set of MCP servers, keyed by the name their tools are namespaced under. */ @@ -190,7 +224,14 @@ export type McpServerDefs = Record; export interface HostMcpCaveat { /** The server's name in `.claude.json`. */ name: string; - /** `dropped` = not passed at all; `degraded` = passed, but not faithfully. */ + /** + * `dropped` = not passed at all; `degraded` = passed, but not faithfully. + * + * Nothing produces `degraded` since herdctl 5.32.0 carries `headers` and + * `type` verbatim (#700) — the variant and its notice branch are kept because + * the class of defect ("the engine has no field for this") is the one #699 was + * filed about and is one schema narrowing away from coming back. + */ kind: "dropped" | "degraded"; /** Human-readable, and specific enough to act on. */ reason: string; @@ -268,6 +309,18 @@ function narrowServer( if (Object.keys(env).length > 0) server.env = env; } if (typeof raw.url === "string" && raw.url !== "") server.url = raw.url; + // Both carried verbatim since herdctl 5.32.0 (#700). Before that they were + // stripped at `addAgent` and this function pushed a `degraded` caveat naming + // the server; the fields reaching `toSDKOptions()` intact is what retired that + // warning. A `type` we do not recognise is left off rather than passed on — + // herdctl's enum would strip it anyway, and the bare-`url` inference is the + // better fallback than nothing. + if (raw.type === "stdio" || raw.type === "sse" || raw.type === "http") server.type = raw.type; + if (isRecord(raw.headers)) { + const headers: Record = {}; + for (const [k, v] of Object.entries(raw.headers)) if (typeof v === "string") headers[k] = v; + if (Object.keys(headers).length > 0) server.headers = headers; + } if (server.command === undefined && server.url === undefined) { caveats.push({ @@ -277,31 +330,6 @@ function narrowServer( }); return undefined; } - // The lossy bits. `type` and `headers` are real keys of Claude Code's own MCP - // config that herdctl's schema has no field for and therefore strips; saying so - // is the difference between "my Notion server does not work" and "my Notion - // server's bearer header was dropped, here is the issue to file". - if (isRecord(raw.headers) && Object.keys(raw.headers).length > 0) { - caveats.push({ - name, - kind: "degraded", - reason: - `its \`headers\` (${Object.keys(raw.headers).join(", ")}) cannot be carried — ` + - `herdctl's MCP schema has no field for them, so the server is passed WITHOUT them, ` + - `any auth they held is gone, and its stored OAuth token (keyed on a hash that ` + - `includes the headers) will not be found either`, - }); - } - if (raw.type === "sse") { - caveats.push({ - name, - kind: "degraded", - reason: - 'it is an `sse` server, and herdctl maps every `url` to `type: "http"` — it will ' + - "be connected to as HTTP, and any OAuth token stored against the `sse` shape " + - "will not be found", - }); - } return server; } diff --git a/packages/server/src/claude-plugins.ts b/packages/server/src/claude-plugins.ts new file mode 100644 index 00000000..b6b516d2 --- /dev/null +++ b/packages/server/src/claude-plugins.ts @@ -0,0 +1,474 @@ +/** + * Host Claude Code **plugins**, handed to each keeper as `agent.plugins` (#700). + * + * The half of MCP inheritance that `claude-mcp.ts` structurally could not reach. + * A plugin bundles commands, agents, skills, hooks and — the reported symptom — + * MCP servers, and none of it is declared in `~/.claude.json`, so reading that + * file finds nothing however carefully it is read. + * + * ## Why the symlink was not enough + * + * `plugins/` IS bridged into paddock's home (under `claude.instructions: host`, + * see `claude-home.ts`) and the SDK really does auto-discover plugins under + * `join(CLAUDE_CONFIG_DIR, "plugins")` — two facts that together made this look + * fixed for months. It is not, because discovery of an *installed* plugin is + * driven by the `enabledPlugins` key, and `enabledPlugins` lives in the **user** + * settings source, which herdctl does not load: every agent with a working + * directory is invoked with `setting_sources: ["project"]`. So the plugin is + * found and then never enabled, silently. + * + * herdctl 5.32.0 (herdctl#446, closing herdctl#444) adds the channel this needs: + * an optional `plugins` array on the agent config, translated to the SDK's own + * `plugins` option and to `--plugin-dir` on the CLI runtime. A plugin passed that + * way is a *session* plugin, and — read out of the CLI bundle rather than + * assumed — a session plugin's enablement is + * `enabledPlugins[`${name}@inline`] ?? manifest.defaultEnabled !== false`, i.e. + * **enabled by default**, with no settings-source grant required. That is why the + * passthrough works where the symlink did not, and why herdctl#446 was right to + * refuse to widen `setting_sources` instead. + * + * ## Which lever gates this, which is NOT the one #700 assumes + * + * #700 (and the brief written from it) scope plugin enumeration to + * `claude.mcpServers: host`. That is half right, and shipping it that way would + * have punched a hole through a different lever: + * + * - `plugins/` is bridged by **`claude.instructions`**, alongside `CLAUDE.md`, + * `agents/` and `commands/` — because most of a plugin IS instructions. Under + * `instructions: own` paddock withdraws that symlink and prints a notice saying + * the host's plugins are not loaded. Passing them anyway because a *different* + * key says `host` would contradict a notice paddock itself emits. + * - the MCP servers inside a plugin are what `claude.mcpServers` is about. + * + * So both levers apply, to the two halves of a plugin, and the SDK has exactly + * the flag needed to split them: `skipMcpDiscovery` loads a plugin's + * skills/hooks/agents/commands but does NOT read its `.mcp.json` or manifest + * `mcpServers`. Hence {@link enumerateHostPlugins}: + * + * | `instructions` | `mcpServers` | result | + * |---|---|---| + * | `host` | `host` | plugins passed whole | + * | `host` | `own` | plugins passed with `skipMcpDiscovery: true` | + * | `own` | *any* | no plugins, and a notice saying which key turns them on | + * + * ## The allowlist, which is where this gets uncomfortable + * + * Both runtimes auto-deny any tool missing from an explicit `allowed_tools`, with + * no prompt and nothing in the logs — the trap `claude-mcp.ts` documents at + * length. A plugin-provided MCP server needs the same widening, so paddock has to + * know its tool prefix *before* the plugin has ever run. + * + * It is knowable, from the CLI bundle: a plugin's servers are registered under + * `` `plugin:${pluginName}:${serverName}` ``, and a server name is normalised + * with `[^a-zA-Z0-9_-] → _` before it becomes a tool prefix. So the pattern is + * `mcp__plugin____*` — matching the SDK's own documented example, + * `mcp__plugin_documents_docs__doc_export`. {@link pluginMcpToolPattern} is that + * one line, and {@link readPluginServerNames} recovers `` by reading the + * two places a plugin declares servers statically: `/.mcp.json` and an + * inline `mcpServers` object in `/.claude-plugin/plugin.json`. + * + * What it CANNOT recover is a manifest whose `mcpServers` is a string or an array + * of strings — a pointer to another file, or to an MCPB source the CLI downloads. + * Resolving a same-directory JSON pointer is easy and is done; anything else is + * recorded as a caveat and warned about at boot, naming the plugin, because the + * failure it produces otherwise is the silent one: the plugin loads, its server + * connects, and every tool call is denied with no log line. Better a warning that + * says "add `mcp__plugin_x_y__*` yourself" than a mystery. + * + * ## What is read, and what is not verified + * + * Enumeration is from `/installed_plugins.json`, the CLI's own + * registry of installs (v2: `{plugins: {"@": [{installPath, + * scope, …}]}}`; v1 keyed the same ids to a single record, whose path was derived + * as `/cache///`). Both are read. The + * host's `settings.json` `enabledPlugins` is consulted only to VETO — an id set + * to `false` is skipped — because a session plugin's own default is "enabled", + * so honouring the veto is the only thing that can respect a host's `/plugin + * disable`. + * + * None of this was validated against a machine with plugins installed: this box + * is Linux with an empty plugin root, and no test here proves a plugin *loads*. + * Every path, file name and naming rule above comes from reading the bundled CLI + * (`node_modules/@anthropic-ai/claude-agent-sdk-linux-x64/claude`), and the tests + * assert what config paddock produces from a planted directory tree, which is a + * strictly weaker claim. A `marketplace`-installed plugin loading via + * `type: "local"` remains unverified end-to-end and needs a real host. + */ +import { promises as fs } from "node:fs"; +import path from "node:path"; +import type { McpServersMode } from "./claude-mcp.js"; +import type { InstructionsMode } from "./claude-instructions.js"; + +/** + * One plugin to load, in herdctl's `PluginSchema` object form. + * + * The schema also accepts a bare path string and normalises it to this, but + * paddock always needs the object because {@link HostPlugin.skipMcpDiscovery} is + * how the two levers are kept apart. + */ +export interface HostPlugin { + type: "local"; + path: string; + skipMcpDiscovery?: boolean; +} + +/** Something about a host plugin worth telling the operator at boot. */ +export interface HostPluginCaveat { + /** The plugin's id (`@`) or its directory name. */ + name: string; + /** `dropped` = not passed at all; `degraded` = passed, but incompletely. */ + kind: "dropped" | "degraded"; + reason: string; +} + +/** What {@link enumerateHostPlugins} found. */ +export interface HostPluginSource { + /** Ready for `agent.plugins`, in registry order. */ + plugins: HostPlugin[]; + /** + * `mcp____*` patterns for every plugin-provided server paddock could + * name statically. Empty whenever MCP discovery is off. + */ + toolPatterns: string[]; + caveats: HostPluginCaveat[]; +} + +/** Nothing inherited: `instructions: own`, and where tests start. */ +export const EMPTY_HOST_PLUGINS: HostPluginSource = Object.freeze({ + plugins: [], + toolPatterns: [], + caveats: [], +}); + +/** The plugin root, `join(CLAUDE_CONFIG_DIR, "plugins")` in the CLI's terms. */ +export function hostPluginRoot(legacyClaudeHome: string): string { + return path.join(legacyClaudeHome, "plugins"); +} + +/** The CLI's own registry of what is installed, under {@link hostPluginRoot}. */ +export const INSTALLED_PLUGINS_FILE = "installed_plugins.json"; + +/** A plugin's manifest, relative to its directory. */ +export const PLUGIN_MANIFEST = path.join(".claude-plugin", "plugin.json"); + +/** A plugin's out-of-manifest MCP declaration, relative to its directory. */ +export const PLUGIN_MCP_FILE = ".mcp.json"; + +/** + * The CLI's server-name normalisation: `[^a-zA-Z0-9_-] → _`, applied to the whole + * `plugin::` name before it becomes a `mcp__…__` prefix. Applied + * to the joined string, not to the parts, so a `.` in a plugin name normalises + * the same way the separators do. + */ +function normalizeServerName(name: string): string { + return name.replace(/[^a-zA-Z0-9_-]/g, "_"); +} + +/** + * The allowlist pattern for one server of one plugin. + * + * Deliberately not `mcpToolPattern` from `claude-mcp.ts`: the name a plugin's + * server is registered under is not the name it is declared under, and that + * derivation is the whole content of this function. + */ +export function pluginMcpToolPattern(pluginName: string, serverName: string): string { + return `mcp__${normalizeServerName(`plugin:${pluginName}:${serverName}`)}__*`; +} + +function isRecord(v: unknown): v is Record { + return typeof v === "object" && v !== null && !Array.isArray(v); +} + +async function readJson(file: string): Promise { + try { + return JSON.parse(await fs.readFile(file, "utf8")); + } catch { + return undefined; + } +} + +/** + * The server names one `.mcp.json`-shaped document declares. + * + * Claude Code accepts both `{mcpServers: {…}}` and a bare map at the top level, + * so both are read — the CLI does `parsed.mcpServers || parsed`. + */ +function serverNamesOf(doc: unknown): string[] { + if (!isRecord(doc)) return []; + const servers = isRecord(doc.mcpServers) ? doc.mcpServers : doc; + return Object.keys(servers).filter((k) => isRecord(servers[k])); +} + +/** One plugin's name and the MCP servers it declares, as far as they are static. */ +export interface PluginServerNames { + /** `manifest.name`, else the directory name — the CLI's own fallback order. */ + pluginName: string; + /** Server names as declared, BEFORE the `plugin::` prefixing. */ + servers: string[]; + /** + * A `mcpServers` the manifest points at rather than spells out, and that could + * not be resolved to a file inside the plugin directory. Non-empty means the + * allowlist for this plugin is incomplete. + */ + unresolved: string[]; +} + +/** + * Read one plugin directory's declared MCP server names. + * + * Mirrors the CLI's own discovery order — `/.mcp.json` first, then whatever + * `manifest.mcpServers` adds — with the one deliberate gap described in the + * module doc: a manifest that points at an MCPB source (or at any path outside + * its own directory) is recorded in {@link PluginServerNames.unresolved} rather + * than fetched. Paddock does not download anything to build an allowlist. + */ +export async function readPluginServerNames(dir: string): Promise { + const manifest = await readJson(path.join(dir, PLUGIN_MANIFEST)); + const pluginName = + isRecord(manifest) && typeof manifest.name === "string" && manifest.name.trim() !== "" + ? manifest.name.trim() + : path.basename(dir); + const servers = new Set(serverNamesOf(await readJson(path.join(dir, PLUGIN_MCP_FILE)))); + const unresolved: string[] = []; + + const declared = isRecord(manifest) ? manifest.mcpServers : undefined; + // A string entry is a path relative to the plugin directory (or an MCPB + // source, which looks the same and is not resolvable without downloading it). + // Resolve it only when it stays inside the directory and parses as JSON; + // anything else is a gap paddock reports rather than guesses at. + const resolveRef = async (ref: string): Promise => { + const target = path.resolve(dir, ref); + if (target !== dir && !target.startsWith(dir + path.sep)) { + unresolved.push(ref); + return; + } + const names = serverNamesOf(await readJson(target)); + if (names.length === 0) unresolved.push(ref); + else for (const n of names) servers.add(n); + }; + + if (typeof declared === "string") await resolveRef(declared); + else if (Array.isArray(declared)) { + for (const entry of declared) { + if (typeof entry === "string") await resolveRef(entry); + else for (const n of serverNamesOf(entry)) servers.add(n); + } + } else if (isRecord(declared)) { + for (const n of serverNamesOf(declared)) servers.add(n); + } + + return { pluginName, servers: [...servers], unresolved }; +} + +/** One installed plugin: its id, and where it landed on disk. */ +interface InstalledPlugin { + id: string; + installPath: string; +} + +/** + * Parse `installed_plugins.json`, v1 or v2. Pure, so the whole selection is + * testable from a string. + * + * v2 keys each `@` id to an ARRAY of installations (one per + * scope), each carrying an explicit `installPath`. v1 keyed it to a single record + * with no path, which the CLI derives as + * `/cache///` with each segment sanitised + * `[^a-zA-Z0-9\-_] → -` (and the version segment additionally allowing `.`). + * + * Fails soft in every direction: this is a file paddock does not own and must + * never turn into a boot failure. + */ +export function parseInstalledPlugins(raw: string, root: string): InstalledPlugin[] { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return []; + } + if (!isRecord(parsed) || !isRecord(parsed.plugins)) return []; + const out: InstalledPlugin[] = []; + for (const [id, entry] of Object.entries(parsed.plugins)) { + if (Array.isArray(entry)) { + for (const install of entry) { + if (isRecord(install) && typeof install.installPath === "string" && install.installPath) { + out.push({ id, installPath: install.installPath }); + } + } + } else if (isRecord(entry)) { + const derived = legacyInstallPath(root, id, entry.version); + if (derived) out.push({ id, installPath: derived }); + } + } + return out; +} + +/** The v1 path derivation, kept beside its only caller. */ +function legacyInstallPath(root: string, id: string, version: unknown): string | undefined { + if (typeof version !== "string" || version === "") return undefined; + const at = id.lastIndexOf("@"); + const name = at > 0 ? id.slice(0, at) : id; + const marketplace = at > 0 ? id.slice(at + 1) : "unknown"; + const seg = (s: string) => s.replace(/[^a-zA-Z0-9\-_]/g, "-"); + let ver = version.replace(/[^a-zA-Z0-9\-_.]/g, "-"); + if (ver === "." || ver === "..") ver = "-"; + return path.join(root, "cache", seg(marketplace || "unknown"), seg(name), ver); +} + +/** Ids the host has explicitly turned off, from its `settings.json`. */ +function disabledIds(settings: unknown): Set { + const out = new Set(); + if (!isRecord(settings) || !isRecord(settings.enabledPlugins)) return out; + for (const [id, value] of Object.entries(settings.enabledPlugins)) { + // Anything that is not an explicit `false` counts as enabled: the key also + // takes an extended object form carrying a version constraint, and a version + // constraint is not a disable. + if (value === false) out.add(id.toLowerCase()); + } + return out; +} + +/** A line for the boot log, at a level (mirrors `HostMcpNotice`). */ +export interface HostPluginNotice { + level: "info" | "warn"; + message: string; +} + +/** What {@link loadHostPlugins} found, ready for the agent config and the log. */ +export interface HostPluginReport { + source: HostPluginSource; + notices: HostPluginNotice[]; +} + +/** + * Enumerate the host's installed plugin directories. Pure apart from reading the + * plugin tree, and takes the two files' contents rather than paths so the whole + * selection is testable without a home directory. + */ +export async function enumerateHostPlugins(opts: { + root: string; + installed: string | undefined; + settings: unknown; + /** Whether the plugins' own MCP servers come too. */ + mcp: boolean; +}): Promise { + const caveats: HostPluginCaveat[] = []; + if (opts.installed === undefined) return { plugins: [], toolPatterns: [], caveats }; + const disabled = disabledIds(opts.settings); + const plugins: HostPlugin[] = []; + const toolPatterns = new Set(); + // One plugin can be recorded under several scopes, and two ids can in principle + // resolve to one directory. `--plugin-dir` the same path twice and the CLI loads + // it twice under one name, so collapse them here. + const seen = new Set(); + + for (const install of parseInstalledPlugins(opts.installed, opts.root)) { + if (disabled.has(install.id.toLowerCase())) continue; + if (seen.has(install.installPath)) continue; + seen.add(install.installPath); + // A registry entry whose directory is gone is the normal aftermath of a + // hand-deleted plugin. The SDK would log "Plugin path does not exist" and + // carry on, but saying it here is what makes an empty `plugins` explicable. + try { + if (!(await fs.stat(install.installPath)).isDirectory()) throw new Error("not a directory"); + } catch { + caveats.push({ + name: install.id, + kind: "dropped", + reason: `its recorded install directory (${install.installPath}) is missing`, + }); + continue; + } + plugins.push( + opts.mcp + ? { type: "local", path: install.installPath } + : { type: "local", path: install.installPath, skipMcpDiscovery: true }, + ); + if (!opts.mcp) continue; + + const { pluginName, servers, unresolved } = await readPluginServerNames(install.installPath); + for (const server of servers) toolPatterns.add(pluginMcpToolPattern(pluginName, server)); + if (unresolved.length > 0) { + caveats.push({ + name: install.id, + kind: "degraded", + reason: + `its manifest points \`mcpServers\` at ${unresolved.join(", ")} rather than declaring ` + + `them inline, so paddock cannot name its servers — the plugin is attached, but any ` + + `tool it provides will be auto-denied with no prompt until you add the matching ` + + `\`mcp__plugin_${normalizeServerName(pluginName)}___*\` to the keeper's ` + + `allowed tools`, + }); + } + } + return { plugins, toolPatterns: [...toolPatterns], caveats }; +} + +/** + * Read the host's plugin registry — once, at boot — and say what came of it. + * + * Under `instructions: own` nothing is opened at all, for the same reason + * `loadHostMcpSource` does not open `.claude.json` under `mcpServers: own`: "own + * everywhere means nothing outside the data dir is read" is a guarantee that has + * to be true in the place someone would check it. + */ +export async function loadHostPlugins(cfg: { + legacyClaudeHome: string; + claude: { instructions: InstructionsMode; mcpServers: McpServersMode }; +}): Promise { + const notices: HostPluginNotice[] = []; + const mcp = cfg.claude.mcpServers === "host"; + if (cfg.claude.instructions !== "host") { + if (mcp) { + // The one combination that silently does less than a user asked for, so it + // is the one that gets a line naming the other key. + notices.push({ + level: "info", + message: + "`claude.mcpServers: host` does not reach MCP servers provided by a Claude Code " + + "plugin: a plugin is mostly commands, agents and skills, so loading one is a " + + "`claude.instructions` decision. Set `claude.instructions: host` (or " + + "PADDOCK_CLAUDE_INSTRUCTIONS=host) as well to inherit your plugins.", + }); + } + return { source: EMPTY_HOST_PLUGINS, notices }; + } + + const root = hostPluginRoot(cfg.legacyClaudeHome); + const file = path.join(root, INSTALLED_PLUGINS_FILE); + let installed: string | undefined; + try { + installed = await fs.readFile(file, "utf8"); + } catch { + // Not a warning: no plugins installed is the overwhelmingly common case, and + // this file does not exist until the first `claude plugin install`. + return { source: EMPTY_HOST_PLUGINS, notices }; + } + const settings = await readJson(path.join(cfg.legacyClaudeHome, "settings.json")); + const source = await enumerateHostPlugins({ root, installed, settings, mcp }); + + if (source.plugins.length > 0) { + notices.push({ + level: "info", + message: + `Claude plugins: host (\`claude.instructions: host\`) — ${source.plugins.length} ` + + `installed plugin${source.plugins.length === 1 ? "" : "s"} attached to every keeper` + + (mcp + ? source.toolPatterns.length > 0 + ? `, including their MCP servers (${source.toolPatterns.join(", ")})` + : ", including their MCP servers (none declared)" + : ". Their MCP servers are NOT attached (`claude.mcpServers: own`)") + + ".", + }); + } + for (const caveat of source.caveats) { + notices.push({ + level: "warn", + message: + caveat.kind === "dropped" + ? `Claude plugin "${caveat.name}" was NOT attached: ${caveat.reason}.` + : `Claude plugin "${caveat.name}" is attached but incomplete: ${caveat.reason}.`, + }); + } + return { source, notices }; +} diff --git a/packages/server/src/herdctl-agent-config.ts b/packages/server/src/herdctl-agent-config.ts index eabfa5cc..2406f29e 100644 --- a/packages/server/src/herdctl-agent-config.ts +++ b/packages/server/src/herdctl-agent-config.ts @@ -21,6 +21,7 @@ import { mcpToolPattern, type McpSources, } from "./claude-mcp.js"; +import { EMPTY_HOST_PLUGINS, type HostPluginSource } from "./claude-plugins.js"; import { DEFAULT_MODEL, SWEEPER_DEFAULT_MODEL, @@ -77,12 +78,18 @@ import { * which is both the isolated mode and what every existing caller means. The * KEEPER is the only agent that gets them: the sweeper is tool-less and each * trigger declares its own narrow allowlist, so neither could call one anyway. + * + * `hostPlugins` is the same story for the host's Claude Code plugins (#700), + * resolved once at boot by `claude-plugins.ts` and gated by `claude.instructions` + * rather than `claude.mcpServers` — see that module for why. Keeper-only for the + * same reasons, and empty unless the lever is on. */ export function buildAgentConfig( cfg: PaddockConfig, project: Project, modelOverride?: string, mcpSources: McpSources = EMPTY_MCP_SOURCES, + hostPlugins: HostPluginSource = EMPTY_HOST_PLUGINS, ): Record & { name: string } { const config: Record & { name: string } = { name: keeperAgentName(project.slug), @@ -172,9 +179,18 @@ export function buildAgentConfig( // Patterns already on the defaults are filtered out, so nothing is restated // for an instance with no external servers (or one whose only server paddock's // own overrode above) — that config stays byte-identical to before this lever. - const extra = Object.keys(external) - .map(mcpToolPattern) - .filter((pattern) => !FLEET_ALLOWED_TOOLS.includes(pattern)); + // + // The host's Claude Code PLUGINS ride on the same two mechanisms (#700, and the + // `plugins` passthrough herdctl 5.32.0 added for it). They are a separate key + // rather than more `mcp_servers` because a plugin is a directory the engine + // loads, not a server paddock can describe — but their servers land in the same + // tool namespace and need the same allowlist widening, under names paddock has + // to derive rather than read (`claude-plugins.ts`). + if (hostPlugins.plugins.length > 0) config.plugins = hostPlugins.plugins; + const extra = [ + ...Object.keys(external).map(mcpToolPattern), + ...hostPlugins.toolPatterns, + ].filter((pattern) => !FLEET_ALLOWED_TOOLS.includes(pattern)); if (extra.length > 0) config.allowed_tools = [...FLEET_ALLOWED_TOOLS, ...extra]; return config; } diff --git a/packages/server/src/herdctl.ts b/packages/server/src/herdctl.ts index 13defe5e..53e4e141 100644 --- a/packages/server/src/herdctl.ts +++ b/packages/server/src/herdctl.ts @@ -89,6 +89,7 @@ import { ensureConfigFile as writeBootConfigFile, } from "./herdctl-agent-config.js"; import { EMPTY_MCP_SOURCES, type McpSources } from "./claude-mcp.js"; +import { EMPTY_HOST_PLUGINS, type HostPluginSource } from "./claude-plugins.js"; import * as jobs from "./herdctl-jobs.js"; import { JobsDirIndex } from "./herdctl-jobs-index.js"; import { AdoptableIndex, type AdoptableSummary, type FilterReason } from "./adoptable.js"; @@ -368,10 +369,15 @@ export class HerdctlService { * all under `mcpServers: own`. Defaults to empty so every existing caller — * including the several tests that construct this with `{} as PaddockConfig` — * keeps meaning "isolated". + * + * `hostPlugins` is the host's Claude Code plugins (#700), read at boot by + * `loadHostPlugins` under `claude.instructions: host`, and handed in for + * exactly the same reasons. */ constructor( private readonly cfg: PaddockConfig, private readonly mcpSources: McpSources = EMPTY_MCP_SOURCES, + private readonly hostPlugins: HostPluginSource = EMPTY_HOST_PLUGINS, ) {} /** @@ -1742,7 +1748,13 @@ export class HerdctlService { project: Project, modelOverride?: string, ): Record & { name: string } { - return buildAgentConfig(this.cfg, project, modelOverride, this.mcpSources); + return buildAgentConfig( + this.cfg, + project, + modelOverride, + this.mcpSources, + this.hostPlugins, + ); } private sweeperAgentConfig(project: Project): Record & { name: string } { diff --git a/packages/server/src/mcp-servers.ts b/packages/server/src/mcp-servers.ts index 65e47772..912ecfcf 100644 --- a/packages/server/src/mcp-servers.ts +++ b/packages/server/src/mcp-servers.ts @@ -48,27 +48,39 @@ * Unlike a management token, an MCP `env` entry is not necessarily secret * (`NOTION_VERSION: "2022-06-28"` is not), so an inline value is a WARNING and * not an error — but the warning is emitted for any key that looks like a - * credential, and nothing in this module ever prints a value. Every diagnostic - * names keys, variable names and server names only; a `url` is reported with its - * query string and userinfo stripped, because that is where an API key rides. - * {@link describeServer} is the single place a server is rendered for a human, - * so there is one function to audit rather than a scattering of template strings. + * credential, and nothing in this module ever prints a value. `headers` follows + * the same rule and for a stronger reason: `Authorization` is the usual reason a + * header is declared at all. Every diagnostic names keys, variable names and + * server names only; a `url` is reported with its query string and userinfo + * stripped, because that is where an API key rides. {@link describeServer} is the + * single place a server is rendered for a human, so there is one function to + * audit rather than a scattering of template strings. * * The resolved values do land in the frozen `PaddockConfig` (as management tokens * already do). They must never reach an API response: `instance-config.ts` * publishes only the fields in its own `FIELDS` table, and this block is * deliberately absent from it. * + * One exposure the `env:` indirection does NOT close: under `driveMode: batch` + * the resolved value reaches a COMMAND LINE. See {@link argvExposure} — and note + * that carrying `headers` (#700) widens it, because an `Authorization` bearer is + * a likelier long-lived credential than an `env` entry. + * * ## Validation is strict, and drops rather than degrades * * Step 5 passes a host server it cannot carry faithfully, with a warning, on the * grounds that a user who declared a server elsewhere should get it plus a * warning rather than nothing plus a warning. That reasoning inverts here, * because the user typed this file: an unusable declaration is a mistake they can - * fix, and a `headers:` block that is silently dropped is an authentication - * failure that looks like a broken server. So anything that cannot be carried is - * an ERROR and that server is not attached — including an unknown key, which is - * how a typo (`arg:` for `args:`) otherwise becomes a server that starts wrong. + * fix. So anything that cannot be carried is an ERROR and that server is not + * attached — including an unknown key, which is how a typo (`arg:` for `args:`) + * otherwise becomes a server that starts wrong. + * + * `headers:` and `type: sse` USED to be in that category, because herdctl's + * schema had no field for either. herdctl 5.32.0 (#446) carries both verbatim, + * so refusing them would now reject a declaration that works — they are accepted, + * and `headers` is a first-class secret-bearing field with the same `env:VAR` + * resolution and the same never-print rule as `env`. * * Never a boot failure, though: one bad server drops itself and the rest attach, * matching `resolveManagementApiConfig`. A typo in a capability must not take an @@ -85,11 +97,11 @@ export interface McpServerConfigFile { args?: string[]; /** Environment for the server process. Values may be `env:VAR_NAME`. */ env?: Record; - /** Endpoint for a streamable-HTTP server. Mutually exclusive with {@link command}. */ + /** Endpoint for a remote server. Mutually exclusive with {@link command}. */ url?: string; - /** Accepted only as `stdio`/`http`, and only when it agrees with the above. */ + /** `stdio` for a {@link command}; `http` or `sse` for a {@link url}. */ type?: string; - /** Present only in a MISCONFIGURED file: the engine cannot carry headers. */ + /** Headers for a remote server. Values may be `env:VAR_NAME`. */ headers?: Record; } @@ -177,10 +189,12 @@ export function redactUrl(raw: string): string { export function describeServer(name: string, def: McpServerDef): string { const parts: string[] = []; if (def.command) parts.push(`stdio: ${def.command.split(/[\\/]/).pop()}`); - if (def.url) parts.push(`http: ${redactUrl(def.url)}`); + if (def.url) parts.push(`${def.type ?? "http"}: ${redactUrl(def.url)}`); if (def.args?.length) parts.push(`${def.args.length} args`); const envCount = Object.keys(def.env ?? {}).length; if (envCount > 0) parts.push(`${envCount} env ${envCount === 1 ? "entry" : "entries"}`); + const headerCount = Object.keys(def.headers ?? {}).length; + if (headerCount > 0) parts.push(`${headerCount} ${headerCount === 1 ? "header" : "headers"}`); return `${name} (${parts.join(", ")})`; } @@ -226,18 +240,18 @@ interface NarrowResult { * Validate + resolve one declaration into the shape herdctl's `McpServerSchema` * carries, or explain why it cannot be. * - * The rejections are the interesting part, and each is a thing step 5 discovered - * the engine strips silently: + * The one rejection left that is about the engine rather than about the file is + * **an unknown key** — the only defence against `arg:`/`envs:`/`commands:`, which + * would otherwise yield a server that starts with the wrong argv. * - * - **`headers`** — there is no field for them, so a bearer-authenticated server - * would arrive unauthenticated AND miss its stored OAuth token, which is keyed - * on a hash that includes the headers. Step 5 passes such a host server with a - * warning; a hand-written declaration gets an error instead, because the user - * typed the header and can act on being told it does nothing. - * - **`type: sse`** — every `url` is mapped to `type: "http"` downstream, so an - * sse server is silently downgraded, with the same OAuth-key consequence. - * - **an unknown key** — the only defence against `arg:`/`envs:`/`commands:`, - * which would otherwise yield a server that starts with the wrong argv. + * `headers` and `type: sse` were rejections too until herdctl 5.32.0, because + * the schema had no field for either: a bearer-authenticated server arrived + * unauthenticated and missed its stored OAuth token (keyed on a hash that + * includes the headers and the type), and an `sse` url was silently connected to + * as HTTP. Both are now carried verbatim, so both are accepted — `headers` + * through the same `env:VAR` resolution as `env`, and `type` checked only for + * agreeing with the declaration (`stdio` for a `command`, `http`/`sse` for a + * `url`). */ function narrowDeclaration( name: string, @@ -269,20 +283,11 @@ function narrowDeclaration( const unknown = Object.keys(raw).filter((k) => !KNOWN_KEYS.has(k)); if (unknown.length > 0) { return fail( - `${where}: unrecognised key(s) ${unknown.join(", ")} (supported: command, args, env, url) ` + + `${where}: unrecognised key(s) ${unknown.join(", ")} ` + + `(supported: command, args, env, url, type, headers) ` + `— a mistyped key would otherwise be dropped without a trace`, ); } - if (raw.headers !== undefined) { - const keys = isRecord(raw.headers) ? Object.keys(raw.headers) : []; - return fail( - `${where}: \`headers\`${keys.length > 0 ? ` (${keys.join(", ")})` : ""} cannot be ` + - `carried — the engine's MCP schema has no field for them, so the server would arrive ` + - `unauthenticated and its stored OAuth token (keyed on a hash that includes the headers) ` + - `would not be found either`, - ); - } - const hasCommand = typeof raw.command === "string" && raw.command.trim().length > 0; const hasUrl = typeof raw.url === "string" && raw.url.trim().length > 0; if (hasCommand && hasUrl) { @@ -296,26 +301,25 @@ function narrowDeclaration( ); } + const server: McpServerDef = {}; + if (raw.type !== undefined) { const type = String(raw.type).trim().toLowerCase(); - if (type === "sse") { - return fail( - `${where}: \`type: sse\` cannot be carried — every \`url\` server is connected to as ` + - `streamable HTTP downstream, so an sse server would be silently downgraded (and its ` + - `stored OAuth token, keyed on a hash that includes the type, not found). Declare it ` + - `as \`http\` if the server supports it`, - ); - } - const expected = hasUrl ? "http" : "stdio"; - if (type !== expected) { + // `sse` joined `http` here in herdctl 5.32.0; an explicit type now wins over + // the bare-`url` ⇒ `http` inference rather than being stripped on the way + // down. It still has to agree with the rest of the declaration, because a + // `type` that disagrees is a typo and starting the wrong transport is a + // confusing failure rather than a loud one. + const expected = hasUrl ? ["http", "sse"] : ["stdio"]; + if (!expected.includes(type)) { return fail( - `${where}: \`type: ${type}\` disagrees with the declaration (expected ${expected})`, + `${where}: \`type: ${type}\` disagrees with the declaration ` + + `(expected ${expected.join(" or ")})`, ); } + server.type = type as McpServerDef["type"]; } - const server: McpServerDef = {}; - if (hasCommand) { const res = resolveLeaf((raw.command as string).trim(), env, `${where}.command`); if (!res.ok) return { errors, warnings: [...warnings, `${res.error} — server not attached`] }; @@ -325,9 +329,9 @@ function narrowDeclaration( const res = resolveLeaf((raw.url as string).trim(), env, `${where}.url`); if (!res.ok) return { errors, warnings: [...warnings, `${res.error} — server not attached`] }; server.url = res.value; - // A key in the query string is the usual way a remote MCP endpoint is - // authenticated now that `headers` cannot be carried, and this file is - // git-tracked. Say so once; never echo the query itself. + // A key in the query string is one of the two ways a remote MCP endpoint is + // authenticated (the other, `headers`, is carryable again since #700), and + // this file is git-tracked. Say so once; never echo the query itself. if (!(raw.url as string).startsWith(ENV_REF_PREFIX)) { let parsed: URL | undefined; try { @@ -381,6 +385,35 @@ function narrowDeclaration( if (Object.keys(resolved).length > 0) server.env = resolved; } + if (raw.headers !== undefined) { + if (!hasUrl) { + return fail(`${where}.headers: only a \`url\` server can carry headers`); + } + if (!isRecord(raw.headers)) { + return fail(`${where}.headers: must be a mapping of header name to value`); + } + const resolved: Record = {}; + for (const [key, value] of Object.entries(raw.headers)) { + if (typeof value !== "string") { + return fail(`${where}.headers.${key}: must be a string`); + } + const res = resolveLeaf(value, env, `${where}.headers.${key}`); + if (!res.ok) return { errors, warnings: [...warnings, `${res.error} — server not attached`] }; + resolved[key] = res.value; + // Same heuristic as `env`, one rule to learn — and `Authorization` is the + // usual reason a header is declared at all, so this fires on the common + // case rather than the exotic one. + if (!value.startsWith(ENV_REF_PREFIX) && SECRET_ISH_KEY_RE.test(key)) { + warnings.push( + `${where}.headers.${key}: looks like a credential and is written into the config file ` + + `itself, which is git-tracked — prefer \`${key}: ${ENV_REF_PREFIX}VAR_NAME\` and set ` + + `the value in the environment`, + ); + } + } + if (Object.keys(resolved).length > 0) server.headers = resolved; + } + return { server, errors, warnings }; } @@ -436,8 +469,9 @@ export function resolveDeclaredMcpServers( * * A process ARGUMENT is not private on Linux: `/proc//cmdline` is * world-readable by default (no `hidepid`), and `ps` prints it. So on the CLI - * runtime every declared server's `env` — the API token included — is legible to - * any local user for the lifetime of each `claude` invocation. Observed, not + * runtime every declared server's `env` — and, since #700 made the field + * carryable, its `headers` — is legible to any local user for the lifetime of + * each `claude` invocation. Observed, not * inferred: `test/integration/declared-mcp-argv.test.ts` drives a real turn and * reads the token back out of the spawned process's argv. * @@ -463,7 +497,8 @@ function argvExposure(names: readonly string[], driveMode: DriveMode): DeclaredM return { level: batch ? "warn" : "info", message: - `${names.join(", ")} ${names.length === 1 ? "declares" : "declare"} \`env\` values, and ` + + `${names.join(", ")} ${names.length === 1 ? "declares" : "declare"} \`env\` values or ` + + `\`headers\`, and ` + `under \`driveMode: batch\` the engine passes the whole server definition to \`claude\` ` + `as a \`--mcp-config\` COMMAND-LINE argument — where any local process can read it via ` + `/proc//cmdline. ` + @@ -523,7 +558,16 @@ export function declaredMcpNotices(opts: { `~/.claude.json (\`claude.mcpServers: host\`); this instance's own declaration wins.`, }); } - const exposed = names.filter((n) => Object.keys(opts.servers[n].env ?? {}).length > 0); + // `headers` joined `env` here in #700: herdctl 5.32.0 carries them, so an + // `Authorization` bearer now rides in the same `--mcp-config` argv element that + // #702 found an `env` token in — and a bearer is the likelier long-lived + // credential of the two. A server declared with headers and no env would + // otherwise be the one case this warning missed. + const exposed = names.filter( + (n) => + Object.keys(opts.servers[n].env ?? {}).length > 0 || + Object.keys(opts.servers[n].headers ?? {}).length > 0, + ); if (exposed.length > 0 && opts.driveMode !== undefined) { // The one place a declared credential escapes paddock, and it escapes // downstream of every rule this module enforces — see {@link argvExposure}. diff --git a/packages/server/test/helpers/app.ts b/packages/server/test/helpers/app.ts index 7eced55f..c39de00b 100644 --- a/packages/server/test/helpers/app.ts +++ b/packages/server/test/helpers/app.ts @@ -68,6 +68,17 @@ interface StartOptions { * Always synthetic: the real one on a dev box holds a user's own servers. */ hostClaudeJson?: Record; + /** + * Synthetic host Claude Code PLUGINS (#700), planted under + * `/.claude/plugins/`: one `installed_plugins.json` registry entry per + * key, each pointing at a directory with a `.claude-plugin/plugin.json` and + * (when given) a `.mcp.json`. + * + * Always synthetic, and nothing ever runs them — a plugin directory is data + * until a turn loads it, and no turn runs in these tests. The real plugin root + * on a dev box holds a user's own plugins and their credentials. + */ + hostPlugins?: Record }>; /** * Extra environment variables to set before build (restored on teardown). * Needed for config that is REFERENCED from the YAML rather than inlined — @@ -225,6 +236,36 @@ export async function startTestApp(opts: StartOptions = {}): Promise { ); } + // The host's plugins (#700). `/.claude/plugins/`, enumerated from the + // CLI's own `installed_plugins.json` registry rather than by scanning — see + // `claude-plugins.ts` for why that file is the source of truth. + if (opts.hostPlugins) { + const root = path.join(home, ".claude", "plugins"); + const registry: Record = {}; + for (const [name, decl] of Object.entries(opts.hostPlugins)) { + const dir = path.join(root, "cache", "test-marketplace", name, "1.0.0"); + await fs.mkdir(path.join(dir, ".claude-plugin"), { recursive: true }); + await fs.writeFile( + path.join(dir, ".claude-plugin", "plugin.json"), + JSON.stringify({ name }), + "utf8", + ); + if (decl.mcpServers) { + await fs.writeFile( + path.join(dir, ".mcp.json"), + JSON.stringify({ mcpServers: decl.mcpServers }), + "utf8", + ); + } + registry[`${name}@test-marketplace`] = [{ scope: "user", installPath: dir }]; + } + await fs.writeFile( + path.join(root, "installed_plugins.json"), + JSON.stringify({ version: 2, plugins: registry }, null, 2), + "utf8", + ); + } + for (const [k, v] of Object.entries(opts.env ?? {})) process.env[k] = v; if (opts.gitRepo) { diff --git a/packages/server/test/integration/claude-mcp-servers.test.ts b/packages/server/test/integration/claude-mcp-servers.test.ts index e92f21bd..ff115c73 100644 --- a/packages/server/test/integration/claude-mcp-servers.test.ts +++ b/packages/server/test/integration/claude-mcp-servers.test.ts @@ -27,7 +27,16 @@ const HOST_CLAUDE_JSON = { // Claude Code writes plenty of unrelated state into this file; carrying some of // it proves the parse ignores what it does not own. firstStartTime: "2026-01-01T00:00:00.000Z", - mcpServers: { notion: { command: "notion-mcp-not-real", args: ["--stdio"] } }, + mcpServers: { + notion: { command: "notion-mcp-not-real", args: ["--stdio"] }, + // A header-authenticated `sse` server: the exact shape #699 had to warn about, + // and the reason this repo needs @herdctl/core >= 5.32.0. + stream: { + type: "sse", + url: "https://mcp.example.invalid/sse", + headers: { Authorization: "Bearer not-a-real-token" }, + }, + }, projects: { "/nonexistent/scoped-project": { mcpServers: { pg: { command: "pg-mcp-not-real" } } }, }, @@ -71,6 +80,11 @@ describe("integration: claude.mcpServers decides what the runtime is handed (#69 const keeper = keeperConfigFor(t, "/nonexistent/plain-project"); expect(keeper.mcp_servers).toEqual({ notion: { command: "notion-mcp-not-real", args: ["--stdio"] }, + stream: { + type: "sse", + url: "https://mcp.example.invalid/sse", + headers: { Authorization: "Bearer not-a-real-token" }, + }, }); // …and the allowlist is widened, or every one of its tools is auto-denied. expect(keeper.allowed_tools).toContain("mcp__notion__*"); @@ -86,12 +100,48 @@ describe("integration: claude.mcpServers decides what the runtime is handed (#69 }); const scoped = keeperConfigFor(t, "/nonexistent/scoped-project"); const other = keeperConfigFor(t, "/nonexistent/plain-project"); - expect(Object.keys(scoped.mcp_servers as object).sort()).toEqual(["notion", "pg"]); - expect(Object.keys(other.mcp_servers as object)).toEqual(["notion"]); + expect(Object.keys(scoped.mcp_servers as object).sort()).toEqual(["notion", "pg", "stream"]); + expect(Object.keys(other.mcp_servers as object).sort()).toEqual(["notion", "stream"]); expect(scoped.allowed_tools).toContain("mcp__pg__*"); expect(other.allowed_tools).not.toContain("mcp__pg__*"); }); + /** + * The assertion that retires #699's stripping warnings, made where the stripping + * used to happen: `AgentConfigSchema` is a `z.object`, so a field it has no slot + * for is dropped at `addAgent` with no error. Reading the agent back out of the + * LIVE fleet is therefore the only place the answer is trustworthy — the unit + * tests above pass identically against 5.31.x, where both fields are gone by + * this point and a bearer-authenticated `sse` server arrives as unauthenticated + * HTTP (and cannot find its stored OAuth token, which is keyed on a hash of + * `{type, url, headers}`). + * + * So this fails on 5.31.x and passes on 5.32.0 — verified by installing both — + * which makes it the guard for the dependency floor, not just for the code. + */ + it("keeps `headers` and `type: sse` through AgentConfigSchema (herdctl 5.32.0)", async () => { + t = await startTestApp({ + hostClaudeJson: HOST_CLAUDE_JSON, + env: { PADDOCK_CLAUDE_MCP_SERVERS: "host" }, + gitRepo: true, + }); + await t.app.inject({ + method: "POST", + url: "/api/projects", + payload: { name: "Streamed", slug: "streamed" }, + }); + const keeper = t.herdctl.manager + .getAgents() + .find((a) => a.name === keeperAgentName("streamed")) as + | { mcp_servers?: Record> } + | undefined; + expect(keeper?.mcp_servers?.stream).toEqual({ + type: "sse", + url: "https://mcp.example.invalid/sse", + headers: { Authorization: "Bearer not-a-real-token" }, + }); + }); + it("reads the key from the config file too, and boots fine with no .claude.json", async () => { // The common first run: `host` asked for, nothing to inherit yet. It must be // an unremarkable boot, not a failure — that file does not exist until the diff --git a/packages/server/test/integration/claude-plugins.test.ts b/packages/server/test/integration/claude-plugins.test.ts new file mode 100644 index 00000000..c0f5ef35 --- /dev/null +++ b/packages/server/test/integration/claude-plugins.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { startTestApp, type TestApp } from "../helpers/app.js"; +import type { Project } from "../../src/projects.js"; + +/** + * Host Claude Code plugins through a REAL boot (#700) — including the one thing + * only the live `FleetManager` can answer: whether `@herdctl/core`'s + * `AgentConfigSchema` actually keeps the `plugins` key paddock now sets. + * + * ## Why that specific assertion is the point of this file + * + * The defect class #700 belongs to is "the layer underneath silently narrowed + * what it accepts". `AgentConfigSchema` is a `z.object`, so a key it has no field + * for is STRIPPED at `addAgent` with no error — which is exactly how `headers` and + * `type: sse` were lost for as long as they were (#699), and how a `plugins` array + * would be lost if this repo were built against anything before `@herdctl/core` + * 5.32.0. Every unit test in `claude-plugins.test.ts` passes in that world. + * + * So the load-bearing test here reads the agent back out of the live fleet, AFTER + * the schema has had its say. It fails against 5.31.x and passes against 5.32.0, + * which makes it the regression guard for the dependency as much as for the code. + * + * ## What it still does not prove + * + * Nothing loads a plugin. There are no plugins on this box, the fixtures are + * planted directories with a manifest and nothing executable in them, and no turn + * runs. Whether the CLI, handed `--plugin-dir `, then loads that directory + * and connects its MCP servers is the SDK's contract and is verified by neither + * this repo nor herdctl. That needs a host with a plugin actually installed. + */ +const PLUGINS = { slack: { mcpServers: { chat: { command: "slack-mcp-not-real" } } } }; + +/** Reach the private builder the same way the other herdctl tests do. */ +function keeperConfigFor(t: TestApp, workingDir: string): Record { + const svc = t.herdctl as unknown as { + keeperAgentConfig: (p: Project, m?: string) => Record; + }; + return svc.keeperAgentConfig({ + slug: "demo", + name: "Demo", + dir: workingDir, + workingDir, + } as unknown as Project); +} + +describe("integration: host Claude Code plugins reach the runtime (#700)", () => { + let t: TestApp | undefined; + + afterEach(async () => { + await t?.teardown(); + t = undefined; + }); + + it("attaches nothing under the default levers, with a plugin sitting right there", async () => { + t = await startTestApp({ hostPlugins: PLUGINS }); + expect(t.cfg.claude.instructions).toBe("own"); + const keeper = keeperConfigFor(t, "/nonexistent/demo"); + expect(keeper.plugins).toBeUndefined(); + expect(keeper.allowed_tools).toBeUndefined(); + }); + + it("passes the plugin and its server's tool pattern under both host levers", async () => { + t = await startTestApp({ + hostPlugins: PLUGINS, + env: { PADDOCK_CLAUDE_INSTRUCTIONS: "host", PADDOCK_CLAUDE_MCP_SERVERS: "host" }, + }); + const keeper = keeperConfigFor(t, "/nonexistent/demo"); + expect(keeper.plugins).toEqual([ + { type: "local", path: expect.stringContaining("plugins/cache/test-marketplace/slack") }, + ]); + // Without this the plugin loads, its server connects, and every call to it is + // auto-denied with no prompt and nothing in the logs. + expect(keeper.allowed_tools).toContain("mcp__plugin_slack_chat__*"); + }); + + /** + * `claude.instructions` gates the plugin; `claude.mcpServers` gates only its MCP + * servers, via the SDK's own `skipMcpDiscovery` flag. This is the combination + * that says the split is real rather than decorative: the plugin's commands, + * agents and skills are inherited, its servers are not, and no tool pattern is + * granted for a namespace nothing occupies. + */ + it("loads the plugin without its servers under `mcpServers: own`", async () => { + t = await startTestApp({ + hostPlugins: PLUGINS, + env: { PADDOCK_CLAUDE_INSTRUCTIONS: "host" }, + }); + expect(t.cfg.claude.mcpServers).toBe("own"); + const keeper = keeperConfigFor(t, "/nonexistent/demo"); + expect(keeper.plugins).toEqual([ + { + type: "local", + path: expect.stringContaining("plugins/cache/test-marketplace/slack"), + skipMcpDiscovery: true, + }, + ]); + expect(keeper.allowed_tools).toBeUndefined(); + }); + + /** + * THE test, per the file header: the agent as the LIVE fleet holds it, i.e. + * after `AgentConfigSchema.parse`. A `plugins` key the schema does not know is + * dropped here and nowhere else, so this is what pins the dependency floor. + */ + it("survives AgentConfigSchema — the key is still there after addAgent", async () => { + t = await startTestApp({ + hostPlugins: PLUGINS, + env: { PADDOCK_CLAUDE_INSTRUCTIONS: "host", PADDOCK_CLAUDE_MCP_SERVERS: "host" }, + gitRepo: true, + }); + await t.app.inject({ + method: "POST", + url: "/api/projects", + payload: { name: "Plugged", slug: "plugged" }, + }); + const keeper = t.herdctl.manager.getAgents().find((a) => a.name === "keeper-plugged") as + | { plugins?: unknown[]; allowed_tools?: string[] } + | undefined; + expect(keeper?.plugins).toEqual([ + { type: "local", path: expect.stringContaining("plugins/cache/test-marketplace/slack") }, + ]); + expect(keeper?.allowed_tools).toContain("mcp__plugin_slack_chat__*"); + }); +}); diff --git a/packages/server/test/unit/claude-mcp.test.ts b/packages/server/test/unit/claude-mcp.test.ts index 958e4e1e..d0685864 100644 --- a/packages/server/test/unit/claude-mcp.test.ts +++ b/packages/server/test/unit/claude-mcp.test.ts @@ -93,45 +93,61 @@ describe("claude-mcp: parsing a .claude.json (#691)", () => { expect(Object.keys(src.byDir)).not.toContain("/home/ed/code/web"); }); - it("keeps only the four fields herdctl's McpServerSchema can carry", () => { + /** + * The inverse of the two assertions this file used to carry (#699 → #700). + * `headers` and `type` were stripped here because herdctl's `McpServerSchema` + * had no field for either and would have dropped them at `addAgent` anyway; + * carrying them was pointless and warning about the loss was the honest thing + * to do instead. herdctl 5.32.0 (#446) added both fields, so the useful + * assertion is now that they SURVIVE — and that nothing warns about them. + * + * Proven at the boundary, not inferred: with 5.32.0 installed, + * `addAgent` → `getAgents()` → `toSDKOptions()` returns + * `{type: "sse", url, headers}` unchanged. This test pins paddock's half of + * that; `packages/core`'s `mcp-and-plugin-passthrough.test.ts` pins herdctl's. + */ + it("carries `headers` and an explicit `type`, which 5.32.0 no longer strips", () => { const src = parseHostMcpConfig( JSON.stringify({ mcpServers: { remote: { - type: "http", - url: "https://mcp.example.com/mcp", + type: "sse", + url: "https://mcp.example.com/sse", headers: { Authorization: "REDACTED" }, env: { TOKEN: "x" }, }, }, }), ); - // `type` and `headers` have no field in herdctl's schema and would be - // stripped at addAgent anyway; dropping them here is what makes the loss - // reportable instead of invisible. - expect(src.user.remote).toEqual({ url: "https://mcp.example.com/mcp", env: { TOKEN: "x" } }); + expect(src.user.remote).toEqual({ + type: "sse", + url: "https://mcp.example.com/sse", + headers: { Authorization: "REDACTED" }, + env: { TOKEN: "x" }, + }); + // The whole point of the bump: no caveat, so no boot warning. An `sse` server + // authenticated with a bearer header used to produce two. + expect(src.caveats).toEqual([]); }); - it("warns that a dropped `headers` also orphans the server's OAuth token", () => { + it("still strips a key herdctl has no field for, without pretending otherwise", () => { + // `tools` (an `McpServerToolPolicy[]`) is real Claude Code config and is still + // not in `McpServerSchema`, so the narrowing is still a narrowing — it just no + // longer breaks authentication. No caveat, because unlike `headers` losing it + // degrades nothing an operator would recognise as a failure. const src = parseHostMcpConfig( - JSON.stringify({ - mcpServers: { remote: { url: "https://x/mcp", headers: { Authorization: "REDACTED" } } }, - }), + JSON.stringify({ mcpServers: { remote: { url: "https://x/mcp", tools: ["a"] } } }), ); - const caveat = src.caveats.find((c) => c.name === "remote"); - expect(caveat?.kind).toBe("degraded"); - // Not a guess: MCP tokens are stored under `mcpOAuth[`${name}|sha256({type,url,headers})`]` - // in the same credential store as the Anthropic login, so losing the headers - // changes the key and the token is not found. See `claude-mcp.ts`. - expect(caveat?.reason).toContain("OAuth token"); + expect(src.user.remote).toEqual({ url: "https://x/mcp" }); }); - it("flags an sse server, which herdctl can only connect to as http", () => { + it("ignores a `type` it does not recognise rather than passing it on", () => { + // herdctl's enum would strip it, and the bare-`url` ⇒ `http` inference is a + // better fallback than a field that vanishes one layer down. const src = parseHostMcpConfig( - JSON.stringify({ mcpServers: { stream: { type: "sse", url: "https://x/sse" } } }), + JSON.stringify({ mcpServers: { odd: { type: "websocket", url: "https://x/ws" } } }), ); - expect(src.user.stream).toEqual({ url: "https://x/sse" }); - expect(src.caveats.find((c) => c.name === "stream")?.reason).toContain("sse"); + expect(src.user.odd).toEqual({ url: "https://x/ws" }); }); it("drops — rather than degrades — a server with nothing to start", () => { @@ -248,13 +264,20 @@ describe("claude-mcp: loading from disk (#691)", () => { expect(report.notices[0].message).toContain("claude mcp add"); }); - it("warns at boot about every server it could not carry faithfully", async () => { + it("warns at boot about a server it cannot start, and only about that", async () => { + // The `degraded` warnings #699 shipped — a dropped `headers`, a coerced + // `sse` — are gone with herdctl 5.32.0. What remains is the one case that is + // genuinely unusable rather than merely narrowed. await writeHostConfig({ - mcpServers: { remote: { url: "https://x/mcp", headers: { Authorization: "REDACTED" } } }, + mcpServers: { + remote: { url: "https://x/mcp", type: "sse", headers: { Authorization: "REDACTED" } }, + broken: { args: ["--x"] }, + }, }); const report = await loadHostMcpSource(cfgFor("host")); - const warn = report.notices.find((n) => n.level === "warn"); - expect(warn?.message).toContain("remote"); - expect(warn?.message).toContain("degraded"); + const warnings = report.notices.filter((n) => n.level === "warn"); + expect(warnings).toHaveLength(1); + expect(warnings[0].message).toContain("broken"); + expect(warnings[0].message).toContain("NOT attached"); }); }); diff --git a/packages/server/test/unit/claude-plugins.test.ts b/packages/server/test/unit/claude-plugins.test.ts new file mode 100644 index 00000000..55485815 --- /dev/null +++ b/packages/server/test/unit/claude-plugins.test.ts @@ -0,0 +1,371 @@ +/** + * Host Claude Code plugin enumeration (#700) — what paddock hands herdctl as + * `agent.plugins`, and the allowlist patterns it derives for the MCP servers a + * plugin provides. + * + * ## What these tests do NOT prove, said up front + * + * **Nothing here loads a plugin.** This box is Linux with an empty plugin root, + * no marketplace and no credentials for a real turn, so every fixture is a + * directory tree planted in a temp dir and every assertion is about the CONFIG + * paddock produces from it. That is one layer below where the interesting failure + * lives: a plugin whose directory reaches `--plugin-dir` and still does not load + * would pass every test in this file. + * + * The naming is deliberate about that — "produces", "derives", "enumerates", + * never "loads". herdctl's own `mcp-and-plugin-passthrough.test.ts` carries the + * next layer (config → the options object handed to the SDK's `query()`), and the + * layer after that — the CLI actually loading the directory — is the SDK's + * contract and is verified by nobody in either repo. A `marketplace`-installed + * plugin loading through `type: "local"` needs a real host with one installed. + * + * What IS load-bearing and testable is the derivation: the tool-name prefix a + * plugin's server ends up under is not the name it is declared under, and getting + * it wrong means every call to that server is auto-denied with no prompt and + * nothing in the logs. {@link pluginMcpToolPattern} is pinned here against the + * SDK's own documented example. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + EMPTY_HOST_PLUGINS, + INSTALLED_PLUGINS_FILE, + PLUGIN_MANIFEST, + PLUGIN_MCP_FILE, + enumerateHostPlugins, + hostPluginRoot, + loadHostPlugins, + parseInstalledPlugins, + pluginMcpToolPattern, + readPluginServerNames, +} from "../../src/claude-plugins.js"; + +let tmp: string; + +beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "paddock-plugins-")); +}); +afterEach(async () => { + await fs.rm(tmp, { recursive: true, force: true }); +}); + +/** Plant a plugin directory: a manifest, and whatever MCP declaration is asked for. */ +async function plantPlugin( + dir: string, + opts: { + name?: string; + mcpJson?: unknown; + manifestMcp?: unknown; + extraFiles?: Record; + } = {}, +): Promise { + await fs.mkdir(path.join(dir, path.dirname(PLUGIN_MANIFEST)), { recursive: true }); + const manifest: Record = { name: opts.name ?? path.basename(dir) }; + if (opts.manifestMcp !== undefined) manifest.mcpServers = opts.manifestMcp; + await fs.writeFile(path.join(dir, PLUGIN_MANIFEST), JSON.stringify(manifest), "utf8"); + if (opts.mcpJson !== undefined) { + await fs.writeFile(path.join(dir, PLUGIN_MCP_FILE), JSON.stringify(opts.mcpJson), "utf8"); + } + for (const [rel, body] of Object.entries(opts.extraFiles ?? {})) { + await fs.writeFile(path.join(dir, rel), JSON.stringify(body), "utf8"); + } + return dir; +} + +describe("pluginMcpToolPattern: the derivation the allowlist depends on", () => { + /** + * THE assertion in this file. A plugin's servers are registered under + * `` `plugin:${pluginName}:${serverName}` ``, and a server name is normalised + * `[^a-zA-Z0-9_-] → _` before it becomes a tool prefix — so the allowlist entry + * for the `docs` server of the `documents` plugin is + * `mcp__plugin_documents_docs__*`, which is exactly the fully-qualified example + * the Agent SDK's own types document (`mcp__plugin_documents_docs__doc_export`). + * That coincidence is the evidence; it is not a guess about a convention. + */ + it("matches the fully-qualified name the SDK documents", () => { + expect(pluginMcpToolPattern("documents", "docs")).toBe("mcp__plugin_documents_docs__*"); + }); + + it("normalises every character the tool namespace cannot hold", () => { + // Applied to the joined `plugin::` string, not to the parts, so the + // separators and a dotted plugin name normalise the same way. + expect(pluginMcpToolPattern("my.plugin", "some server")).toBe( + "mcp__plugin_my_plugin_some_server__*", + ); + }); +}); + +describe("parseInstalledPlugins: the CLI's own registry, both formats", () => { + it("reads a v2 entry's explicit installPath, one per install scope", () => { + const plugins = parseInstalledPlugins( + JSON.stringify({ + version: 2, + plugins: { + "slack@acme": [ + { scope: "user", installPath: "/home/ed/.claude/plugins/cache/acme/slack/1.2.0" }, + { scope: "project", projectPath: "/x", installPath: "/x/.claude/plugins/slack" }, + ], + }, + }), + "/home/ed/.claude/plugins", + ); + expect(plugins).toEqual([ + { id: "slack@acme", installPath: "/home/ed/.claude/plugins/cache/acme/slack/1.2.0" }, + { id: "slack@acme", installPath: "/x/.claude/plugins/slack" }, + ]); + }); + + /** + * v1 held no path at all; the CLI derived it as + * `/cache///` with each segment sanitised. + * Reproduced rather than ignored because a host that has not run a recent + * `claude` still has the old file, and enumerating nothing there would look + * exactly like "you have no plugins". + */ + it("derives a v1 entry's path the way the CLI does", () => { + const plugins = parseInstalledPlugins( + JSON.stringify({ plugins: { "slack@acme": { version: "1.2.0" } } }), + "/root", + ); + expect(plugins).toEqual([{ id: "slack@acme", installPath: "/root/cache/acme/slack/1.2.0" }]); + }); + + it("returns nothing rather than throwing on a file it cannot read", () => { + // This is a file paddock does not own and must never turn into a boot + // failure — the same posture `parseHostMcpConfig` takes with `.claude.json`. + expect(parseInstalledPlugins("{not json", "/root")).toEqual([]); + expect(parseInstalledPlugins(JSON.stringify({ plugins: "nope" }), "/root")).toEqual([]); + const noPath = JSON.stringify({ plugins: { "a@b": [{}] } }); + expect(parseInstalledPlugins(noPath, "/root")).toEqual([]); + }); +}); + +describe("readPluginServerNames: the two places a plugin declares servers", () => { + it("reads `.mcp.json`, in both the wrapped and the bare shape", async () => { + const wrapped = await plantPlugin(path.join(tmp, "a"), { + mcpJson: { mcpServers: { docs: { command: "docs-mcp" } } }, + }); + const bare = await plantPlugin(path.join(tmp, "b"), { + mcpJson: { docs: { command: "docs-mcp" } }, + }); + expect((await readPluginServerNames(wrapped)).servers).toEqual(["docs"]); + expect((await readPluginServerNames(bare)).servers).toEqual(["docs"]); + }); + + it("unions an inline manifest `mcpServers` with `.mcp.json`", async () => { + const dir = await plantPlugin(path.join(tmp, "p"), { + name: "slack", + mcpJson: { mcpServers: { docs: { command: "x" } } }, + manifestMcp: { chat: { url: "https://x/mcp" } }, + }); + const read = await readPluginServerNames(dir); + expect(read.pluginName).toBe("slack"); + expect(read.servers.sort()).toEqual(["chat", "docs"]); + expect(read.unresolved).toEqual([]); + }); + + it("follows a manifest pointer that stays inside the plugin directory", async () => { + const dir = await plantPlugin(path.join(tmp, "p"), { + manifestMcp: "./servers.json", + extraFiles: { "servers.json": { mcpServers: { chat: { command: "x" } } } }, + }); + const read = await readPluginServerNames(dir); + expect(read.servers).toEqual(["chat"]); + expect(read.unresolved).toEqual([]); + }); + + /** + * The honest gap, and the reason it is reported rather than guessed at: a + * manifest may point `mcpServers` at an MCPB bundle the CLI downloads, or at a + * path outside the plugin. Paddock will not fetch anything to build an + * allowlist, so it records the pointer — which is what turns the silent + * auto-deny into a boot warning naming the plugin. + */ + it("records a pointer it will not resolve rather than inventing a name", async () => { + const outside = await plantPlugin(path.join(tmp, "p"), { manifestMcp: "../elsewhere.json" }); + expect(await readPluginServerNames(outside)).toMatchObject({ + servers: [], + unresolved: ["../elsewhere.json"], + }); + const bundle = await plantPlugin(path.join(tmp, "q"), { manifestMcp: "server.mcpb" }); + expect((await readPluginServerNames(bundle)).unresolved).toEqual(["server.mcpb"]); + }); + + it("falls back to the directory name when the manifest names nothing", async () => { + const dir = path.join(tmp, "unnamed"); + await fs.mkdir(path.join(dir, path.dirname(PLUGIN_MANIFEST)), { recursive: true }); + await fs.writeFile(path.join(dir, PLUGIN_MANIFEST), JSON.stringify({}), "utf8"); + expect((await readPluginServerNames(dir)).pluginName).toBe("unnamed"); + }); +}); + +describe("enumerateHostPlugins: what reaches agent.plugins", () => { + /** A registry naming one planted plugin that declares one MCP server. */ + async function fixture(id = "slack@acme"): Promise<{ root: string; installed: string }> { + const root = path.join(tmp, "plugins"); + const dir = path.join(root, "cache", "acme", "slack", "1.0.0"); + await plantPlugin(dir, { name: "slack", mcpJson: { mcpServers: { chat: { command: "x" } } } }); + return { + root, + installed: JSON.stringify({ + version: 2, + plugins: { [id]: [{ scope: "user", installPath: dir }] }, + }), + }; + } + + /** + * THE test for the lever split. A plugin is instructions AND (maybe) MCP + * servers, and `skipMcpDiscovery` is the SDK's own flag for loading the first + * half without the second — so `mcpServers: own` still gets the plugin's + * commands/skills/agents and gets NO tool patterns, while `host` gets both. + */ + it("passes the plugin whole with MCP on, and skipMcpDiscovery with it off", async () => { + const { root, installed } = await fixture(); + const withMcp = await enumerateHostPlugins({ root, installed, settings: {}, mcp: true }); + const without = await enumerateHostPlugins({ root, installed, settings: {}, mcp: false }); + + expect(withMcp.plugins).toEqual([ + { type: "local", path: path.join(root, "cache", "acme", "slack", "1.0.0") }, + ]); + expect(withMcp.toolPatterns).toEqual(["mcp__plugin_slack_chat__*"]); + + expect(without.plugins).toEqual([ + { + type: "local", + path: path.join(root, "cache", "acme", "slack", "1.0.0"), + skipMcpDiscovery: true, + }, + ]); + // No pattern, because no server is attached to need one. Widening the + // allowlist here would grant a namespace nothing occupies. + expect(without.toolPatterns).toEqual([]); + }); + + /** + * A `--plugin-dir` plugin is enabled BY DEFAULT — its enablement is + * `enabledPlugins["@inline"] ?? manifest.defaultEnabled !== false`, and + * the host's key is `@`, which never matches. So honouring + * an explicit `false` here is the only thing that can respect a host's + * `/plugin disable`, and it is the only thing this reads `settings.json` for. + */ + it("honours an explicit `enabledPlugins: false` and nothing else about it", async () => { + const { root, installed } = await fixture(); + const off = await enumerateHostPlugins({ + root, + installed, + settings: { enabledPlugins: { "slack@acme": false } }, + mcp: true, + }); + expect(off.plugins).toEqual([]); + + // An extended entry (a version constraint) is not a disable, and neither is + // an absent one — an installed plugin is passed unless it is turned off. + for (const enabledPlugins of [{ "slack@acme": { version: "^1" } }, {}]) { + const on = await enumerateHostPlugins({ + root, + installed, + settings: { enabledPlugins }, + mcp: true, + }); + expect(on.plugins).toHaveLength(1); + } + }); + + it("reports a registry entry whose directory is gone instead of passing it", async () => { + const root = path.join(tmp, "plugins"); + const installed = JSON.stringify({ + version: 2, + plugins: { "ghost@acme": [{ scope: "user", installPath: path.join(root, "gone") }] }, + }); + const src = await enumerateHostPlugins({ root, installed, settings: {}, mcp: true }); + expect(src.plugins).toEqual([]); + expect(src.caveats).toEqual([ + { + name: "ghost@acme", + kind: "dropped", + reason: `its recorded install directory (${path.join(root, "gone")}) is missing`, + }, + ]); + }); + + it("attaches a plugin it cannot name servers for, and says what is missing", async () => { + const root = path.join(tmp, "plugins"); + const dir = path.join(root, "cache", "acme", "bundled", "1.0.0"); + await plantPlugin(dir, { name: "bundled", manifestMcp: "server.mcpb" }); + const src = await enumerateHostPlugins({ + root, + installed: JSON.stringify({ + version: 2, + plugins: { "bundled@acme": [{ scope: "user", installPath: dir }] }, + }), + settings: {}, + mcp: true, + }); + // Attached — refusing it would lose its commands and skills too, and the + // plugin may well provide no MCP tools the keeper needs. + expect(src.plugins).toHaveLength(1); + expect(src.toolPatterns).toEqual([]); + expect(src.caveats[0].kind).toBe("degraded"); + // The warning has to be actionable: it names the prefix an operator would + // have to add, because the failure it prevents is a silent auto-deny. + expect(src.caveats[0].reason).toContain("mcp__plugin_bundled___*"); + }); +}); + +describe("loadHostPlugins: the boot read, and the levers that gate it", () => { + const cfgFor = (instructions: "own" | "host", mcpServers: "own" | "host") => + ({ + legacyClaudeHome: path.join(tmp, ".claude"), + claude: { instructions, mcpServers }, + }) as Parameters[0]; + + /** + * The divergence from #700 worth pinning, because it is a judgement call and not + * a detail: #700 scopes plugin enumeration to `claude.mcpServers: host`, but + * `plugins/` is bridged by `claude.instructions` and most of a plugin IS + * instructions. Under `instructions: own` paddock already prints "your ~/.claude + * plugins are NOT loaded"; loading them because a different key says `host` + * would contradict its own boot log. So it reads nothing, and says which key to + * flip instead of leaving an empty `plugins` to be discovered. + */ + it("reads nothing under `instructions: own`, and names the key that turns it on", async () => { + const report = await loadHostPlugins(cfgFor("own", "host")); + expect(report.source).toBe(EMPTY_HOST_PLUGINS); + expect(report.notices[0].message).toContain("claude.instructions: host"); + }); + + it("says nothing at all when neither lever is on", async () => { + const report = await loadHostPlugins(cfgFor("own", "own")); + expect(report.source).toBe(EMPTY_HOST_PLUGINS); + expect(report.notices).toEqual([]); + }); + + it("stays quiet when the host has never installed a plugin", async () => { + // `installed_plugins.json` does not exist until the first `claude plugin + // install`, which is the overwhelmingly common case and not worth a warning. + await fs.mkdir(path.join(tmp, ".claude"), { recursive: true }); + const report = await loadHostPlugins(cfgFor("host", "host")); + expect(report.source.plugins).toEqual([]); + expect(report.notices).toEqual([]); + }); + + it("enumerates from the registry and names the patterns it widened by", async () => { + const root = hostPluginRoot(path.join(tmp, ".claude")); + const dir = path.join(root, "cache", "acme", "slack", "1.0.0"); + await plantPlugin(dir, { name: "slack", mcpJson: { mcpServers: { chat: { command: "x" } } } }); + await fs.writeFile( + path.join(root, INSTALLED_PLUGINS_FILE), + JSON.stringify({ + version: 2, + plugins: { "slack@acme": [{ scope: "user", installPath: dir }] }, + }), + "utf8", + ); + const report = await loadHostPlugins(cfgFor("host", "host")); + expect(report.source.plugins).toEqual([{ type: "local", path: dir }]); + expect(report.notices[0].message).toContain("mcp__plugin_slack_chat__*"); + }); +}); diff --git a/packages/server/test/unit/herdctl-host-mcp.test.ts b/packages/server/test/unit/herdctl-host-mcp.test.ts index 888adfb0..1e50a909 100644 --- a/packages/server/test/unit/herdctl-host-mcp.test.ts +++ b/packages/server/test/unit/herdctl-host-mcp.test.ts @@ -20,6 +20,7 @@ import { } from "../../src/herdctl-agent-config.js"; import { FLEET_ALLOWED_TOOLS, BROWSER_MCP_TOOL } from "../../src/herdctl-agent-names.js"; import { EMPTY_MCP_SOURCES, parseHostMcpConfig, type McpSources } from "../../src/claude-mcp.js"; +import { EMPTY_HOST_PLUGINS, type HostPluginSource } from "../../src/claude-plugins.js"; import type { PaddockConfig } from "../../src/config.js"; import type { Project } from "../../src/projects.js"; @@ -243,3 +244,71 @@ describe("buildAgentConfig: instance-declared mcpServers (#691 step 6)", () => { ).toBeUndefined(); }); }); + +/** + * The host's PLUGINS on the same seam (#700, consuming herdctl 5.32.0's `plugins` + * passthrough). Asserted here rather than in `claude-plugins.test.ts` because the + * thing that can silently break is not the enumeration — it is the two keys + * landing on the same agent config together, since a plugin whose server is + * attached without its `mcp__…__*` pattern has every call auto-denied with no + * prompt and nothing in the logs. + * + * Nothing here loads a plugin; the paths are synthetic and never touched. + */ +describe("buildAgentConfig: host plugins (#700)", () => { + const hostPlugins: HostPluginSource = { + plugins: [{ type: "local", path: "/home/ed/.claude/plugins/cache/acme/slack/1.0.0" }], + toolPatterns: ["mcp__plugin_slack_chat__*"], + caveats: [], + }; + + it("passes them as `plugins` and widens the allowlist by their tool patterns", () => { + const config = buildAgentConfig(cfg(), project(), undefined, EMPTY_MCP_SOURCES, hostPlugins); + expect(config.plugins).toEqual(hostPlugins.plugins); + expect(config.allowed_tools).toEqual([...FLEET_ALLOWED_TOOLS, "mcp__plugin_slack_chat__*"]); + }); + + /** + * The allowlist is ONE array and herdctl's `mergeAgentConfig` REPLACES arrays + * rather than merging them, so an agent that widens it inherits nothing from the + * fleet defaults. Both sources therefore have to be restated together or the + * second one silently removes the first. + */ + it("widens by the plugins' patterns AND the servers', in one array", () => { + const config = buildAgentConfig(cfg(), project(), undefined, hostConfig, hostPlugins); + expect(config.allowed_tools).toEqual([ + ...FLEET_ALLOWED_TOOLS, + "mcp__notion__*", + "mcp__pg__*", + "mcp__plugin_slack_chat__*", + ]); + }); + + it("leaves a config with no plugins byte-identical to before the lever", () => { + // The default argument means every existing caller — and every project on an + // instance with `instructions: own` — keeps meaning "no plugins". + const before = buildAgentConfig(cfg(), project(), undefined, EMPTY_MCP_SOURCES); + const after = buildAgentConfig( + cfg(), + project(), + undefined, + EMPTY_MCP_SOURCES, + EMPTY_HOST_PLUGINS, + ); + expect(after).toEqual(before); + expect("plugins" in after).toBe(false); + expect(after.allowed_tools).toBeUndefined(); + }); + + it("never attaches a plugin to the sweeper or to a trigger", () => { + // Same scope decision as both MCP sources, and the same structural guarantee: + // neither builder takes a HostPluginSource at all. + expect(buildSweeperConfig(cfg(), project()).plugins).toBeUndefined(); + expect( + buildTriggerConfig(cfg(), project(), "nightly", { + trigger: { type: "schedule", cron: "0 3 * * *" }, + run: {}, + } as unknown as Parameters[3]).plugins, + ).toBeUndefined(); + }); +}); diff --git a/packages/server/test/unit/mcp-servers.test.ts b/packages/server/test/unit/mcp-servers.test.ts index ea370323..0e01e7a6 100644 --- a/packages/server/test/unit/mcp-servers.test.ts +++ b/packages/server/test/unit/mcp-servers.test.ts @@ -62,9 +62,11 @@ describe("mcpServers: the shape that reaches the runtime", () => { NO_ENV, ); expect(errors).toEqual([]); - // `type` is not a field herdctl's schema has, so it is consumed here rather - // than passed on to be stripped downstream. - expect(servers).toEqual({ linear: { url: "https://mcp.example.test/mcp" } }); + // `type` used to be consumed here — herdctl's schema had no field for it, so + // passing it on only meant being stripped one layer down. 5.32.0 has one, and + // an explicit type wins over the bare-`url` ⇒ `http` inference, so it is + // carried through even when it agrees with what would be inferred anyway. + expect(servers).toEqual({ linear: { url: "https://mcp.example.test/mcp", type: "http" } }); }); it("ignores an absent block entirely", () => { @@ -78,37 +80,71 @@ describe("mcpServers: the shape that reaches the runtime", () => { describe("mcpServers: what is refused, and why refusing beats degrading", () => { /** - * The case the brief singles out. herdctl's `McpServerSchema` has no `headers` - * field, so a bearer-authenticated server would arrive unauthenticated — and - * its stored OAuth token, keyed on a hash that includes the headers, would not - * be found either. Step 5 warns and passes it; a hand-written declaration gets - * an error, because the user typed the header and can act on being told it does - * nothing. + * These two used to be refusals — the cases the #691-step-6 brief singled out — + * because herdctl's `McpServerSchema` had no `headers` field and mapped every + * `url` to `type: "http"`, so declaring either produced a server that arrived + * unauthenticated and could not find its stored OAuth token. herdctl 5.32.0 + * (#446) carries both verbatim, so refusing them would now reject a declaration + * that works. The invariant that has to survive the change is the secrets one: + * a header value is a credential and must never be echoed. */ - it("refuses `headers`, naming the header keys and never their values", () => { - const { servers, errors } = resolveDeclaredMcpServers( + it("accepts `headers` and `type: sse`, and never echoes a header value", () => { + const { servers, errors, warnings } = resolveDeclaredMcpServers( { notion: { - url: "https://mcp.example.test/mcp", + url: "https://mcp.example.test/sse", + type: "sse", headers: { Authorization: `Bearer ${SECRET}` }, }, }, NO_ENV, ); + expect(errors).toEqual([]); + expect(servers.notion).toEqual({ + url: "https://mcp.example.test/sse", + type: "sse", + headers: { Authorization: `Bearer ${SECRET}` }, + }); + // Inline and credential-shaped, so it earns the same advice `env` values get… + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("Authorization"); + // …and the advice does not contain the thing it is advising about. + expect([...errors, ...warnings].join("\n")).not.toContain(SECRET); + }); + + it("resolves an `env:VAR_NAME` header, and says nothing when it is used", () => { + const { servers, errors, warnings } = resolveDeclaredMcpServers( + { + notion: { + url: "https://mcp.example.test/mcp", + headers: { Authorization: `${ENV_REF_PREFIX}NOTION_BEARER` }, + }, + }, + { NOTION_BEARER: `Bearer ${SECRET}` }, + ); + expect(errors).toEqual([]); + expect(warnings).toEqual([]); + expect(servers.notion.headers).toEqual({ Authorization: `Bearer ${SECRET}` }); + }); + + it("refuses headers on a stdio server, where they mean nothing", () => { + const { servers, errors } = resolveDeclaredMcpServers( + { local: { command: "x", headers: { Authorization: "y" } } }, + NO_ENV, + ); expect(servers).toEqual({}); - expect(errors).toHaveLength(1); - expect(errors[0]).toContain("Authorization"); - expect(errors[0]).toContain("not attached"); - expect(errors.join("\n")).not.toContain(SECRET); + expect(errors[0]).toContain("only a `url` server can carry headers"); }); - it("refuses `type: sse`, which would otherwise be silently connected to as HTTP", () => { + it("still refuses a `type` that disagrees with the declaration", () => { + // `sse` is legal now; `stdio` on a `url` server is still a typo, and starting + // the wrong transport is a confusing failure rather than a loud one. const { servers, errors } = resolveDeclaredMcpServers( - { legacy: { url: "https://mcp.example.test/sse", type: "sse" } }, + { legacy: { url: "https://mcp.example.test/sse", type: "stdio" } }, NO_ENV, ); expect(servers).toEqual({}); - expect(errors[0]).toContain("sse"); + expect(errors[0]).toContain("expected http or sse"); }); /** @@ -170,7 +206,7 @@ describe("mcpServers: what is refused, and why refusing beats degrading", () => /** One bad server must not take the instance — or the other servers — down. */ it("drops only the offending server", () => { const { servers, errors } = resolveDeclaredMcpServers( - { good: { command: "ok-mcp" }, bad: { url: "https://x.test", type: "sse" } }, + { good: { command: "ok-mcp" }, bad: { url: "https://x.test", type: "stdio" } }, NO_ENV, ); expect(Object.keys(servers)).toEqual(["good"]); @@ -269,7 +305,10 @@ describe("mcpServers: secrets", () => { args: [`--token=${SECRET}`], env: { API_KEY: SECRET }, }, - remote: { url: `https://user:${SECRET}@mcp.example.test/mcp?key=${SECRET}` }, + remote: { + url: `https://user:${SECRET}@mcp.example.test/mcp?key=${SECRET}`, + headers: { Authorization: `Bearer ${SECRET}`, "X-Api-Key": SECRET }, + }, }, NO_ENV, ); @@ -290,6 +329,15 @@ describe("mcpServers: secrets", () => { expect(line).toBe("notion (stdio: npx, 2 args, 1 env entry)"); }); + it("describeServer counts headers and names the declared transport", () => { + const line = describeServer("notion", { + url: "https://mcp.example.test/sse", + type: "sse", + headers: { Authorization: `Bearer ${SECRET}` }, + }); + expect(line).toBe("notion (sse: https://mcp.example.test/sse, 1 header)"); + }); + it("redactUrl strips query, fragment and userinfo", () => { expect(redactUrl(`https://u:${SECRET}@host.test/mcp?key=${SECRET}#f`)).toBe( "https://@host.test/mcp?", @@ -338,6 +386,28 @@ describe("mcpServers: what the boot log says", () => { expect([...batch, ...session].map((n) => n.message).join("\n")).not.toContain(SECRET); }); + /** + * #700 made `headers` carryable, which puts an `Authorization` bearer into the + * same argv element #702 read an `env` token out of — and a bearer is the + * likelier long-lived credential. A url server with headers and no `env` would + * have been the one shape this warning missed. + */ + it("warns about `headers` too, which is the field #700 made carryable", () => { + const servers = { + notion: { + url: "https://mcp.example.test/sse", + type: "sse" as const, + headers: { Authorization: `Bearer ${SECRET}` }, + }, + }; + const batch = declaredMcpNotices({ servers, driveMode: "batch" }); + const warned = batch.find((n) => n.message.includes("/proc//cmdline")); + expect(warned?.level).toBe("warn"); + expect(warned?.message).toContain("notion"); + expect(warned?.message).toContain("headers"); + expect(batch.map((n) => n.message).join("\n")).not.toContain(SECRET); + }); + it("says nothing about the command line for a server with no env, or with no drive mode", () => { const noEnv = { docs: { url: "https://mcp.example.test/mcp" } }; expect(declaredMcpNotices({ servers: noEnv, driveMode: "batch" }).length).toBe(1);