fix(coding-agent): align auto-discovery with loader rules

This commit is contained in:
Mario Zechner 2026-01-26 13:49:21 +01:00
parent b270e7b585
commit 585ce73be1
5 changed files with 139 additions and 63 deletions

View file

@ -169,7 +169,7 @@ function collectFiles(dir: string, filePattern: RegExp, skipNodeModules = true):
return files; return files;
} }
function collectSkillEntries(dir: string): string[] { function collectSkillEntries(dir: string, includeRootFiles = true): string[] {
const entries: string[] = []; const entries: string[] = [];
if (!existsSync(dir)) return entries; if (!existsSync(dir)) return entries;
@ -194,14 +194,13 @@ function collectSkillEntries(dir: string): string[] {
} }
if (isDir) { if (isDir) {
const skillMd = join(fullPath, "SKILL.md"); entries.push(...collectSkillEntries(fullPath, false));
if (existsSync(skillMd)) { } else if (isFile) {
const isRootMd = includeRootFiles && entry.name.endsWith(".md");
const isSkillMd = !includeRootFiles && entry.name === "SKILL.md";
if (isRootMd || isSkillMd) {
entries.push(fullPath); entries.push(fullPath);
} else {
entries.push(...collectSkillEntries(fullPath));
} }
} else if (isFile && entry.name.endsWith(".md")) {
entries.push(fullPath);
} }
} }
} catch { } catch {
@ -211,48 +210,8 @@ function collectSkillEntries(dir: string): string[] {
return entries; return entries;
} }
function collectAutoSkillEntries(dir: string, isRoot = true): string[] { function collectAutoSkillEntries(dir: string, includeRootFiles = true): string[] {
const entries: string[] = []; return collectSkillEntries(dir, includeRootFiles);
if (!existsSync(dir)) return entries;
try {
const dirEntries = readdirSync(dir, { withFileTypes: true });
for (const entry of dirEntries) {
if (entry.name.startsWith(".")) continue;
if (entry.name === "node_modules") continue;
const fullPath = join(dir, entry.name);
let isDir = entry.isDirectory();
let isFile = entry.isFile();
if (entry.isSymbolicLink()) {
try {
const stats = statSync(fullPath);
isDir = stats.isDirectory();
isFile = stats.isFile();
} catch {
continue;
}
}
if (isDir) {
const skillMd = join(fullPath, "SKILL.md");
if (existsSync(skillMd)) {
entries.push(fullPath);
} else {
entries.push(...collectAutoSkillEntries(fullPath, false));
}
} else if (isFile && entry.name.endsWith(".md")) {
if (isRoot || entry.name === "SKILL.md") {
entries.push(fullPath);
}
}
}
} catch {
// Ignore errors
}
return entries;
} }
function collectAutoPromptEntries(dir: string): string[] { function collectAutoPromptEntries(dir: string): string[] {
@ -400,9 +359,18 @@ function collectAutoExtensionEntries(dir: string): string[] {
function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string): boolean { function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string): boolean {
const rel = relative(baseDir, filePath); const rel = relative(baseDir, filePath);
const name = basename(filePath); const name = basename(filePath);
return patterns.some( const isSkillFile = name === "SKILL.md";
(pattern) => minimatch(rel, pattern) || minimatch(name, pattern) || minimatch(filePath, pattern), const parentDir = isSkillFile ? dirname(filePath) : undefined;
); const parentRel = isSkillFile ? relative(baseDir, parentDir!) : undefined;
const parentName = isSkillFile ? basename(parentDir!) : undefined;
return patterns.some((pattern) => {
if (minimatch(rel, pattern) || minimatch(name, pattern) || minimatch(filePath, pattern)) {
return true;
}
if (!isSkillFile) return false;
return minimatch(parentRel!, pattern) || minimatch(parentName!, pattern) || minimatch(parentDir!, pattern);
});
} }
function normalizeExactPattern(pattern: string): string { function normalizeExactPattern(pattern: string): string {
@ -415,9 +383,18 @@ function normalizeExactPattern(pattern: string): string {
function matchesAnyExactPattern(filePath: string, patterns: string[], baseDir: string): boolean { function matchesAnyExactPattern(filePath: string, patterns: string[], baseDir: string): boolean {
if (patterns.length === 0) return false; if (patterns.length === 0) return false;
const rel = relative(baseDir, filePath); const rel = relative(baseDir, filePath);
const name = basename(filePath);
const isSkillFile = name === "SKILL.md";
const parentDir = isSkillFile ? dirname(filePath) : undefined;
const parentRel = isSkillFile ? relative(baseDir, parentDir!) : undefined;
return patterns.some((pattern) => { return patterns.some((pattern) => {
const normalized = normalizeExactPattern(pattern); const normalized = normalizeExactPattern(pattern);
return normalized === rel || normalized === filePath; if (normalized === rel || normalized === filePath) {
return true;
}
if (!isSkillFile) return false;
return normalized === parentRel || normalized === parentDir;
}); });
} }

View file

@ -268,24 +268,52 @@ export class DefaultResourceLoader implements ResourceLoader {
}); });
// Helper to extract enabled paths and store metadata // Helper to extract enabled paths and store metadata
const getEnabledPaths = ( const getEnabledResources = (
resources: Array<{ path: string; enabled: boolean; metadata: PathMetadata }>, resources: Array<{ path: string; enabled: boolean; metadata: PathMetadata }>,
): string[] => { ): Array<{ path: string; enabled: boolean; metadata: PathMetadata }> => {
for (const r of resources) { for (const r of resources) {
if (!this.pathMetadata.has(r.path)) { if (!this.pathMetadata.has(r.path)) {
this.pathMetadata.set(r.path, r.metadata); this.pathMetadata.set(r.path, r.metadata);
} }
} }
return resources.filter((r) => r.enabled).map((r) => r.path); return resources.filter((r) => r.enabled);
}; };
const getEnabledPaths = (
resources: Array<{ path: string; enabled: boolean; metadata: PathMetadata }>,
): string[] => getEnabledResources(resources).map((r) => r.path);
// Store metadata and get enabled paths // Store metadata and get enabled paths
this.pathMetadata = new Map(); this.pathMetadata = new Map();
const enabledExtensions = getEnabledPaths(resolvedPaths.extensions); const enabledExtensions = getEnabledPaths(resolvedPaths.extensions);
const enabledSkills = getEnabledPaths(resolvedPaths.skills); const enabledSkillResources = getEnabledResources(resolvedPaths.skills);
const enabledPrompts = getEnabledPaths(resolvedPaths.prompts); const enabledPrompts = getEnabledPaths(resolvedPaths.prompts);
const enabledThemes = getEnabledPaths(resolvedPaths.themes); const enabledThemes = getEnabledPaths(resolvedPaths.themes);
const mapSkillPath = (resource: { path: string; metadata: PathMetadata }): string => {
if (resource.metadata.source !== "auto" && resource.metadata.origin !== "package") {
return resource.path;
}
try {
const stats = statSync(resource.path);
if (!stats.isDirectory()) {
return resource.path;
}
} catch {
return resource.path;
}
const skillFile = join(resource.path, "SKILL.md");
if (existsSync(skillFile)) {
if (!this.pathMetadata.has(skillFile)) {
this.pathMetadata.set(skillFile, resource.metadata);
}
return skillFile;
}
return resource.path;
};
const enabledSkills = enabledSkillResources.map(mapSkillPath);
// Add CLI paths metadata // Add CLI paths metadata
for (const r of cliExtensionPaths.extensions) { for (const r of cliExtensionPaths.extensions) {
if (!this.pathMetadata.has(r.path)) { if (!this.pathMetadata.has(r.path)) {

View file

@ -98,12 +98,14 @@ function buildGroups(resolved: ResolvedPaths): ResourceGroup[] {
group.subgroups.push(subgroup); group.subgroups.push(subgroup);
} }
const displayName =
resourceType === "skills" && basename(path) === "SKILL.md" ? basename(dirname(path)) : basename(path);
subgroup.items.push({ subgroup.items.push({
path, path,
enabled, enabled,
metadata, metadata,
resourceType, resourceType,
displayName: basename(path), displayName,
groupKey, groupKey,
subgroupKey, subgroupKey,
}); });

View file

@ -59,8 +59,9 @@ describe("DefaultPackageManager", () => {
it("should resolve skill paths from settings", async () => { it("should resolve skill paths from settings", async () => {
const skillDir = join(agentDir, "skills", "my-skill"); const skillDir = join(agentDir, "skills", "my-skill");
mkdirSync(skillDir, { recursive: true }); mkdirSync(skillDir, { recursive: true });
const skillFile = join(skillDir, "SKILL.md");
writeFileSync( writeFileSync(
join(skillDir, "SKILL.md"), skillFile,
`--- `---
name: test-skill name: test-skill
description: A test skill description: A test skill
@ -71,8 +72,8 @@ Content`,
settingsManager.setSkillPaths(["skills"]); settingsManager.setSkillPaths(["skills"]);
const result = await packageManager.resolve(); const result = await packageManager.resolve();
// Skills with SKILL.md are returned as directory paths // Skills with SKILL.md are returned as file paths
expect(result.skills.some((r) => r.path === skillDir && r.enabled)).toBe(true); expect(result.skills.some((r) => r.path === skillFile && r.enabled)).toBe(true);
}); });
it("should resolve project paths relative to .pi", async () => { it("should resolve project paths relative to .pi", async () => {
@ -144,8 +145,10 @@ Content`,
const result = await packageManager.resolveExtensionSources([pkgDir]); const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.extensions.some((r) => r.path === join(pkgDir, "src", "index.ts") && r.enabled)).toBe(true); expect(result.extensions.some((r) => r.path === join(pkgDir, "src", "index.ts") && r.enabled)).toBe(true);
// Skills with SKILL.md are returned as directory paths // Skills with SKILL.md are returned as file paths
expect(result.skills.some((r) => r.path === join(pkgDir, "skills", "my-skill") && r.enabled)).toBe(true); expect(result.skills.some((r) => r.path === join(pkgDir, "skills", "my-skill", "SKILL.md") && r.enabled)).toBe(
true,
);
}); });
it("should handle directories with auto-discovery layout", async () => { it("should handle directories with auto-discovery layout", async () => {

View file

@ -3,6 +3,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path"; import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { DefaultResourceLoader } from "../src/core/resource-loader.js"; import { DefaultResourceLoader } from "../src/core/resource-loader.js";
import { SettingsManager } from "../src/core/settings-manager.js";
import type { Skill } from "../src/core/skills.js"; import type { Skill } from "../src/core/skills.js";
describe("DefaultResourceLoader", () => { describe("DefaultResourceLoader", () => {
@ -51,6 +52,27 @@ Skill content here.`,
expect(skills.some((s) => s.name === "test-skill")).toBe(true); expect(skills.some((s) => s.name === "test-skill")).toBe(true);
}); });
it("should ignore extra markdown files in auto-discovered skill dirs", async () => {
const skillDir = join(agentDir, "skills", "pi-skills", "browser-tools");
mkdirSync(skillDir, { recursive: true });
writeFileSync(
join(skillDir, "SKILL.md"),
`---
name: browser-tools
description: Browser tools
---
Skill content here.`,
);
writeFileSync(join(skillDir, "EFFICIENCY.md"), "No frontmatter here");
const loader = new DefaultResourceLoader({ cwd, agentDir });
await loader.reload();
const { skills, diagnostics } = loader.getSkills();
expect(skills.some((s) => s.name === "browser-tools")).toBe(true);
expect(diagnostics.some((d) => d.path?.endsWith("EFFICIENCY.md"))).toBe(false);
});
it("should discover prompts from agentDir", async () => { it("should discover prompts from agentDir", async () => {
const promptsDir = join(agentDir, "prompts"); const promptsDir = join(agentDir, "prompts");
mkdirSync(promptsDir, { recursive: true }); mkdirSync(promptsDir, { recursive: true });
@ -69,6 +91,50 @@ Prompt content.`,
expect(prompts.some((p) => p.name === "test-prompt")).toBe(true); expect(prompts.some((p) => p.name === "test-prompt")).toBe(true);
}); });
it("should honor overrides for auto-discovered resources", async () => {
const settingsManager = SettingsManager.inMemory();
settingsManager.setExtensionPaths(["-extensions/disabled.ts"]);
settingsManager.setSkillPaths(["-skills/skip-skill"]);
settingsManager.setPromptTemplatePaths(["-prompts/skip.md"]);
settingsManager.setThemePaths(["-themes/skip.json"]);
const extensionsDir = join(agentDir, "extensions");
mkdirSync(extensionsDir, { recursive: true });
writeFileSync(join(extensionsDir, "disabled.ts"), "export default function() {}");
const skillDir = join(agentDir, "skills", "skip-skill");
mkdirSync(skillDir, { recursive: true });
writeFileSync(
join(skillDir, "SKILL.md"),
`---
name: skip-skill
description: Skip me
---
Content`,
);
const promptsDir = join(agentDir, "prompts");
mkdirSync(promptsDir, { recursive: true });
writeFileSync(join(promptsDir, "skip.md"), "Skip prompt");
const themesDir = join(agentDir, "themes");
mkdirSync(themesDir, { recursive: true });
writeFileSync(join(themesDir, "skip.json"), "{}");
const loader = new DefaultResourceLoader({ cwd, agentDir, settingsManager });
await loader.reload();
const { extensions } = loader.getExtensions();
const { skills } = loader.getSkills();
const { prompts } = loader.getPrompts();
const { themes } = loader.getThemes();
expect(extensions.some((e) => e.path.endsWith("disabled.ts"))).toBe(false);
expect(skills.some((s) => s.name === "skip-skill")).toBe(false);
expect(prompts.some((p) => p.name === "skip")).toBe(false);
expect(themes.some((t) => t.sourcePath?.endsWith("skip.json"))).toBe(false);
});
it("should discover AGENTS.md context files", async () => { it("should discover AGENTS.md context files", async () => {
writeFileSync(join(cwd, "AGENTS.md"), "# Project Guidelines\n\nBe helpful."); writeFileSync(join(cwd, "AGENTS.md"), "# Project Guidelines\n\nBe helpful.");