diff --git a/.changeset/sharp-moles-howl.md b/.changeset/sharp-moles-howl.md new file mode 100644 index 000000000..8131889fe --- /dev/null +++ b/.changeset/sharp-moles-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3483 +--- +ZCode installs now strip mcp__* tool grants from installed GSD subagents at install time. ZCode's dispatcher treats every mcp____* entry in an agent's tools: frontmatter as a required MCP server and hard-fails the subagent spawn (CONFIGURATION_ERROR) when it is not connected, so /gsd-quick --full and plan/execute-phase flows failed out of the box with zero MCP servers configured. Installed ZCode agents now declare only core tools; MCP tools remain available when servers are connected. Claude Code installs are unchanged. diff --git a/bin/install.js b/bin/install.js index 3008f1ed2..d6542486d 100755 --- a/bin/install.js +++ b/bin/install.js @@ -11089,7 +11089,12 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // brandingRewrites-only branch). // cline remains excluded: rules-only local branch + local/global complication // that the descriptor-driven path does not handle correctly. - const _DESCRIPTOR_AGENTS_RUNTIMES = new Set(['cursor', 'windsurf', 'augment', 'trae', 'codebuddy', 'copilot', 'antigravity', 'qwen', 'kimi']); + // #3384: zcode cut over — its agents kind now declares + // convertClaudeAgentToZcodeAgent (strips mcp__* grants ZCode's dispatcher + // treats as required MCP servers). Without this exclusion the legacy inline + // loop below deletes+re-copies zcode's agents RAW, bypassing the converter + // (the same hazard the qwen comment above documents). + const _DESCRIPTOR_AGENTS_RUNTIMES = new Set(['cursor', 'windsurf', 'augment', 'trae', 'codebuddy', 'copilot', 'antigravity', 'qwen', 'kimi', 'zcode']); // Always remove stale gsd-* agents first so re-installing with // `--minimal` actually shrinks a previously-full install. diff --git a/capabilities/zcode/capability.json b/capabilities/zcode/capability.json index fe832bc0c..6805fc5fd 100644 --- a/capabilities/zcode/capability.json +++ b/capabilities/zcode/capability.json @@ -43,7 +43,7 @@ "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ], "local": [ @@ -69,7 +69,7 @@ "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ] }, diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index f33abd815..6ec6cd305 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -3799,7 +3799,7 @@ const capabilities = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ], "local": [ @@ -3825,7 +3825,7 @@ const capabilities = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ] }, @@ -7041,7 +7041,7 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ], "local": [ @@ -7067,7 +7067,7 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": null + "converter": "convertClaudeAgentToZcodeAgent" } ] }, diff --git a/gsd-core/bin/lib/capability-validator.cjs b/gsd-core/bin/lib/capability-validator.cjs index de98d4421..b00396bb4 100644 --- a/gsd-core/bin/lib/capability-validator.cjs +++ b/gsd-core/bin/lib/capability-validator.cjs @@ -727,6 +727,9 @@ const VALID_CONVERTER_NAMES = new Set([ 'convertClaudeAgentToCodexAgent', // ADR-1239 / #2092 Phase B Upgrade 1 — native .qwen/agents/*.md subagent projection. 'convertClaudeAgentToQwenAgent', + // #3384 — ZCode agents are Claude-shaped but its dispatcher treats mcp__* tools + // grants as required MCP servers; this converter strips them at install time. + 'convertClaudeAgentToZcodeAgent', ]); // C3: Validate role:runtime body diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index 03e7bcf36..175f5fa4f 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -2370,6 +2370,102 @@ function convertClaudeAgentToQwenAgent(content) { return `${fm}\n${body}`; } +/** + * Convert a Claude Code agent .md for ZCode (#3384). + * + * ZCode is Claude-shaped (same frontmatter, same named-dispatch subagents), so + * the file is preserved verbatim EXCEPT the `tools:` grant list: ZCode's + * dispatcher treats every `mcp____*` entry as a REQUIRED MCP server and + * hard-fails the subagent spawn (CONFIGURATION_ERROR: "Required MCP server is + * not connected") whenever it is not connected, whereas Claude Code treats the + * same entries as an optional allowlist. The `mcp__*` entries are stripped at + * install time — the same exclusion Kimi's converter applies via + * convertKimiToolName — so subagent spawns succeed with zero MCP servers + * configured; connected servers' tools remain reachable (auto-discovered by the + * host, not granted by frontmatter). + * + * Line-surgical by design: ONLY `tools:` lines inside the frontmatter are + * touched, so every other byte (description, color, commented-out blocks, the + * body) survives identically. Handles both shapes GSD emits — the inline comma + * list (`tools: A, B, C`) and the YAML block list (`tools:` + `- A` items). + * An agent whose filtered grant list becomes empty (every grant was `mcp__*`) + * drops the `tools:` key entirely: an absent key inherits the full toolkit, + * which is the degrade-gracefully outcome, never a toolless subagent. + * + * Byte-identical for an agent with no `mcp__*` grants (the common case) and + * for an agent with no frontmatter at all. + */ +function convertClaudeAgentToZcodeAgent(content) { + // Fast path: no MCP grant token anywhere means nothing to strip. (A body + // mention alone is not a grant — the line scan below finds no tools-line + // change and returns `content` unchanged anyway; this just skips the scan.) + if (!content.includes('mcp__')) return content; + + const lines = content.split('\n'); + if (lines[0] !== '---') return content; + let fmEnd = -1; + for (let i = 1; i < lines.length; i++) { + if (lines[i] === '---') { + fmEnd = i; + break; + } + } + if (fmEnd === -1) return content; // unterminated frontmatter — leave verbatim + + const out = []; + let changed = false; + let i = 1; + while (i < fmEnd) { + const line = lines[i]; + const inlineTools = /^tools:[ \t]*(.+)$/.exec(line); + if (inlineTools) { + const grants = inlineTools[1].split(',').map((tool) => tool.trim()).filter((tool) => tool !== ''); + const kept = grants.filter((tool) => !tool.startsWith('mcp__')); + if (kept.length === grants.length) { + out.push(line); // no mcp__* grants — keep the line byte-identical + } else if (kept.length > 0) { + out.push(`tools: ${kept.join(', ')}`); + changed = true; + } else { + changed = true; // every grant was mcp__*: drop the tools key entirely + } + i++; + continue; + } + if (/^tools:[ \t]*$/.test(line)) { + // Block-list form: collect the following `- item` lines. + const items = []; + let j = i + 1; + while (j < fmEnd && /^([ \t]*)-[ \t]*(\S.*)$/.test(lines[j])) { + items.push(lines[j]); + j++; + } + const kept = items.filter((item) => { + const name = /^([ \t]*)-[ \t]*(\S.*)$/.exec(item)[2].trim(); + return !name.startsWith('mcp__'); + }); + if (kept.length !== items.length) { + changed = true; + if (kept.length > 0) { + out.push(line); + out.push(...kept); + } // else: drop the tools key and all its items + } else { + out.push(line, ...items); + } + i = j; + continue; + } + out.push(line); + i++; + } + if (!changed) return content; + // Opening delimiter + transformed frontmatter + closing delimiter + body. + out.unshift(lines[0]); + out.push(...lines.slice(fmEnd)); + return out.join('\n'); +} + function convertClaudeAgentToCodebuddyAgent(content) { const converted = convertClaudeToCodebuddyMarkdown(content); @@ -3139,6 +3235,11 @@ export = { // conversionExports[converterName] dispatch (runtime-artifact-layout.cts) // can resolve it from capabilities/qwen/capability.json's agents kind. convertClaudeAgentToQwenAgent, + // #3384: ZCode agents are Claude-shaped but its dispatcher treats mcp__* + // tools grants as required MCP servers — registered by name for the same + // conversionExports[converterName] dispatch, resolved from + // capabilities/zcode/capability.json's agents kind. + convertClaudeAgentToZcodeAgent, // #1511 ADR-1508 Phase 2: rewrite engine deep seam // Low-level walkers (pathPrefix + attribution pre-resolved by caller): applyRuntimeContentRewritesInPlace, diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 6f98b5256..4a0b6b03e 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -4041,9 +4041,9 @@ describe('ADR-1016 phase 5a: closed-vocab set exports', () => { // ─── 25. ADR-857 phase 5e: closed ConverterName enum (Part B) ───────────────── describe('ADR-857 phase 5e: VALID_CONVERTER_NAMES closed enum', () => { - test('VALID_CONVERTER_NAMES has exactly 26 entries (16 command/skill/workflow + 10 agent converters)', () => { + test('VALID_CONVERTER_NAMES has exactly 27 entries (16 command/skill/workflow + 11 agent converters)', () => { assert.ok(VALID_CONVERTER_NAMES instanceof Set, 'VALID_CONVERTER_NAMES must be a Set'); - assert.strictEqual(VALID_CONVERTER_NAMES.size, 26, 'VALID_CONVERTER_NAMES must have exactly 26 entries, got: ' + VALID_CONVERTER_NAMES.size); + assert.strictEqual(VALID_CONVERTER_NAMES.size, 27, 'VALID_CONVERTER_NAMES must have exactly 27 entries, got: ' + VALID_CONVERTER_NAMES.size); }); test('VALID_CONVERTER_NAMES contains all expected converter names', () => { @@ -4076,6 +4076,8 @@ describe('ADR-857 phase 5e: VALID_CONVERTER_NAMES closed enum', () => { 'convertClaudeAgentToCodexAgent', // ADR-1239 / #2092 Phase B Upgrade 1 — native .qwen/agents/*.md subagent projection. 'convertClaudeAgentToQwenAgent', + // #3384 — ZCode agent converter (strips mcp__* grants at install time). + 'convertClaudeAgentToZcodeAgent', ]; for (const name of expected) { assert.ok(VALID_CONVERTER_NAMES.has(name), 'VALID_CONVERTER_NAMES must contain "' + name + '"'); diff --git a/tests/zcode-agent-mcp-grants.install.test.cjs b/tests/zcode-agent-mcp-grants.install.test.cjs new file mode 100644 index 000000000..a48436882 --- /dev/null +++ b/tests/zcode-agent-mcp-grants.install.test.cjs @@ -0,0 +1,168 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product (see #3384) — this suite asserts on the literal +// frontmatter of EMITTED install artifacts (installed agent .md files), which ARE the deployed +// contract: an mcp__* grant that survives install is treated by ZCode's dispatcher as a +// required-but-must-be-connected MCP server and hard-fails every subagent spawn. + +/** + * zcode-agent-mcp-grants.install.test.cjs — 50-test-matrix.md rows 1-4 (issue #3384). + * + * ZCode's subagent dispatcher treats every `mcp____*` entry in an agent's + * `tools:` frontmatter as a REQUIRED MCP server and throws CONFIGURATION_ERROR on + * spawn when it is not connected. Claude Code treats the same grants as an optional + * allowlist, which is why the GSD sources carry them. The install must therefore + * filter `mcp__*` entries out of the tools list for ZCode — the same outcome Kimi + * already gets via its own conversion path — while leaving Claude Code's verbatim + * copy untouched. + * + * Every row spawns a REAL installer and asserts on the parsed frontmatter of what + * actually reached disk (behavioral seam — never on which internal converter ran). + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { runNode } = require('./helpers/process-seam.cjs'); + +const { cleanup } = require('./helpers.cjs'); +const { installerEnv } = require('./helpers/install-shared.cjs'); +const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); + +/** The 8 MCP-granted agents named in the issue. Every one hard-fails spawn on + * ZCode with zero MCP servers configured, so every one must install clean. */ +const MCP_GRANTED_AGENTS = [ + 'gsd-phase-researcher', + 'gsd-project-researcher', + 'gsd-ui-researcher', + 'gsd-advisor-researcher', + 'gsd-domain-researcher', + 'gsd-ai-researcher', + 'gsd-planner', + 'gsd-executor', +]; + +/** Parse the `tools:` grant list out of an agent .md's YAML frontmatter. + * Handles both shapes GSD emits: inline comma list (`tools: A, B, C`) and + * YAML block list (`tools:\n - A\n - B`). Returns [] when the agent + * declares no tools. */ +function parseFrontmatterTools(content) { + const lines = content.split(/\r?\n/); + if (lines[0] !== '---') return []; + const tools = []; + let collecting = false; + for (let i = 1; i < lines.length; i++) { + const line = lines[i]; + if (line === '---') break; // end of frontmatter + if (collecting) { + const itemMatch = /^(\s*)-\s*(\S.*)$/.exec(line); + if (itemMatch && itemMatch[1].length <= 2) { + tools.push(itemMatch[2].trim()); + continue; + } + collecting = false; // a non-list line ends the block list + } + const inlineMatch = /^tools:[ \t]*(.*)$/.exec(line); + if (inlineMatch) { + const value = inlineMatch[1].trim(); + if (value) { + for (const tool of value.split(',')) { + const name = tool.trim(); + if (name) tools.push(name); + } + } else { + collecting = true; // `tools:` alone starts a block list + } + } + } + return tools; +} + +/** Spawn a real install of one runtime at one scope. Mirrors the seam shape of + * tests/agent-fragments-emission.install.test.cjs. Returns { result, root } + * where root is the install root (config dir for global, project cwd for local). */ +function spawnInstall(runtime, scope) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-3384-${runtime}-${scope}-`)); + const args = ['--preserve-symlinks', '--preserve-symlinks-main', path.join(REPO_ROOT, 'bin', 'install.js'), `--${runtime}`]; + if (scope === 'global') { + args.push('--global', '--config-dir', root); + } else { + args.push('--local'); + } + const seamResult = runNode(args, { + cwd: root, + env: installerEnv({ HOME: root, USERPROFILE: root }), + timeoutMs: INSTALL_TIMEOUT_MS, + }); + return { result: seamResult, root }; +} + +/** Locate an installed agent file for a runtime install root: global installs + * land at /agents/.md, local installs at /.zcode/agents/.md. */ +function installedAgentPath(root, scope, agentName) { + return scope === 'global' + ? path.join(root, 'agents', `${agentName}.md`) + : path.join(root, '.zcode', 'agents', `${agentName}.md`); +} + +/** Rows 1/2 + row 3 (anti-vacuity) for one scope. Caller owns root cleanup. */ +function assertZcodeAgentsClean(t, scope) { + const { result, root } = spawnInstall('zcode', scope); + t.after(() => cleanup(root)); + + assert.strictEqual(result.exitCode, 0, + `zcode ${scope} install must succeed\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); + + for (const agentName of MCP_GRANTED_AGENTS) { + const agentPath = installedAgentPath(root, scope, agentName); + assert.ok(fs.existsSync(agentPath), `zcode ${scope} install must write ${path.relative(root, agentPath)}`); + const content = fs.readFileSync(agentPath, 'utf8'); + assert.ok(content.startsWith('---\n'), + `${agentName} (${scope}): installed agent must carry YAML frontmatter (starts: ${JSON.stringify(content.slice(0, 20))})`); + const tools = parseFrontmatterTools(content); + assert.ok(tools.length > 0, + `${agentName} (${scope}): tools list must not be emptied by the mcp__ filter`); + + const mcpGrants = tools.filter((tool) => tool.startsWith('mcp__')); + assert.deepStrictEqual(mcpGrants, [], + `${agentName} (${scope}): installed tools must declare zero mcp__* grants — ZCode's dispatcher ` + + `treats each as a required MCP server and hard-fails the spawn when unconnected. Found: ${mcpGrants.join(', ')}`); + + // Row 3 anti-vacuity: the filter must strip ONLY the mcp__* entries, never the + // whole grant list — every one of these agents needs its core tools to function. + for (const coreTool of ['Read', 'Bash']) { + assert.ok(tools.includes(coreTool), + `${agentName} (${scope}): non-MCP tool '${coreTool}' must survive the filter (found: ${tools.join(', ')})`); + } + } +} + +// ─── Rows 1/2: a fresh ZCode install carries zero mcp__* grants (both scopes) ── + +test('zcode global install: all 8 MCP-granted agents install with zero mcp__* tool grants (#3384)', (t) => { + assertZcodeAgentsClean(t, 'global'); +}); + +test('zcode local install: all 8 MCP-granted agents install with zero mcp__* tool grants (#3384)', (t) => { + assertZcodeAgentsClean(t, 'local'); +}); + +// ─── Row 4: Claude Code parity — its mcp__* grants are an OPTIONAL allowlist ─── + +test('claude global install still carries mcp__* grants (optional-allowlist semantics untouched by #3384)', (t) => { + const { result, root } = spawnInstall('claude', 'global'); + t.after(() => cleanup(root)); + assert.strictEqual(result.exitCode, 0, + `claude global install must succeed\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); + const agentPath = path.join(root, 'agents', 'gsd-phase-researcher.md'); + assert.ok(fs.existsSync(agentPath), 'claude global install must write agents/gsd-phase-researcher.md'); + const tools = parseFrontmatterTools(fs.readFileSync(agentPath, 'utf8')); + assert.ok( + tools.some((tool) => tool.startsWith('mcp__')), + `claude's own install must keep its mcp__* optional-allowlist grants verbatim (found tools: ${tools.join(', ')})`, + ); +});