fix(coding-agent): resource collision precedence, user/project skills override package skills

Sort resolved resources by precedence rank (project settings > project auto >
user settings > user auto > package) so first-wins collision logic produces
correct results. Also reorder CLI paths to prepend instead of append.

fixes #2781
This commit is contained in:
Mario Zechner
2026-04-04 21:28:38 +02:00
parent fbd35a4660
commit a7acef92a7
3 changed files with 144 additions and 4 deletions

View File

@@ -118,6 +118,24 @@ interface ResourceAccumulator {
themes: Map<string, { metadata: PathMetadata; enabled: boolean }>;
}
/**
* Compute a numeric precedence rank for a resource based on its metadata.
* Lower rank = higher precedence. Used to sort resolved resources so that
* name-collision resolution ("first wins") produces the correct outcome.
*
* Precedence (highest to lowest):
* 0 project + settings entry (source: "local", scope: "project")
* 1 project + auto-discovered (source: "auto", scope: "project")
* 2 user + settings entry (source: "local", scope: "user")
* 3 user + auto-discovered (source: "auto", scope: "user")
* 4 package resource (origin: "package")
*/
function resourcePrecedenceRank(m: PathMetadata): number {
if (m.origin === "package") return 4;
const scopeBase = m.scope === "project" ? 0 : 2;
return scopeBase + (m.source === "local" ? 0 : 1);
}
interface PackageFilter {
extensions?: string[];
skills?: string[];
@@ -2146,11 +2164,13 @@ export class DefaultPackageManager implements PackageManager {
private toResolvedPaths(accumulator: ResourceAccumulator): ResolvedPaths {
const toResolved = (entries: Map<string, { metadata: PathMetadata; enabled: boolean }>): ResolvedResource[] => {
return Array.from(entries.entries()).map(([path, { metadata, enabled }]) => ({
const resolved = Array.from(entries.entries()).map(([path, { metadata, enabled }]) => ({
path,
enabled,
metadata,
}));
resolved.sort((a, b) => resourcePrecedenceRank(a.metadata) - resourcePrecedenceRank(b.metadata));
return resolved;
};
return {

View File

@@ -413,7 +413,7 @@ export class DefaultResourceLoader implements ResourceLoader {
const skillPaths = this.noSkills
? this.mergePaths(cliEnabledSkills, this.additionalSkillPaths)
: this.mergePaths([...enabledSkills, ...cliEnabledSkills], this.additionalSkillPaths);
: this.mergePaths([...cliEnabledSkills, ...enabledSkills], this.additionalSkillPaths);
this.lastSkillPaths = skillPaths;
this.updateSkillsFromPaths(skillPaths, metadataByPath);
@@ -425,7 +425,7 @@ export class DefaultResourceLoader implements ResourceLoader {
const promptPaths = this.noPromptTemplates
? this.mergePaths(cliEnabledPrompts, this.additionalPromptTemplatePaths)
: this.mergePaths([...enabledPrompts, ...cliEnabledPrompts], this.additionalPromptTemplatePaths);
: this.mergePaths([...cliEnabledPrompts, ...enabledPrompts], this.additionalPromptTemplatePaths);
this.lastPromptPaths = promptPaths;
this.updatePromptsFromPaths(promptPaths, metadataByPath);
@@ -437,7 +437,7 @@ export class DefaultResourceLoader implements ResourceLoader {
const themePaths = this.noThemes
? this.mergePaths(cliEnabledThemes, this.additionalThemePaths)
: this.mergePaths([...enabledThemes, ...cliEnabledThemes], this.additionalThemePaths);
: this.mergePaths([...cliEnabledThemes, ...enabledThemes], this.additionalThemePaths);
this.lastThemePaths = themePaths;
this.updateThemesFromPaths(themePaths, metadataByPath);

View File

@@ -0,0 +1,120 @@
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { DefaultResourceLoader } from "../../../src/core/resource-loader.js";
describe("issue #2781 skill collision precedence: user skills should override package skills", () => {
let tempDir: string;
let agentDir: string;
let cwd: string;
beforeEach(() => {
tempDir = join(tmpdir(), `pi-2781-${Date.now()}-${Math.random().toString(36).slice(2)}`);
agentDir = join(tempDir, "agent");
cwd = join(tempDir, "project");
mkdirSync(agentDir, { recursive: true });
mkdirSync(cwd, { recursive: true });
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
function createPackageWithSkill(name: string, description: string): string {
const pkgDir = join(tempDir, `fake-package-${name}`);
const skillDir = join(pkgDir, "skills", name);
mkdirSync(skillDir, { recursive: true });
writeFileSync(
join(pkgDir, "package.json"),
JSON.stringify({ name: `fake-pkg-${name}`, version: "1.0.0", pi: { skills: [`skills/${name}`] } }, null, 2),
);
writeFileSync(
join(skillDir, "SKILL.md"),
`---\nname: ${name}\ndescription: ${description}\n---\nPackage skill content`,
);
return pkgDir;
}
function createUserSkill(name: string, description: string): string {
const skillDir = join(agentDir, "skills", name);
mkdirSync(skillDir, { recursive: true });
const skillPath = join(skillDir, "SKILL.md");
writeFileSync(skillPath, `---\nname: ${name}\ndescription: ${description}\n---\nUser skill content`);
return skillPath;
}
function createProjectSkill(name: string, description: string): string {
const skillDir = join(cwd, ".pi", "skills", name);
mkdirSync(skillDir, { recursive: true });
const skillPath = join(skillDir, "SKILL.md");
writeFileSync(skillPath, `---\nname: ${name}\ndescription: ${description}\n---\nProject skill content`);
return skillPath;
}
function createSettingsWithPackage(pkgDir: string, scope: "user" | "project"): void {
const settingsDir = scope === "user" ? agentDir : join(cwd, ".pi");
mkdirSync(settingsDir, { recursive: true });
writeFileSync(join(settingsDir, "settings.json"), JSON.stringify({ packages: [pkgDir] }, null, 2));
}
it("user auto-discovered skill should override package skill with same name", async () => {
const pkgDir = createPackageWithSkill("web-fetch", "Package web-fetch skill");
const userSkillPath = createUserSkill("web-fetch", "User web-fetch override");
createSettingsWithPackage(pkgDir, "user");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { skills } = loader.getSkills();
const webFetch = skills.find((s) => s.name === "web-fetch");
expect(webFetch).toBeDefined();
expect(webFetch!.filePath).toBe(userSkillPath);
expect(webFetch!.description).toBe("User web-fetch override");
});
it("project auto-discovered skill should override package skill with same name", async () => {
const pkgDir = createPackageWithSkill("web-fetch", "Package web-fetch skill");
const projectSkillPath = createProjectSkill("web-fetch", "Project web-fetch override");
createSettingsWithPackage(pkgDir, "user");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { skills } = loader.getSkills();
const webFetch = skills.find((s) => s.name === "web-fetch");
expect(webFetch).toBeDefined();
expect(webFetch!.filePath).toBe(projectSkillPath);
expect(webFetch!.description).toBe("Project web-fetch override");
});
it("project skill should override user skill which should override package skill", async () => {
const pkgDir = createPackageWithSkill("web-fetch", "Package web-fetch skill");
createUserSkill("web-fetch", "User web-fetch override");
const projectSkillPath = createProjectSkill("web-fetch", "Project web-fetch override");
createSettingsWithPackage(pkgDir, "user");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { skills } = loader.getSkills();
const webFetch = skills.find((s) => s.name === "web-fetch");
expect(webFetch).toBeDefined();
expect(webFetch!.filePath).toBe(projectSkillPath);
expect(webFetch!.description).toBe("Project web-fetch override");
});
it("collision diagnostics should report package skill as loser when user skill wins", async () => {
const pkgDir = createPackageWithSkill("web-fetch", "Package web-fetch skill");
createUserSkill("web-fetch", "User web-fetch override");
createSettingsWithPackage(pkgDir, "user");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { diagnostics } = loader.getSkills();
const collision = diagnostics.find((d) => d.type === "collision" && d.collision?.name === "web-fetch");
expect(collision).toBeDefined();
expect(collision!.collision!.loserPath).toContain("fake-package");
});
});