diff --git a/.changeset/tidy-tunas-wander.md b/.changeset/tidy-tunas-wander.md new file mode 100644 index 000000000..d13c6f16c --- /dev/null +++ b/.changeset/tidy-tunas-wander.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 4238 +--- +Allow configured agent tool grants to augment installed agent definitions across supported runtimes. diff --git a/CONTEXT.md b/CONTEXT.md index e913c8ce7..f0bc8109a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -286,7 +286,7 @@ Leaf module owning the **static** model tables and the closed vocabularies deriv Module owning model and effort resolution policy: resolves the model, runtime tier, planning granularity, reasoning effort, and fast-mode for a given agent by reading project config and resolving against the model profiles and catalog (`resolveModelInternal`, `resolveModelPolicy`, `resolveTierEntry`, `resolveModelForTier`, `resolveGranularityInternal`, `resolveEffortInternal`, `resolveFastModeInternal`, `resolveEffortForTier`, `nextEffort`, `assertValidGranularityOverride`). Depends only on leaf modules (`config-loader` for `loadConfig`, `configuration` for defaults, `model-profiles` and `model-catalog` for the static tables) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2f (#888) — the final core.cts decomposition step; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. **`CLAUDE_AGENT_ALIASES` no longer lives here** — it moved down to the Model Catalog Module (#3241, ADR-2313 Phase 1) so the Agent Install Check and Codex-sync surfaces can consume the alias rule without taking a `config-loader` dependency this module would have dragged with it; it is still **re-exported** from here, so existing importers (`bin/install.js`, `tests/codex-config.test.cjs`) are unaffected and a parity test asserts both modules expose the same set. Source of truth: `gsd-core/bin/lib/model-resolver.cjs` (generated from `src/model-resolver.cts`). ### Install Model Override Resolver Module -Module owning install-time per-agent model-override resolution (#2256 / #2794), extracted from `bin/install.js`'s inline agent-staging loop (#2875 Part 2 / J8), which duplicated this EXACT precedence chain across two runtime branches (OpenCode/Kilo, ~24 lines each): `model_overrides[agent]` > `model_profile_overrides..` > omit. Interface: `readGsdGlobalModelOverrides`/`readGsdEffectiveModelOverrides`/`readGsdRuntimeProfileResolver` (impure config-file reads, mirroring `install-effort-resolver.cts`'s existing extraction precedent) and `resolveAgentModelOverride` (pure given their pre-resolved outputs). Reached from `runtime-artifact-layout.cts`'s agents-kind `stage()` for the opencode/kilo converters, so it is on the `installRuntimeArtifacts` call tree — every fs touch routes through `installFs()` (Install Fs Adapter Module) rather than calling `node:fs` directly, so a fake adapter injected via `withInstallFs` is honored. A single source of truth means the descriptor-driven agents pipeline and `bin/install.js`'s own callers can only diverge if this module changes, not silently across two hand-maintained copies (the Generative Fix Divergence class CLAUDE.md's "Known Defects" section warns about). Source: `src/install-model-override-resolver.cts` -> `gsd-core/bin/lib/install-model-override-resolver.cjs`. See Install Engine Module, Model Resolver Module. +Module owning install-time per-agent model-override resolution (#2256 / #2794), extracted from `bin/install.js`'s inline agent-staging loop (#2875 Part 2 / J8), which duplicated this EXACT precedence chain across two runtime branches (OpenCode/Kilo, ~24 lines each): `model_overrides[agent]` > `model_profile_overrides..` > omit. Interface: `readGsdGlobalModelOverrides`/`readGsdEffectiveModelOverrides`/`readGsdRuntimeProfileResolver` (impure config-file reads, mirroring `install-effort-resolver.cts`'s existing extraction precedent) and `resolveAgentModelOverride` (pure given their pre-resolved outputs). Reached from `runtime-artifact-layout.cts`'s agents-kind `stage()` for the opencode/kilo converters, so it is on the `installRuntimeArtifacts` call tree — every fs touch routes through `installFs()` (Install Fs Adapter Module) rather than calling `node:fs` directly, so a fake adapter injected via `withInstallFs` is honored. A single source of truth means the descriptor-driven agents pipeline and `bin/install.js`'s own callers can only diverge if this module changes, not silently across two hand-maintained copies (the Generative Fix Divergence class CLAUDE.md's "Known Defects" section warns about). Source: `src/install-model-override-resolver.cts` -> `gsd-core/bin/lib/install-model-override-resolver.cjs`. See Install Engine Module, Model Resolver Module. #4032 added a sibling read path, `readGsdEffectiveAgentTools`, resolving `agent_tools` (`*`/per-agent selectors, read from BOTH `~/.gsd/defaults.json` and the nearest `.planning/config.json`) with the same global-then-project precedence as `resolveAgentModelOverride` — a plain object-spread merge keyed by the SAME selector name in both files, project winning; there is no `project:` prefix syntax. Consumed pre-converter by `appendAgentTools` (Runtime Artifact Conversion Module) via `stageAgentsForRuntimeWithConverter` (`src/install-profiles.cts`). ### Codex Agent TOML Module A **genuine leaf** (node builtins only) owning the typed IR for `~/.codex/agents/.toml` (#3243, ADR-2313 Phase 3). It is a **document model, not a policy** — it knows how to parse/render/strip the two keys the posture owns (`model`, `model_reasoning_effort`); it does NOT know which `model` values are illegal for Codex (that predicate, `isAnthropicFlavoredModel`, stays in the Model Catalog Module and the caller decides what to strip). `parseCodexAgentToml(content) → {ok:true,doc} | {ok:false,reason}` is the STRICT half: `PARSE_REASON.UNTERMINATED_BLOCK` when the `developer_instructions` block is opened but never closed, because a writer that proceeds on a malformed document risks rewriting it. `renderCodexAgentToml(doc)` round-trips **byte-identically** for an unmodified doc — the load-bearing property that stops a sync from silently reformatting a user's file — by keeping the original `lines` array (never re-derived) plus the detected `eol`/BOM/trailing-newline metadata, and rejoining rather than reconstructing. `stripModel`/`stripReasoningEffort` remove exactly one targeted line, re-indexing the block range and the sibling key's line index; every other line (comments, hand-added keys, the prompt block, line endings) is untouched. `scanTomlLines`/`stripBOM`/`findDeveloperInstructionsBlockRange`/`unquoteTomlValue` are the LENIENT reader primitives — moved here (not copied) from the Agent Install Check Module (#3242, Phase 2), which still imports and calls them directly, unchanged in behavior: an unterminated block falls back to "rest of file is inside the block" rather than failing, because misreading prompt prose as a pin is only a false positive. One block-range detector (`findDeveloperInstructionsBlockRange`, now carrying a `terminated` flag the strict parser reads and the lenient scanner ignores) serves both policies, so the reader and the writer can never silently diverge on where the block ends. Consumed by the Codex `.toml` sync (Commands Module's `cmdEffortSyncCodex`, ADR-2313 D7) and the Agent Install Check Module's `checkCodexModelPosture`. Source of truth: `gsd-core/bin/lib/codex-agent-toml.cjs` (generated from `src/codex-agent-toml.cts`). See Agent Install Check Module, Model Catalog Module, ADR-2313. @@ -319,7 +319,7 @@ Pure, no-I/O `when=` evaluator over `InvocationFacts`, mapping a document-order Module owning the per-runtime mapping from artifact kind to filesystem placement. ADR-3660 defines the typed `kinds` per runtime (`commands`, `agents`, `skills`) with destination subpath, prefix, and stage adapter (with per-runtime converters in `bin/install.js`: `convertClaudeCommandToClaudeSkill`, `…CodexSkill`, `…CopilotSkill`, `…AntigravitySkill`). Owns the per-runtime `nested` skill-bundle decision (#69): a `skillsKind` flag in `src/runtime-artifact-layout.cts` drives whether a runtime receives the nested router layout (6 `gsd-ns-*` routers + concrete skills under `/skills//`) or the flat `skills/gsd-/` layout; the evidence/doc-link matrix is recorded in a comment above `resolveRuntimeArtifactLayout`. Phase 1 applies this seam to the Runtime Surface Module (`surface.cjs:applySurface`); as of #813, `applySurface` applies the same per-runtime skill-body path rewrites as `installRuntimeArtifacts` for `skills` kinds — re-surfacing no longer overwrites installed SKILL.md bodies with converter-default `~/.claude` paths. Per ADR-1508 / #1511 the former `getInstallExports`/`loadInstallExports` relay (a `GSD_TEST_MODE`-guarded `require('bin/install.js')` by which `surface.cjs` reached `computePathPrefix`/`applyRuntimeContentRewritesInPlace`) was DELETED from this module; content rewriting now lives in the Runtime Artifact Conversion Module and `surface.cjs:applySurface` calls its `rewriteStagedSkillBodies` directly. The resolved `scope` is still carried on the `Layout` object so `applySurface` derives the same `pathPrefix` (global `$HOME` form vs. absolute) as a fresh install. Phase 2 is planned to migrate install/uninstall in `bin/install.js` so all lifecycle sites iterate one shared layout table instead of re-encoding runtime layout logic. This design is intended to remove the #3659 class of omissions. Migrations remain under the Installer Migration Module (ADR-0008). The `.gsd-source` marker (#1477) is a two-party provisioning contract that lets source resolution succeed on the Claude global skills layout, which ships `gsd-core/{bin,contexts,references,templates,workflows}` but no `commands/gsd` source tree for `findInstallSourceRoot` to walk up to: the writer is `bin/install.js`, which writes `/.gsd-source` (content: the absolute path to its own `commands/gsd`, terminated by a newline) when `runtime === 'claude' && isGlobal`, guarded by `fs.existsSync` so a half-published package never writes a dangling marker; the reader is `findInstallSourceRoot(configDir)`, which prefers the marker over its walk-up but falls through to the walk-up if the marker is absent, dangling, or empty/whitespace-only. #2871 Phase 2 widens the Module from placement to placement **+** trigger resolution: `resolveTriggerSurface(runtime, scopes, { stems, routerStems?, childToRouters?, registry? }) -> TriggerSurface[]` answers "what does a user type" rather than "where does a file land" — a new, pure function alongside `resolveRuntimeArtifactLayout` (untouched, still 7 callers), never a widened signature. Only `commands` and `skills` are trigger-bearing; `agents`/`kimi-agents` are excluded entirely — an agent is invoked through the Agent/Task tool's `subagent_type`, a separate dispatch interface point, never a `/gsd-` a user types (ADR-2866 amendment below). Each `TriggerSurface` names its `trigger`, `kind`, `scope`, `destPath` (computed through the SAME `namespacedByDir` branch `_copyStaged` uses), `registration` (`'direct'` | `'via-router'`, the latter naming the owning router's `routerTrigger` for a nested-router runtime's concrete child skill — #69), and `shadowedBy` (the winning sibling entry, or `null`). The winner across scopes and kinds is decided by scope rank first (Install Scope Module's `scopeRank`, consumed not re-derived — global outranks local) then by the runtime's new `runtime.triggerPrecedence` descriptor axis (ordered kind names, highest priority first; default `['skills', 'commands']`, required-with-default so a pre-#2871 `capability.json` keeps validating). `shadowedBy` ships unread this phase — Phase 4 (#2873) is its first consumer. See ADR-3660. ### Runtime Artifact Conversion Module -Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical Claude-authored command/agent/skill markdown into runtime-specific artifact bodies, including converter selection, frontmatter/body normalization, runtime path rewrites, and staged artifact generation. Runtime Artifact Layout remains responsible for filesystem placement (`kind`, destination subpath, prefix, nesting); Runtime Artifact Conversion owns the content Implementation behind that placement seam so install, uninstall/surface parity, and future plugin/package projections stop reaching back through `bin/install.js` for converter functions or `GSD_TEST_MODE`-guarded installer exports. Chosen direction: sibling Module, not an expanded Layout Module, to preserve ADR-3660's narrow placement responsibility while deepening artifact content locality. First slice: relocate only the layout-reached conversion family (`convertClaudeCommandTo*Skill`, converted command-file emitters, `buildKimiAgentArtifacts`) plus the minimal helper closure they need; do not leave helper dependencies in `bin/install.js` because that would preserve the same shallow seam under a new filename. Installer integration decision: `bin/install.js` imports the conversion Module at top level and re-exports the moved names for compatibility; the conversion Module must not import `bin/install.js` or Runtime Artifact Layout, so the dependency direction becomes installer/layout Adapters -> conversion Module, never conversion -> installer. First-slice Interface decision: export the existing compatibility names only; do not introduce a grouped `convertRuntimeArtifact` Interface until after relocation proves byte-for-byte behavior. SHIPPED (ADR-1508): the converter family relocated in #1510 Phase 1 (`getDirName`→runtime-name-policy, `processAttribution` here); #1511 Phase 2 moved the content-rewrite engine here in full — `_applyRuntimeRewrites` (per-runtime switch, injected attribution), the staged-content walkers `applyRuntimeContentRewritesInPlace`/`applyRuntimeContentRewritesForCommandsInPlace`, `computePathPrefix` (private; `_computePathPrefix` for tests), and the deep public seam `rewriteStagedSkillBodies`/`rewriteStagedCommandBodies({runtime,configDir,scope,homedir?,platform?,resolveAttribution?})`. `bin/install.js` binds these back (single owner, exports preserved); `getCommitAttribution` stays in `bin/install.js` (impure install-time config I/O) and is injected. The `getInstallExports` relay in Runtime Artifact Layout Module was deleted; the dependency direction installer/layout → conversion (never upward) is now enforced. Exception: opencode and kilo path-prefix rewriting is a deliberate `bin/install.js`-owned pre-conversion step (`applyOpencodeFamilyPathPrefix`) per #784, not a violation of the single-owner rule. Source: `gsd-core/bin/lib/runtime-artifact-conversion.cjs`. Also exports `resolveVersionFrom(libDir)` — a lazy, defensive GSD-version resolver (installed-tree `gsd-core/VERSION` first, then the source/npm `package.json` three dirs up, both validated against the repo's shared semver-prefix shape, degrading to `''` on failure) that replaced a module-load-time `require('../../../package.json')` which crashed on runtimes whose root carries no `package.json` (e.g. Codex) (#1383). +Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical Claude-authored command/agent/skill markdown into runtime-specific artifact bodies, including converter selection, frontmatter/body normalization, runtime path rewrites, and staged artifact generation. Runtime Artifact Layout remains responsible for filesystem placement (`kind`, destination subpath, prefix, nesting); Runtime Artifact Conversion owns the content Implementation behind that placement seam so install, uninstall/surface parity, and future plugin/package projections stop reaching back through `bin/install.js` for converter functions or `GSD_TEST_MODE`-guarded installer exports. Chosen direction: sibling Module, not an expanded Layout Module, to preserve ADR-3660's narrow placement responsibility while deepening artifact content locality. First slice: relocate only the layout-reached conversion family (`convertClaudeCommandTo*Skill`, converted command-file emitters, `buildKimiAgentArtifacts`) plus the minimal helper closure they need; do not leave helper dependencies in `bin/install.js` because that would preserve the same shallow seam under a new filename. Installer integration decision: `bin/install.js` imports the conversion Module at top level and re-exports the moved names for compatibility; the conversion Module must not import `bin/install.js` or Runtime Artifact Layout, so the dependency direction becomes installer/layout Adapters -> conversion Module, never conversion -> installer. First-slice Interface decision: export the existing compatibility names only; do not introduce a grouped `convertRuntimeArtifact` Interface until after relocation proves byte-for-byte behavior. SHIPPED (ADR-1508): the converter family relocated in #1510 Phase 1 (`getDirName`→runtime-name-policy, `processAttribution` here); #1511 Phase 2 moved the content-rewrite engine here in full — `_applyRuntimeRewrites` (per-runtime switch, injected attribution), the staged-content walkers `applyRuntimeContentRewritesInPlace`/`applyRuntimeContentRewritesForCommandsInPlace`, `computePathPrefix` (private; `_computePathPrefix` for tests), and the deep public seam `rewriteStagedSkillBodies`/`rewriteStagedCommandBodies({runtime,configDir,scope,homedir?,platform?,resolveAttribution?})`. `bin/install.js` binds these back (single owner, exports preserved); `getCommitAttribution` stays in `bin/install.js` (impure install-time config I/O) and is injected. The `getInstallExports` relay in Runtime Artifact Layout Module was deleted; the dependency direction installer/layout → conversion (never upward) is now enforced. Exception: opencode and kilo path-prefix rewriting is a deliberate `bin/install.js`-owned pre-conversion step (`applyOpencodeFamilyPathPrefix`) per #784, not a violation of the single-owner rule. Source: `gsd-core/bin/lib/runtime-artifact-conversion.cjs`. Also exports `resolveVersionFrom(libDir)` — a lazy, defensive GSD-version resolver (installed-tree `gsd-core/VERSION` first, then the source/npm `package.json` three dirs up, both validated against the repo's shared semver-prefix shape, degrading to `''` on failure) that replaced a module-load-time `require('../../../package.json')` which crashed on runtimes whose root carries no `package.json` (e.g. Codex) (#1383). #4032 added `appendAgentTools` as step 3 of the ADR-1235 pre-converter pipeline (`stageAgentsForRuntimeWithConverter`, `src/install-profiles.cts` — not this module): appends `readGsdEffectiveAgentTools` grants (Install Model Override Resolver Module) into the canonical agent's `tools:` frontmatter — line-surgical, both inline-comma and YAML block-list forms — before the runtime-specific converter runs, so every converter (including Kilo's `mcp__*`→`{server}_{tool}` permission mapping and ZCode's `mcp__*` omission policy) sees configured grants without a duplicated per-runtime write path. ### Runtime Artifact Install Plan Module Module owning install-time staging and content-rewrite selection for a pre-resolved Runtime Artifact Layout. Interface: `createRuntimeArtifactInstallPlan({ layout, resolvedProfile, homedir?, platform?, resolveAttribution?, deps? }) -> { ok:true, plan:{ items, cleanupDirs } } | { ok:false, kind:'stage_failed'|'rewrite_failed', message, cleanupDirs, failedKind? }`. It iterates `layout.kinds` in order, calls each kind's `stage(resolvedProfile)`, delegates `commands` to Runtime Artifact Conversion `rewriteStagedCommandBodies`, delegates `skills` and `kimi-agents` to `rewriteStagedSkillBodies`, leaves non-rewritten kinds unchanged, and projects copy items as `{ kind, sourceDir, destDir }`. It deliberately does not prune, copy, run legacy migrations, print output, or execute cleanup; those remain Installer Module adapter responsibilities until later slices wire the plan into `bin/install.js`. **Write-confinement (ADR-1239 Phase B / #1679):** the exported pure `assertDestWithinConfigHome(configDir, destSubpath) -> resolvedDest` is the security gate — every kind's `destDir` is computed through it on both the install and uninstall plan paths, so a `destSubpath` that escapes `configHome` (`../../etc`, a NUL byte, etc.) is rejected at plan-build time with a clear error; `surface.cjs:applySurface` and `bin/install.js:installOpencodeFamilySkills` route their joins through the same helper, and `_copyStaged` carries a defense-in-depth containment check. This is security-load-bearing for the Phase C third-party-descriptor loader (which is where an untrusted `destSubpath` could arrive). Source: `gsd-core/bin/lib/runtime-artifact-install-plan.cjs`. See Runtime Artifact Layout Module and Runtime Artifact Conversion Module. diff --git a/bin/install.js b/bin/install.js index f3f8cc325..dca6318f4 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1821,8 +1821,20 @@ const kiloAgentPermissionOrder = [ 'lsp', ]; +const kiloMcpPermissionPattern = /^mcp__([A-Za-z0-9_-]+)__((?:[A-Za-z0-9_-]+)|\*)$/; + +// Derives Kilo's native `{server}_{tool}` MCP permission key (kilo.ai/docs — +// external, fixed format). Not injective: both capture groups allow `_`, so +// e.g. `mcp__a_b__c` and `mcp__a__b_c` derive the same key. The `Set` below +// resolves any such collision deterministically to first-seen-wins — see +// src/runtime-artifact-conversion.cts's regression test. Mirrors that file's +// convertClaudeToKiloPermissionTool exactly (#4032). function convertClaudeToKiloPermissionTool(claudeTool) { - return claudeToKiloAgentPermissions[claudeTool] || null; + const builtinPermission = claudeToKiloAgentPermissions[claudeTool]; + if (builtinPermission) return builtinPermission; + + const mcpPermission = kiloMcpPermissionPattern.exec(claudeTool); + return mcpPermission ? `${mcpPermission[1]}_${mcpPermission[2]}` : null; } function buildKiloAgentPermissionBlock(claudeTools) { @@ -1839,6 +1851,10 @@ function buildKiloAgentPermissionBlock(claudeTools) { for (const permission of kiloAgentPermissionOrder) { lines.push(` ${permission}: ${allowedPermissions.has(permission) ? 'allow' : 'deny'}`); } + for (const permission of allowedPermissions) { + if (kiloAgentPermissionOrder.includes(permission)) continue; + lines.push(` ${permission}: allow`); + } return lines; } @@ -7397,7 +7413,8 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (isAgent && inAgentTools) { if (trimmed.startsWith('- ')) { - agentTools.push(trimmed.substring(2).trim()); + const tool = runtimeArtifactConversion._decodeToolScalar(trimmed.substring(2)); + if (tool !== null) agentTools.push(tool); continue; } if (trimmed && !trimmed.startsWith('-')) { @@ -7409,8 +7426,12 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (trimmed.startsWith('tools:')) { if (isAgent) { const toolsValue = trimmed.substring(6).trim(); - if (toolsValue) { - const tools = toolsValue.split(',').map(t => t.trim()).filter(t => t); + // A comment-only value (`tools: # note`) is not real inline content — + // fall through to the block-list scan instead of decoding the + // comment as a bogus tool name and dropping the list (mirrors + // src/runtime-artifact-conversion.cts's convertClaudeToKiloFrontmatter, #4032). + if (toolsValue && !toolsValue.startsWith('#')) { + const tools = runtimeArtifactConversion._splitToolScalars(toolsValue).map(runtimeArtifactConversion._decodeToolScalar).filter(tool => tool !== null); agentTools.push(...tools); } else { inAgentTools = true; @@ -7476,7 +7497,8 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (trimmed.startsWith('- ')) { const tool = trimmed.substring(2).trim(); if (isAgent) { - agentTools.push(tool); + const decoded = runtimeArtifactConversion._decodeToolScalar(tool); + if (decoded !== null) agentTools.push(decoded); } else { allowedTools.push(tool); } @@ -11351,7 +11373,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } else if (_isSkillsRuntime) { console.log(` ${dim}↳${reset} Agents installed via descriptor-driven layout (${runtime})`); } else { - const _standaloneAgentsResult = installAgentsKindStandalone(runtime, targetDir, _installScopeId, _resolvedProfile, pathPrefix, getCommitAttribution, _installedCapabilityRegistry); + const _standaloneProjectDir = isGlobal ? process.cwd() : targetDir; + const _standaloneAgentsResult = installAgentsKindStandalone(runtime, targetDir, _installScopeId, _resolvedProfile, pathPrefix, getCommitAttribution, _installedCapabilityRegistry, _standaloneProjectDir); if (_standaloneAgentsResult) { // #2875 defect fix: installAgentsKindStandalone now returns `null` // (rather than a truthy result pointing at an empty destDir) whenever a diff --git a/capabilities/kimi/capability.json b/capabilities/kimi/capability.json index 6e88d246d..f2f07c550 100644 --- a/capabilities/kimi/capability.json +++ b/capabilities/kimi/capability.json @@ -97,7 +97,8 @@ "verificationStyle": "kimi", "agentManifestStyle": "kimi-nested", "doneBannerStyle": "kimi-agent-file", - "skipSharedHooksInstall": true + "skipSharedHooksInstall": true, + "noPathRewrite": true } } } diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 608ac63c1..82f39a2d9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -18,6 +18,7 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new "granularity": "standard", "model_profile": "balanced", "model_overrides": {}, + "agent_tools": {}, "models": {}, "dynamic_routing": null, "planning": { @@ -165,12 +166,66 @@ project one is reported, since that is the file you are most likely able to fix. **If you see this warning:** your config was not applied. Validate the file, for example with `node -e "JSON.parse(require('fs').readFileSync('.planning/config.json','utf8'))"`, then re-run. +## Agent tool grants + +`agent_tools` is an opt-in, install-time addition to the tools already declared by shipped +agents. Put defaults shared by your projects in `~/.gsd/defaults.json` and project-specific +choices in the nearest `.planning/config.json`: + +```json +{ + "agent_tools": { + "*": ["mcp__docs__search"], + "gsd-executor": ["WebFetch"] + } +} +``` + +Selectors are agent names; `"*"` applies to every agent. For an agent, GSD appends wildcard +grants before its named grants, after the agent's existing tools, in first-seen order. Re-running +the same install is idempotent: it does not add another copy of an existing grant. + +Project configuration replaces only selectors it names. For example, this project setting keeps +the global wildcard but replaces the global `gsd-executor` list: + +```json +{ + "agent_tools": { + "gsd-executor": ["WebSearch"] + } +} +``` + +Each selector value must be an array. A usable entry is a single tool token which, after trimming, +is non-empty and contains no whitespace, comma, `#`, quote, U+0000–U+001F, U+007F–U+009F, +U+2028, or U+2029, and does not end with `:`. +Invalid entries are ignored. An explicitly present but invalid project selector resolves to no +grant for that selector; it does not restore the global value. A present but invalid project +`agent_tools` container suppresses all global grants. Inline grants remain plain comma-separated +tool names as required by Claude; block-sequence entries are YAML-quoted. Agents without a +`tools:` key inherit the runtime's default tool surface, so GSD leaves those agents unchanged. + +A `--global` install still discovers the nearest `.planning/config.json` from the current working +directory, so `gsd install --global` run from inside a project applies that project's +`agent_tools` selectors to the global install too — not just to that project's own local install. + +Run `gsd install ` again after changing `agent_tools`; installed artifacts do not read +configuration at agent-spawn time. The shared staging path gives Claude, Codex, and Qwen their +existing host representations. Kimi maps supported canonical tools and continues to omit MCP +grants with its existing diagnostic. ZCode continues to omit `mcp__*` entries because its +dispatcher treats them as required MCP servers, and OpenCode keeps its converter-owned tools +omission. These are converter-specific output rules, not a claim that every runtime authorizes a +tool identically. Codex custom agents inherit the parent session's MCP servers natively; +`agent_tools` does not encode an allowlist into their TOML or widen `sandbox_mode`, which remains +derived from the shipped agent declaration. + ## Core Settings | Setting | Type | Options | Default | Description | |---------|------|---------|---------|-------------| | `mode` | enum | `interactive`, `yolo` | `interactive` | `yolo` auto-approves decisions; `interactive` confirms at each step | | `granularity` | enum | `coarse`, `standard`, `fine` | `standard` | Controls phase count: `coarse` (2-4), `standard` (4-6), `fine` (6-10) | +| `agent_tools.` | string[] | tool names meeting the [agent tool grant validation rules](#agent-tool-grants) | (none) | Additive install-time grants for `"*"` or a named agent. A project selector replaces the corresponding global selector; wildcard grants precede named grants. Re-run `gsd install ` after changing it. | | `model_profile` | enum | `quality`, `balanced`, `budget`, `adaptive`, `inherit` | `balanced` | Model tier for each agent (see [Model Profiles](#model-profiles)). `adaptive` was added per [#1713](https://github.com/open-gsd/gsd-core/issues/1713) / [#1806](https://github.com/open-gsd/gsd-core/issues/1806) and resolves the same way as the other tiers under runtime-aware profiles. | | `runtime` | string | `claude`, `codex`, or any string | (none) | Active runtime for [runtime-aware profile resolution](#runtime-aware-profiles-2517). When set, profile tiers (opus/sonnet/haiku) resolve to runtime-native model IDs. The resolved ID is embedded into each agent's static frontmatter at install time on `opencode` (whose `spawn_agent` interface does not accept an inline `model` parameter, so editing `model_overrides` requires re-running `gsd install ` to take effect — see [Per-Agent Overrides](#per-agent-overrides)); other runtimes consume the resolver at spawn time. **`codex` is the exception: it embeds no per-tier model at all.** Codex is a passive / session-only model host ([ADR-2313](adr/2313-codex-passive-model-posture.md)) — a ChatGPT-account session exposes only its own model, so a pinned tier model returns `400 invalid_request_error` and the agent fails to spawn. Codex agents therefore inherit the session model, and only an explicit real-Codex id in `model_overrides` (e.g. `"gpt-5.6-sol"`) is written into the `.toml`. When unset (default), model resolution is unchanged from prior versions — but the runtime GSD *reports* (`agent_runtime`) then falls through to [host detection](how-to/control-the-reported-host-runtime.md), which can resolve `codex` from Codex's own session environment. Detection affects reporting and the agent-installation check only; it never feeds tier resolution, which still reads this key alone. Added in v1.39; Codex behavior changed in v1.11; reporting-only host detection added in v1.11 | | `model_profile_overrides..` | string \| object | per-runtime tier override | (none) | Override the runtime-aware tier mapping for a specific `(runtime, tier)`. Tier is one of `opus`, `sonnet`, `haiku`. Value is either a model ID string (e.g. `"gpt-5-pro"`) or `{ model, reasoning_effort }`. See [Runtime-Aware Profiles](#runtime-aware-profiles-2517). Added in v1.39 | diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index ae3046341..58413b523 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -2259,7 +2259,8 @@ const capabilities = { "verificationStyle": "kimi", "agentManifestStyle": "kimi-nested", "doneBannerStyle": "kimi-agent-file", - "skipSharedHooksInstall": true + "skipSharedHooksInstall": true, + "noPathRewrite": true } } }, @@ -6933,7 +6934,8 @@ const runtimes = { "verificationStyle": "kimi", "agentManifestStyle": "kimi-nested", "doneBannerStyle": "kimi-agent-file", - "skipSharedHooksInstall": true + "skipSharedHooksInstall": true, + "noPathRewrite": true } } }, diff --git a/gsd-core/bin/shared/config-schema.manifest.json b/gsd-core/bin/shared/config-schema.manifest.json index f9bda88b0..2ea6033c9 100644 --- a/gsd-core/bin/shared/config-schema.manifest.json +++ b/gsd-core/bin/shared/config-schema.manifest.json @@ -53,6 +53,7 @@ "planning.pr_strict", "planning.search_gitignored", "planning.sub_repos", + "agent_tools", "review.default_reviewers", "review.max_prompt_tokens", "review.max_prompt_tokens_per_reviewer", @@ -118,6 +119,11 @@ "workflow._auto_chain_active" ], "dynamicKeyPatterns": [ + { + "topLevel": "agent_tools", + "source": "^agent_tools\\.(\\*|[a-zA-Z0-9_-]+)$", + "description": "agent_tools." + }, { "topLevel": "agent_skills", "source": "^agent_skills\\.[a-zA-Z0-9_-]+$", diff --git a/src/config-loader.cts b/src/config-loader.cts index 3684d8b7e..111630551 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -928,6 +928,7 @@ function loadConfigResolved(cwd: string, options: Record = {}): project_code: get('project_code') ?? defaults.project_code, subagent_timeout: get('subagent_timeout', { section: 'workflow', field: 'subagent_timeout' }) ?? defaults.subagent_timeout, model_overrides: (parsed['model_overrides']) || null, + agent_tools: (parsed['agent_tools']) || null, models: (parsed['models']) || null, granularity: parsed['granularity'] !== undefined ? parsed['granularity'] : null, granularities: (parsed['granularities']) || null, diff --git a/src/install-engine.cts b/src/install-engine.cts index b7fcbf5f1..b5df6f169 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -1055,6 +1055,7 @@ function installRuntimeArtifacts( // generic layout-driven loop below, mirroring the bespoke install path that // previously lived inline in bin/install.js. const behaviors = _hostBehaviors(runtime); + const projectDir = scope === 'global' ? process.cwd() : configDir; if (behaviors.combinedFamilyInstall) { // #2329: combined-family runtimes (OpenCode/Kilo) bypass // _runLegacyInstallMigrations below entirely (early return), so their @@ -1063,7 +1064,7 @@ function installRuntimeArtifacts( // #2874 design row 2: this early return must ALSO return an executed // plan — installOpencodeFamilyArtifacts reports what it wrote, so a // whole runtime family returning undefined is no longer a hole. - return installOpencodeFamilyArtifacts(runtime, configDir, scope, resolvedProfile, resolveAttribution, behaviors, capabilityRegistry); + return installOpencodeFamilyArtifacts(runtime, configDir, scope, resolvedProfile, resolveAttribution, behaviors, capabilityRegistry, projectDir); } // Legacy cleanup before layout-driven writes @@ -1083,6 +1084,7 @@ function installRuntimeArtifacts( homedir: () => os.homedir(), platform: process.platform, resolveAttribution, + projectDir, }); const cleanupDirs = planResult.ok ? planResult.plan.cleanupDirs : planResult.cleanupDirs; @@ -1480,6 +1482,7 @@ function installOpencodeFamilySkills( * @param capabilityRegistry - #2362: optional composed capability registry, threaded * straight through to resolveRuntimeArtifactLayout (unused by the agents kind today, * but kept for signature parity with the skills/commands siblings on this call tree) + * @param projectDir - project/config discovery root, distinct from the artifact destination * @returns `{ sourceDir, destDir }` describing what was written, or `null` when the * runtime's layout declares no `agents` kind. */ @@ -1491,6 +1494,7 @@ function installAgentsKindStandalone( pathPrefix: string, resolveAttribution: ResolveAttribution = () => undefined, capabilityRegistry?: any, + projectDir?: string | null, ): { sourceDir: string; destDir: string } | null { const layout: any = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, targetDir, scope as 'global' | 'local', capabilityRegistry); const agentsKindEntry = layout.kinds.find((k: any) => k.kind === 'agents'); @@ -1508,7 +1512,7 @@ function installAgentsKindStandalone( // for the generic layout-driven loop (runtime-artifact-install-plan.cts) — // targetDir IS the install root the inline agent loop called `targetDir`. const attribution = resolveAttribution ? resolveAttribution(runtime) : undefined; - const agentCtx = { runtime, pathPrefix, attribution, targetDir }; + const agentCtx = { runtime, pathPrefix, attribution, targetDir, projectDir: projectDir ?? targetDir }; const stagedDir: string = agentsKindEntry.stage(resolvedProfile, agentCtx); const stagedAgentFiles: string[] = installFs().existsSync(stagedDir) @@ -1792,6 +1796,7 @@ function _migrateLegacyOpencodeCommandDir(runtime: string, configDir: string, be * installOpencodeFamilySkills so an installed third-party capability skill * materializes for this combined-family (OpenCode/Kilo) install path too. * Absent -> no third-party skills staged (fail closed). + * @param projectDir - project/config discovery root, distinct from configDir for global installs * @returns #2874 design row 2: an executed-plan value, same top-level shape * (`runtime`/`scope`/`kinds`/`cleanup`/`postSteps`) as the generic * `installRuntimeArtifacts` branch — this was the one early return a @@ -1805,6 +1810,7 @@ function installOpencodeFamilyArtifacts( resolveAttribution: ResolveAttribution = () => undefined, behaviors: any = {}, capabilityRegistry?: any, + projectDir?: string | null, ): any { // #2870: `scope` keeps its exported required `string` signature (no // signature change). It is always the `installRuntimeArtifacts`-forwarded @@ -1846,7 +1852,7 @@ function installOpencodeFamilyArtifacts( // generic layout-driven loop uses (see installAgentsKindStandalone's own // doc). A `null` result means this runtime's layout declares no `agents` // kind — nothing written, nothing reported (no #1879-F15 inert claim). - const agentsResult = installAgentsKindStandalone(runtime, configDir, scope, resolvedProfile, pathPrefix, resolveAttribution, capabilityRegistry); + const agentsResult = installAgentsKindStandalone(runtime, configDir, scope, resolvedProfile, pathPrefix, resolveAttribution, capabilityRegistry, projectDir); _installNativePluginIfDeclared(runtime, configDir, behaviors, src); diff --git a/src/install-model-override-resolver.cts b/src/install-model-override-resolver.cts index 1ec43ed02..0f4eb373a 100644 --- a/src/install-model-override-resolver.cts +++ b/src/install-model-override-resolver.cts @@ -126,6 +126,54 @@ function readGsdEffectiveModelOverrides(targetDir: string | null = null, options return { ...(global || {}), ...(projectOverrides || {}) }; } +type AgentTools = Record; + +function readGsdAgentTools(config: Record | null): AgentTools | null { + const raw = config?.agent_tools; + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return null; + + const result: AgentTools = {}; + for (const [selector, value] of Object.entries(raw as Record)) { + // An explicitly present selector always has a verdict. Invalid values are + // empty so a project config cannot accidentally restore a global grant. + result[selector] = Array.isArray(value) + ? value.filter((entry): entry is string => typeof entry === 'string') + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0 + && !entry.endsWith(':') + && !/[\s\u0000-\u001F\u007F-\u009F,#"'\u2028\u2029]/.test(entry)) + : []; + } + return result; +} + +/** + * Resolve valid install-time `agent_tools` grants from global defaults and + * the nearest project config. Project selectors replace only matching global + * selectors; malformed whole files remain harmless absence like the existing + * model override resolver. + */ +function readGsdEffectiveAgentTools(targetDir: string | null = null, options: ReadOptions = {}): AgentTools | null { + const home = options.homedir ? options.homedir() : os.homedir(); + const globalConfig = _readGsdConfigFile(path.join(home, '.gsd', 'defaults.json'), 'global defaults'); + + let projectConfig: Record | null = null; + if (targetDir) { + const candidate = _findAncestorGsdConfigPath(targetDir); + if (candidate) { + projectConfig = _readGsdConfigFile(candidate, 'project config'); + } + } + + const global = readGsdAgentTools(globalConfig); + const project = readGsdAgentTools(projectConfig); + if (projectConfig + && Object.prototype.hasOwnProperty.call(projectConfig, 'agent_tools') + && project === null) return {}; + if (!global && !project) return null; + return { ...(global || {}), ...(project || {}) }; +} + interface RuntimeProfileMergedConfig { runtime: string | null; model_profile: string; @@ -267,6 +315,7 @@ function resolveAgentModelOverride( export = { readGsdGlobalModelOverrides, readGsdEffectiveModelOverrides, + readGsdEffectiveAgentTools, readGsdRuntimeProfileResolver, resolveAgentModelOverride, }; diff --git a/src/install-profiles.cts b/src/install-profiles.cts index f43140a9f..073aff418 100644 --- a/src/install-profiles.cts +++ b/src/install-profiles.cts @@ -36,6 +36,7 @@ const { readGsdCommandNames: _readGsdCommandNames, deriveAgentName: _deriveAgentName, applyAgentFrontmatterExtensions: _applyAgentFrontmatterExtensions, + appendAgentTools: _appendAgentTools, } = conversionModule as { applyAgentPathRewrites: (content: string, runtime: string, pathPrefix: string) => string; processAttribution: (content: string, attribution: string | null | undefined) => string; @@ -46,6 +47,7 @@ const { // runtime descriptor's hostBehaviors.agentFrontmatterExtensions. deriveAgentName: (fileName: string) => string; applyAgentFrontmatterExtensions: (content: string, opts: { runtime: string; agentName: string; targetDir?: string | null }) => string; + appendAgentTools: (content: string, grants: string[]) => string; }; // #2995 (epic #1671 Phase 6.4): agent bodies join the fragment model. Markers are @@ -57,6 +59,8 @@ import workflowFragmentsModule = require('./workflow-fragments.cjs'); const { composeWorkflow: _composeWorkflow } = workflowFragmentsModule as { composeWorkflow: (content: string, opts?: { sourcePath?: string }) => string; }; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installModelOverrideResolver = require('./install-model-override-resolver.cjs'); // --------------------------------------------------------------------------- // Profile definitions @@ -907,13 +911,16 @@ function stageSkillsForRuntimeAsSkills( * did with its own `targetDir` variable. Optional — a caller with no * `targetDir` in scope (e.g. the feat-1173 synthetic-descriptor seam tests) * degrades to `null`, matching `readGsdEffectiveEffortConfig(null)`'s own - * global-only-config contract. + * global-only-config contract. `projectDir` separates project config discovery + * from that artifact destination for global installs (#4032). */ interface AgentCtx { runtime: string; pathPrefix: string; attribution: string | null | undefined; targetDir?: string | null; + /** Project/config discovery root; defaults to targetDir for compatibility. */ + projectDir?: string | null; } /** @@ -936,14 +943,14 @@ interface AgentCtx { * agent loop in bin/install.js exactly: * 1. applyAgentPathRewrites (4 base ~/.claude/ regexes; skipped for copilot/antigravity) * 2. processAttribution (Co-Authored-By policy) - * 3. converter (runtime-specific frontmatter/body transform) - * 4. applyAgentFrontmatterExtensions (#2875 Part 2: effort/disallowedTools, + * 3. appendAgentTools (#4032: validated agent_tools grants, before host conversion) + * 4. converter (runtime-specific frontmatter/body transform) + * 5. applyAgentFrontmatterExtensions (#2875 Part 2: effort/disallowedTools, * gated by hostBehaviors.agentFrontmatterExtensions — no-op for a runtime * that declares nothing, e.g. every non-Claude runtime today) - * 5. normalizeAgentBodyForRuntime (colon→hyphen refs; no-op for trivial group) - * When `agentCtx` is absent, only the converter is applied (backward-compat for - * the feat-1173 synthetic-descriptor tests and the copilot/antigravity paths - * that handle cross-cutting inside their converters). + * 6. normalizeAgentBodyForRuntime (colon→hyphen refs; no-op for trivial group) + * When `agentCtx` is absent, only global `agent_tools` augmentation and the + * converter run; other cross-cutting remains absent for backward compatibility. * * @param srcAgentsDir source agents directory (e.g. agents/) * @param resolvedProfile profile filter from resolveProfile() @@ -957,8 +964,7 @@ interface AgentCtx { * every OTHER converter's contract — converters that don't declare a * 3rd parameter simply never read it. * @param isGlobal install scope passed through to the converter - * @param agentCtx optional cross-cutting context (ADR-1235 §1); when absent, - * only the converter is applied (backward compat) + * @param agentCtx optional cross-cutting context (ADR-1235 §1) */ function stageAgentsForRuntimeWithConverter( srcAgentsDir: string, @@ -994,6 +1000,7 @@ function stageAgentsForRuntimeWithConverter( try { // Resolve cmdNames once per staging call (not per file) for performance. const cmdNames = agentCtx ? _readGsdCommandNames() : []; + const agentTools = installModelOverrideResolver.readGsdEffectiveAgentTools(agentCtx?.projectDir ?? agentCtx?.targetDir ?? null); for (const entry of entries) { if (!entry.isFile()) continue; if (!entry.name.endsWith('.md')) continue; @@ -1011,24 +1018,30 @@ function stageAgentsForRuntimeWithConverter( // throws loudly naming the file for a malformed marker, never emitting a // half-composed agent. content = _composeWorkflow(content, { sourcePath: agentSourcePath }); + const agentName = _deriveAgentName(entry.name); + const grants = [ + ...(agentTools?.['*'] || []), + ...(agentTools?.[agentName] || []), + ]; if (agentCtx) { // #2875 Part 2 / row I3: derived exactly as the inline loop does — // single-sourced via deriveAgentName (runtime-artifact-conversion.cts). - const agentName = _deriveAgentName(entry.name); // ADR-1235 §1: pre-converter cross-cutting (matches inline loop order exactly) // Step 1: path rewrites (4 base ~/.claude/ regexes; skipped for copilot/antigravity) content = _applyAgentPathRewrites(content, agentCtx.runtime, agentCtx.pathPrefix); // Step 2: attribution content = _processAttribution(content, agentCtx.attribution); - // Step 3: converter (runtime-specific frontmatter/body transform) + // Step 3: validated canonical grants, before host conversion. + content = _appendAgentTools(content, grants); + // Step 4: converter (runtime-specific frontmatter/body transform) content = converter(content, isGlobal, { agentName }); - // Step 4: frontmatter extensions (effort/disallowedTools; no-op unless + // Step 5: frontmatter extensions (effort/disallowedTools; no-op unless // the runtime declares hostBehaviors.agentFrontmatterExtensions) content = _applyAgentFrontmatterExtensions(content, { runtime: agentCtx.runtime, agentName, targetDir: agentCtx.targetDir }); - // Step 5: normalize colon→hyphen refs (no-op for trivial group) + // Step 6: normalize colon→hyphen refs (no-op for trivial group) content = _normalizeAgentBodyForRuntime(content, agentCtx.runtime, cmdNames); } else { - // Backward-compat: only apply the converter (no cross-cutting) + content = _appendAgentTools(content, grants); content = converter(content, isGlobal); } installFs().writeFileSync(path.join(stageDir, entry.name), content, 'utf8'); diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index 5fe609e9e..c7e7c344e 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -288,12 +288,27 @@ const kiloAgentPermissionOrder = [ 'lsp', ]; +const kiloMcpPermissionPattern = /^mcp__([A-Za-z0-9_-]+)__((?:[A-Za-z0-9_-]+)|\*)$/; + +/** + * Derive Kilo's native `{server}_{tool}` MCP permission key (kilo.ai/docs — + * external, fixed format we don't control). Not injective: both capture + * groups allow `_`, so e.g. `mcp__a_b__c` and `mcp__a__b_c` derive the same + * key. `buildKiloAgentPermissionBlock`'s `Set` resolves any such collision + * deterministically to first-seen-wins — see its regression test. Changing + * the derivation to avoid the collision isn't an option: Kilo's own runtime + * only recognizes this exact key shape. + */ function convertClaudeToKiloPermissionTool(claudeTool) { - return claudeToKiloAgentPermissions[claudeTool] || null; + const builtinPermission = claudeToKiloAgentPermissions[claudeTool]; + if (builtinPermission) return builtinPermission; + + const mcpPermission = kiloMcpPermissionPattern.exec(claudeTool); + return mcpPermission ? `${mcpPermission[1]}_${mcpPermission[2]}` : null; } function buildKiloAgentPermissionBlock(claudeTools) { - const allowedPermissions = new Set(); + const allowedPermissions = new Set(); for (const tool of claudeTools) { const mapped = convertClaudeToKiloPermissionTool(tool); @@ -306,6 +321,10 @@ function buildKiloAgentPermissionBlock(claudeTools) { for (const permission of kiloAgentPermissionOrder) { lines.push(` ${permission}: ${allowedPermissions.has(permission) ? 'allow' : 'deny'}`); } + for (const permission of allowedPermissions) { + if (kiloAgentPermissionOrder.includes(permission)) continue; + lines.push(` ${permission}: allow`); + } return lines; } @@ -675,6 +694,122 @@ function convertClaudeCommandToKimiCodeSkill(content, skillName, _runtime = null const KIMI_CANONICAL_GSD_AGENT_RE = /^gsd-[a-z0-9-]+$/; +/** Split a `tools:` comma list without tearing a quoted scalar that contains a literal comma. */ +function splitToolScalars(text: string): string[] { + const parts: string[] = []; + let current = ''; + let quote: string | null = null; + for (const ch of text) { + if (quote) { + current += ch; + if (ch === quote) quote = null; + } else if (ch === '"' || ch === "'") { + quote = ch; + current += ch; + } else if (ch === ',') { + parts.push(current); + current = ''; + } else { + current += ch; + } + } + parts.push(current); + return parts; +} + +/** Normalize one complete frontmatter tool scalar through the shared YAML parser. */ +function decodeToolScalar(raw: unknown): string | null { + if (typeof raw !== 'string') return null; + const value = raw.trim(); + if (!value) return null; + if (value.startsWith('"') || value.startsWith("'")) { + try { + const decoded = frontmatterModule.parseFrontmatter('---\ntool: ' + value + '\n---\n').tool; + return typeof decoded === 'string' && decoded.trim() ? decoded.trim() : null; + } catch { + return null; + } + } + // A bare (unquoted) scalar may carry a trailing ` # comment` (YAML requires + // whitespace before `#` to start a comment) — every caller here passes a + // single already-comma-split token, never the whole `tools:` line, so this + // is safe to strip unconditionally rather than pushing the concern onto + // each call site. Strip BEFORE the malformed-quote check below: a comment + // may itself contain a `"`/`'` (e.g. `Bash # note: "internal"`), which is + // not a real YAML quote delimiter and must not cause a false rejection. + const commentIndex = value.search(/[ \t]#/); + const stripped = commentIndex === -1 ? value : value.slice(0, commentIndex).trimEnd(); + if (!stripped) return null; + if (stripped.endsWith('"') || stripped.endsWith("'")) return null; + return stripped; +} + +/** Append validated canonical grants without reserializing unrelated frontmatter. */ +function appendAgentTools(content: string, grants: string[]): string { + if (grants.length === 0) return content; + const eol = content.includes('\r\n') ? '\r\n' : '\n'; + const lines = content.split(eol); + if (lines[0] !== '---') return content; + const frontmatterEnd = lines.indexOf('---', 1); + if (frontmatterEnd === -1) return content; + const toolsIndex = lines.findIndex((line, index) => index < frontmatterEnd && /^tools:[ \t]*(.*)$/.test(line)); + if (toolsIndex === -1) return content; + + const toolsMatch = /^tools:[ \t]*(.*)$/.exec(lines[toolsIndex]); + if (!toolsMatch) return content; + // Plain (non-leading-quote) YAML scalars have no internal quoting — a `#` + // after whitespace is a real comment start regardless of nearby quote + // characters (verified: real YAML truncates `Read, "x #1"` at the space + // before `#` too), so this scan does not need to be quote-aware. The one + // case that DOES need protection — a value that IS a leading quoted scalar + // — is refused outright below rather than parsed past. + const commentIndex = toolsMatch[1].search(/[ \t]#/); + let inlineValue = commentIndex === -1 ? toolsMatch[1] : toolsMatch[1].slice(0, commentIndex); + let inlineComment = commentIndex === -1 ? '' : toolsMatch[1].slice(commentIndex); + // A header that is ONLY a comment (`tools: # note`) has no leading space in + // the captured group — the outer regex's `[ \t]*` already consumed it — so + // the `[ \t]#` scan above never fires. Reclassify as no inline value so the + // block-list scan below runs instead of swallowing the comment as content. + if (inlineValue.trim().startsWith('#')) { + inlineComment = toolsMatch[1]; + inlineValue = ''; + } + // A value that STARTS with a quote is a YAML quoted scalar occupying the + // whole node — nothing may follow it on the same line except a comment + // (`tools: "Bash"` is valid; `tools: "Bash", Read` is not, even before this + // function touches it). Appending in place would corrupt otherwise-valid + // frontmatter, so refuse rather than emit invalid YAML. A value that STARTS + // with `[` is a YAML flow sequence (`tools: [Bash, Read]`) — its own commas + // are node-internal, not scalar separators, and content cannot follow its + // closing `]` on the same line either, so the same refusal applies. + if (/^["'[]/.test(inlineValue.trim())) return content; + const existing: string[] = []; + let insertAt = toolsIndex + 1; + if (inlineValue.trim()) { + existing.push(...splitToolScalars(inlineValue).map(decodeToolScalar).filter((value): value is string => value !== null)); + } else { + while (insertAt < frontmatterEnd) { + const item = /^([ \t]+)-[ \t]*(\S.*)$/.exec(lines[insertAt]); + if (!item) break; + const decoded = decodeToolScalar(item[2]); + if (decoded !== null) existing.push(decoded); + insertAt += 1; + } + } + const present = new Set(existing); + const additions = grants.filter((grant) => !present.has(grant) && (present.add(grant), true)); + if (additions.length === 0) return content; + + if (inlineValue.trim()) { + lines[toolsIndex] = `tools: ${inlineValue.trimEnd()}, ${additions.join(', ')}${inlineComment}`; + } else { + const firstItem = /^([ \t]+)-/.exec(lines[toolsIndex + 1]); + const indent = firstItem ? firstItem[1] : ' '; + lines.splice(insertAt, 0, ...additions.map((grant) => `${indent}- ${JSON.stringify(grant)}`)); + } + return lines.join(eol); +} + function parseKimiAgentSource(source) { if (typeof source === 'string') { return { @@ -703,7 +838,8 @@ function parseFrontmatterTools(frontmatter) { if (collecting) { if (trimmed.startsWith('- ')) { - tools.push(trimmed.slice(2).trim()); + const tool = decodeToolScalar(trimmed.slice(2)); + if (tool !== null) tools.push(tool); continue; } collecting = false; @@ -716,10 +852,13 @@ function parseFrontmatterTools(frontmatter) { if (trimmed.startsWith('tools:') || trimmed.startsWith('allowed-tools:')) { const value = trimmed.slice(trimmed.indexOf(':') + 1).trim(); - if (value) { - for (const tool of value.split(',')) { - const name = tool.trim(); - if (name) tools.push(name); + // A comment-only value (`tools: # note`) has no real inline content — + // fall through to the block-list scan below instead of decoding the + // comment text as a bogus tool name and silently dropping the list. + if (value && !value.startsWith('#')) { + for (const tool of splitToolScalars(value)) { + const name = decodeToolScalar(tool); + if (name !== null) tools.push(name); } } else { collecting = true; @@ -2100,7 +2239,8 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (isAgent && inAgentTools) { if (trimmed.startsWith('- ')) { - agentTools.push(trimmed.substring(2).trim()); + const tool = decodeToolScalar(trimmed.substring(2)); + if (tool !== null) agentTools.push(tool); continue; } if (trimmed && !trimmed.startsWith('-')) { @@ -2112,8 +2252,11 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (trimmed.startsWith('tools:')) { if (isAgent) { const toolsValue = trimmed.substring(6).trim(); - if (toolsValue) { - const tools = toolsValue.split(',').map(t => t.trim()).filter(t => t); + // A comment-only value (`tools: # note`) is not real inline content — + // fall through to the block-list scan (`inAgentTools`) instead of + // decoding the comment as a bogus tool name and dropping the list. + if (toolsValue && !toolsValue.startsWith('#')) { + const tools = splitToolScalars(toolsValue).map(decodeToolScalar).filter((tool): tool is string => tool !== null); agentTools.push(...tools); } else { inAgentTools = true; @@ -2179,7 +2322,8 @@ function convertClaudeToKiloFrontmatter(content, { isAgent = false, modelOverrid if (trimmed.startsWith('- ')) { const tool = trimmed.substring(2).trim(); if (isAgent) { - agentTools.push(tool); + const decoded = decodeToolScalar(tool); + if (decoded !== null) agentTools.push(decoded); } else { allowedTools.push(tool); } @@ -2545,11 +2689,17 @@ function convertClaudeAgentToQwenAgent(content) { * Byte-identical for an agent with no `mcp__*` grants (the common case) and * for an agent with no frontmatter at all. */ +/** Fail-closed: an undecodable scalar is dropped, never kept (ZCode's contract is "never emit mcp__*"). */ +function zcodeKeepsGrant(rawTool: string): boolean { + const decoded = decodeToolScalar(rawTool); + return decoded !== null && !decoded.startsWith('mcp__'); +} + 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; + // A double-quoted YAML scalar may encode the leading "m" in mcp__ as an + // escape, so only skip the shared scalar-decoder scan when neither form is + // present. The unchanged scan below still preserves byte-identical content. + if (!content.includes('mcp__') && !content.includes('\\')) return content; const lines = content.split('\n'); if (lines[0] !== '---') return content; @@ -2568,9 +2718,14 @@ function convertClaudeAgentToZcodeAgent(content) { 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__')); + // A header that is ONLY a comment (`tools: # note`) is not real inline + // content — fall through to the block-list scan below instead of + // matching here, or a following block list's mcp__* items never get + // scanned and leak through unstripped. + const commentOnlyHeader = inlineTools !== null && inlineTools[1].trim().startsWith('#'); + if (inlineTools && !commentOnlyHeader) { + const grants = splitToolScalars(inlineTools[1]).map((tool) => tool.trim()).filter((tool) => tool !== ''); + const kept = grants.filter((tool) => zcodeKeepsGrant(tool)); if (kept.length === grants.length) { out.push(line); // no mcp__* grants — keep the line byte-identical } else if (kept.length > 0) { @@ -2582,7 +2737,7 @@ function convertClaudeAgentToZcodeAgent(content) { i++; continue; } - if (/^tools:[ \t]*$/.test(line)) { + if (/^tools:[ \t]*$/.test(line) || commentOnlyHeader) { // Block-list form: collect the following `- item` lines. const items = []; let j = i + 1; @@ -2592,7 +2747,7 @@ function convertClaudeAgentToZcodeAgent(content) { } const kept = items.filter((item) => { const name = /^([ \t]*)-[ \t]*(\S.*)$/.exec(item)[2].trim(); - return !name.startsWith('mcp__'); + return zcodeKeepsGrant(name); }); if (kept.length !== items.length) { changed = true; @@ -3644,6 +3799,7 @@ function processAttribution( export = { processAttribution, + appendAgentTools, // #2103: public accessor for hostBehaviors.agentFileExtension, exported so // surface.cts's _syncGsdDir can derive the .agent.md rename from the SAME // descriptor read as install-engine.cts (folds a duplicated hardcoded @@ -3700,6 +3856,8 @@ export = { // opencode/kilo command install through the engine instead of the bespoke path). convertClaudeToOpencodeFrontmatter, convertClaudeToKiloFrontmatter, + _decodeToolScalar: decodeToolScalar, + _splitToolScalars: splitToolScalars, readGsdCommandNames, transformContentToHyphen, // #1383: version resolver (exported for regression test of the Codex diff --git a/src/runtime-artifact-install-plan.cts b/src/runtime-artifact-install-plan.cts index 786224bbe..8d1e97f96 100644 --- a/src/runtime-artifact-install-plan.cts +++ b/src/runtime-artifact-install-plan.cts @@ -36,6 +36,8 @@ interface AgentCtx { * resolution can read config exactly as the inline agent loop's own * `targetDir` variable did. */ targetDir?: string | null; + /** Project/config discovery root, distinct from global artifact destinations. */ + projectDir?: string | null; } interface ArtifactKind { @@ -114,6 +116,7 @@ interface CreateRuntimeArtifactInstallPlanArgs { homedir?: () => string; platform?: NodeJS.Platform; resolveAttribution?: (runtime: string) => string | null | undefined; + projectDir?: string | null; deps?: Dependencies; } @@ -164,6 +167,7 @@ function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlan homedir, platform, resolveAttribution, + projectDir, deps = {}, } = args; const conversionExports = _require('./runtime-artifact-conversion.cjs') as RuntimeArtifactConversionExports; @@ -203,12 +207,18 @@ function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlan const attribution = resolveAttribution ? resolveAttribution(layout.runtime) : undefined; // #2875 Part 2 (row I1): layout.configDir IS the install root the inline // agent loop called `targetDir` — same value, same resolution. - const agentCtx: AgentCtx = { runtime: layout.runtime, pathPrefix, attribution, targetDir: layout.configDir }; + const agentCtx: AgentCtx = { + runtime: layout.runtime, + pathPrefix, + attribution, + targetDir: layout.configDir, + projectDir: projectDir ?? layout.configDir, + }; for (const kind of layout.kinds) { let stagedDir: string; try { - if (kind.kind === 'agents') { + if (kind.kind === 'agents' || kind.kind === 'kimi-agents') { // ADR-1235 §1: pass agentCtx so stageAgentsForRuntimeWithConverter applies // the full inline-loop order: pathRewrites → attribution → converter → normalize. // The cross-cutting is now PRE-converter (inside staging), not POST. @@ -229,7 +239,7 @@ function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlan const rewrittenDir = rewriteStagedSkillBodies(stagedDir, rewriteOpts); sourceDir = addCleanupDir(cleanupDirs, stagedDir, rewrittenDir); } - // agents kind: cross-cutting already applied INSIDE kind.stage() via agentCtx. + // Agent kinds: cross-cutting already applied INSIDE kind.stage() via agentCtx. // No POST-step needed. sourceDir stays as stagedDir. } catch (err) { return { ok: false, kind: 'rewrite_failed', message: errorMessage(err), cleanupDirs, failedKind: kind.kind }; diff --git a/src/runtime-artifact-layout.cts b/src/runtime-artifact-layout.cts index 99f5a353f..2755dc8d4 100644 --- a/src/runtime-artifact-layout.cts +++ b/src/runtime-artifact-layout.cts @@ -105,6 +105,8 @@ interface AgentCtx { * per-agent model-override resolution below. Mirrors install-profiles.cts's * identically-named AgentCtx field — see its doc comment. */ targetDir?: string | null; + /** Project/config discovery root, distinct from global artifact destinations. */ + projectDir?: string | null; } interface ArtifactKind { @@ -514,7 +516,7 @@ function kimiAgentsKind(destSubpath: string, prefix: string, configDir: string): kind: 'kimi-agents', destSubpath, prefix, - stage: (resolved) => { + stage: (resolved, agentCtx) => { const buildKimiAgentArtifacts = conversionExports['buildKimiAgentArtifacts'] as (opts: { rootAgent?: string; subagents?: Array<{ path: string; content: string }>; @@ -528,6 +530,8 @@ function kimiAgentsKind(destSubpath: string, prefix: string, configDir: string): findAgentsSourceRoot(configDir), resolved, (content: string) => content, + false, + agentCtx, ); const subagents: Array<{ path: string; content: string }> = []; if (installFs().existsSync(stagedAgents)) { diff --git a/tests/agent-tools.install.test.cjs b/tests/agent-tools.install.test.cjs new file mode 100644 index 000000000..af652d880 --- /dev/null +++ b/tests/agent-tools.install.test.cjs @@ -0,0 +1,522 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product (#4032) — these assertions read +// emitted installer artifacts, the deployed agent contract. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const fc = require('fast-check'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { createTempDir, cleanup, runGsdTools } = require('./helpers.cjs'); +const { installerEnv, RUNTIME_META } = require('./helpers/install-shared.cjs'); +const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const { appendAgentTools, buildKimiAgentArtifacts, convertClaudeAgentToQwenAgent, convertClaudeAgentToZcodeAgent, _decodeToolScalar } = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); +const { parseFrontmatter } = require('../gsd-core/bin/lib/frontmatter.cjs'); + +function parseTools(content) { + const tools = parseFrontmatter(content).tools; + if (tools === undefined || tools === null) return []; + if (typeof tools === 'object' && !Array.isArray(tools)) return []; + return (Array.isArray(tools) ? tools : [tools]) + .flatMap((value) => String(value).replace(/\s+#.*$/, '').split(/[,\s]+/)) + .map((value) => value.trim()) + .filter(Boolean); +} + +function installClaude(t, { defaults, projectConfig, root = createTempDir('gsd-4032-claude-') } = {}) { + t.after(() => cleanup(root)); + if (defaults !== undefined) { + fs.mkdirSync(path.join(root, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(root, '.gsd', 'defaults.json'), JSON.stringify(defaults), 'utf8'); + } + if (projectConfig !== undefined) { + fs.mkdirSync(path.join(root, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(root, '.planning', 'config.json'), JSON.stringify(projectConfig), 'utf8'); + } + const args = ['--preserve-symlinks', '--preserve-symlinks-main', path.join(REPO_ROOT, 'bin', 'install.js'), '--claude', '--local']; + const result = runNode(args, { + cwd: root, + env: installerEnv({ HOME: root, USERPROFILE: root }), + timeoutMs: INSTALL_TIMEOUT_MS, + }); + assert.strictEqual(result.exitCode, 0, `Claude install failed:\n${result.stderr}`); + return { + root, + agent(name) { + return fs.readFileSync(path.join(root, '.claude', 'agents', `${name}.md`), 'utf8'); + }, + }; +} + +function installRuntime(t, runtime, { defaults, projectConfig, repeat = false, scope = 'local' } = {}) { + const root = createTempDir(`gsd-4032-${runtime}-project-`); + const home = createTempDir(`gsd-4032-${runtime}-home-`); + t.after(() => { + cleanup(root); + cleanup(home); + }); + if (defaults !== undefined) { + fs.mkdirSync(path.join(home, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(home, '.gsd', 'defaults.json'), JSON.stringify(defaults), 'utf8'); + } + if (projectConfig !== undefined) { + fs.mkdirSync(path.join(root, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(root, '.planning', 'config.json'), JSON.stringify(projectConfig), 'utf8'); + } + const configDir = scope === 'global' + ? path.join(home, RUNTIME_META[runtime].globalSuffix) + : path.join(root, RUNTIME_META[runtime].localDir); + const args = ['--preserve-symlinks', '--preserve-symlinks-main', path.join(REPO_ROOT, 'bin', 'install.js'), `--${runtime}`]; + if (scope === 'global') args.push('--global', '--config-dir', configDir); + else args.push('--local'); + const run = () => runNode(args, { + cwd: root, + env: installerEnv({ HOME: home, USERPROFILE: home }), + timeoutMs: INSTALL_TIMEOUT_MS, + }); + const result = run(); + assert.strictEqual(result.exitCode, 0, `${runtime} install failed:\n${result.stderr}`); + if (repeat) { + const rerun = run(); + assert.strictEqual(rerun.exitCode, 0, `${runtime} reinstall failed:\n${rerun.stderr}`); + } + return { root, home, configDir }; +} + +function emittedAgentArtifacts(install, agentName, root = install.configDir) { + const artifacts = []; + const visit = (dir) => { + if (!fs.existsSync(dir)) return; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const candidate = path.join(dir, entry.name); + if (entry.isDirectory()) visit(candidate); + else if (entry.isFile() && new RegExp(`^${agentName.replace(/[.*+?^${}()|[\\]\\]/g, '\\$&')}(?:\\.|$)`).test(entry.name)) { + artifacts.push(fs.readFileSync(candidate, 'utf8')); + } + } + }; + visit(root); + assert.ok(artifacts.length > 0, `install must emit at least one ${agentName} artifact`); + return artifacts; +} + +test('Claude installer appends wildcard then named grants exactly once (#4032)', (t) => { + const installed = installClaude(t, { + defaults: { + agent_tools: { + '*': ['mcp__global__one', 'mcp__shared__tool'], + 'gsd-executor': ['mcp__agent__two', 'mcp__shared__tool'], + }, + }, + }); + + const tools = parseTools(installed.agent('gsd-executor')); + assert.match( + installed.agent('gsd-executor'), + /^tools:.*mcp__global__one, mcp__shared__tool, mcp__agent__two$/m, + 'Claude inline tools must remain plain comma-separated tool names', + ); + assert.deepStrictEqual( + tools.slice(-3), + ['mcp__global__one', 'mcp__shared__tool', 'mcp__agent__two'], + ); + assert.strictEqual(tools.filter((tool) => tool === 'mcp__shared__tool').length, 1); +}); + +test('project selectors override only their matching global selector (#4032)', (t) => { + const installed = installClaude(t, { + defaults: { agent_tools: { '*': ['mcp__global__wildcard'], 'gsd-executor': ['mcp__global__executor'] } }, + projectConfig: { agent_tools: { 'gsd-executor': ['mcp__project__executor'] } }, + }); + + const tools = parseTools(installed.agent('gsd-executor')); + assert.ok(tools.includes('mcp__global__wildcard')); + assert.ok(tools.includes('mcp__project__executor')); + assert.ok(!tools.includes('mcp__global__executor')); +}); + +test('an invalid project selector fails closed instead of restoring a global grant (#4032)', (t) => { + const installed = installClaude(t, { + defaults: { agent_tools: { 'gsd-executor': ['mcp__global__executor'] } }, + projectConfig: { agent_tools: { 'gsd-executor': 'not-an-array' } }, + }); + + assert.ok(!parseTools(installed.agent('gsd-executor')).includes('mcp__global__executor')); +}); + +test('an invalid project agent_tools container suppresses global grants (#4032)', (t) => { + const installed = installClaude(t, { + defaults: { agent_tools: { '*': ['mcp__global__wildcard'] } }, + projectConfig: { agent_tools: ['not-an-object'] }, + }); + + assert.ok(!parseTools(installed.agent('gsd-executor')).includes('mcp__global__wildcard')); +}); + +test('config-set accepts named and wildcard agent_tools selectors (#4032)', (t) => { + const root = createTempDir('gsd-4032-config-'); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(root, '.planning', 'config.json'), '{}\n', 'utf8'); + const env = { HOME: root, USERPROFILE: root }; + + for (const selector of ['gsd-executor', '*']) { + const result = runGsdTools( + ['config-set', `agent_tools.${selector}`, '["mcp__configured__grant"]'], + root, + env, + ); + assert.ok(result.success, `config-set agent_tools.${selector} failed: ${result.error}`); + } + + const config = JSON.parse(fs.readFileSync(path.join(root, '.planning', 'config.json'), 'utf8')); + assert.deepStrictEqual(config.agent_tools, { + 'gsd-executor': ['mcp__configured__grant'], + '*': ['mcp__configured__grant'], + }); +}); + +test('inline and block tools forms keep their form after installer augmentation (#4032)', (t) => { + const installed = installClaude(t, { + defaults: { agent_tools: { '*': ['mcp__form__grant'] } }, + }); + + const inline = installed.agent('gsd-executor'); + const block = installed.agent('gsd-nyquist-auditor'); + assert.match(inline, /^tools:[^\n]+mcp__form__grant/m); + assert.match(block, /^tools:\r?\n(?:[ \t]+- [^\n]+\r?\n)*[ \t]+- "mcp__form__grant"$/m); +}); + +test('missing or invalid agent_tools leave installed agent bytes unchanged (#4032)', (t) => { + const baselineInstall = installClaude(t); + const baseline = baselineInstall.agent('gsd-executor'); + const invalidInstall = installClaude(t, { + root: baselineInstall.root, + defaults: { agent_tools: { '*': [null, '', ' '] } }, + }); + const invalid = invalidInstall.agent('gsd-executor'); + assert.strictEqual(invalid, baseline); +}); + +test('host converters receive canonical grants without changing their omissions (#4032)', (t) => { + const defaults = { agent_tools: { 'gsd-executor': ['mcp__configured__grant', 'mcp__first__*'] } }; + for (const runtime of ['claude', 'codex']) { + const artifacts = emittedAgentArtifacts(installRuntime(t, runtime, { defaults }), 'gsd-executor'); + assert.ok(artifacts.some((artifact) => artifact.includes('mcp__configured__grant')), + `${runtime} must expose the configured canonical grant in its existing host form`); + } + const kiloArtifacts = emittedAgentArtifacts(installRuntime(t, 'kilo', { defaults }), 'gsd-executor'); + assert.ok(kiloArtifacts.some((artifact) => { + const configured = artifact.indexOf(' configured_grant: allow'); + const wildcard = artifact.indexOf(' first_*: allow'); + return configured >= 0 && wildcard > configured; + }), + 'Kilo must translate safe canonical MCP grants into native permission keys in first-seen order'); + for (const runtime of ['zcode', 'opencode']) { + const artifacts = emittedAgentArtifacts(installRuntime(t, runtime, { defaults }), 'gsd-executor'); + assert.ok(artifacts.every((artifact) => !artifact.includes('mcp__configured__grant')), + `${runtime} must preserve its existing tool omission policy`); + } +}); + +test('Kimi receives canonical grants before its existing mapper runs (#4032)', (t) => { + const install = installRuntime(t, 'kimi', { + defaults: { agent_tools: { 'gsd-executor': ['WebFetch'] } }, + scope: 'global', + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor'); + assert.ok(artifacts.some((artifact) => artifact.includes('kimi_cli.tools.web:FetchURL')), + 'Kimi must map a configured canonical WebFetch tool through its existing converter'); +}); + +test('Kimi project selectors override matching global selectors (#4032)', (t) => { + const install = installRuntime(t, 'kimi', { + defaults: { agent_tools: { 'gsd-executor': ['WebFetch'] } }, + projectConfig: { agent_tools: { 'gsd-executor': ['WebSearch'] } }, + scope: 'global', + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor'); + assert.ok(artifacts.some((artifact) => artifact.includes('kimi_cli.tools.web:SearchWeb')), + 'Kimi must map the project grant through its existing converter'); + assert.ok(artifacts.every((artifact) => !artifact.includes('kimi_cli.tools.web:FetchURL')), + 'the matching global selector must not survive the project override'); +}); + +test('Kimi neutralizes ~/.claude/gsd-core references instead of leaking a repointed path (#4032)', (t) => { + // Routing Kimi through the ADR-1235 pre-converter pipeline (needed so + // project-scoped agent_tools selectors reach it, see the test above) also + // runs _applyAgentPathRewrites. Kimi's own neutralizeKimiAgentPrompt expects + // to see the ORIGINAL `~/.claude/gsd-core` text; kimi/kimi-code capability.json + // now opt out via hostBehaviors.noPathRewrite so that still holds. + const install = installRuntime(t, 'kimi', { scope: 'global' }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor'); + assert.ok(artifacts.some((artifact) => artifact.includes('GSD core')), + 'a ~/.claude/gsd-core reference must neutralize to prose, not a repointed Kimi path'); + assert.ok(artifacts.every((artifact) => + !artifact.includes('~/.claude/gsd-core') && + !artifact.includes('$HOME/.claude/gsd-core') && + !artifact.includes('config/agents/gsd-core/references')), + 'no raw Claude- or Kimi-prefixed gsd-core path may leak into the Kimi prompt body'); +}); + +test('Kilo global install resolves project agent_tools from the working directory (#4032)', (t) => { + const install = installRuntime(t, 'kilo', { + defaults: { agent_tools: { 'gsd-executor': ['mcp__global__loser'] } }, + projectConfig: { agent_tools: { 'gsd-executor': ['mcp__project__winner'] } }, + scope: 'global', + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor'); + assert.ok(artifacts.some((artifact) => artifact.includes(' project_winner: allow')), + 'the combined-family path must discover project config from cwd during a global install'); + assert.ok(artifacts.every((artifact) => !artifact.includes('global_loser')), + 'the matching global selector must not survive the project override'); +}); + +test('Kilo permission-key collision resolves deterministically to first-seen (#4032)', (t) => { + // mcp__a_b__c and mcp__a__b_c both derive Kilo's native "a_b_c" key — the + // `{server}_{tool}` format is Kilo's own fixed contract, not ours to widen. + const install = installRuntime(t, 'kilo', { + defaults: { agent_tools: { 'gsd-executor': ['mcp__a_b__c', 'mcp__a__b_c'] } }, + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor'); + assert.ok(artifacts.some((artifact) => (artifact.match(/^ {2}a_b_c: allow$/gm) || []).length === 1), + 'a colliding pair must still emit exactly one permission line'); +}); + +test('every installable runtime accepts a configured MCP grant without crashing (#4032)', (t) => { + // Shallow, broad: appendAgentTools runs pre-converter for every runtime, but + // only 6 have deep per-runtime assertions elsewhere in this file. This locks + // in that the other runtimes' own converters don't choke or mangle output + // when a canonical grant is appended into their frontmatter dialect. + // scope: 'global' — universally supported (cline is global-only; local + // support varies per runtime, global does not). Search from `install.home`, + // not `install.configDir`: a nested-home runtime (e.g. antigravity) places + // agents in a sibling directory outside its own configDir subtree. + const NO_SUBAGENT_TOOLKIT = new Set(['pi']); // programmatic dispatch, no named-dispatch agent files + // Runtimes empirically verified (see PR #4238 remediation) to pass an + // arbitrary mcp__ grant through recognizably — either verbatim or via + // Kilo's {server}_{tool} transform. Every other runtime filters unknown + // tool names through its own built-in vocabulary (a legitimate, unrelated + // per-runtime design choice, not an agent_tools omission) and is checked + // for a clean, non-crashing install only. This is an allowlist, not a + // guess-based omit-list, so it can't silently drift as runtimes are added. + const GRANT_SURVIVES_RECOGNIZABLY = new Set(['claude', 'codex', 'copilot', 'hermes', 'kimi-code', 'kilo', 'qwen']); + for (const runtime of Object.keys(RUNTIME_META)) { + if (NO_SUBAGENT_TOOLKIT.has(runtime)) continue; + const install = installRuntime(t, runtime, { + defaults: { agent_tools: { 'gsd-executor': ['mcp__smoke__probe'] } }, + scope: 'global', + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-executor', install.home); + assert.ok(artifacts.every((artifact) => artifact.length > 0), `${runtime} must emit non-empty gsd-executor artifact(s)`); + if (!GRANT_SURVIVES_RECOGNIZABLY.has(runtime)) continue; + assert.ok(artifacts.some((artifact) => artifact.includes('mcp__smoke__probe') || artifact.includes('smoke_probe')), + `${runtime} must carry the configured grant (raw or Kilo-style transformed) — this must fail if agent_tools is reverted`); + } +}); + +test('Codex grants do not widen the generated TOML sandbox (#4032)', (t) => { + const install = installRuntime(t, 'codex', { + defaults: { agent_tools: { 'gsd-plan-checker': ['Write'] } }, + }); + const toml = fs.readFileSync(path.join(install.configDir, 'agents', 'gsd-plan-checker.toml'), 'utf8'); + assert.match(toml, /^sandbox_mode = "read-only"$/m); + assert.doesNotMatch(toml, /Write/, + 'Codex tool availability is inherited from the parent session, not encoded in agent TOML'); +}); + +test('hostile values fail closed while inline Claude tools remain valid tokens (#4032)', (t) => { + const rejected = [ + null, 1, '', ' ', 'mcp__bad,comma', 'mcp__bad\0nul', 'mcp__bad\nline', 'mcp__bad\u0085nel', 'mcp__bad\u2028line', + '#comment', 'tool:', 'tool: value', 'Bash(git log:*)', '"quote"', "'quote'", + ]; + const accepted = ['mcp__safe__:terminal', 'Agent(worker)', '\\backslash']; + const installed = installClaude(t, { defaults: { agent_tools: { '*': [...rejected, ...accepted] } } }); + const content = installed.agent('gsd-executor'); + const frontmatter = content.slice(4, content.indexOf('\n---', 4)); + const parsed = require('js-yaml').load(frontmatter); + assert.strictEqual(typeof parsed.tools, 'string', 'the emitted inline tools scalar must remain valid YAML'); + assert.deepStrictEqual(parseTools(content).slice(-accepted.length), accepted); + assert.ok(rejected.every((value) => typeof value !== 'string' || !parseTools(content).includes(value))); + assert.ok(rejected.every((value) => typeof value !== 'string' || !value.trim() || !content.includes(value.trim())), + 'rejected entries must not leak into the installed artifact under a different tokenization'); +}); + +test('reinstall remains idempotent and preserves Claude read-only restrictions (#4032)', (t) => { + const install = installRuntime(t, 'claude', { + defaults: { agent_tools: { '*': ['mcp__idempotent__grant'] } }, + repeat: true, + }); + const artifacts = emittedAgentArtifacts(install, 'gsd-plan-checker'); + assert.ok(artifacts.every((artifact) => parseTools(artifact).filter((tool) => tool === 'mcp__idempotent__grant').length === 1)); + assert.ok(artifacts.some((artifact) => artifact.includes('disallowedTools:')), + 'the existing Claude read-only deny-list must survive augmentation'); +}); + +test('quoted scalar identity is shared by append, Kimi, and Qwen (#4191)', () => { + for (const [raw, expected] of [ + ['"\\x6dcp__server__tool"', 'mcp__server__tool'], + ['"\\u006dcp__server__tool"', 'mcp__server__tool'], + ['"\\U0000006dcp__server__tool"', 'mcp__server__tool'], + ['"\\x6gcp__server__tool"', null], + ]) { + assert.strictEqual(_decodeToolScalar(raw), expected); + } + + const inline = '---\nname: gsd-test\ndescription: test\ntools: "WebFetch", \'WebSearch\', "unterminated\n---\n'; + const block = '---\nname: gsd-test\ndescription: test\ntools:\n - "WebFetch"\n - \'WebSearch\'\n - "unterminated\n---\n'; + + for (const content of [inline, block]) { + const once = appendAgentTools(content, ['WebFetch', 'WebSearch']); + assert.strictEqual(once, content, 'quoted values must already satisfy append idempotency'); + + const kimi = buildKimiAgentArtifacts({ subagents: [{ path: 'agents/gsd-test.md', content }] }); + assert.ok(kimi.subagents[0].yaml.includes('kimi_cli.tools.web:FetchURL')); + assert.ok(kimi.subagents[0].yaml.includes('kimi_cli.tools.web:SearchWeb')); + assert.ok(!kimi.subagents[0].yaml.includes('unterminated')); + + const qwen = convertClaudeAgentToQwenAgent(content); + assert.match(qwen, /^ {2}- WebFetch$/m); + assert.match(qwen, /^ {2}- WebSearch$/m); + assert.doesNotMatch(qwen, /unterminated/); + } +}); + +test('appendAgentTools preserves inline YAML comments without swallowing grants (#4032)', () => { + const content = '---\nname: gsd-test\ntools: Read # keep this note\n---\n'; + const augmented = appendAgentTools(content, ['WebFetch']); + assert.match(augmented, /^tools: Read, WebFetch # keep this note$/m); + assert.deepStrictEqual(parseTools(augmented), ['Read', 'WebFetch']); +}); + +test('appendAgentTools leaves agents without a tools key unchanged (#4032)', () => { + const content = '---\nname: gsd-test\ndescription: inherits the runtime tool surface\n---\n'; + assert.strictEqual(appendAgentTools(content, ['WebFetch']), content); +}); + +test('fast-check: append preserves stable first-seen order and converges (#4032)', () => { + const token = fc.constantFrom('Read', 'Write', 'WebFetch', 'mcp__server__tool', 'Skill'); + fc.assert( + fc.property( + fc.constantFrom('inline', 'block'), + fc.array(token, { maxLength: 8 }), + fc.array(token, { maxLength: 8 }), + fc.array(token, { maxLength: 8 }), + (form, existing, wildcard, named) => { + const frontmatter = form === 'inline' + ? `---\ntools: ${existing.join(', ')}\n---\n` + : `---\ntools:\n${existing.map((tool) => ` - ${tool}`).join('\n')}\n---\n`; + const present = new Set(existing); + const additions = [...wildcard, ...named].filter((tool) => !present.has(tool) && (present.add(tool), true)); + const expected = [...existing, ...additions]; + const once = appendAgentTools(frontmatter, [...wildcard, ...named]); + assert.deepStrictEqual(parseTools(once), expected); + assert.strictEqual(appendAgentTools(once, [...wildcard, ...named]), once); + }, + ), + { numRuns: 100 }, + ); +}); + +test('appendAgentTools applies grants under a comment-only tools: header (#4032)', () => { + const frontmatter = '---\ntools: # TODO: fill in\n - Read\n---\n'; + const once = appendAgentTools(frontmatter, ['Write']); + assert.deepStrictEqual(parseTools(once), ['Read', 'Write']); + assert.match(once, /^tools: # TODO: fill in$/m, 'the comment-only header line must survive untouched'); +}); + +test('appendAgentTools does not tear a quoted scalar containing a literal comma (#4032)', () => { + // parseTools (this file's own helper) naive-splits on comma/space too, so it + // can't round-trip a quoted comma scalar — assert the raw line instead. + const frontmatter = '---\ntools: Read, "mcp__x, y"\n---\n'; + const once = appendAgentTools(frontmatter, ['Write']); + assert.match(once, /^tools: Read, "mcp__x, y", Write$/m, + 'the quoted scalar must survive intact and Write must be appended once'); + const twice = appendAgentTools(once, ['Write']); + assert.strictEqual(twice, once, 'reapplying the same grant must be a no-op (Write not duplicated)'); +}); + +test('appendAgentTools refuses to extend a value that IS a leading quoted scalar (#4032)', () => { + // Regression found in PR #4238 remediation (Opus review): a YAML quoted + // scalar occupies the whole node — `tools: "Read"` is valid, but + // `tools: "Read", Write` is not, even before this function touches it. + // Naively appending after it produced invalid frontmatter. There is no + // safe line-surgical rewrite here (that would require re-serializing the + // scalar), so the correct behavior is to leave the line untouched. + const frontmatter = '---\ntools: "Read"\n---\n'; + const once = appendAgentTools(frontmatter, ['Write']); + assert.strictEqual(once, frontmatter, 'a leading quoted scalar must be left byte-identical, not corrupted'); +}); + +test('appendAgentTools refuses to extend a value that IS a YAML flow sequence (#4032)', () => { + // Regression found in PR #4238 remediation (CodeRabbit review): a flow + // sequence occupies the whole node — `tools: [Bash, Read]` is valid, but + // `tools: [Bash, Read], Write` is not (content cannot follow a closed flow + // collection on the same line). The naive append produced invalid + // frontmatter, same failure mode as the leading-quoted-scalar case above. + const frontmatter = '---\ntools: [Bash, Read]\n---\n'; + const once = appendAgentTools(frontmatter, ['Write']); + assert.strictEqual(once, frontmatter, 'a leading flow sequence must be left byte-identical, not corrupted'); +}); + +test('appendAgentTools recognizes an existing block item with a trailing comment (no duplicate) (#4032)', () => { + // Regression found in PR #4238 remediation: decodeToolScalar did not strip + // a trailing ` # note` from a bare block-list item, so `- Read # note` + // decoded to `'Read # note'` — `present.has('Read')` then missed, and a + // second `- "Read"` item was inserted alongside the original. + const frontmatter = '---\ntools:\n - Read # note\n---\n'; + const once = appendAgentTools(frontmatter, ['Read', 'Write']); + assert.doesNotMatch(once, /- "Read"/, 'Read must not be duplicated as a new quoted item'); + assert.match(once, /^ {2}- Read # note$/m, 'the original commented item must survive untouched'); + assert.match(once, /^ {2}- "Write"$/m, 'the genuinely new grant must still be appended'); +}); + +test('parseFrontmatterTools (Qwen conversion) does not silently drop a block list under a comment-only header (#4032)', () => { + // Sibling of the appendAgentTools fix (finding 4): parseFrontmatterTools's + // own comment-only-header check (`if (value) {...}` treated `# note` as + // truthy content instead of falling through to `collecting = true`) is a + // separate parser reached by Kimi and Qwen conversion, downstream of + // appendAgentTools's own output. + const agent = '---\nname: gsd-executor\ndescription: test\ntools: # note\n - Read\n - Write\n---\nbody\n'; + const qwen = convertClaudeAgentToQwenAgent(agent); + assert.match(qwen, /- Read/); + assert.match(qwen, /- Write/); +}); + +test('parseFrontmatterTools (Qwen conversion) does not tear a quoted scalar containing a literal comma (#4032)', () => { + const agent = appendAgentTools( + '---\nname: gsd-executor\ndescription: test\ntools: Read, "mcp__x, y"\n---\nbody\n', + [], + ); + const qwen = convertClaudeAgentToQwenAgent(agent); + assert.match(qwen, /- "mcp__x, y"/, 'the quoted scalar must survive as one tool, not torn on its internal comma'); +}); + +test('ZCode strips an undecodable mcp__ scalar instead of keeping it (fail-closed) (#4032)', () => { + // An unterminated quote makes decodeToolScalar return null. ZCode's contract + // is "never emit mcp__*" (a required-MCP-server hard-fail otherwise), so a + // decode failure must be treated as unsafe-and-stripped, never safe-and-kept. + const inline = '---\ntools: Read, "mcp__server__tool\n---\n'; + assert.doesNotMatch(convertClaudeAgentToZcodeAgent(inline), /mcp__/, + 'an undecodable inline scalar must not survive ZCode conversion'); + + const block = '---\ntools:\n - Read\n - "mcp__server__tool\n---\n'; + assert.doesNotMatch(convertClaudeAgentToZcodeAgent(block), /mcp__/, + 'an undecodable block-list scalar must not survive ZCode conversion'); +}); + +test('ZCode strips a block-list mcp__ item under a comment-only tools: header (#4032)', () => { + // Regression found in PR #4238 remediation: a `tools: # comment` header line + // matched the INLINE-value regex (comment text treated as content), so the + // block-list scan below it never ran and `mcp__server__tool` leaked through + // verbatim — breaking ZCode's "never emit mcp__*" invariant. + const content = '---\ntools: # comment-only header\n - Read\n - mcp__server__tool\n---\n'; + const converted = convertClaudeAgentToZcodeAgent(content); + assert.doesNotMatch(converted, /mcp__/, 'mcp__server__tool must be stripped, not leaked through'); + assert.match(converted, /^tools: # comment-only header$/m, 'the comment-only header line must survive untouched'); + assert.match(converted, /^ {2}- Read$/m, 'the non-mcp__ item must be kept'); +}); diff --git a/tests/config-field-docs.test.cjs b/tests/config-field-docs.test.cjs index 0ed1279c6..9f516e274 100644 --- a/tests/config-field-docs.test.cjs +++ b/tests/config-field-docs.test.cjs @@ -214,6 +214,21 @@ describe('config-field-docs', () => { ); }); + test('agent_tools is registered in the central schema and public configuration docs (#4032)', () => { + const manifest = JSON.parse(fs.readFileSync(CONFIG_SCHEMA_MANIFEST_PATH, 'utf-8')); + assert.ok(manifest.validKeys.includes('agent_tools'), + 'agent_tools must be accepted by the central config schema'); + const selectorPattern = manifest.dynamicKeyPatterns.find((entry) => entry.topLevel === 'agent_tools'); + assert.ok(selectorPattern, 'agent_tools must register a dynamic selector pattern'); + assert.ok(new RegExp(selectorPattern.source).test('agent_tools.gsd-executor')); + assert.ok(new RegExp(selectorPattern.source).test('agent_tools.*')); + const publicDocs = fs.readFileSync(DOCS_CONFIG_PATH, 'utf-8'); + assert.ok(tableRowForKey(publicDocs, 'agent_tools.'), + 'agent_tools must have a public configuration table row'); + assert.match(publicDocs, /agents without a\s+`tools:` key inherit/i); + assert.match(publicDocs, /Codex.*parent.*MCP servers.*sandbox_mode/is); + }); + test('documents sub_repos field (CONFIG_DEFAULTS, no namespace form)', () => { // sub_repos is in CONFIG_DEFAULTS but has no NAMESPACE_MAP entry // (it uses a planning.sub_repos nested lookup but is documented as a diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index 4614223c0..89d8a0aad 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -233,6 +233,17 @@ describe('loadConfig — unknown-key warning dedup', () => { assert.ok(warnings.length <= 1, `warning emitted more than once: ${warnings.length} times`); }); + test('agent_tools is accepted without an unknown-key warning (#4032)', () => { + writeConfig(tmpDir, { agent_tools: { 'gsd-executor': ['WebFetch'] } }); + const config = loadConfig(tmpDir); + assert.deepEqual(config.agent_tools, { 'gsd-executor': ['WebFetch'] }); + assert.equal( + stderrLines.some((line) => line.includes('agent_tools')), + false, + 'a documented agent_tools config must not be reported as unknown', + ); + }); + // #2674: the two cases above only pass because each picks a key name no other // case reuses — so neither can observe whether the documented reset actually // runs. _resetRuntimeWarningCacheForTests is documented as resetting diff --git a/tests/slug-derivation-drift-guard.test.cjs b/tests/slug-derivation-drift-guard.test.cjs index 837dc8eac..b76f174e5 100644 --- a/tests/slug-derivation-drift-guard.test.cjs +++ b/tests/slug-derivation-drift-guard.test.cjs @@ -129,7 +129,7 @@ describe('findSlugDerivationDrift — MAJOR-1: allowlist exemption is scoped to const sanctionedRealEndLines = [ { file: path.join('src', 'core-utils.cts'), fn: 'generateSlugInternal', realEndLine: 199 }, { file: path.join('src', 'gsd2-import.cts'), fn: 'slugify', realEndLine: 103 }, - { file: path.join('src', 'runtime-artifact-conversion.cts'), fn: 'normalizeKimiSkillName', realEndLine: 616 }, + { file: path.join('src', 'runtime-artifact-conversion.cts'), fn: 'normalizeKimiSkillName', realEndLine: 635 }, { file: path.join('scripts', 'generate-package-identity.cjs'), fn: 'slugifyPackageName', realEndLine: 42 }, ]; diff --git a/tests/zcode-agent-mcp-grants.install.test.cjs b/tests/zcode-agent-mcp-grants.install.test.cjs index a48436882..6b27d6d63 100644 --- a/tests/zcode-agent-mcp-grants.install.test.cjs +++ b/tests/zcode-agent-mcp-grants.install.test.cjs @@ -29,6 +29,7 @@ const { runNode } = require('./helpers/process-seam.cjs'); const { cleanup } = require('./helpers.cjs'); const { installerEnv } = require('./helpers/install-shared.cjs'); +const { buildOverlayRepo } = require('./helpers/overlay-repo.cjs'); const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.join(__dirname, '..'); @@ -85,9 +86,9 @@ function parseFrontmatterTools(content) { /** 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) { +function spawnInstall(runtime, scope, repoRoot = REPO_ROOT) { 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}`]; + const args = ['--preserve-symlinks', '--preserve-symlinks-main', path.join(repoRoot, 'bin', 'install.js'), `--${runtime}`]; if (scope === 'global') { args.push('--global', '--config-dir', root); } else { @@ -151,6 +152,45 @@ test('zcode local install: all 8 MCP-granted agents install with zero mcp__* too assertZcodeAgentsClean(t, 'local'); }); +function zcodeFixture(tools) { + return `---\nname: gsd-phase-researcher\ndescription: ZCode quoted-MCP fixture\ntools: ${tools}\n---\n\nfixture body survives\n`; +} + +function zcodeBlockFixture(items) { + return `---\nname: gsd-phase-researcher\ndescription: ZCode quoted-MCP fixture\ntools:\n${items.map((item) => ` - ${item}`).join('\n')}\n---\n\nfixture body survives\n`; +} + +test('zcode treats quoted MCP scalars as equivalent to plain scalars (#4189)', (t) => { + const cases = [ + { name: 'mixed inline', source: zcodeFixture('Read, mcp__server__plain, \'mcp__server__single\', "mcp__server__double"'), expected: 'tools: Read' }, + { name: 'escaped inline', source: zcodeFixture('Read, "\\x6dcp__server__tool"'), expected: 'tools: Read' }, + { name: 'escaped inline unicode-16', source: zcodeFixture('Read, "\\u006dcp__server__tool"'), expected: 'tools: Read' }, + { name: 'escaped inline unicode-32', source: zcodeFixture('Read, "\\U0000006dcp__server__tool"'), expected: 'tools: Read' }, + { name: 'commented inline', source: zcodeFixture('Read, "mcp__server__double" # note'), expected: 'tools: Read' }, + { name: 'all inline', source: zcodeFixture('\'mcp__server__single\', "mcp__server__double"'), expected: null }, + { name: 'mixed block', source: zcodeBlockFixture(['Read', 'mcp__server__plain', "'mcp__server__single'", '"mcp__server__double"']), expected: 'tools:\n - Read' }, + { name: 'escaped block', source: zcodeBlockFixture(['Read', '"\\x6dcp__server__tool"']), expected: 'tools:\n - Read' }, + { name: 'escaped block unicode-16', source: zcodeBlockFixture(['Read', '"\\u006dcp__server__tool"']), expected: 'tools:\n - Read' }, + { name: 'escaped block unicode-32', source: zcodeBlockFixture(['Read', '"\\U0000006dcp__server__tool"']), expected: 'tools:\n - Read' }, + { name: 'commented block', source: zcodeBlockFixture(['Read', '"mcp__server__double" # note']), expected: 'tools:\n - Read' }, + { name: 'all block', source: zcodeBlockFixture(["'mcp__server__single'", '"mcp__server__double"']), expected: null }, + ]; + + for (const row of cases) { + const overlay = buildOverlayRepo({ 'agents/gsd-phase-researcher.md': row.source }); + t.after(() => cleanup(overlay)); + const { result, root } = spawnInstall('zcode', 'global', overlay); + t.after(() => cleanup(root)); + assert.strictEqual(result.exitCode, 0, `${row.name}: zcode install must succeed\n${result.stderr}`); + const emitted = fs.readFileSync(path.join(root, 'agents', 'gsd-phase-researcher.md'), 'utf8'); + assert.ok(!emitted.includes('mcp__server__'), `${row.name}: all semantic MCP entries must be removed`); + assert.ok(!emitted.includes('\\x6dcp__server__'), `${row.name}: YAML-escaped semantic MCP entries must be removed`); + assert.ok(emitted.includes('fixture body survives'), `${row.name}: unrelated body bytes must survive`); + if (row.expected === null) assert.ok(!/^tools:/m.test(emitted), `${row.name}: all-MCP lists must drop tools`); + else assert.ok(emitted.includes(row.expected), `${row.name}: non-MCP tool formatting must survive`); + } +}); + // ─── 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) => {