* fix(#3384): strip mcp__* tool grants from zcode-installed subagents ZCode's dispatcher treats every mcp__<server>__* 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, whereas Claude Code treats the same grants as an optional allowlist. ZCode shared Claude's verbatim agents copy (converter: null), so all 8 MCP-granted agents failed to spawn out of the box with zero MCP servers configured. Add convertClaudeAgentToZcodeAgent — a line-surgical converter that filters mcp__* entries out of the frontmatter tools: grant list (both inline comma and YAML block-list shapes) and preserves every other byte. Declare it on both of zcode's capability.json agents entries and cut zcode over to the descriptor-driven agents path (_DESCRIPTOR_AGENTS_RUNTIMES) so the legacy inline loop stops deleting+re-copying the converted agents raw. Claude Code, Kimi, and Gemini install behavior is unchanged. * chore(#3384): link changeset fragment to pr 3483 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/sharp-moles-howl.md
Normal file
5
.changeset/sharp-moles-howl.md
Normal file
@@ -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__<server>__* 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.
|
||||
@@ -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.
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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__<server>__*` 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,
|
||||
|
||||
@@ -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 + '"');
|
||||
|
||||
168
tests/zcode-agent-mcp-grants.install.test.cjs
Normal file
168
tests/zcode-agent-mcp-grants.install.test.cjs
Normal file
@@ -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__<server>__*` 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 <configDir>/agents/<name>.md, local installs at <cwd>/.zcode/agents/<name>.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(', ')})`,
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user