From 77f562db1a90d94b5eb713a8c0cf3cf53c55733b Mon Sep 17 00:00:00 2001 From: xuyunfang Date: Mon, 12 Oct 2026 14:57:47 +0800 Subject: [PATCH] fix(skills): quiet startup skill conflicts for duplicated shared skill dirs Users who run both Codex and Claude Code often have the same skills copied into ~/.agents/skills and ~/.claude/skills. Each copy produced a collision entry, flooding startup even though the shadowed copy is never loaded. - Skip a colliding skill silently when its SKILL.md is byte-identical to the winner, the same way symlinked duplicates are already skipped. - Without the full resource listing, collapse remaining skill/prompt collisions into one line per (skipped dir, winning dir) pair; --verbose still lists each one. Warnings and errors are shown in full as before. --- CHANGELOG.md | 2 + apps/cli/src/ui/interactive-mode.ts | 55 ++++++++++++++++++- apps/cli/test/interactive-mode-status.test.ts | 53 ++++++++++++++++++ packages/coding-agent/src/core/skills.ts | 14 +++++ .../test/skill-loading-regressions.test.ts | 31 +++++++++++ 5 files changed, 152 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7917b1ea..259faa17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### Fixed +- Stop flooding startup with `[Skill conflicts]` when the same skills are installed in both `~/.agents/skills` and `~/.claude/skills`. Byte-identical copies are now skipped silently, and remaining name collisions collapse to one summary line per directory pair; `--verbose` still lists each one. + - Stop appending Ultracode capability tutorials on every turn. Announce session-mode transitions after user input, reuse unchanged state, and restore necessary reminders when context is compacted or replaced. - Recognize `/exit` as an alias for `/quit`, including while the agent is running or compacting. - Recognize `search_files`, `find_files`, and `list_directory` when selecting default system-prompt file-exploration guidance. diff --git a/apps/cli/src/ui/interactive-mode.ts b/apps/cli/src/ui/interactive-mode.ts index 13fe39b1..15520bef 100644 --- a/apps/cli/src/ui/interactive-mode.ts +++ b/apps/cli/src/ui/interactive-mode.ts @@ -1890,7 +1890,11 @@ export class InteractiveMode { return this.formatDisplayPath(p); } - private formatDiagnostics(diagnostics: readonly ResourceDiagnostic[], sourceInfos: Map): string { + private formatDiagnostics( + diagnostics: readonly ResourceDiagnostic[], + sourceInfos: Map, + options?: { summarizeCollisions?: boolean }, + ): string { const lines: string[] = []; // Group collision diagnostics by name @@ -1907,6 +1911,11 @@ export class InteractiveMode { } } + if (options?.summarizeCollisions && collisions.size > 0) { + lines.push(...this.formatCollisionSummary(collisions)); + collisions.clear(); + } + // Format collision diagnostics grouped by name for (const [name, collisionList] of collisions) { const first = collisionList[0]?.collision; @@ -1954,6 +1963,44 @@ export class InteractiveMode { return lines.join("\n"); } + /** + * One line per (skipped directory, winning directory) pair. Shadowed resources are + * informational - the winner already loaded - so the per-name breakdown is left to + * --verbose instead of filling the startup screen. + */ + private formatCollisionSummary(collisions: Map): string[] { + const resourceRoot = (p: string) => { + // //SKILL.md belongs to ; a loose /.md does too. + const dir = path.dirname(p); + return path.basename(p) === "SKILL.md" ? path.dirname(dir) : dir; + }; + + const groups = new Map(); + for (const collisionList of collisions.values()) { + for (const d of collisionList) { + if (!d.collision) continue; + const loserRoot = resourceRoot(d.collision.loserPath); + const winnerRoot = resourceRoot(d.collision.winnerPath); + const key = `${loserRoot}\0${winnerRoot}`; + const group = groups.get(key) ?? { loserRoot, winnerRoot, noun: d.collision.resourceType, count: 0 }; + group.count++; + groups.set(key, group); + } + } + + const lines: string[] = []; + for (const { loserRoot, winnerRoot, noun, count } of groups.values()) { + lines.push( + theme.fg( + "warning", + ` ${count} ${noun}${count === 1 ? "" : "s"} in ${this.formatDisplayPath(loserRoot)} skipped (same name already loaded from ${this.formatDisplayPath(winnerRoot)})`, + ), + ); + } + lines.push(theme.fg("dim", " Run with --verbose to list them.")); + return lines; + } + private showLoadedResources(options?: { extensions?: Array<{ path: string; sourceInfo?: SourceInfo }>; force?: boolean; @@ -2127,9 +2174,11 @@ export class InteractiveMode { } if (showDiagnostics) { + // Without the full listing, shadowed skills/prompts collapse to a one-line summary. + const summarizeCollisions = !showListing && !this.options.verbose; const skillDiagnostics = skillsResult.diagnostics; if (skillDiagnostics.length > 0) { - const warningLines = this.formatDiagnostics(skillDiagnostics, sourceInfos); + const warningLines = this.formatDiagnostics(skillDiagnostics, sourceInfos, { summarizeCollisions }); this.loadedResourcesContainer.addChild( new Text(`${theme.fg("warning", "[Skill conflicts]")}\n${warningLines}`, 0, 0), ); @@ -2138,7 +2187,7 @@ export class InteractiveMode { const promptDiagnostics = promptsResult.diagnostics; if (promptDiagnostics.length > 0) { - const warningLines = this.formatDiagnostics(promptDiagnostics, sourceInfos); + const warningLines = this.formatDiagnostics(promptDiagnostics, sourceInfos, { summarizeCollisions }); this.loadedResourcesContainer.addChild( new Text(`${theme.fg("warning", "[Prompt conflicts]")}\n${warningLines}`, 0, 0), ); diff --git a/apps/cli/test/interactive-mode-status.test.ts b/apps/cli/test/interactive-mode-status.test.ts index 82781abe..06b775e8 100644 --- a/apps/cli/test/interactive-mode-status.test.ts +++ b/apps/cli/test/interactive-mode-status.test.ts @@ -833,6 +833,59 @@ describe("InteractiveMode.showLoadedResources", () => { expect(output).not.toContain("commit"); }); + test("summarizes collisions in Step presentation unless verbose", () => { + for (const verbose of [false, true]) { + const fakeThis = createShowLoadedResourcesThis({ + quietStartup: false, + tuiStyle: "step", + verbose, + skillDiagnostics: [{ type: "collision", message: 'name "commit" collision' }], + }); + const formatOptions: unknown[] = []; + fakeThis.formatDiagnostics = (_diagnostics: unknown, _sourceInfos: unknown, opts: unknown) => { + formatOptions.push(opts); + return "diagnostics"; + }; + + (InteractiveMode as any).prototype.showLoadedResources.call(fakeThis, { + force: false, + showDiagnosticsWhenQuiet: true, + }); + + expect(formatOptions).toEqual([{ summarizeCollisions: !verbose }]); + } + }); + + test("collision summary collapses shadowed skills into one line per directory pair", () => { + const collision = (name: string, loserPath: string, winnerPath: string) => ({ + type: "collision" as const, + message: `name "${name}" collision`, + path: loserPath, + collision: { resourceType: "skill" as const, name, winnerPath, loserPath }, + }); + const fakeThis: any = { + formatDisplayPath: (p: string) => p, + }; + fakeThis.formatCollisionSummary = (collisions: unknown) => + (InteractiveMode as any).prototype.formatCollisionSummary.call(fakeThis, collisions); + + const output = (InteractiveMode as any).prototype.formatDiagnostics.call( + fakeThis, + [ + collision("oo", "/u/.claude/skills/oo/SKILL.md", "/u/.agents/skills/oo/SKILL.md"), + collision("wrangler", "/u/.claude/skills/wrangler/SKILL.md", "/u/.agents/skills/wrangler/SKILL.md"), + { type: "warning", message: "skill path does not exist" }, + ], + new Map(), + { summarizeCollisions: true }, + ); + + expect(output).toContain("2 skills in /u/.claude/skills skipped (same name already loaded from /u/.agents/skills)"); + expect(output).toContain("--verbose"); + expect(output).toContain("skill path does not exist"); + expect(output).not.toContain("collision:"); + }); + test("shows full resource listing on verbose startup even when tool output is collapsed", () => { const fakeThis = createShowLoadedResourcesThis({ quietStartup: true, diff --git a/packages/coding-agent/src/core/skills.ts b/packages/coding-agent/src/core/skills.ts index 007fe030..97ab24e6 100644 --- a/packages/coding-agent/src/core/skills.ts +++ b/packages/coding-agent/src/core/skills.ts @@ -363,6 +363,14 @@ function escapeXml(str: string): string { .replace(/'/g, "'"); } +function hasSameContent(a: string, b: string): boolean { + try { + return readFileSync(a).equals(readFileSync(b)); + } catch { + return false; + } +} + export interface LoadSkillsOptions { /** Working directory for project-local skills. */ cwd: string; @@ -406,6 +414,12 @@ export function loadSkills(options: LoadSkillsOptions): LoadSkillsResult { const existing = skillMap.get(skill.name); if (existing) { + // Tools that install one skill for several agents (e.g. ~/.agents/skills and + // ~/.claude/skills) often copy instead of symlink. An identical copy changes + // nothing about what gets loaded, so skip it as silently as a symlink. + if (hasSameContent(existing.filePath, skill.filePath)) { + continue; + } collisionDiagnostics.push({ type: "collision", message: `name "${skill.name}" collision`, diff --git a/packages/coding-agent/test/skill-loading-regressions.test.ts b/packages/coding-agent/test/skill-loading-regressions.test.ts index 43c146fe..f160cf2e 100644 --- a/packages/coding-agent/test/skill-loading-regressions.test.ts +++ b/packages/coding-agent/test/skill-loading-regressions.test.ts @@ -138,6 +138,37 @@ describe("skill loading regressions", () => { expect(winner(apiResult.skills)).toBe(winner(loader.getSkills().skills)); }); + it("identical copies of a skill are skipped without a collision diagnostic", () => { + const { root } = fixture(); + const agentsPath = putSkill(join(root, ".agents", "skills", "audit-skill")); + putSkill(join(root, ".claude", "skills", "audit-skill")); + putSkill(join(root, ".claude", "skills", "other-skill"), "other-skill"); + const result = loadSkills({ + cwd: root, + agentDir: join(root, "agent"), + skillPaths: [join(root, ".agents", "skills"), join(root, ".claude", "skills")], + includeDefaults: false, + }); + expect(result.skills.find((s) => s.name === "audit-skill")?.filePath).toBe(agentsPath); + expect(result.skills.map((s) => s.name).sort()).toEqual(["audit-skill", "other-skill"]); + expect(result.diagnostics.filter((d) => d.type === "collision")).toEqual([]); + }); + + it("differing copies of a skill still report a collision", () => { + const { root } = fixture(); + const agentsPath = putSkill(join(root, ".agents", "skills", "audit-skill"), "audit-skill", "AGENTS"); + const claudePath = putSkill(join(root, ".claude", "skills", "audit-skill"), "audit-skill", "CLAUDE"); + const result = loadSkills({ + cwd: root, + agentDir: join(root, "agent"), + skillPaths: [join(root, ".agents", "skills"), join(root, ".claude", "skills")], + includeDefaults: false, + }); + const collisions = result.diagnostics.filter((d) => d.type === "collision"); + expect(collisions).toHaveLength(1); + expect(collisions[0]?.collision).toMatchObject({ winnerPath: agentsPath, loserPath: claudePath }); + }); + it("both loaders should accept the same UTF-8 BOM skill file", async () => { const { root, agentDir } = fixture(); const skillsDir = join(agentDir, "skills");