diff --git a/CHANGELOG.md b/CHANGELOG.md index 308bedb0..c106e0d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### 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. - Persist Ultracode on/off state in the current session journal so reopening that session restores its mode without changing global or project settings. - 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. 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");