* test(4032): add failing installed-agent grants contract Cover global and project agent_tools precedence at the real Claude installer seam before adding implementation. * feat(4032): apply configured agent tool grants during staging Resolve selector-level global and project config once per staging call, then append validated grants before runtime conversion. * test(4032): cover host grant and quoted MCP contracts Exercise installed host artifacts and prove ZCode must treat quoted MCP scalars like plain MCP grants. * feat(4032): apply configured agent tool grants across runtimes Move augmentation and scalar identity into the converter seam so every staged artifact preserves host policy. * fix(4032): register agent tool grants in configuration Accept documented agent_tools config without unknown-key warnings.\n\nKeep installer fixtures on the shared temporary-directory helper. * fix(4032): translate configured MCP grants for Kilo Reuse the converter-owned scalar decoder so quoted canonical grants reach Kilo's native permission keys without altering other host policies. * fix(4032): decode YAML-escaped tool grants * fix(4032): emit valid inline agent tool grants * fix(4032): reject invalid trailing-colon grants * test(#4032): cover cross-review remediation gaps * fix(#4032): close cross-runtime grant gaps * test(#4032): expose Kimi global project context * fix(#4032): preserve Kimi project config context * chore(#4032): add release note * test(#4032): expose fork review regressions * fix(#4032): address fork review findings * test(#4032): make byte-stability assertion portable Compare repeat installs at one root so platform-specific path rendering cannot masquerade as an agent_tools behavior change. * chore(#4032): bind changeset to upstream PR 4238 * fix(#4032): address trek-e review findings (2,3,4,5,6,7,8) Fixes fail-closed decode-failure handling in ZCode's mcp__ stripper, a comment-only `tools:` header mis-parse that silently dropped configured grants, and a naive comma-split that could tear a quoted scalar containing a literal comma. Documents Kilo's inherent `{server}_{tool}` MCP-permission-key collision (external, fixed format — not ours to widen) and locks the existing first-seen-wins resolution in with a regression test. Opts kimi/kimi-code out of the ADR-1235 pre-converter path-rewrite step: routing Kimi through that pipeline (needed so project-scoped agent_tools selectors reach it) was short-circuiting Kimi's own neutralizeKimiAgentPrompt, which expects the original ~/.claude/gsd-core text rather than a pre-rewritten Kimi path. Extends the fast-check token pool and per-runtime install coverage with the missing comment/comma/broad-runtime cases the prior review flagged as untested. * docs(#4032): add CONTEXT.md glossary entries for agent_tools resolver + pre-converter step Documents readGsdEffectiveAgentTools (Install Model Override Resolver Module) and the appendAgentTools pre-converter pipeline step (Runtime Artifact Conversion Module), per contributor-standards.md's new-seam glossary requirement (finding 1). * fix(#4032): address agy adversarial review findings An agy (gemini-3.8-flash-high) adversarial pass over the prior review-fix commit found the fixes for findings 3, 4, 6 and 8 had unfixed sibling gaps, plus a genuine new regression and two CONTEXT.md inaccuracies: - ZCode's comment-only `tools: # note` header matched the inline-value branch instead of falling through to the block-list scan, so a following mcp__* item leaked through unstripped — the exact defect finding 4 fixed in appendAgentTools, unfixed in this sibling function. - Reverted capabilities/kimi-code/capability.json's noPathRewrite: true. kimi-code uses the standard 'agents' kind with converter: null (not kimi-agents — confirmed by reading the descriptor, not its prose description), so it never went through the pipeline change finding 5 fixed, and disabling its path rewrite broke every ~/.claude/ embed in its shipped agents instead. - decodeToolScalar never stripped a trailing ` # comment` from a bare (unquoted) scalar, so a comment after a block-list item, or after an appended grant on an inline line, became part of the "tool name" — fixed at the source (one call site fixes every consumer). - appendAgentTools's comment-index scan wasn't quote-aware, so a `#` inside a quoted scalar (`"mcp__server #1"`) was mistaken for a comment start and corrupted the quote. - parseFrontmatterTools (Kimi/Qwen's tool-list reader, downstream of appendAgentTools's own output) had the same naive comma-split and comment-only-header gaps as findings 4 and 6, unpatched. - The all-runtime smoke test's presence assertion was built on a guessed omit-list; empirically only 7 of 17 runtimes keep an arbitrary mcp__ grant recognizable, replaced with a verified allowlist. - CONTEXT.md claimed a `project:<agent>` selector prefix that does not exist (project override is a same-key merge across two config files) and mislabeled stageAgentsForRuntimeWithConverter's module. * fix(#4032): address full-PR review (Opus critical/ponytail + agy) A whole-PR pass (critical-code-reviewer + ponytail-review on Opus, plus a second agy full-source adversarial pass) surfaced defects the earlier finding-scoped passes couldn't reach: - appendAgentTools corrupted a `tools:` line whose ENTIRE value is a leading quoted scalar (`tools: "Read"` -> `tools: "Read", Write`, invalid YAML) — there is no safe line-surgical rewrite here, so it now refuses to touch that shape instead of emitting broken frontmatter. - decodeToolScalar's malformed-trailing-quote check ran BEFORE comment stripping, so a bare tool name with a quote inside its own trailing comment (`Bash # note: "internal"`) was wrongly rejected. Reordered. - findUnquotedCommentIndex (added in the prior remediation commit) was built on a wrong model of YAML: a `#` after whitespace starts a real comment in a plain scalar regardless of nearby quote characters — verified against the actual parser. The one case that DOES need protection (a leading quoted scalar) is now refused outright above, so the quote-tracking scan was dead weight solving a problem that no longer reaches it. Removed; reverted to the plain `[ \t]#` scan. - Kilo has a SEPARATE agent-frontmatter parser (convertClaudeToKiloFrontmatter, distinct from the buildKiloAgentPermissionBlock fixed earlier) with the same comment-only-header and naive-comma-split gaps as findings 4 and 6 — unfixed in both its src/ and bin/install.js copies. Fixed in both, exporting splitToolScalars for bin/install.js to reuse rather than reimplementing it. - Pipeline docstring in stageAgentsForRuntimeWithConverter still listed 5 steps, omitting appendAgentTools (now step 3 of 6). - docs/CONFIGURATION.md didn't state that a --global install still discovers agent_tools from the cwd's .planning/config.json (confirmed intentional and already covered by a dedicated test, not a bug). - Removed install-engine.cts's deps.cwd injection seam: zero callers or tests ever populated it. Two claims from this round were verified and rejected, not fixed: prototype pollution via a `__proto__` selector key (empirically confirmed `Object.prototype` is never touched — only reassigns the resolver's own local object's prototype, with no observable effect), and a `*` grant value crashing YAML parsing as an alias reference (empirically confirmed it parses as plain scalar text, no crash). A pre-existing, unrelated defect (extractFrontmatterField returns null for block-list `tools:` on Copilot/Antigravity/Cursor/Codex/Qwen, affecting two shipped agents today) was filed as a follow-up rather than fixed here — it predates #4032 and isn't caused or worsened by this PR. * fix(#4032): update stale slug-derivation-drift-guard fixture line normalizeKimiSkillName's real closing brace moved from line 616 to 635 as a side effect of this PR's edits to runtime-artifact-conversion.cts; the MAJOR-1 fixture's hardcoded realEndLine had gone stale. * fix(#4032): address CodeRabbit findings on projectDir threading and flow-sequence tools bin/install.js's installAgentsKindStandalone call site omitted the projectDir argument the function already supports, so a global install through this legacy branch silently fell back to the runtime config dir instead of process.cwd() when resolving project-scoped agent_tools grants — inconsistent with the sibling installOpencodeFamilyArtifacts call site, which already threads it correctly. appendAgentTools' leading-quoted-scalar bailout did not cover a YAML flow sequence (`tools: [Bash, Read]`): splitToolScalars tore it apart on the in-sequence commas and appended past its closing bracket, producing invalid frontmatter. Extended the bailout regex to also refuse a value starting with `[`, matching the same "whole node, nothing may follow" reasoning already applied to quoted scalars. --------- Co-authored-by: CI Rebase Check <ci@gsd-redux> Co-authored-by: Test <test@test.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
268 lines
9.8 KiB
TypeScript
268 lines
9.8 KiB
TypeScript
'use strict';
|
|
|
|
/**
|
|
* Runtime Artifact Install Plan Module.
|
|
*
|
|
* Turns a pre-resolved runtime artifact layout into staged copy inputs. The
|
|
* installer adapter still owns pruning, copying, migrations, output, and final
|
|
* cleanup execution.
|
|
*/
|
|
|
|
// In .cts (CommonJS output) files, `require` is available as a global.
|
|
const _require: NodeRequire = require;
|
|
const path = _require('node:path') as typeof import('node:path');
|
|
|
|
// #2870: InstallScope is owned by install-scope.cts, not re-declared here.
|
|
// `isGlobalScope` centralizes the `scope === 'global'` boolean projection
|
|
// this module needs at `_computePathPrefix`'s `isGlobal: boolean` boundary
|
|
// (see the module-level doc comment on `isGlobalScope` for why the
|
|
// projection is centralized rather than eliminated).
|
|
import { isGlobalScope, type InstallScope } from './install-scope.cjs';
|
|
|
|
type ArtifactKindName = 'commands' | 'agents' | 'skills' | 'kimi-agents';
|
|
|
|
interface ResolvedProfile {
|
|
name?: string;
|
|
skills?: Set<string> | '*';
|
|
agents?: Set<string>;
|
|
}
|
|
|
|
interface AgentCtx {
|
|
runtime: string;
|
|
pathPrefix: string;
|
|
attribution: string | null | undefined;
|
|
/** #2875 Part 2 (row I1): install root, threaded through so the
|
|
* descriptor pipeline's frontmatter-extensions step and model-override
|
|
* 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 {
|
|
kind: ArtifactKindName;
|
|
destSubpath: string;
|
|
prefix?: string;
|
|
stage: (resolvedProfile: ResolvedProfile, agentCtx?: AgentCtx) => string;
|
|
/** Resolved absolute alternate install root for this kind, if the descriptor
|
|
* specifies one (e.g. codex skills → $HOME/.agents). Undefined means the
|
|
* kind installs under the runtime's normal configDir. */
|
|
home?: string;
|
|
}
|
|
|
|
interface Layout {
|
|
runtime: string;
|
|
configDir: string;
|
|
scope?: InstallScope;
|
|
kinds: ArtifactKind[];
|
|
}
|
|
|
|
interface RewriteOpts {
|
|
runtime: string;
|
|
configDir: string;
|
|
scope: InstallScope;
|
|
homedir?: () => string;
|
|
platform?: NodeJS.Platform;
|
|
resolveAttribution?: (runtime: string) => string | null | undefined;
|
|
}
|
|
|
|
interface Dependencies {
|
|
rewriteStagedSkillBodies?: (stagedDir: string, opts: RewriteOpts) => string | void;
|
|
rewriteStagedCommandBodies?: (stagedDir: string, opts: RewriteOpts) => string | void;
|
|
}
|
|
|
|
interface ComputePathPrefixOpts {
|
|
isGlobal: boolean;
|
|
isOpencode: boolean;
|
|
isWindowsHost: boolean;
|
|
resolvedTarget: string;
|
|
homeDir: string;
|
|
}
|
|
|
|
interface RuntimeArtifactConversionExports {
|
|
rewriteStagedSkillBodies: (stagedDir: string, opts: RewriteOpts) => string | void;
|
|
rewriteStagedCommandBodies: (stagedDir: string, opts: RewriteOpts) => string | void;
|
|
_computePathPrefix: (opts: ComputePathPrefixOpts) => string;
|
|
}
|
|
|
|
interface PlanItem {
|
|
kind: ArtifactKindName;
|
|
sourceDir: string;
|
|
destDir: string;
|
|
}
|
|
|
|
interface InstallPlan {
|
|
items: PlanItem[];
|
|
cleanupDirs: string[];
|
|
}
|
|
|
|
interface UninstallPlanItem {
|
|
kind: ArtifactKindName;
|
|
destDir: string;
|
|
}
|
|
|
|
interface UninstallPlan {
|
|
items: UninstallPlanItem[];
|
|
}
|
|
|
|
type InstallPlanResult =
|
|
| { ok: true; plan: InstallPlan }
|
|
| { ok: false; kind: 'stage_failed' | 'rewrite_failed'; message: string; cleanupDirs: string[]; failedKind?: ArtifactKindName };
|
|
|
|
interface CreateRuntimeArtifactInstallPlanArgs {
|
|
layout: Layout;
|
|
resolvedProfile: ResolvedProfile;
|
|
homedir?: () => string;
|
|
platform?: NodeJS.Platform;
|
|
resolveAttribution?: (runtime: string) => string | null | undefined;
|
|
projectDir?: string | null;
|
|
deps?: Dependencies;
|
|
}
|
|
|
|
/**
|
|
* Asserts that `destSubpath` resolves to a path inside `configDir`.
|
|
*
|
|
* Rejects any path that escapes the configDir root (e.g. "../../etc") and any
|
|
* path containing a NUL byte. This is a security gate for Phase B of
|
|
* ADR-1239: third-party descriptors must never be able to write outside the
|
|
* designated config home directory.
|
|
*
|
|
* @param configDir - The root config directory (e.g. ~/.claude).
|
|
* @param destSubpath - The relative path declared by the runtime descriptor.
|
|
* @returns The resolved absolute path under configDir.
|
|
* @throws {Error} if destSubpath escapes configDir or contains a NUL byte.
|
|
*/
|
|
function assertDestWithinConfigHome(configDir: string, destSubpath: string): string {
|
|
if (destSubpath.includes('\0')) {
|
|
throw new Error(
|
|
`destSubpath "${destSubpath}" contains a NUL byte and is not valid`,
|
|
);
|
|
}
|
|
const root = path.resolve(configDir);
|
|
const resolved = path.resolve(configDir, destSubpath);
|
|
if (resolved === root || !resolved.startsWith(root + path.sep)) {
|
|
throw new Error(
|
|
`destSubpath "${destSubpath}" must be a strict subpath of configHome "${configDir}" — not configHome itself or outside it (escapes configHome)`,
|
|
);
|
|
}
|
|
return resolved;
|
|
}
|
|
|
|
function errorMessage(err: unknown): string {
|
|
if (err instanceof Error) return err.message;
|
|
return String(err);
|
|
}
|
|
|
|
function addCleanupDir(cleanupDirs: string[], stagedDir: string, rewrittenDir: string | void): string {
|
|
const sourceDir = rewrittenDir ?? stagedDir;
|
|
if (sourceDir !== stagedDir) cleanupDirs.push(sourceDir);
|
|
return sourceDir;
|
|
}
|
|
|
|
function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlanArgs): InstallPlanResult {
|
|
const {
|
|
layout,
|
|
resolvedProfile,
|
|
homedir,
|
|
platform,
|
|
resolveAttribution,
|
|
projectDir,
|
|
deps = {},
|
|
} = args;
|
|
const conversionExports = _require('./runtime-artifact-conversion.cjs') as RuntimeArtifactConversionExports;
|
|
const rewriteStagedSkillBodies = deps.rewriteStagedSkillBodies ?? conversionExports.rewriteStagedSkillBodies;
|
|
const rewriteStagedCommandBodies = deps.rewriteStagedCommandBodies ?? conversionExports.rewriteStagedCommandBodies;
|
|
const cleanupDirs: string[] = [];
|
|
const items: PlanItem[] = [];
|
|
const scope = layout.scope ?? 'global';
|
|
const rewriteOpts: RewriteOpts = {
|
|
runtime: layout.runtime,
|
|
configDir: layout.configDir,
|
|
scope,
|
|
homedir,
|
|
platform,
|
|
resolveAttribution,
|
|
};
|
|
|
|
// ADR-1235 §1: build agentCtx once per plan so agents kind entries can apply
|
|
// the CORRECT pre-converter cross-cutting (path rewrites → attribution → converter
|
|
// → normalize). This mirrors the exact per-file order in the inline agent loop
|
|
// in bin/install.js (lines 9330-9415). agentCtx is passed as the second arg
|
|
// to kind.stage() for agents kind entries with a converter (convertedAgentsKind).
|
|
// NO _stampNonClaudeRuntimeDefaults — agents are NOT stamped in the inline loop.
|
|
const os = _require('node:os') as typeof import('node:os');
|
|
const { posixNormalize } = _require('./shell-command-projection.cjs') as { posixNormalize: (p: string) => string };
|
|
const homedirFn: () => string = homedir ?? (() => os.homedir());
|
|
const resolvedTarget = posixNormalize(path.resolve(layout.configDir));
|
|
const homeDir = posixNormalize(homedirFn());
|
|
// #2870: `scope` above is already the module-owned `InstallScope` value
|
|
// (`layout.scope ?? 'global'`, defaulted before this point, so it is never
|
|
// `undefined` here) — `isGlobalScope` projects it to the boolean
|
|
// `_computePathPrefix`'s existing `isGlobal: boolean` API requires.
|
|
const isGlobal = isGlobalScope(scope);
|
|
const isOpencode = layout.runtime === 'opencode';
|
|
const isWindowsHost = (platform ?? process.platform) === 'win32';
|
|
const pathPrefix = conversionExports._computePathPrefix({ isGlobal, isOpencode, isWindowsHost, resolvedTarget, homeDir });
|
|
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,
|
|
projectDir: projectDir ?? layout.configDir,
|
|
};
|
|
|
|
for (const kind of layout.kinds) {
|
|
let stagedDir: string;
|
|
try {
|
|
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.
|
|
stagedDir = kind.stage(resolvedProfile, agentCtx);
|
|
} else {
|
|
stagedDir = kind.stage(resolvedProfile);
|
|
}
|
|
} catch (err) {
|
|
return { ok: false, kind: 'stage_failed', message: errorMessage(err), cleanupDirs, failedKind: kind.kind };
|
|
}
|
|
|
|
let sourceDir = stagedDir;
|
|
try {
|
|
if (kind.kind === 'commands') {
|
|
const rewrittenDir = rewriteStagedCommandBodies(stagedDir, rewriteOpts);
|
|
sourceDir = addCleanupDir(cleanupDirs, stagedDir, rewrittenDir);
|
|
} else if (kind.kind === 'skills' || kind.kind === 'kimi-agents') {
|
|
const rewrittenDir = rewriteStagedSkillBodies(stagedDir, rewriteOpts);
|
|
sourceDir = addCleanupDir(cleanupDirs, stagedDir, rewrittenDir);
|
|
}
|
|
// 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 };
|
|
}
|
|
|
|
items.push({
|
|
kind: kind.kind,
|
|
sourceDir,
|
|
destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath),
|
|
});
|
|
}
|
|
|
|
return { ok: true, plan: { items, cleanupDirs } };
|
|
}
|
|
|
|
function createRuntimeArtifactUninstallPlan(layout: Layout): UninstallPlan {
|
|
return {
|
|
items: layout.kinds.map((kind) => ({
|
|
kind: kind.kind,
|
|
destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath),
|
|
})),
|
|
};
|
|
}
|
|
|
|
export = { assertDestWithinConfigHome, createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };
|