Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions cli/commands/skills/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@ import { validateSkillDirectory } from "./validate.ts";
async function withTempSkill(
files: Record<string, string>,
fn: (dir: string) => Promise<void>,
skillDirName = "test-skill",
): Promise<void> {
const dir = await Deno.makeTempDir({ prefix: "vf-skill-validate-" });
const rootDir = await Deno.makeTempDir({ prefix: "vf-skill-validate-" });
const dir = join(rootDir, skillDirName);
try {
await Deno.mkdir(dir, { recursive: true });
for (const [path, content] of Object.entries(files)) {
const target = join(dir, path);
await Deno.mkdir(join(target, ".."), { recursive: true });
await Deno.writeTextFile(target, content);
}
await fn(dir);
} finally {
await Deno.remove(dir, { recursive: true });
await Deno.remove(rootDir, { recursive: true });
}
}

Expand All @@ -37,7 +40,7 @@ Review the submitted changes.
}, async (dir) => {
const issues = await validateSkillDirectory(dir);
assertEquals(issues, []);
});
}, "code-review");
});

it("reports a missing SKILL.md", async () => {
Expand All @@ -60,8 +63,26 @@ description: Invalid name.
const issues = await validateSkillDirectory(dir);
assertEquals(issues.length, 1);
assertEquals(issues[0]?.severity, "error");
assertEquals(issues[0]?.message.includes("Invalid skill name"), true);
});
assertEquals(issues[0]?.message.includes('Invalid skill name "BadName"'), true);
}, "bad-name");
});

it("reports SKILL.md frontmatter name mismatch with directory", async () => {
await withTempSkill({
"SKILL.md": `---
name: email
description: Mismatched name.
---

# Email
`,
}, async (dir) => {
const issues = await validateSkillDirectory(dir);
assertEquals(issues, [{
severity: "error",
message: 'Skill name "email" does not match directory name "process-email"',
}]);
}, "process-email");
});

it("warns when SKILL.md has no instruction body", async () => {
Expand All @@ -74,6 +95,6 @@ description: Empty instruction body.
}, async (dir) => {
const issues = await validateSkillDirectory(dir);
assertEquals(issues, [{ severity: "warning", message: "SKILL.md body is empty" }]);
});
}, "empty-body");
});
});
24 changes: 22 additions & 2 deletions cli/commands/skills/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { createSuccessEnvelope, isJsonMode, outputJson } from "../../shared/json
import { exitProcess, logError, logSuccess, logWarning } from "#cli/utils";
import { createFileSystem } from "veryfront/platform";
import { basename } from "#std/path.ts";
import { parseSkillFrontmatter, validateSkillMetadata } from "veryfront/skill";
import { parseSkillFrontmatter, SKILL_NAME_REGEX, validateSkillMetadata } from "veryfront/skill";

interface ValidationIssue {
severity: "error" | "warning";
Expand All @@ -36,7 +36,9 @@ export async function validateSkillDirectory(dir: string): Promise<ValidationIss

try {
const parsed = await parseSkillFrontmatter(content);
validateSkillMetadata(parsed.frontmatter, basename(dir));
const directoryName = basename(dir);
validateCanonicalFrontmatterName(parsed.frontmatter, directoryName);
validateSkillMetadata(parsed.frontmatter, directoryName);
if (!parsed.body.trim()) {
issues.push({ severity: "warning", message: "SKILL.md body is empty" });
}
Expand All @@ -48,6 +50,24 @@ export async function validateSkillDirectory(dir: string): Promise<ValidationIss
return issues;
}

function validateCanonicalFrontmatterName(
frontmatter: Record<string, unknown>,
directoryName: string,
): void {
if (typeof frontmatter.name !== "string") return;

const name = frontmatter.name.trim();
if (!SKILL_NAME_REGEX.test(name)) {
throw new Error(
`Invalid skill name "${name}": must be lowercase alphanumeric with hyphens, 1-64 characters`,
);
}

if (name !== directoryName) {
throw new Error(`Skill name "${name}" does not match directory name "${directoryName}"`);
}
}

async function outputResults(
dir: string,
issues: ValidationIssue[],
Expand Down
1 change: 0 additions & 1 deletion scripts/lint/test-typecheck-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"src/cache/registry.test.ts",
"src/chat/upload-handler.test.ts",
"src/config/env.test.ts",
"src/discovery/agent-scoped-capabilities.test.ts",
"src/embedding/chunk.test.ts",
"src/embedding/rag-store.test.ts",
"src/errors/middleware/wrap-unknown.test.ts",
Expand Down
30 changes: 30 additions & 0 deletions src/agent/runtime/project-skill-catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,36 @@ Deno.test("catalog includes colocated skills with owner metadata and source path
assertEquals(own?.sourcePath, "agents/researcher/SKILL.md");
});

Deno.test("catalog accepts provider-safe colocated skill ids for dotted agent ids", async () => {
const { catalog } = createSkillCatalog({
paths: [
"agents/a.b/AGENT.md",
"agents/a.b/skills/x_y/SKILL.md",
"agents/a.b/skills/x_y/references/styles.md",
],
contentsByPath: {
"agents/a.b/skills/x_y/SKILL.md": `---
name: X Y
description: Owned underscore helper
metadata:
display_name: X Y
---
Use X Y.
`,
},
});

const skills = await catalog();
assertEquals(skills.map((skill) => skill.id), ["a_b--x_y"]);
const nested = skills[0];
assertEquals(nested?.name, "a_b--x_y");
assertEquals(nested?.displayName, "X Y");
assertEquals(nested?.ownerAgentId, "a.b");
assertEquals(nested?.shortName, "x_y");
assertEquals(nested?.sourcePath, "agents/a.b/skills/x_y/SKILL.md");
assertEquals(nested?.references, ["references/styles.md"]);
});

Deno.test("catalog keeps global skills unowned and carries their source paths", async () => {
const { catalog } = createSkillCatalog({
paths: ["skills/gmail/SKILL.md"],
Expand Down
69 changes: 66 additions & 3 deletions src/agent/runtime/skill-metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@ Deno.test("parseRuntimeSkillMetadata parses valid frontmatter", () => {
const content = `---
name: My Skill
description: A useful skill
metadata:
display_name: My Display Skill
tier: project
---
Body content here`;
const metadata = parseRuntimeSkillMetadata(content);
assertExists(metadata);
assertEquals(metadata.name, "My Skill");
assertEquals(metadata.metadata, { display_name: "My Display Skill", tier: "project" });
assertEquals(metadata.description, "A useful skill");
});

Expand All @@ -33,22 +37,79 @@ Deno.test("parseRuntimeSkillMetadata returns empty metadata for empty content",
assertEquals(metadata.name, undefined);
});

Deno.test("buildRuntimeSkillDefinition builds a skill definition from valid content", () => {
Deno.test("buildRuntimeSkillDefinition builds a canonical skill definition from valid content", () => {
const content = `---
name: Code Review
name: code-review
description: Reviews code quality
metadata:
display_name: Code Review
---
# Code Review Skill
Review the code for quality issues.`;

const skill = buildRuntimeSkillDefinition({ id: "code-review", content });
assertExists(skill);
assertEquals(skill.id, "code-review");
assertEquals(skill.name, "Code Review");
assertEquals(skill.name, "code-review");
assertEquals(skill.displayName, "Code Review");
assertEquals(skill.description, "Reviews code quality");
assertEquals(skill.metadata, { display_name: "Code Review" });
assertEquals(skill.instructions, content);
});

Deno.test("buildRuntimeSkillDefinition recovers a legacy display-style frontmatter name", () => {
const content = `---
name: Process Email
description: Process email
---
Body`;
const skill = buildRuntimeSkillDefinition({ id: "process-email", content });
assertExists(skill);
assertEquals(skill.id, "process-email");
assertEquals(skill.name, "process-email");
assertEquals(skill.displayName, "Process Email");
});

Deno.test("buildRuntimeSkillDefinition rejects invalid canonical ids", () => {
const errors: Array<Record<string, unknown> | undefined> = [];
const skill = buildRuntimeSkillDefinition({
id: "Process Email",
content: `---
description: Process email
---
Body`,
logger: {
error: (_message, metadata) => errors.push(metadata),
},
});

assertEquals(skill, null);
assertEquals(errors[0]?.id, "Process Email");
});

Deno.test("buildRuntimeSkillDefinition accepts provider-safe owned namespaced ids", () => {
const content = `---
name: x_y
description: Owned helper
metadata:
display_name: Owned Helper
---
Body`;
const skill = buildRuntimeSkillDefinition({
id: "a_b--x_y",
content,
ownerAgentId: "a.b",
shortName: "x_y",
});

assertExists(skill);
assertEquals(skill.id, "a_b--x_y");
assertEquals(skill.name, "a_b--x_y");
assertEquals(skill.displayName, "Owned Helper");
assertEquals(skill.ownerAgentId, "a.b");
assertEquals(skill.shortName, "x_y");
});

Deno.test("buildRuntimeSkillDefinition uses id as fallback name", () => {
const content = `---
description: A skill
Expand All @@ -65,6 +126,8 @@ name: Test
# This is the heading
Some body text`;
const skill = buildRuntimeSkillDefinition({ id: "test", content });
assertEquals(skill?.name, "test");
assertEquals(skill?.displayName, "Test");
assertEquals(skill?.description, "This is the heading");
});

Expand Down
Loading