refactor(discovery): extracted skill loading logic into reusable helper to eliminate duplication
- Extracted skill loading logic into reusable `loadSkillsFromDir` helper function to eliminate code duplication across builtin, claude, and codex discovery providers. - Refactored builtin provider to use new `loadSkillsFromDir` helper instead of inline `loadSkillsRecursive` function. - Refactored claude provider to use new `loadSkillsFromDir` helper, reducing function from 78 lines to 25 lines. - Refactored codex provider to use new `loadSkillsFromDir` helper, consolidating duplicate skill transformation and frontmatter parsing logic. - Updated extension module discovery to skip `node_modules` directories in addition to hidden directories. - Added guideline to Task tool documentation about isolating file scopes for concurrent agents to prevent conflicts.
This commit is contained in:
can1357
@@ -1,7 +1,6 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added automatic discovery and listing of AGENTS.md files in the system prompt, providing agents with an authoritative list of project-specific instruction files without runtime searching
|
||||
@@ -9,10 +8,14 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Refactored skill discovery to use unified `loadSkillsFromDir` helper across all providers, reducing code duplication
|
||||
- Updated skill discovery to scan only `skills/*/SKILL.md` entries instead of recursive walks in Codex provider
|
||||
- Added guidance to Task tool documentation to isolate file scopes when assigning tasks to prevent agent conflicts
|
||||
- Updated Task tool documentation to emphasize that subagents have no access to conversation history and require all relevant context to be explicitly passed
|
||||
- Revised task agent prompt to clarify that subagents have full tool access and can make file edits, run commands, and create files
|
||||
- OpenAI Codex: updated to use bundled system prompt from upstream
|
||||
- Changed `complete` tool to make `data` parameter optional when aborting, while still requiring it for successful completions
|
||||
- Skills discovery now scans only `skills/*/SKILL.md` entries instead of recursive walks
|
||||
|
||||
### Removed
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* .pi is an alias for backwards compatibility.
|
||||
*/
|
||||
|
||||
import { basename, dirname, isAbsolute, join, resolve } from "path";
|
||||
import { dirname, isAbsolute, join, resolve } from "path";
|
||||
import { type ContextFile, contextFileCapability } from "../capability/context-file";
|
||||
import { type Extension, type ExtensionManifest, extensionCapability } from "../capability/extension";
|
||||
import { type ExtensionModule, extensionModuleCapability } from "../capability/extension-module";
|
||||
@@ -16,7 +16,7 @@ import { type MCPServer, mcpCapability } from "../capability/mcp";
|
||||
import { type Prompt, promptCapability } from "../capability/prompt";
|
||||
import { type Rule, ruleCapability } from "../capability/rule";
|
||||
import { type Settings, settingsCapability } from "../capability/settings";
|
||||
import { type Skill, type SkillFrontmatter, skillCapability } from "../capability/skill";
|
||||
import { type Skill, skillCapability } from "../capability/skill";
|
||||
import { type SlashCommand, slashCommandCapability } from "../capability/slash-command";
|
||||
import { type SystemPrompt, systemPromptCapability } from "../capability/system-prompt";
|
||||
import { type CustomTool, toolCapability } from "../capability/tool";
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
expandEnvVarsDeep,
|
||||
getExtensionNameFromPath,
|
||||
loadFilesFromDir,
|
||||
loadSkillsFromDir,
|
||||
parseFrontmatter,
|
||||
parseJSON,
|
||||
SOURCE_PATHS,
|
||||
@@ -190,64 +191,18 @@ registerProvider<SystemPrompt>(systemPromptCapability.id, {
|
||||
});
|
||||
|
||||
// Skills
|
||||
function loadSkillFromFile(ctx: LoadContext, path: string, level: "user" | "project"): Skill | null {
|
||||
const content = ctx.fs.readFile(path);
|
||||
if (!content) return null;
|
||||
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
const skillDir = dirname(path);
|
||||
const parentDirName = basename(skillDir);
|
||||
const name = (frontmatter.name as string) || parentDirName;
|
||||
|
||||
if (!frontmatter.description) return null;
|
||||
|
||||
return {
|
||||
name,
|
||||
path,
|
||||
content: body,
|
||||
frontmatter: frontmatter as SkillFrontmatter,
|
||||
level,
|
||||
_source: createSourceMeta(PROVIDER_ID, path, level),
|
||||
};
|
||||
}
|
||||
|
||||
function loadSkillsRecursive(ctx: LoadContext, dir: string, level: "user" | "project"): LoadResult<Skill> {
|
||||
const items: Skill[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
if (!ctx.fs.isDir(dir)) return { items, warnings };
|
||||
|
||||
for (const name of ctx.fs.readDir(dir)) {
|
||||
if (name.startsWith(".") || name === "node_modules") continue;
|
||||
|
||||
const path = join(dir, name);
|
||||
|
||||
if (ctx.fs.isDir(path)) {
|
||||
const skillFile = join(path, "SKILL.md");
|
||||
if (ctx.fs.isFile(skillFile)) {
|
||||
const skill = loadSkillFromFile(ctx, skillFile, level);
|
||||
if (skill) items.push(skill);
|
||||
}
|
||||
|
||||
const sub = loadSkillsRecursive(ctx, path, level);
|
||||
items.push(...sub.items);
|
||||
if (sub.warnings) warnings.push(...sub.warnings);
|
||||
} else if (name === "SKILL.md") {
|
||||
const skill = loadSkillFromFile(ctx, path, level);
|
||||
if (skill) items.push(skill);
|
||||
}
|
||||
}
|
||||
|
||||
return { items, warnings };
|
||||
}
|
||||
|
||||
function loadSkills(ctx: LoadContext): LoadResult<Skill> {
|
||||
const items: Skill[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
for (const { dir, level } of getConfigDirs(ctx)) {
|
||||
const skillsDir = join(dir, "skills");
|
||||
const result = loadSkillsRecursive(ctx, skillsDir, level);
|
||||
const result = loadSkillsFromDir(ctx, {
|
||||
dir: skillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level,
|
||||
requireDescription: true,
|
||||
});
|
||||
items.push(...result.items);
|
||||
if (result.warnings) warnings.push(...result.warnings);
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ import {
|
||||
expandEnvVarsDeep,
|
||||
getExtensionNameFromPath,
|
||||
loadFilesFromDir,
|
||||
parseFrontmatter,
|
||||
loadSkillsFromDir,
|
||||
parseJSON,
|
||||
} from "./helpers";
|
||||
|
||||
@@ -218,78 +218,25 @@ function loadSkills(ctx: LoadContext): LoadResult<Skill> {
|
||||
const items: Skill[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
// User-level: ~/.claude/skills/*/SKILL.md
|
||||
const userBase = getUserClaude(ctx);
|
||||
const userSkillsDir = join(userBase, "skills");
|
||||
const userSkillsDir = join(getUserClaude(ctx), "skills");
|
||||
const userResult = loadSkillsFromDir(ctx, {
|
||||
dir: userSkillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level: "user",
|
||||
});
|
||||
items.push(...userResult.items);
|
||||
if (userResult.warnings) warnings.push(...userResult.warnings);
|
||||
|
||||
if (ctx.fs.isDir(userSkillsDir)) {
|
||||
const skillDirs = ctx.fs.readDir(userSkillsDir);
|
||||
|
||||
for (const dirName of skillDirs) {
|
||||
if (dirName.startsWith(".")) continue;
|
||||
|
||||
const skillDir = join(userSkillsDir, dirName);
|
||||
if (!ctx.fs.isDir(skillDir)) continue;
|
||||
|
||||
const skillFile = join(skillDir, "SKILL.md");
|
||||
if (!ctx.fs.isFile(skillFile)) continue;
|
||||
|
||||
const content = ctx.fs.readFile(skillFile);
|
||||
if (!content) {
|
||||
warnings.push(`Failed to read ${skillFile}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
const name = (frontmatter.name as string) || dirName;
|
||||
|
||||
items.push({
|
||||
name,
|
||||
path: skillFile,
|
||||
content: body,
|
||||
frontmatter,
|
||||
level: "user",
|
||||
_source: createSourceMeta(PROVIDER_ID, skillFile, "user"),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Project-level: <project>/.claude/skills/*/SKILL.md
|
||||
const projectBase = getProjectClaude(ctx);
|
||||
if (projectBase) {
|
||||
const projectSkillsDir = join(projectBase, "skills");
|
||||
|
||||
if (ctx.fs.isDir(projectSkillsDir)) {
|
||||
const skillDirs = ctx.fs.readDir(projectSkillsDir);
|
||||
|
||||
for (const dirName of skillDirs) {
|
||||
if (dirName.startsWith(".")) continue;
|
||||
|
||||
const skillDir = join(projectSkillsDir, dirName);
|
||||
if (!ctx.fs.isDir(skillDir)) continue;
|
||||
|
||||
const skillFile = join(skillDir, "SKILL.md");
|
||||
if (!ctx.fs.isFile(skillFile)) continue;
|
||||
|
||||
const content = ctx.fs.readFile(skillFile);
|
||||
if (!content) {
|
||||
warnings.push(`Failed to read ${skillFile}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
const name = (frontmatter.name as string) || dirName;
|
||||
|
||||
items.push({
|
||||
name,
|
||||
path: skillFile,
|
||||
content: body,
|
||||
frontmatter,
|
||||
level: "project",
|
||||
_source: createSourceMeta(PROVIDER_ID, skillFile, "project"),
|
||||
});
|
||||
}
|
||||
}
|
||||
const projectResult = loadSkillsFromDir(ctx, {
|
||||
dir: projectSkillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level: "project",
|
||||
});
|
||||
items.push(...projectResult.items);
|
||||
if (projectResult.warnings) warnings.push(...projectResult.warnings);
|
||||
}
|
||||
|
||||
return { items, warnings };
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
discoverExtensionModulePaths,
|
||||
getExtensionNameFromPath,
|
||||
loadFilesFromDir,
|
||||
loadSkillsFromDir,
|
||||
parseFrontmatter,
|
||||
SOURCE_PATHS,
|
||||
} from "./helpers";
|
||||
@@ -209,51 +210,25 @@ function loadSkills(ctx: LoadContext): LoadResult<Skill> {
|
||||
const items: Skill[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
// User level: ~/.codex/skills/
|
||||
const userSkillsDir = join(ctx.home, SOURCE_PATHS.codex.userBase, "skills");
|
||||
const userResult = loadFilesFromDir(ctx, userSkillsDir, PROVIDER_ID, "user", {
|
||||
extensions: ["md"],
|
||||
recursive: true,
|
||||
transform: (name, content, path, source) => {
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
const skillName = frontmatter.name || name.replace(/\.md$/, "");
|
||||
|
||||
return {
|
||||
name: String(skillName),
|
||||
path,
|
||||
content: body,
|
||||
frontmatter,
|
||||
level: "user" as const,
|
||||
_source: source,
|
||||
};
|
||||
},
|
||||
const userResult = loadSkillsFromDir(ctx, {
|
||||
dir: userSkillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level: "user",
|
||||
});
|
||||
items.push(...userResult.items);
|
||||
warnings.push(...(userResult.warnings || []));
|
||||
if (userResult.warnings) warnings.push(...userResult.warnings);
|
||||
|
||||
// Project level: .codex/skills/
|
||||
const codexDir = ctx.fs.walkUp(".codex", { dir: true });
|
||||
if (codexDir) {
|
||||
const projectSkillsDir = join(codexDir, "skills");
|
||||
const projectResult = loadFilesFromDir(ctx, projectSkillsDir, PROVIDER_ID, "project", {
|
||||
extensions: ["md"],
|
||||
recursive: true,
|
||||
transform: (name, content, path, source) => {
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
const skillName = frontmatter.name || name.replace(/\.md$/, "");
|
||||
|
||||
return {
|
||||
name: String(skillName),
|
||||
path,
|
||||
content: body,
|
||||
frontmatter,
|
||||
level: "project" as const,
|
||||
_source: source,
|
||||
};
|
||||
},
|
||||
const projectResult = loadSkillsFromDir(ctx, {
|
||||
dir: projectSkillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level: "project",
|
||||
});
|
||||
items.push(...projectResult.items);
|
||||
warnings.push(...(projectResult.warnings || []));
|
||||
if (projectResult.warnings) warnings.push(...projectResult.warnings);
|
||||
}
|
||||
|
||||
return { items, warnings };
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
|
||||
import { join, resolve } from "path";
|
||||
import { parse as parseYAML } from "yaml";
|
||||
import type { Skill, SkillFrontmatter } from "../capability/skill";
|
||||
import type { LoadContext, LoadResult, SourceMeta } from "../capability/types";
|
||||
|
||||
/**
|
||||
@@ -126,6 +127,56 @@ export function parseFrontmatter(content: string): {
|
||||
}
|
||||
}
|
||||
|
||||
export function loadSkillsFromDir(
|
||||
ctx: LoadContext,
|
||||
options: {
|
||||
dir: string;
|
||||
providerId: string;
|
||||
level: "user" | "project";
|
||||
requireDescription?: boolean;
|
||||
},
|
||||
): LoadResult<Skill> {
|
||||
const items: Skill[] = [];
|
||||
const warnings: string[] = [];
|
||||
const { dir, level, providerId, requireDescription = false } = options;
|
||||
|
||||
if (!ctx.fs.isDir(dir)) {
|
||||
return { items, warnings };
|
||||
}
|
||||
|
||||
for (const name of ctx.fs.readDir(dir)) {
|
||||
if (name.startsWith(".") || name === "node_modules") continue;
|
||||
|
||||
const skillDir = join(dir, name);
|
||||
if (!ctx.fs.isDir(skillDir)) continue;
|
||||
|
||||
const skillFile = join(skillDir, "SKILL.md");
|
||||
if (!ctx.fs.isFile(skillFile)) continue;
|
||||
|
||||
const content = ctx.fs.readFile(skillFile);
|
||||
if (!content) {
|
||||
warnings.push(`Failed to read ${skillFile}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const { frontmatter, body } = parseFrontmatter(content);
|
||||
if (requireDescription && !frontmatter.description) {
|
||||
continue;
|
||||
}
|
||||
|
||||
items.push({
|
||||
name: (frontmatter.name as string) || name,
|
||||
path: skillFile,
|
||||
content: body,
|
||||
frontmatter: frontmatter as SkillFrontmatter,
|
||||
level,
|
||||
_source: createSourceMeta(providerId, skillFile, level),
|
||||
});
|
||||
}
|
||||
|
||||
return { items, warnings };
|
||||
}
|
||||
|
||||
/**
|
||||
* Expand environment variables in a string.
|
||||
* Supports ${VAR} and ${VAR:-default} syntax.
|
||||
@@ -286,7 +337,7 @@ export function discoverExtensionModulePaths(ctx: LoadContext, dir: string): str
|
||||
const discovered: string[] = [];
|
||||
|
||||
for (const name of ctx.fs.readDir(dir)) {
|
||||
if (name.startsWith(".")) continue;
|
||||
if (name.startsWith(".") || name === "node_modules") continue;
|
||||
|
||||
const entryPath = join(dir, name);
|
||||
|
||||
|
||||
@@ -33,6 +33,7 @@ If you discussed requirements, plans, schemas, or decisions with the user, you M
|
||||
- **Minimize tool chatter**: Avoid repeating large context; use Output tool with output ids for full logs
|
||||
- **Structured completion**: If `output` is provided, subagents must call `complete` to finish
|
||||
- **Parallelize**: Launch multiple agents concurrently whenever possible
|
||||
- **Isolate file scopes**: Assign each task distinct files or directories so agents don't conflict
|
||||
- **Results are intermediate data**: Agent findings provide context for YOU to perform actual work. Do not treat agent reports as "task complete" signals.
|
||||
- **Stateless invocations**: Subagents have zero memory of your conversation. Pass ALL relevant context: requirements discussed, decisions made, schemas agreed upon, file paths mentioned. If you reference something from earlier discussion without including it, the subagent will fail.
|
||||
- **Trust outputs**: Agent results should generally be trusted
|
||||
|
||||
Reference in New Issue
Block a user