From cb1d9df63b7a03ba9024092c242b7b1642670c35 Mon Sep 17 00:00:00 2001 From: Harsh Singh Date: Mon, 10 Aug 2026 13:42:09 +0530 Subject: [PATCH] fix(compile): stop writing the same skill twice for agents that read .agents/skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repo with both .agents/ and .github/ (or .gemini/) got byte-identical SKILL.md files in both places — the exact duplication the comment beside the agents adapter claimed could not happen. Its narrow detection only covers the case where .agents/ is absent, not the common one where a repo has it and a native skills directory too. The duplicate is never harmless, and it fails differently per client: Copilot searches .github/skills first and dedupes by name, so the .agents copy was silently ignored; Gemini CLI loads the .agents alias after .gemini/skills, overrides it, and prints "Skill conflict detected" for every duplicated name — noise kitbash itself was causing; Codex dedupes root paths but not skill names, so it loaded the skill twice. copilot and gemini now yield to .agents/skills/ when it is written, with a note saying so, and a copy left by an earlier version is pruned on the next compile. A repo without .agents/ is unaffected — copilot still gets .github/skills/, because the dedup must never cost an agent its only copy. Also corrects two claims in that comment that were never verified: .agents/skills is not Codex's only repo path (/.codex/skills also loads at repo scope), and Roo, Amp, OpenCode and Antigravity were listed as readers without being checked. The readers now named — Codex, Copilot, Gemini CLI, Cursor, Zed, Cline — were each confirmed against that client's own source or docs. The benchmark harness read copilot and gemini from their own skills dirs, which are no longer written when the shared path serves them; it now reads the path that serves them. Published numbers are unchanged, the bytes being identical. Adds 7 tests. 0.22.0. --- CHANGELOG.md | 9 ++++++ packages/cli/package.json | 2 +- packages/cli/scripts/benchmark.mjs | 13 +++++---- packages/cli/scripts/test.mjs | 47 ++++++++++++++++++++++++++++-- packages/cli/src/adapters.ts | 41 ++++++++++++++++++++++---- packages/cli/src/commands.ts | 19 +++++++++++- site/changelog.html | 15 ++++++++-- site/index.html | 2 +- 8 files changed, 130 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a3ffe..6ad86ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ Format: [Keep a Changelog](https://keepachangelog.com). Versioning: semver — for skills *and* for this CLI, breaking prompt changes are breaking changes. +## [0.22.0] — 2026-08-09 + +### Fixed +- **A repo with both `.agents/` and `.github/` (or `.gemini/`) got the same skill written twice.** The `agents` adapter's detection is narrow so the vendor-neutral path is not forced on repos that never asked for it — but that only covers the case where `.agents/` is absent, not the far more common one where a repo has it *and* a native skills directory. The result was byte-identical `SKILL.md` files in both places, which the source comment beside that adapter had explicitly claimed would not happen. + + The duplicate is never harmless, and it fails differently per client: **Copilot** searches `.github/skills` first and dedupes by name, so the `.agents/skills` copy is silently ignored; **Gemini CLI** loads the `.agents` alias *after* `.gemini/skills`, overrides it, and prints a `Skill conflict detected` warning for every duplicated name — noise Kitbash itself was generating; and **Codex** dedupes root paths but *not* skill names, so it loaded the skill twice. `copilot` and `gemini` now yield to `.agents/skills/` when it is being written, with a note saying so, and any copy left by an earlier version is pruned on the next compile. A repo without `.agents/` is unaffected: Copilot still gets `.github/skills/`, because the dedup must never cost an agent its only copy. +- **Corrected two unverified claims in the adapter source.** `.agents/skills` is not Codex's only repo path (`/.codex/skills` also loads at repo scope), and Roo, Amp, OpenCode and Antigravity were listed as readers of the vendor-neutral path without ever having been checked. The readers now named — Codex, Copilot, Gemini CLI, Cursor, Zed and Cline — were each confirmed against that client's own source or documentation. +- The benchmark harness read Copilot's and Gemini's cost from their own skills directories, which are no longer written when the vendor-neutral path serves them; it now reads the path that actually serves them. The published numbers are unchanged, because the bytes are identical. + ## [0.21.1] — 2026-08-09 ### Fixed diff --git a/packages/cli/package.json b/packages/cli/package.json index 958150f..47a0fb3 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "kitbash", - "version": "0.21.1", + "version": "0.22.0", "description": "The package manager and compiler for AI agent skills — write once, run in every coding agent", "license": "Apache-2.0", "author": "Harsh Singh", diff --git a/packages/cli/scripts/benchmark.mjs b/packages/cli/scripts/benchmark.mjs index 0efe0e7..291454d 100644 --- a/packages/cli/scripts/benchmark.mjs +++ b/packages/cli/scripts/benchmark.mjs @@ -51,17 +51,18 @@ function measure(tmp, target, skillName) { return estimateTokens(read(`.claude/skills/${skillName}/SKILL.md`)); case "cursor": return estimateTokens(read(`.cursor/rules/${skillName}.mdc`)); - // zed and cline read the same vendor-neutral path and emit the same bytes, - // so they measure identically — listed rather than folded in, because a - // reader looking up "what does Zed cost" must find a row. + // Every one of these is served by the vendor-neutral path and reads the same + // bytes, so they measure identically. Zed and cline compile there directly; + // copilot and gemini also read it, so their own skills directory is not + // written when it is present (see VENDOR_NEUTRAL_ALIASES). Listed separately + // rather than folded together, because a reader looking up "what does Zed + // cost" must find a row. case "agents": case "zed": case "cline": - return estimateTokens(read(`.agents/skills/${skillName}/SKILL.md`)); case "copilot": - return estimateTokens(read(`.github/skills/${skillName}/SKILL.md`)); case "gemini": - return estimateTokens(read(`.gemini/skills/${skillName}/SKILL.md`)); + return estimateTokens(read(`.agents/skills/${skillName}/SKILL.md`)); case "windsurf": return estimateTokens(read(`.windsurf/rules/${skillName}.md`)); case "aider": diff --git a/packages/cli/scripts/test.mjs b/packages/cli/scripts/test.mjs index a69199d..05d08d9 100644 --- a/packages/cli/scripts/test.mjs +++ b/packages/cli/scripts/test.mjs @@ -67,7 +67,9 @@ try { check("claude-code output exists", existsSync(claude)); check("cursor output exists", existsSync(cursor)); check("agentsmd output exists", existsSync(agents)); - check("copilot output exists", existsSync(join(tmp, ".github/skills/prereview/SKILL.md"))); + // This repo has .agents/, and Copilot reads .agents/skills/ as well as its own + // dir, so the skill is served once from the shared path rather than duplicated. + check("copilot is served by the vendor-neutral path, not a second copy", !existsSync(join(tmp, ".github/skills/prereview/SKILL.md")) && existsSync(join(tmp, ".agents/skills/prereview/SKILL.md"))); check("copilot uses the lazy skills dir, not always-on instructions", !existsSync(join(tmp, ".github/instructions/prereview.instructions.md"))); check("cline compiles to the lazy skills path, not a .clinerules rule", existsSync(join(tmp, ".agents/skills/prereview/SKILL.md"))); check("cline no longer emits an always-on .clinerules rule", !existsSync(join(tmp, ".clinerules/prereview.md"))); @@ -77,7 +79,9 @@ try { check("agents (vendor-neutral) output exists", existsSync(join(tmp, ".agents/skills/prereview/SKILL.md"))); const agentsSkill = readFileSync(join(tmp, ".agents/skills/prereview/SKILL.md"), "utf8"); check("agents output carries spec frontmatter", /^---\nname: prereview\ndescription: "/.test(agentsSkill), agentsSkill.slice(0, 120)); - check("gemini output exists in the lazy skills dir", existsSync(join(tmp, ".gemini/skills/prereview/SKILL.md"))); + // Gemini CLI loads .agents/skills/ as a workspace alias that overrides + // .gemini/skills/ and warns on every duplicated name, so only one is written. + check("gemini is served by the vendor-neutral path, not a second copy", !existsSync(join(tmp, ".gemini/skills/prereview/SKILL.md"))); const geminiMd = readFileSync(join(tmp, "GEMINI.md"), "utf8"); check("gemini no longer merges into GEMINI.md, user content untouched", !geminiMd.includes("kitbash:begin") && geminiMd.startsWith("# Project notes"), geminiMd.slice(0, 120)); const aiderOut = readFileSync(join(tmp, "CONVENTIONS.md"), "utf8"); @@ -1771,6 +1775,45 @@ try { rmSync(escTmp, { recursive: true, force: true }); } +// ── the vendor-neutral path deduplicates the clients that also read it ─────── +// Copilot and Gemini CLI both read .agents/skills/ in addition to their own +// skills dir, verified against Copilot's documented search order and Gemini's +// skillManager.ts. Emitting both is never harmless: Copilot silently ignores the +// second copy, Gemini warns per duplicated name, and Codex loads it twice. +const aliasTmp = mkdtempSync(join(tmpdir(), "kitbash-alias-")); +try { + mkdirSync(join(aliasTmp, ".agents"), { recursive: true }); + mkdirSync(join(aliasTmp, ".github"), { recursive: true }); + run(["init"], aliasTmp); + run(["install", `file:${fixture}`, "--yes"], aliasTmp); + const al = run(["compile"], aliasTmp); + check("alias: .agents/skills is written", existsSync(join(aliasTmp, ".agents/skills/prereview/SKILL.md")), al.out); + check("alias: the redundant copilot dir is not", !existsSync(join(aliasTmp, ".github/skills/prereview/SKILL.md")), al.out); + check("alias: and the reason is stated", al.out.includes("served by .agents/skills/"), al.out); + check("alias: it is a note, so --strict still passes", run(["compile", "--strict"], aliasTmp).status === 0); +} finally { + rmSync(aliasTmp, { recursive: true, force: true }); +} + +// Without .agents/, copilot must still get its own directory — the dedup must +// never cost an agent its only copy. +const onlyTmp = mkdtempSync(join(tmpdir(), "kitbash-onlygh-")); +try { + mkdirSync(join(onlyTmp, ".github"), { recursive: true }); + run(["init"], onlyTmp); + run(["install", `file:${fixture}`, "--yes"], onlyTmp); + run(["compile"], onlyTmp); + check("alias: copilot keeps .github/skills when .agents is absent", existsSync(join(onlyTmp, ".github/skills/prereview/SKILL.md"))); + + // Adding .agents/ later must prune the now-redundant copy rather than leave two. + mkdirSync(join(onlyTmp, ".agents"), { recursive: true }); + const after = run(["compile"], onlyTmp); + check("alias: the now-stale copy is pruned on the next compile", !existsSync(join(onlyTmp, ".github/skills/prereview/SKILL.md")), after.out); + check("alias: pruning is reported", after.out.includes("removed .github/skills/prereview/SKILL.md"), after.out); +} finally { + rmSync(onlyTmp, { recursive: true, force: true }); +} + if (failures) { console.error(`\n${failures} test(s) failed`); process.exit(1); diff --git a/packages/cli/src/adapters.ts b/packages/cli/src/adapters.ts index 9cd28da..9d69d7a 100644 --- a/packages/cli/src/adapters.ts +++ b/packages/cli/src/adapters.ts @@ -212,12 +212,21 @@ function skillDirAdapter( } /** - * The vendor-neutral skills path, read by Codex (its only repo path), Cursor, - * Copilot, Gemini CLI, Roo, Amp, OpenCode, Zed and Antigravity. + * The vendor-neutral skills path. Confirmed against each client's own source or + * docs to be read by Codex (`codex-rs/ext/skills/src/host_roots.rs`), Copilot + * (project search position 2, after `.github/skills`), Gemini CLI + * (`skillManager.ts`, as an explicit workspace alias), Cursor, Zed — for which + * it is the *only* location — and Cline (`skill-directories.ts`). * - * Detection is deliberately narrow (`.agents/` or `.codex/`): agents that also - * have a native path already get their own adapter, and emitting both would - * duplicate the skill for no benefit. + * Two claims that used to sit here were wrong and are worth not repeating: + * `.agents/skills` is NOT Codex's only repo path (`/.codex/skills` also + * loads at repo scope when a trusted project layer exists), and Roo, Amp, + * OpenCode and Antigravity were listed without ever being verified. They may + * well read it; nothing here should assume so until someone checks. + * + * Detection is deliberately narrow (`.agents/` or `.codex/`). That keeps the + * path out of repos that never asked for it, but it does NOT prevent the + * both-present case — see VENDOR_NEUTRAL_ALIASES. */ const agents = skillDirAdapter( "agents", @@ -225,6 +234,28 @@ const agents = skillDirAdapter( (root) => existsSync(join(root, ".agents")) || existsSync(join(root, ".codex")), ); +/** + * Targets whose own skills directory is redundant once `.agents/skills/` is + * being written, because the same agent reads both paths. + * + * Emitting both is not merely wasteful, and the failure differs per client: + * Copilot searches `.github/skills` first and dedupes by name, so the + * `.agents/skills` copy is silently ignored; Gemini CLI loads the `.agents` + * alias *after* `.gemini/skills` and overrides it while printing a + * "Skill conflict detected" warning for every duplicated name — noise Kitbash + * itself would be causing. Codex is worse still: it dedupes root paths but not + * skill names, so a skill present at two roots is loaded twice. + * + * Zed and Cline are absent from this map because they already compile to + * `.agents/skills/` and so cannot duplicate. Cursor is absent because it + * compiles to `.cursor/rules/*.mdc`, a rules file rather than a skill + * directory — a different mechanism, not a second copy of the same one. + */ +export const VENDOR_NEUTRAL_ALIASES: { id: string; dir: string }[] = [ + { id: "copilot", dir: ".github/skills" }, + { id: "gemini", dir: ".gemini/skills" }, +]; + /** * Zed's skill loader (`crates/agent_skills/agent_skills.rs`) is stricter than * KSF about frontmatter, and it fails *silently* — a skill that violates either diff --git a/packages/cli/src/commands.ts b/packages/cli/src/commands.ts index adfd9f2..b1ab01c 100644 --- a/packages/cli/src/commands.ts +++ b/packages/cli/src/commands.ts @@ -6,7 +6,7 @@ import { createRequire } from "node:module"; import { createInterface } from "node:readline"; import { tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; -import { ADAPTERS, AGENT_PLUGIN_DIR, agentPluginManifest, GENERATED_MARK, mergeSection, pruneSections, readFileIfExists, type CompiledFile } from "./adapters.js"; +import { ADAPTERS, AGENT_PLUGIN_DIR, agentPluginManifest, GENERATED_MARK, mergeSection, pruneSections, readFileIfExists, VENDOR_NEUTRAL_ALIASES, type CompiledFile } from "./adapters.js"; import { dropLock, integrityOf, readLock, upsertLock, walk, LOCK_FILE } from "./lock.js"; import { fileChanges, manifestDelta, textOf, unifiedDiff } from "./diff.js"; import { collectImports, driftGroups, type ImportedSource } from "./importers.js"; @@ -936,6 +936,23 @@ export async function cmdCompile(args: string[]): Promise { } } + // A target whose own skills directory is redundant once `.agents/skills/` is + // written — the same agent reads both paths — is dropped rather than duplicated. + // The duplicate is never harmless: Copilot silently ignores the second copy, + // Gemini prints a "Skill conflict detected" warning for every duplicated name, + // and Codex dedupes root paths but not skill names, so it loads the skill twice. + // Any existing copy is removed by the prune pass below, since nothing wrote it. + if (adapters.some((a) => a.id === "agents")) { + for (const alias of VENDOR_NEUTRAL_ALIASES) { + if (!adapters.some((a) => a.id === alias.id)) continue; + const dropped = [...files.keys()].filter((p) => p.startsWith(`${alias.dir}/`)); + for (const p of dropped) files.delete(p); + if (dropped.length) { + notes.push(`${alias.id}: served by .agents/skills/, which it also reads — ${alias.dir}/ not written, so the skill is not duplicated.`); + } + } + } + const written: CompiledFile[] = [...files.entries()].map(([path, content]) => ({ path, content })); // Every emitted path must land inside the project. Adapters build filenames from // manifest values, so a containment check here is the last line before a write: diff --git a/site/changelog.html b/site/changelog.html index dad1a3c..74a2022 100644 --- a/site/changelog.html +++ b/site/changelog.html @@ -91,7 +91,7 @@

Changelog

Releases follow Keep a Changelog and semver — for skills and for this CLI, breaking prompt changes are breaking changes. The CLI is published to npm as kitbash and to Homebrew via singhharsh1708/tap. Tagged builds are on the GitHub releases page.

-
v0.21.1Current CLI version
+
v0.22.0Current CLI version
8Compile targets
Apache-2.0License
@@ -105,10 +105,21 @@

Changelog

Confirm with kitbash --version, which reads the installed package.json. Install and uninstall routes are covered on the installation page.

+
+
+

v0.22.0

+ 2026-08-09latest +
+

Fixed

+
  • A repo with both .agents/ and .github/ (or .gemini/) got the same skill written twice. The agents adapter's detection is narrow so the vendor-neutral path is not forced on repos that never asked for it — but that only covers the case where .agents/ is absent, not the far more common one where a repo has it and a native skills directory. The result was byte-identical SKILL.md files in both places, which the source comment beside that adapter had explicitly claimed would not happen.
+

The duplicate is never harmless, and it fails differently per client: Copilot searches .github/skills first and dedupes by name, so the .agents/skills copy is silently ignored; Gemini CLI loads the .agents alias after .gemini/skills, overrides it, and prints a Skill conflict detected warning for every duplicated name — noise Kitbash itself was generating; and Codex dedupes root paths but not skill names, so it loaded the skill twice. copilot and gemini now yield to .agents/skills/ when it is being written, with a note saying so, and any copy left by an earlier version is pruned on the next compile. A repo without .agents/ is unaffected: Copilot still gets .github/skills/, because the dedup must never cost an agent its only copy.

+
  • Corrected two unverified claims in the adapter source. .agents/skills is not Codex's only repo path (<repo>/.codex/skills also loads at repo scope), and Roo, Amp, OpenCode and Antigravity were listed as readers of the vendor-neutral path without ever having been checked. The readers now named — Codex, Copilot, Gemini CLI, Cursor, Zed and Cline — were each confirmed against that client's own source or documentation.
  • The benchmark harness read Copilot's and Gemini's cost from their own skills directories, which are no longer written when the vendor-neutral path serves them; it now reads the path that actually serves them. The published numbers are unchanged, because the bytes are identical.
+
+

v0.21.1

- 2026-08-09latest + 2026-08-09

Fixed

  • An update that added or swapped an MCP server passed review unflagged. update and diff print a field-by-field manifest delta and mark escalations — permissions.network: no → YES and friends — but the delta covered no MCP fields at all, so the single largest escalation a skill can make was visible only as "skill.toml changed" in the file diff. It is now reported server by server, with four escalations called out: a new server (a new program running with your agent's permissions, which outranks any permission flag); a server keeping its name while its command, args or url change — the rug-pull shape, where the thing you approved is not the thing that will run; a widened tool allowlist, including any narrowing to *; and a new env or header key, which is a new value handed to third-party code. Removing a server is reported as a change, not an escalation, and an unchanged declaration produces no output.
diff --git a/site/index.html b/site/index.html index c0fd6b3..a31a5c0 100644 --- a/site/index.html +++ b/site/index.html @@ -151,7 +151,7 @@ -

Open format for AI agent skills · v0.21.1 · stable spec (RFC 0002)

+

Open format for AI agent skills · v0.22.0 · stable spec (RFC 0002)

Write an agent skill once. Run it everywhere.