From 585ce73be17b70a5b5351bc00b884bca3694cc4a Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Mon, 26 Jan 2026 13:49:21 +0100 Subject: [PATCH] fix(coding-agent): align auto-discovery with loader rules --- .../coding-agent/src/core/package-manager.ts | 83 +++++++------------ .../coding-agent/src/core/resource-loader.ts | 36 +++++++- .../interactive/components/config-selector.ts | 4 +- .../coding-agent/test/package-manager.test.ts | 13 +-- .../coding-agent/test/resource-loader.test.ts | 66 +++++++++++++++ 5 files changed, 139 insertions(+), 63 deletions(-) diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 18f21759..86ccaf42 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -169,7 +169,7 @@ function collectFiles(dir: string, filePattern: RegExp, skipNodeModules = true): return files; } -function collectSkillEntries(dir: string): string[] { +function collectSkillEntries(dir: string, includeRootFiles = true): string[] { const entries: string[] = []; if (!existsSync(dir)) return entries; @@ -194,14 +194,13 @@ function collectSkillEntries(dir: string): string[] { } if (isDir) { - const skillMd = join(fullPath, "SKILL.md"); - if (existsSync(skillMd)) { + entries.push(...collectSkillEntries(fullPath, false)); + } else if (isFile) { + const isRootMd = includeRootFiles && entry.name.endsWith(".md"); + const isSkillMd = !includeRootFiles && entry.name === "SKILL.md"; + if (isRootMd || isSkillMd) { entries.push(fullPath); - } else { - entries.push(...collectSkillEntries(fullPath)); } - } else if (isFile && entry.name.endsWith(".md")) { - entries.push(fullPath); } } } catch { @@ -211,48 +210,8 @@ function collectSkillEntries(dir: string): string[] { return entries; } -function collectAutoSkillEntries(dir: string, isRoot = true): string[] { - const entries: string[] = []; - 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 collectAutoSkillEntries(dir: string, includeRootFiles = true): string[] { + return collectSkillEntries(dir, includeRootFiles); } function collectAutoPromptEntries(dir: string): string[] { @@ -400,9 +359,18 @@ function collectAutoExtensionEntries(dir: string): string[] { function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string): boolean { const rel = relative(baseDir, filePath); const name = basename(filePath); - return patterns.some( - (pattern) => minimatch(rel, pattern) || minimatch(name, pattern) || minimatch(filePath, pattern), - ); + const isSkillFile = name === "SKILL.md"; + 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 { @@ -415,9 +383,18 @@ function normalizeExactPattern(pattern: string): string { function matchesAnyExactPattern(filePath: string, patterns: string[], baseDir: string): boolean { if (patterns.length === 0) return false; 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) => { 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; }); } diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 02560733..d1517681 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -268,24 +268,52 @@ export class DefaultResourceLoader implements ResourceLoader { }); // Helper to extract enabled paths and store metadata - const getEnabledPaths = ( + const getEnabledResources = ( resources: Array<{ path: string; enabled: boolean; metadata: PathMetadata }>, - ): string[] => { + ): Array<{ path: string; enabled: boolean; metadata: PathMetadata }> => { for (const r of resources) { if (!this.pathMetadata.has(r.path)) { 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 this.pathMetadata = new Map(); const enabledExtensions = getEnabledPaths(resolvedPaths.extensions); - const enabledSkills = getEnabledPaths(resolvedPaths.skills); + const enabledSkillResources = getEnabledResources(resolvedPaths.skills); const enabledPrompts = getEnabledPaths(resolvedPaths.prompts); 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 for (const r of cliExtensionPaths.extensions) { if (!this.pathMetadata.has(r.path)) { diff --git a/packages/coding-agent/src/modes/interactive/components/config-selector.ts b/packages/coding-agent/src/modes/interactive/components/config-selector.ts index 2533b3ad..bf42f6e2 100644 --- a/packages/coding-agent/src/modes/interactive/components/config-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/config-selector.ts @@ -98,12 +98,14 @@ function buildGroups(resolved: ResolvedPaths): ResourceGroup[] { group.subgroups.push(subgroup); } + const displayName = + resourceType === "skills" && basename(path) === "SKILL.md" ? basename(dirname(path)) : basename(path); subgroup.items.push({ path, enabled, metadata, resourceType, - displayName: basename(path), + displayName, groupKey, subgroupKey, }); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 3977aaa3..ca3c4b31 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -59,8 +59,9 @@ describe("DefaultPackageManager", () => { it("should resolve skill paths from settings", async () => { const skillDir = join(agentDir, "skills", "my-skill"); mkdirSync(skillDir, { recursive: true }); + const skillFile = join(skillDir, "SKILL.md"); writeFileSync( - join(skillDir, "SKILL.md"), + skillFile, `--- name: test-skill description: A test skill @@ -71,8 +72,8 @@ Content`, settingsManager.setSkillPaths(["skills"]); const result = await packageManager.resolve(); - // Skills with SKILL.md are returned as directory paths - expect(result.skills.some((r) => r.path === skillDir && r.enabled)).toBe(true); + // Skills with SKILL.md are returned as file paths + expect(result.skills.some((r) => r.path === skillFile && r.enabled)).toBe(true); }); it("should resolve project paths relative to .pi", async () => { @@ -144,8 +145,10 @@ Content`, const result = await packageManager.resolveExtensionSources([pkgDir]); 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 - expect(result.skills.some((r) => r.path === join(pkgDir, "skills", "my-skill") && r.enabled)).toBe(true); + // Skills with SKILL.md are returned as file paths + 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 () => { diff --git a/packages/coding-agent/test/resource-loader.test.ts b/packages/coding-agent/test/resource-loader.test.ts index dc91fc3f..567073c8 100644 --- a/packages/coding-agent/test/resource-loader.test.ts +++ b/packages/coding-agent/test/resource-loader.test.ts @@ -3,6 +3,7 @@ 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"; +import { SettingsManager } from "../src/core/settings-manager.js"; import type { Skill } from "../src/core/skills.js"; describe("DefaultResourceLoader", () => { @@ -51,6 +52,27 @@ Skill content here.`, 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 () => { const promptsDir = join(agentDir, "prompts"); mkdirSync(promptsDir, { recursive: true }); @@ -69,6 +91,50 @@ Prompt content.`, 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 () => { writeFileSync(join(cwd, "AGENTS.md"), "# Project Guidelines\n\nBe helpful.");