fix: quote scoped package names in skill frontmatter and update validator (#79)
- Wrapped unquoted @scope/pkg values in double quotes across 19 SKILL.md files. - Added 'package' to ALLOWED_FIELDS in JS validator. - Added YAML validity regression test to test suite. - Updated package-lock.json. Fixes #79 Closes #80
This commit is contained in:
@@ -1,11 +1,11 @@
|
||||
const assert = require('assert');
|
||||
const { hasUseSection } = require('../validate-skills');
|
||||
const assert = require("assert");
|
||||
const { hasUseSection } = require("../validate-skills");
|
||||
|
||||
const samples = [
|
||||
['## When to Use', true],
|
||||
['## Use this skill when', true],
|
||||
['## When to Use This Skill', true],
|
||||
['## Overview', false],
|
||||
["## When to Use", true],
|
||||
["## Use this skill when", true],
|
||||
["## When to Use This Skill", true],
|
||||
["## Overview", false],
|
||||
];
|
||||
|
||||
for (const [heading, expected] of samples) {
|
||||
@@ -13,4 +13,31 @@ for (const [heading, expected] of samples) {
|
||||
assert.strictEqual(hasUseSection(content), expected, heading);
|
||||
}
|
||||
|
||||
console.log('ok');
|
||||
// Regression test for YAML validity in frontmatter (Issue #79)
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
const { listSkillIds, parseFrontmatter } = require("../../lib/skill-utils");
|
||||
|
||||
const SKILLS_DIR = path.join(__dirname, "../../skills");
|
||||
const skillIds = listSkillIds(SKILLS_DIR);
|
||||
|
||||
console.log(`Checking YAML validity for ${skillIds.length} skills...`);
|
||||
|
||||
for (const skillId of skillIds) {
|
||||
const skillPath = path.join(SKILLS_DIR, skillId, "SKILL.md");
|
||||
const content = fs.readFileSync(skillPath, "utf8");
|
||||
const { errors, hasFrontmatter } = parseFrontmatter(content);
|
||||
|
||||
if (!hasFrontmatter) {
|
||||
console.warn(`[WARN] No frontmatter in ${skillId}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
assert.strictEqual(
|
||||
errors.length,
|
||||
0,
|
||||
`YAML parse errors in ${skillId}: ${errors.join(", ")}`,
|
||||
);
|
||||
}
|
||||
|
||||
console.log("ok");
|
||||
|
||||
@@ -2,13 +2,13 @@
|
||||
* Legacy / alternative validator. For CI and PR checks, use scripts/validate_skills.py.
|
||||
* Run: npm run validate (or npm run validate:strict)
|
||||
*/
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { listSkillIds, parseFrontmatter } = require('../lib/skill-utils');
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
const { listSkillIds, parseFrontmatter } = require("../lib/skill-utils");
|
||||
|
||||
const ROOT = path.resolve(__dirname, '..');
|
||||
const SKILLS_DIR = path.join(ROOT, 'skills');
|
||||
const BASELINE_PATH = path.join(ROOT, 'validation-baseline.json');
|
||||
const ROOT = path.resolve(__dirname, "..");
|
||||
const SKILLS_DIR = path.join(ROOT, "skills");
|
||||
const BASELINE_PATH = path.join(ROOT, "validation-baseline.json");
|
||||
|
||||
const errors = [];
|
||||
const warnings = [];
|
||||
@@ -17,12 +17,14 @@ const missingDoNotUseSection = [];
|
||||
const missingInstructionsSection = [];
|
||||
const longFiles = [];
|
||||
const unknownFieldSkills = [];
|
||||
const isStrict = process.argv.includes('--strict')
|
||||
|| process.env.STRICT === '1'
|
||||
|| process.env.STRICT === 'true';
|
||||
const writeBaseline = process.argv.includes('--write-baseline')
|
||||
|| process.env.WRITE_BASELINE === '1'
|
||||
|| process.env.WRITE_BASELINE === 'true';
|
||||
const isStrict =
|
||||
process.argv.includes("--strict") ||
|
||||
process.env.STRICT === "1" ||
|
||||
process.env.STRICT === "true";
|
||||
const writeBaseline =
|
||||
process.argv.includes("--write-baseline") ||
|
||||
process.env.WRITE_BASELINE === "1" ||
|
||||
process.env.WRITE_BASELINE === "true";
|
||||
|
||||
const NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/;
|
||||
const MAX_NAME_LENGTH = 64;
|
||||
@@ -30,14 +32,15 @@ const MAX_DESCRIPTION_LENGTH = 1024;
|
||||
const MAX_COMPATIBILITY_LENGTH = 500;
|
||||
const MAX_SKILL_LINES = 500;
|
||||
const ALLOWED_FIELDS = new Set([
|
||||
'name',
|
||||
'description',
|
||||
'risk',
|
||||
'source',
|
||||
'license',
|
||||
'compatibility',
|
||||
'metadata',
|
||||
'allowed-tools',
|
||||
"name",
|
||||
"description",
|
||||
"risk",
|
||||
"source",
|
||||
"license",
|
||||
"compatibility",
|
||||
"metadata",
|
||||
"allowed-tools",
|
||||
"package",
|
||||
]);
|
||||
|
||||
const USE_SECTION_PATTERNS = [
|
||||
@@ -47,15 +50,19 @@ const USE_SECTION_PATTERNS = [
|
||||
];
|
||||
|
||||
function hasUseSection(content) {
|
||||
return USE_SECTION_PATTERNS.some(pattern => pattern.test(content));
|
||||
return USE_SECTION_PATTERNS.some((pattern) => pattern.test(content));
|
||||
}
|
||||
|
||||
function isPlainObject(value) {
|
||||
return value && typeof value === 'object' && !Array.isArray(value);
|
||||
return value && typeof value === "object" && !Array.isArray(value);
|
||||
}
|
||||
|
||||
function validateStringField(fieldName, value, { min = 1, max = Infinity } = {}) {
|
||||
if (typeof value !== 'string') {
|
||||
function validateStringField(
|
||||
fieldName,
|
||||
value,
|
||||
{ min = 1, max = Infinity } = {},
|
||||
) {
|
||||
if (typeof value !== "string") {
|
||||
return `${fieldName} must be a string.`;
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
@@ -90,24 +97,37 @@ function loadBaseline() {
|
||||
}
|
||||
|
||||
try {
|
||||
const parsed = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8'));
|
||||
const parsed = JSON.parse(fs.readFileSync(BASELINE_PATH, "utf8"));
|
||||
return {
|
||||
useSection: Array.isArray(parsed.useSection) ? parsed.useSection : [],
|
||||
doNotUseSection: Array.isArray(parsed.doNotUseSection) ? parsed.doNotUseSection : [],
|
||||
instructionsSection: Array.isArray(parsed.instructionsSection) ? parsed.instructionsSection : [],
|
||||
doNotUseSection: Array.isArray(parsed.doNotUseSection)
|
||||
? parsed.doNotUseSection
|
||||
: [],
|
||||
instructionsSection: Array.isArray(parsed.instructionsSection)
|
||||
? parsed.instructionsSection
|
||||
: [],
|
||||
longFile: Array.isArray(parsed.longFile) ? parsed.longFile : [],
|
||||
};
|
||||
} catch (err) {
|
||||
addWarning('Failed to parse validation-baseline.json; strict mode may fail.');
|
||||
return { useSection: [], doNotUseSection: [], instructionsSection: [], longFile: [] };
|
||||
addWarning(
|
||||
"Failed to parse validation-baseline.json; strict mode may fail.",
|
||||
);
|
||||
return {
|
||||
useSection: [],
|
||||
doNotUseSection: [],
|
||||
instructionsSection: [],
|
||||
longFile: [],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
function addStrictSectionErrors(label, missing, baselineSet) {
|
||||
if (!isStrict) return;
|
||||
const strictMissing = missing.filter(skillId => !baselineSet.has(skillId));
|
||||
const strictMissing = missing.filter((skillId) => !baselineSet.has(skillId));
|
||||
if (strictMissing.length) {
|
||||
addError(`Missing "${label}" section (strict): ${strictMissing.length} skills (examples: ${strictMissing.slice(0, 5).join(', ')})`);
|
||||
addError(
|
||||
`Missing "${label}" section (strict): ${strictMissing.length} skills (examples: ${strictMissing.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,15 +140,19 @@ function run() {
|
||||
const baselineLongFile = new Set(baseline.longFile || []);
|
||||
|
||||
for (const skillId of skillIds) {
|
||||
const skillPath = path.join(SKILLS_DIR, skillId, 'SKILL.md');
|
||||
const skillPath = path.join(SKILLS_DIR, skillId, "SKILL.md");
|
||||
|
||||
if (!fs.existsSync(skillPath)) {
|
||||
addError(`Missing SKILL.md: ${skillId}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const content = fs.readFileSync(skillPath, 'utf8');
|
||||
const { data, errors: fmErrors, hasFrontmatter } = parseFrontmatter(content);
|
||||
const content = fs.readFileSync(skillPath, "utf8");
|
||||
const {
|
||||
data,
|
||||
errors: fmErrors,
|
||||
hasFrontmatter,
|
||||
} = parseFrontmatter(content);
|
||||
const lineCount = content.split(/\r?\n/).length;
|
||||
|
||||
if (!hasFrontmatter) {
|
||||
@@ -136,7 +160,9 @@ function run() {
|
||||
}
|
||||
|
||||
if (fmErrors && fmErrors.length) {
|
||||
fmErrors.forEach(error => addError(`Frontmatter parse error (${skillId}): ${error}`));
|
||||
fmErrors.forEach((error) =>
|
||||
addError(`Frontmatter parse error (${skillId}): ${error}`),
|
||||
);
|
||||
}
|
||||
|
||||
if (!NAME_PATTERN.test(skillId)) {
|
||||
@@ -144,7 +170,10 @@ function run() {
|
||||
}
|
||||
|
||||
if (data.name !== undefined) {
|
||||
const nameError = validateStringField('name', data.name, { min: 1, max: MAX_NAME_LENGTH });
|
||||
const nameError = validateStringField("name", data.name, {
|
||||
min: 1,
|
||||
max: MAX_NAME_LENGTH,
|
||||
});
|
||||
if (nameError) {
|
||||
addError(`${nameError} (${skillId})`);
|
||||
} else {
|
||||
@@ -158,15 +187,22 @@ function run() {
|
||||
}
|
||||
}
|
||||
|
||||
const descError = data.description === undefined
|
||||
? 'description is required.'
|
||||
: validateStringField('description', data.description, { min: 1, max: MAX_DESCRIPTION_LENGTH });
|
||||
const descError =
|
||||
data.description === undefined
|
||||
? "description is required."
|
||||
: validateStringField("description", data.description, {
|
||||
min: 1,
|
||||
max: MAX_DESCRIPTION_LENGTH,
|
||||
});
|
||||
if (descError) {
|
||||
addError(`${descError} (${skillId})`);
|
||||
}
|
||||
|
||||
if (data.license !== undefined) {
|
||||
const licenseError = validateStringField('license', data.license, { min: 1, max: 128 });
|
||||
const licenseError = validateStringField("license", data.license, {
|
||||
min: 1,
|
||||
max: 128,
|
||||
});
|
||||
if (licenseError) {
|
||||
addError(`${licenseError} (${skillId})`);
|
||||
}
|
||||
@@ -174,7 +210,7 @@ function run() {
|
||||
|
||||
if (data.compatibility !== undefined) {
|
||||
const compatibilityError = validateStringField(
|
||||
'compatibility',
|
||||
"compatibility",
|
||||
data.compatibility,
|
||||
{ min: 1, max: MAX_COMPATIBILITY_LENGTH },
|
||||
);
|
||||
@@ -183,10 +219,12 @@ function run() {
|
||||
}
|
||||
}
|
||||
|
||||
if (data['allowed-tools'] !== undefined) {
|
||||
if (typeof data['allowed-tools'] !== 'string') {
|
||||
addError(`allowed-tools must be a space-delimited string. (${skillId})`);
|
||||
} else if (!data['allowed-tools'].trim()) {
|
||||
if (data["allowed-tools"] !== undefined) {
|
||||
if (typeof data["allowed-tools"] !== "string") {
|
||||
addError(
|
||||
`allowed-tools must be a space-delimited string. (${skillId})`,
|
||||
);
|
||||
} else if (!data["allowed-tools"].trim()) {
|
||||
addError(`allowed-tools cannot be empty. (${skillId})`);
|
||||
}
|
||||
}
|
||||
@@ -196,7 +234,7 @@ function run() {
|
||||
addError(`metadata must be a string map/object. (${skillId})`);
|
||||
} else {
|
||||
for (const [key, value] of Object.entries(data.metadata)) {
|
||||
if (typeof value !== 'string') {
|
||||
if (typeof value !== "string") {
|
||||
addError(`metadata.${key} must be a string. (${skillId})`);
|
||||
}
|
||||
}
|
||||
@@ -204,10 +242,14 @@ function run() {
|
||||
}
|
||||
|
||||
if (data && Object.keys(data).length) {
|
||||
const unknownFields = Object.keys(data).filter(key => !ALLOWED_FIELDS.has(key));
|
||||
const unknownFields = Object.keys(data).filter(
|
||||
(key) => !ALLOWED_FIELDS.has(key),
|
||||
);
|
||||
if (unknownFields.length) {
|
||||
unknownFieldSkills.push(skillId);
|
||||
addError(`Unknown frontmatter fields (${skillId}): ${unknownFields.join(', ')}`);
|
||||
addError(
|
||||
`Unknown frontmatter fields (${skillId}): ${unknownFields.join(", ")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,39 +261,61 @@ function run() {
|
||||
missingUseSection.push(skillId);
|
||||
}
|
||||
|
||||
if (!content.includes('## Do not use')) {
|
||||
if (!content.includes("## Do not use")) {
|
||||
missingDoNotUseSection.push(skillId);
|
||||
}
|
||||
|
||||
if (!content.includes('## Instructions')) {
|
||||
if (!content.includes("## Instructions")) {
|
||||
missingInstructionsSection.push(skillId);
|
||||
}
|
||||
}
|
||||
|
||||
if (missingUseSection.length) {
|
||||
addWarning(`Missing "Use this skill when" section: ${missingUseSection.length} skills (examples: ${missingUseSection.slice(0, 5).join(', ')})`);
|
||||
addWarning(
|
||||
`Missing "Use this skill when" section: ${missingUseSection.length} skills (examples: ${missingUseSection.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
|
||||
if (missingDoNotUseSection.length) {
|
||||
addWarning(`Missing "Do not use" section: ${missingDoNotUseSection.length} skills (examples: ${missingDoNotUseSection.slice(0, 5).join(', ')})`);
|
||||
addWarning(
|
||||
`Missing "Do not use" section: ${missingDoNotUseSection.length} skills (examples: ${missingDoNotUseSection.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
|
||||
if (missingInstructionsSection.length) {
|
||||
addWarning(`Missing "Instructions" section: ${missingInstructionsSection.length} skills (examples: ${missingInstructionsSection.slice(0, 5).join(', ')})`);
|
||||
addWarning(
|
||||
`Missing "Instructions" section: ${missingInstructionsSection.length} skills (examples: ${missingInstructionsSection.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
|
||||
if (longFiles.length) {
|
||||
addWarning(`SKILL.md over ${MAX_SKILL_LINES} lines: ${longFiles.length} skills (examples: ${longFiles.slice(0, 5).join(', ')})`);
|
||||
addWarning(
|
||||
`SKILL.md over ${MAX_SKILL_LINES} lines: ${longFiles.length} skills (examples: ${longFiles.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
|
||||
if (unknownFieldSkills.length) {
|
||||
addWarning(`Unknown frontmatter fields detected: ${unknownFieldSkills.length} skills (examples: ${unknownFieldSkills.slice(0, 5).join(', ')})`);
|
||||
addWarning(
|
||||
`Unknown frontmatter fields detected: ${unknownFieldSkills.length} skills (examples: ${unknownFieldSkills.slice(0, 5).join(", ")})`,
|
||||
);
|
||||
}
|
||||
|
||||
addStrictSectionErrors('Use this skill when', missingUseSection, baselineUse);
|
||||
addStrictSectionErrors('Do not use', missingDoNotUseSection, baselineDoNotUse);
|
||||
addStrictSectionErrors('Instructions', missingInstructionsSection, baselineInstructions);
|
||||
addStrictSectionErrors(`SKILL.md line count <= ${MAX_SKILL_LINES}`, longFiles, baselineLongFile);
|
||||
addStrictSectionErrors("Use this skill when", missingUseSection, baselineUse);
|
||||
addStrictSectionErrors(
|
||||
"Do not use",
|
||||
missingDoNotUseSection,
|
||||
baselineDoNotUse,
|
||||
);
|
||||
addStrictSectionErrors(
|
||||
"Instructions",
|
||||
missingInstructionsSection,
|
||||
baselineInstructions,
|
||||
);
|
||||
addStrictSectionErrors(
|
||||
`SKILL.md line count <= ${MAX_SKILL_LINES}`,
|
||||
longFiles,
|
||||
baselineLongFile,
|
||||
);
|
||||
|
||||
if (writeBaseline) {
|
||||
const baselineData = {
|
||||
@@ -266,14 +330,14 @@ function run() {
|
||||
}
|
||||
|
||||
if (warnings.length) {
|
||||
console.warn('Warnings:');
|
||||
console.warn("Warnings:");
|
||||
for (const warning of warnings) {
|
||||
console.warn(`- ${warning}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (errors.length) {
|
||||
console.error('\nErrors:');
|
||||
console.error("\nErrors:");
|
||||
for (const error of errors) {
|
||||
console.error(`- ${error}`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user