From 9e2ef2c94dea3f1267deba570c9a423fb38c2f85 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 12 Jun 2026 00:20:28 -0400 Subject: [PATCH] fix(#1091): thread install scope into skill converters so local Antigravity/Copilot installs use workspace paths (#1092) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skills layout wrapper (skillsKind) invoked every per-runtime skill converter as realConverter(content, skillName, runtime, cmdNames). The 3rd positional arg is overloaded: claude/kimi/cline converters read `runtime` there, but the copilot/antigravity converters read `isGlobal` there — so they received the truthy runtime string and always took the global path branch, leaking ~/.gemini/antigravity/ and ~/.copilot/ into local/workspace installs instead of .agent/ and .github/. Thread `scope` from resolveRuntimeArtifactLayout -> dispatchKindEntry -> skillsKind, derive isGlobal = scope === 'global', and pass it as a non-colliding 5th positional arg. Move isGlobal out of the colliding 3rd slot in the two converter signatures (3rd/4th become ignored _runtime/_cmdNames, matching the kimi convention). The fix flows through the shared ArtifactKind.stage closure, so applySurface re-apply inherits it via the same seam. Regression test exercises the wrapper seam (installRuntimeArtifacts at local scope) for both runtimes and asserts workspace paths, not global. Closes #1091 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 --- .changeset/steady-otters-leap.md | 5 +++ bin/install.js | 6 ++- src/runtime-artifact-layout.cts | 22 +++++++--- tests/copilot-install.test.cjs | 2 +- tests/install-runtime-artifacts.test.cjs | 56 ++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 .changeset/steady-otters-leap.md diff --git a/.changeset/steady-otters-leap.md b/.changeset/steady-otters-leap.md new file mode 100644 index 000000000..c84be2959 --- /dev/null +++ b/.changeset/steady-otters-leap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1092 +--- +**Workspace (local) Antigravity and Copilot skill installs no longer point at the global config home** — a local install rewrote `~/.claude/` references in `SKILL.md` bodies to the global `~/.gemini/antigravity/` / `~/.copilot/` paths instead of the workspace-relative `.agent/` / `.github/`, because the skills layout wrapper passed the runtime name into the converter's `isGlobal` parameter slot. (#1092) diff --git a/bin/install.js b/bin/install.js index 0aa0f4ab5..3cd70ac60 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1737,12 +1737,13 @@ function convertClaudeToCopilotContent(content, isGlobal = false) { return c; } +// isGlobal is the 5th positional arg (3rd/4th are runtime/cmdNames passed by the skills wrapper). See runtime-artifact-layout skillsKind. /** * Convert a Claude command (.md) to a Copilot skill (SKILL.md). * Transforms frontmatter only — body passes through with CONV-06/07 applied. * Skills keep original tool names (no mapping) per CONTEXT.md decision. */ -function convertClaudeCommandToCopilotSkill(content, skillName, isGlobal = false) { +function convertClaudeCommandToCopilotSkill(content, skillName, _runtime = null, _cmdNames = null, isGlobal = false) { const converted = convertClaudeToCopilotContent(content, isGlobal); const { frontmatter, body } = extractFrontmatterAndBody(converted); if (!frontmatter) return converted; @@ -2240,12 +2241,13 @@ function convertClaudeToAntigravityContent(content, isGlobal = false) { return c; } +// isGlobal is the 5th positional arg (3rd/4th are runtime/cmdNames passed by the skills wrapper). See runtime-artifact-layout skillsKind. /** * Convert a Claude command (.md) to an Antigravity skill (SKILL.md). * Transforms frontmatter to minimal name + description only. * Body passes through with path/command conversions applied. */ -function convertClaudeCommandToAntigravitySkill(content, skillName, isGlobal = false) { +function convertClaudeCommandToAntigravitySkill(content, skillName, _runtime = null, _cmdNames = null, isGlobal = false) { const converted = convertClaudeToAntigravityContent(content, isGlobal); const { frontmatter, body } = extractFrontmatterAndBody(converted); if (!frontmatter) return converted; diff --git a/src/runtime-artifact-layout.cts b/src/runtime-artifact-layout.cts index 289f4c160..64df12e9c 100644 --- a/src/runtime-artifact-layout.cts +++ b/src/runtime-artifact-layout.cts @@ -251,6 +251,10 @@ function kimiAgentsKind(destSubpath: string, prefix: string, configDir: string): * @param runtime canonical runtime ID (gates Hermes/Qwen branding in converter) * @param configDir runtime config dir (for .gsd-source marker resolution) * @param nested if true, nest concrete skills under their ns-* routers (#69) + * @param scope install scope; converted to isGlobal and passed as 5th positional + * arg so scope-aware converters (antigravity, copilot) can choose + * between global home paths and workspace-relative paths without + * colliding with the `runtime` string at position 3. */ function skillsKind( destSubpath: string, @@ -259,6 +263,7 @@ function skillsKind( runtime: string, configDir: string, nested = false, + scope: 'local' | 'global' = 'global', ): ArtifactKind { return { kind: 'skills', @@ -266,12 +271,17 @@ function skillsKind( prefix, stage: (resolved) => { const installExports = getInstallExports(); - const realConverter = installExports[converterName] as (content: string, skillName: string, runtime: string, cmdNames: string[]) => string; + const realConverter = installExports[converterName] as (content: string, skillName: string, runtime: string, cmdNames: string[], isGlobal: boolean) => string; // Compute cmdNames once per stage call for performance (#3583). - // Extra args are ignored by converters that don't need runtime/cmdNames. + // Extra trailing args are ignored by converters that don't need them. The + // isGlobal flag is the 5th positional (NOT the 3rd): the 3rd positional is + // `runtime` for the claude/kimi/cline converters, so the scope-aware + // converters (antigravity, copilot) read isGlobal from position 5 to avoid + // colliding with `runtime` and always taking the global branch. const cmdNames = installExports.readGsdCommandNames(); + const isGlobal = scope === 'global'; const wrappedConverter = (content: string, skillName: string): string => - realConverter(content, skillName, runtime, cmdNames); + realConverter(content, skillName, runtime, cmdNames, isGlobal); return stageSkillsForRuntimeAsSkills(findInstallSourceRoot(configDir), resolved, wrappedConverter, prefix, nested); }, }; @@ -382,7 +392,7 @@ function getRegistry(): { runtimes: Record dispatchKindEntry(entry, runtime, configDir)); + const kinds: ArtifactKind[] = entries.map((entry) => dispatchKindEntry(entry, runtime, configDir, scope)); return { runtime, configDir, scope, kinds }; } diff --git a/tests/copilot-install.test.cjs b/tests/copilot-install.test.cjs index a9c5e9f2b..295bacb00 100644 --- a/tests/copilot-install.test.cjs +++ b/tests/copilot-install.test.cjs @@ -538,7 +538,7 @@ description: Test skill Check ~/.claude/settings and ./.claude/local and $HOME/.claude/global.`; - const result = convertClaudeCommandToCopilotSkill(input, 'gsd-test', true); + const result = convertClaudeCommandToCopilotSkill(input, 'gsd-test', null, null, true); assert.ok(result.includes('~/.copilot/settings'), 'tilde path converted to global'); assert.ok(result.includes('./.github/local'), 'dot-slash path converted'); assert.ok(result.includes('$HOME/.copilot/global'), '$HOME path converted to global'); diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index b981aa2bc..0ba9776b1 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -75,6 +75,21 @@ function writeCommandEntry(destDir, prefix, stem) { fs.writeFileSync(path.join(destDir, `${prefix}${stem}.md`), `# ${stem}\n`); } +function readAllSkillMd(dir) { + const out = []; + if (!fs.existsSync(dir)) return ''; + const stack = [dir]; + while (stack.length) { + const cur = stack.pop(); + for (const ent of fs.readdirSync(cur, { withFileTypes: true })) { + const p = path.join(cur, ent.name); + if (ent.isDirectory()) stack.push(p); + else if (ent.name === 'SKILL.md') out.push(fs.readFileSync(p, 'utf8')); + } + } + return out.join('\n'); +} + describe('installRuntimeArtifacts — skills runtimes write gsd-prefixed skill dirs', () => { for (const runtime of SKILLS_RUNTIMES_LAYOUT) { test(`${runtime}: gsd-prefixed skill dirs in skills/`, (t) => { @@ -516,3 +531,44 @@ describe('uninstallRuntimeArtifacts — legacy cleanup runs before layout remova assert.ok(fs.existsSync(path.join(userSkill, 'SKILL.md'))); }); }); + +describe('skills wrapper threads install scope into converter isGlobal (regression: local installs must not leak global home paths)', () => { + // Bug: the skills wrapper in runtime-artifact-layout passed `runtime` (a truthy + // string) as the converter's 3rd positional arg. For antigravity/copilot that + // param was `isGlobal`, so LOCAL installs always took the GLOBAL path branch and + // leaked ~/.gemini/antigravity or ~/.copilot instead of the workspace path. + for (const { runtime, globalMarker, localMarker } of [ + { runtime: 'antigravity', globalMarker: '~/.gemini/antigravity', localMarker: '.agents' }, + { runtime: 'copilot', globalMarker: '~/.copilot', localMarker: '.github' }, + ]) { + test(`${runtime}: local skill content uses workspace path, not global home`, (t) => { + const globalDir = createTempDir(`gsd-ial-g-${runtime}-`); + const localDir = createTempDir(`gsd-ial-l-${runtime}-`); + t.after(() => { cleanup(globalDir); cleanup(localDir); }); + + installRuntimeArtifacts(runtime, globalDir, 'global', RESOLVED_CORE); + installRuntimeArtifacts(runtime, localDir, 'local', RESOLVED_CORE); + + const gSkills = resolveRuntimeArtifactLayout(runtime, globalDir, 'global').kinds.find(k => k.kind === 'skills'); + const lSkills = resolveRuntimeArtifactLayout(runtime, localDir, 'local').kinds.find(k => k.kind === 'skills'); + assert.ok(gSkills && lSkills, `${runtime}: must resolve a skills kind for both scopes`); + + const gCombined = readAllSkillMd(path.join(globalDir, gSkills.destSubpath)); + const lCombined = readAllSkillMd(path.join(localDir, lSkills.destSubpath)); + + // Precondition (non-vacuity guard): some core skill carries a ~/.claude + // reference, so the GLOBAL install surfaces the global home marker. If this + // assertion ever fails, the source skills lost their path references — fix + // the fixture/source, do not delete this test. + assert.ok(gCombined.includes(globalMarker), + `${runtime}: precondition — global install should contain '${globalMarker}'`); + + // The actual regression: a LOCAL install must NOT leak the global home path… + assert.ok(!lCombined.includes(globalMarker), + `${runtime}: local install must NOT leak global home path '${globalMarker}'`); + // …and SHOULD reference the workspace-relative path. + assert.ok(lCombined.includes(localMarker), + `${runtime}: local install must reference workspace path '${localMarker}'`); + }); + } +});