Phase 12: skill name collision warn — SPEC §11.
Duplicate names in resourceLoader resolve to the first skill with a one-time UI warning during re-inject expansion. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
+30
-3
@@ -18,11 +18,38 @@ import type { ExtensionState } from "./state.js";
|
||||
|
||||
export const DEFERRED_REINJECT_CUSTOM_TYPE = "skill-reinject:inject";
|
||||
|
||||
function notifySkippedSkill(ctx: ExtensionContext | undefined, skillName: string, reason: string): void {
|
||||
function notifyWarning(ctx: ExtensionContext | undefined, message: string): void {
|
||||
if (!ctx?.hasUI) {
|
||||
return;
|
||||
}
|
||||
ctx.ui.notify(`skill-reinject: skipped "${skillName}" — ${reason}`, "warning");
|
||||
ctx.ui.notify(message, "warning");
|
||||
}
|
||||
|
||||
function notifySkippedSkill(ctx: ExtensionContext | undefined, skillName: string, reason: string): void {
|
||||
notifyWarning(ctx, `skill-reinject: skipped "${skillName}" — ${reason}`);
|
||||
}
|
||||
|
||||
/** First registered skill per name; warn on resourceLoader collisions (SPEC §11). */
|
||||
export function registeredSkillsByName<T extends Pick<Skill, "name">>(
|
||||
registeredSkills: readonly T[],
|
||||
ctx?: ExtensionContext,
|
||||
): Map<string, T> {
|
||||
const byName = new Map<string, T>();
|
||||
const warned = new Set<string>();
|
||||
for (const skill of registeredSkills) {
|
||||
if (byName.has(skill.name)) {
|
||||
if (!warned.has(skill.name)) {
|
||||
warned.add(skill.name);
|
||||
notifyWarning(
|
||||
ctx,
|
||||
`skill-reinject: duplicate skill name "${skill.name}" — using first from resourceLoader`,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
byName.set(skill.name, skill);
|
||||
}
|
||||
return byName;
|
||||
}
|
||||
|
||||
/** Names still registered in resourceLoader (SPEC §5.2). */
|
||||
@@ -77,7 +104,7 @@ export function buildReinjectBlocks(
|
||||
registeredSkills: readonly Pick<Skill, "name" | "filePath" | "baseDir">[],
|
||||
ctx?: ExtensionContext,
|
||||
): string[] {
|
||||
const registeredByName = new Map(registeredSkills.map((skill) => [skill.name, skill]));
|
||||
const registeredByName = registeredSkillsByName(registeredSkills, ctx);
|
||||
const blocks: string[] = [];
|
||||
for (const name of skillNames) {
|
||||
const tracked = state.skills.find((skill) => skill.name === name);
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { registeredSkillsByName } from "../src/reinject";
|
||||
|
||||
describe("registeredSkillsByName", () => {
|
||||
it("keeps the first skill when names collide", () => {
|
||||
const first = { name: "alpha", filePath: "/a/SKILL.md", baseDir: "/a" };
|
||||
const second = { name: "alpha", filePath: "/b/SKILL.md", baseDir: "/b" };
|
||||
|
||||
const byName = registeredSkillsByName([first, second]);
|
||||
|
||||
expect(byName.get("alpha")).toBe(first);
|
||||
});
|
||||
|
||||
it("warns once per duplicate name", () => {
|
||||
const notify = vi.fn();
|
||||
const ctx = { hasUI: true, ui: { notify } } as never;
|
||||
const skills = [
|
||||
{ name: "alpha", filePath: "/a/SKILL.md", baseDir: "/a" },
|
||||
{ name: "alpha", filePath: "/b/SKILL.md", baseDir: "/b" },
|
||||
{ name: "alpha", filePath: "/c/SKILL.md", baseDir: "/c" },
|
||||
];
|
||||
|
||||
registeredSkillsByName(skills, ctx);
|
||||
|
||||
expect(notify).toHaveBeenCalledTimes(1);
|
||||
expect(notify).toHaveBeenCalledWith(
|
||||
'skill-reinject: duplicate skill name "alpha" — using first from resourceLoader',
|
||||
"warning",
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user