mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix: dont load root mds as skills in settings (#8012)
* fix: dont load root mds as skills in settings * changelogololol --------- Co-authored-by: Mario Zechner <badlogicgames@gmail.com>
This commit is contained in:
parent
ca21c16861
commit
8c2529daeb
5 changed files with 96 additions and 44 deletions
|
|
@ -43,8 +43,9 @@ export function formatSkillInvocation(skill: Skill, additionalInstructions?: str
|
|||
/**
|
||||
* Load skills from one or more directories.
|
||||
*
|
||||
* Traverses directories recursively, loads `SKILL.md` files, loads direct root `.md` files as skills, honors ignore files,
|
||||
* and returns diagnostics for invalid skill files. Missing input directories are skipped.
|
||||
* Traverses directories recursively, loads `SKILL.md` files, loads direct root `.md` files with skill
|
||||
* frontmatter, honors ignore files, and returns diagnostics for invalid declared skill files. Missing input
|
||||
* directories are skipped.
|
||||
*/
|
||||
export async function loadSkills(
|
||||
env: ExecutionEnv,
|
||||
|
|
@ -246,6 +247,11 @@ async function loadSkillFromFile(
|
|||
parentDirName: string,
|
||||
): Promise<{ skill: Skill | null; diagnostics: SkillDiagnostic[] }> {
|
||||
const diagnostics: SkillDiagnostic[] = [];
|
||||
const isDeclaredSkill =
|
||||
filePath
|
||||
.replace(/[\\/]+$/, "")
|
||||
.split(/[\\/]/)
|
||||
.pop() === "SKILL.md";
|
||||
const rawContent = await env.readTextFile(filePath);
|
||||
if (!rawContent.ok) {
|
||||
diagnostics.push({ type: "warning", code: "read_failed", message: rawContent.error.message, path: filePath });
|
||||
|
|
@ -254,12 +260,17 @@ async function loadSkillFromFile(
|
|||
|
||||
const parsed = parseFrontmatter<SkillFrontmatter>(rawContent.value);
|
||||
if (!parsed.ok) {
|
||||
diagnostics.push({ type: "warning", code: "parse_failed", message: parsed.error.message, path: filePath });
|
||||
if (isDeclaredSkill) {
|
||||
diagnostics.push({ type: "warning", code: "parse_failed", message: parsed.error.message, path: filePath });
|
||||
}
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
const { frontmatter, body } = parsed.value;
|
||||
const description = typeof frontmatter.description === "string" ? frontmatter.description : undefined;
|
||||
if (!isDeclaredSkill && (!description || description.trim() === "")) {
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
for (const error of validateDescription(description)) {
|
||||
diagnostics.push({ type: "warning", code: "invalid_metadata", message: error, path: filePath });
|
||||
|
|
|
|||
|
|
@ -113,4 +113,23 @@ Use this skill.
|
|||
expect(skills.map((skill) => skill.name)).toEqual(["skills"]);
|
||||
expect(skills[0]?.content).toBe("Root content");
|
||||
});
|
||||
|
||||
it("ignores root markdown docs that do not declare skills", async () => {
|
||||
const root = createTempDir();
|
||||
const env = new NodeExecutionEnv({ cwd: root });
|
||||
await env.createDir("skills/nested-skill", { recursive: true });
|
||||
await env.writeFile("skills/README.md", "# Shared skills\n\nDocumentation.");
|
||||
await env.writeFile("skills/AGENTS.md", "# Agent notes\n\nDocumentation.");
|
||||
await env.writeFile("skills/CLAUDE.md", "---\ndescription: [invalid\n---\n\nDocumentation.");
|
||||
await env.writeFile("skills/root.md", "---\ndescription: Root skill\n---\nRoot content");
|
||||
await env.writeFile(
|
||||
"skills/nested-skill/SKILL.md",
|
||||
"---\nname: nested-skill\ndescription: Nested skill\n---\nNested content",
|
||||
);
|
||||
|
||||
const { skills, diagnostics } = await loadSkills(env, "skills");
|
||||
|
||||
expect(diagnostics).toEqual([]);
|
||||
expect(skills.map((skill) => skill.name).sort()).toEqual(["nested-skill", "skills"]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Fixed root Markdown files such as `README.md` and `AGENTS.md` in skill directories being reported as broken skills unless they declare valid skill frontmatter ([#7805](https://github.com/earendil-works/pi/issues/7805)).
|
||||
- Fixed single-object `edit` tool inputs failing validation by accepting them as one-edit arrays in both coding-agent and harness edit tools ([#7835](https://github.com/earendil-works/pi/issues/7835)).
|
||||
- Fixed managed-tool downloads delaying TUI startup and hiding diagnostics in fullscreen mode by mounting the TUI first and showing download progress and warnings inside it.
|
||||
- Fixed opening a model selector immediately after startup cancelling and restarting the in-progress model catalog refresh.
|
||||
|
|
|
|||
|
|
@ -34,9 +34,10 @@ Pi loads skills from:
|
|||
- CLI: `--skill <path>` (repeatable, additive even with `--no-skills`)
|
||||
|
||||
Discovery rules:
|
||||
- In `~/.pi/agent/skills/` and `.pi/skills/`, direct root `.md` files are discovered as individual skills
|
||||
- In `~/.pi/agent/skills/` and `.pi/skills/`, direct root `.md` files are discovered as individual skills when they have valid skill frontmatter with a non-empty `description`
|
||||
- In all skill locations, directories containing `SKILL.md` are discovered recursively
|
||||
- In `~/.agents/skills/` and project `.agents/skills/`, root `.md` files are ignored
|
||||
- Root Markdown files other than `SKILL.md` that do not look like skills are ignored silently
|
||||
|
||||
Disable discovery with `--no-skills` (explicit `--skill` paths still load).
|
||||
|
||||
|
|
@ -183,7 +184,7 @@ Pi validates skills against the Agent Skills standard. Most issues produce warni
|
|||
|
||||
Unknown frontmatter fields are ignored.
|
||||
|
||||
**Exception:** Skills with missing description are not loaded.
|
||||
Declared skills with missing descriptions are not loaded. Malformed `SKILL.md` files and `SKILL.md` files without a description produce warnings and are not loaded. Other Markdown files without valid skill frontmatter are ignored.
|
||||
|
||||
Name collisions (same name from different locations) warn and keep the first skill found.
|
||||
|
||||
|
|
|
|||
|
|
@ -114,10 +114,10 @@ function validateName(name: string): string[] {
|
|||
/**
|
||||
* Validate description per Agent Skills spec.
|
||||
*/
|
||||
function validateDescription(description: string | undefined): string[] {
|
||||
function validateDescription(description: unknown): string[] {
|
||||
const errors: string[] = [];
|
||||
|
||||
if (!description || description.trim() === "") {
|
||||
if (typeof description !== "string" || description.trim() === "") {
|
||||
errors.push("description is required");
|
||||
} else if (description.length > MAX_DESCRIPTION_LENGTH) {
|
||||
errors.push(`description exceeds ${MAX_DESCRIPTION_LENGTH} characters (${description.length})`);
|
||||
|
|
@ -279,49 +279,69 @@ function loadSkillFromFile(
|
|||
source: string,
|
||||
): { skill: Skill | null; diagnostics: ResourceDiagnostic[] } {
|
||||
const diagnostics: ResourceDiagnostic[] = [];
|
||||
const isDeclaredSkill = basename(filePath) === "SKILL.md";
|
||||
|
||||
let rawContent: string;
|
||||
try {
|
||||
const rawContent = readFileSync(filePath, "utf-8");
|
||||
const { frontmatter } = parseFrontmatter<SkillFrontmatter>(rawContent);
|
||||
const skillDir = dirname(filePath);
|
||||
const parentDirName = basename(skillDir);
|
||||
|
||||
// Validate description
|
||||
const descErrors = validateDescription(frontmatter.description);
|
||||
for (const error of descErrors) {
|
||||
diagnostics.push({ type: "warning", message: error, path: filePath });
|
||||
}
|
||||
|
||||
// Use name from frontmatter, or fall back to parent directory name
|
||||
const name = frontmatter.name || parentDirName;
|
||||
|
||||
// Validate name
|
||||
const nameErrors = validateName(name);
|
||||
for (const error of nameErrors) {
|
||||
diagnostics.push({ type: "warning", message: error, path: filePath });
|
||||
}
|
||||
|
||||
// Still load the skill even with warnings (unless description is completely missing)
|
||||
if (!frontmatter.description || frontmatter.description.trim() === "") {
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
return {
|
||||
skill: {
|
||||
name,
|
||||
description: frontmatter.description,
|
||||
filePath,
|
||||
baseDir: skillDir,
|
||||
sourceInfo: createSkillSourceInfo(filePath, skillDir, source),
|
||||
disableModelInvocation: frontmatter["disable-model-invocation"] === true,
|
||||
},
|
||||
diagnostics,
|
||||
};
|
||||
rawContent = readFileSync(filePath, "utf-8");
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : "failed to parse skill file";
|
||||
const message = error instanceof Error ? error.message : "failed to read skill file";
|
||||
diagnostics.push({ type: "warning", message, path: filePath });
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
let frontmatter: SkillFrontmatter;
|
||||
try {
|
||||
({ frontmatter } = parseFrontmatter<SkillFrontmatter>(rawContent));
|
||||
} catch (error) {
|
||||
if (isDeclaredSkill) {
|
||||
const message = error instanceof Error ? error.message : "failed to parse skill file";
|
||||
diagnostics.push({ type: "warning", message, path: filePath });
|
||||
}
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
const description = frontmatter.description;
|
||||
const hasDescription = typeof description === "string" && description.trim() !== "";
|
||||
if (!isDeclaredSkill && !hasDescription) {
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
const skillDir = dirname(filePath);
|
||||
const parentDirName = basename(skillDir);
|
||||
|
||||
// Validate description
|
||||
const descErrors = validateDescription(description);
|
||||
for (const error of descErrors) {
|
||||
diagnostics.push({ type: "warning", message: error, path: filePath });
|
||||
}
|
||||
|
||||
// Use name from frontmatter, or fall back to parent directory name
|
||||
const frontmatterName = typeof frontmatter.name === "string" ? frontmatter.name : undefined;
|
||||
const name = frontmatterName || parentDirName;
|
||||
|
||||
// Validate name
|
||||
const nameErrors = validateName(name);
|
||||
for (const error of nameErrors) {
|
||||
diagnostics.push({ type: "warning", message: error, path: filePath });
|
||||
}
|
||||
|
||||
// Still load the skill even with warnings, unless description is missing or empty.
|
||||
if (!hasDescription) {
|
||||
return { skill: null, diagnostics };
|
||||
}
|
||||
|
||||
return {
|
||||
skill: {
|
||||
name,
|
||||
description,
|
||||
filePath,
|
||||
baseDir: skillDir,
|
||||
sourceInfo: createSkillSourceInfo(filePath, skillDir, source),
|
||||
disableModelInvocation: frontmatter["disable-model-invocation"] === true,
|
||||
},
|
||||
diagnostics,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue