Skip to content

Commit be70ac4

Browse files
committed
ci: add lint-skill-entry structural validator for skill contributions
Adds a CI gate that validates skill contributions against the documented schema, reusing the installer's own parser (collectSkills / parseFrontmatter) so it checks what actually ships rather than a parallel model. - tools/skill-schema.mjs: single source for the frontmatter fields, maturity vocabulary, bundle-dir allowlist, description ceiling, and name pattern. - .github/scripts/lint-skill-entry.mjs: errors on layout/schema violations, warns on advisory issues. On-demand-only contract bans alwaysApply, and the sibling-dir allowlist rejects knowledge/. - .github/workflows/lint-skill-entry.yml: gates PRs on changed skills only, so pre-existing drift never blocks an unrelated change. - test/lint-skill-entry.test.mjs: fixture tests for pass and each violation.
1 parent d23becf commit be70ac4

5 files changed

Lines changed: 333 additions & 1 deletion

File tree

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
#!/usr/bin/env node
2+
//
3+
// Structural validator for skill contributions.
4+
//
5+
// Reuses the installer's own parser (bin/metamask-skills.mjs collectSkills /
6+
// parseFrontmatter) so that it validates exactly what ships, rather than a
7+
// parallel model. Errors block; warnings advise. Exits non-zero on any error.
8+
//
9+
// Run against the repo: node .github/scripts/lint-skill-entry.mjs
10+
// Run against another tree: SKILLS_LINT_ROOT=/path node .github/scripts/lint-skill-entry.mjs
11+
12+
import { readFileSync, readdirSync } from 'node:fs';
13+
import path from 'node:path';
14+
import { fileURLToPath } from 'node:url';
15+
16+
import { collectSkills, parseFrontmatter } from '../../bin/metamask-skills.mjs';
17+
import {
18+
ALLOWED_SIBLING_DIRS,
19+
DESCRIPTION_MAX,
20+
INSTALLED_PREFIX,
21+
KNOWN_FRONTMATTER,
22+
KNOWN_REPOS,
23+
MATURITY_VALUES,
24+
NAME_PATTERN,
25+
RECOMMENDED_SECTIONS,
26+
} from '../../tools/skill-schema.mjs';
27+
28+
const ROOT = process.env.SKILLS_LINT_ROOT
29+
? path.resolve(process.env.SKILLS_LINT_ROOT)
30+
: path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
31+
32+
const allowedSiblings = new Set(ALLOWED_SIBLING_DIRS);
33+
const TRUTHY = new Set(['1', 'true', 'yes', 'on']);
34+
35+
export function lintSkill(skill) {
36+
const errors = [];
37+
const warnings = [];
38+
const dirName = skill.id.slice(skill.domain.length + 1);
39+
40+
let raw;
41+
try {
42+
raw = parseFrontmatter(readFileSync(path.join(skill.path, 'skill.md'), 'utf8'));
43+
} catch (error) {
44+
return { errors: [`could not read skill.md: ${error.message}`], warnings };
45+
}
46+
47+
if (!raw.name) {
48+
errors.push('missing required `name` in frontmatter');
49+
} else {
50+
if (raw.name !== dirName) {
51+
errors.push(`\`name\` "${raw.name}" must match the directory "${dirName}"`);
52+
}
53+
if (!NAME_PATTERN.test(raw.name)) {
54+
errors.push(`\`name\` "${raw.name}" must be kebab-case`);
55+
}
56+
if (raw.name.startsWith(INSTALLED_PREFIX)) {
57+
errors.push(`source \`name\` must not carry the \`${INSTALLED_PREFIX}\` prefix; the installer adds it`);
58+
}
59+
}
60+
61+
if (!raw.description) {
62+
errors.push('missing required `description` in frontmatter');
63+
} else if (raw.description.length > DESCRIPTION_MAX) {
64+
errors.push(`\`description\` is ${raw.description.length} chars, over the ${DESCRIPTION_MAX}-char operator ceiling`);
65+
}
66+
67+
if (raw.maturity && !MATURITY_VALUES.includes(raw.maturity)) {
68+
errors.push(`\`maturity\` "${raw.maturity}" must be one of: ${MATURITY_VALUES.join(', ')}`);
69+
}
70+
71+
// On-demand-only contract: a source skill must not force persistent loading.
72+
if (raw.alwaysApply !== undefined && TRUTHY.has(String(raw.alwaysApply).toLowerCase())) {
73+
errors.push('skills are on-demand only; remove `alwaysApply: true` (always-on guidance belongs in AGENTS.md)');
74+
}
75+
76+
// Sibling directories: bundle dirs and repos/ only. knowledge/ is rejected.
77+
let entries = [];
78+
try {
79+
entries = readdirSync(skill.path, { withFileTypes: true });
80+
} catch {
81+
// skill dir vanished mid-run; nothing to check
82+
}
83+
for (const entry of entries) {
84+
if (entry.isDirectory() && !allowedSiblings.has(entry.name)) {
85+
errors.push(`unexpected directory "${entry.name}/" beside skill.md (allowed: ${[...allowedSiblings].join(', ')}); domain knowledge belongs in references/`);
86+
}
87+
}
88+
89+
for (const repo of skill.repos) {
90+
if (!KNOWN_REPOS.includes(repo)) {
91+
warnings.push(`repos/${repo}.md targets an unknown consumer (known: ${KNOWN_REPOS.join(', ')})`);
92+
}
93+
}
94+
95+
for (const key of Object.keys(raw)) {
96+
if (!KNOWN_FRONTMATTER.includes(key) && key !== 'alwaysApply') {
97+
warnings.push(`unknown frontmatter key "${key}"; operators silently ignore unrecognised keys (typo?)`);
98+
}
99+
}
100+
101+
for (const section of RECOMMENDED_SECTIONS) {
102+
if (!new RegExp(`^#{1,4}\\s+${section}\\b`, 'imu').test(skill.body)) {
103+
warnings.push(`missing recommended section "## ${section}"`);
104+
}
105+
}
106+
107+
return { errors, warnings };
108+
}
109+
110+
// Restrict to skills touched by the given file paths (the CI gate passes the
111+
// PR's changed files, so pre-existing drift in untouched skills never blocks an
112+
// unrelated change). With no paths, every skill is linted (a full audit).
113+
function skillsForPaths(skills, paths) {
114+
const resolved = paths.map((file) => path.resolve(ROOT, file));
115+
return skills.filter((skill) =>
116+
resolved.some((file) => file === skill.path || file.startsWith(`${skill.path}${path.sep}`)),
117+
);
118+
}
119+
120+
function main() {
121+
const paths = process.argv.slice(2).filter((arg) => !arg.startsWith('-'));
122+
const all = collectSkills([ROOT]);
123+
const skills = paths.length > 0 ? skillsForPaths(all, paths) : all;
124+
let errorCount = 0;
125+
let warningCount = 0;
126+
127+
for (const skill of skills) {
128+
const { errors, warnings } = lintSkill(skill);
129+
if (errors.length > 0 || warnings.length > 0) {
130+
console.log(`\n${skill.id}`);
131+
for (const message of errors) {
132+
console.log(` error: ${message}`);
133+
}
134+
for (const message of warnings) {
135+
console.log(` warning: ${message}`);
136+
}
137+
}
138+
errorCount += errors.length;
139+
warningCount += warnings.length;
140+
}
141+
142+
console.log(`\n${skills.length} skill(s) checked, ${errorCount} error(s), ${warningCount} warning(s).`);
143+
// Set exitCode rather than process.exit() so buffered stdout flushes when it
144+
// is a pipe (e.g. under CI or execFileSync), instead of being truncated.
145+
process.exitCode = errorCount > 0 ? 1 : 0;
146+
}
147+
148+
const invokedDirectly =
149+
process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url);
150+
if (invokedDirectly) {
151+
main();
152+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: Lint skill entries
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- 'domains/**'
7+
- 'tools/**'
8+
- '.github/scripts/lint-skill-entry.mjs'
9+
- '.github/workflows/lint-skill-entry.yml'
10+
11+
permissions:
12+
contents: read
13+
14+
jobs:
15+
lint-skills:
16+
name: Lint skill entries
17+
runs-on: ubuntu-latest
18+
steps:
19+
- uses: actions/checkout@v4
20+
21+
- uses: actions/setup-node@v4
22+
with:
23+
node-version: 24.x
24+
25+
- name: Get changed skill files
26+
id: changed
27+
uses: tj-actions/changed-files@v45
28+
with:
29+
files: domains/**
30+
31+
- name: Lint changed skills
32+
if: steps.changed.outputs.any_changed == 'true'
33+
run: node .github/scripts/lint-skill-entry.mjs ${{ steps.changed.outputs.all_changed_files }}

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
"test": "node --test test/*.test.mjs",
2222
"pack:dry-run": "yarn pack --dry-run",
2323
"lint": "yarn lint:changelog",
24-
"lint:changelog": "auto-changelog validate --formatter oxfmt"
24+
"lint:changelog": "auto-changelog validate --formatter oxfmt",
25+
"lint:skills": "node .github/scripts/lint-skill-entry.mjs"
2526
},
2627
"publishConfig": {
2728
"access": "public",

test/lint-skill-entry.test.mjs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import assert from 'node:assert/strict';
2+
import { spawnSync } from 'node:child_process';
3+
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
4+
import os from 'node:os';
5+
import path from 'node:path';
6+
import { fileURLToPath } from 'node:url';
7+
import { afterEach, describe, test } from 'node:test';
8+
9+
const LINTER = path.resolve(
10+
path.dirname(fileURLToPath(import.meta.url)),
11+
'..',
12+
'.github',
13+
'scripts',
14+
'lint-skill-entry.mjs',
15+
);
16+
17+
const roots = [];
18+
19+
function makeRoot() {
20+
const root = mkdtempSync(path.join(os.tmpdir(), 'skill-lint-'));
21+
roots.push(root);
22+
return root;
23+
}
24+
25+
afterEach(() => {
26+
while (roots.length > 0) {
27+
rmSync(roots.pop(), { recursive: true, force: true });
28+
}
29+
});
30+
31+
function writeSkill(root, domain, name, frontmatter, body) {
32+
const dir = path.join(root, 'domains', domain, 'skills', name);
33+
mkdirSync(dir, { recursive: true });
34+
const defaultBody = '## When To Use\n\n- always\n\n## Workflow\n\n1. do the thing\n';
35+
writeFileSync(path.join(dir, 'skill.md'), `---\n${frontmatter}\n---\n\n${body ?? defaultBody}`);
36+
return dir;
37+
}
38+
39+
function lint(root) {
40+
const result = spawnSync(process.execPath, [LINTER], {
41+
env: { ...process.env, SKILLS_LINT_ROOT: root },
42+
encoding: 'utf8',
43+
});
44+
return { code: result.status, output: `${result.stdout}${result.stderr}` };
45+
}
46+
47+
describe('lint-skill-entry', () => {
48+
test('a well-formed skill passes', () => {
49+
const root = makeRoot();
50+
writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: Write unit tests');
51+
assert.equal(lint(root).code, 0);
52+
});
53+
54+
test('missing name fails', () => {
55+
const root = makeRoot();
56+
writeSkill(root, 'testing', 'unit-testing', 'description: x');
57+
const { code, output } = lint(root);
58+
assert.equal(code, 1);
59+
assert.match(output, /missing required `name`/u);
60+
});
61+
62+
test('name not matching the directory fails', () => {
63+
const root = makeRoot();
64+
writeSkill(root, 'testing', 'unit-testing', 'name: wrong-name\ndescription: x');
65+
const { code, output } = lint(root);
66+
assert.equal(code, 1);
67+
assert.match(output, /must match the directory/u);
68+
});
69+
70+
test('a knowledge/ sibling directory fails (the conversion guarantee)', () => {
71+
const root = makeRoot();
72+
const dir = writeSkill(root, 'perps', 'fix-bug', 'name: fix-bug\ndescription: x');
73+
mkdirSync(path.join(dir, 'knowledge'));
74+
const { code, output } = lint(root);
75+
assert.equal(code, 1);
76+
assert.match(output, /knowledge/u);
77+
});
78+
79+
test('an mms- prefix in the source name fails', () => {
80+
const root = makeRoot();
81+
writeSkill(root, 'testing', 'mms-unit', 'name: mms-unit\ndescription: x');
82+
const { code, output } = lint(root);
83+
assert.equal(code, 1);
84+
assert.match(output, /prefix/u);
85+
});
86+
87+
test('a description over the operator ceiling fails', () => {
88+
const root = makeRoot();
89+
writeSkill(root, 'testing', 'unit-testing', `name: unit-testing\ndescription: ${'x'.repeat(1100)}`);
90+
const { code, output } = lint(root);
91+
assert.equal(code, 1);
92+
assert.match(output, /operator ceiling/u);
93+
});
94+
95+
test('an invalid maturity value fails', () => {
96+
const root = makeRoot();
97+
writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: x\nmaturity: beta');
98+
const { code, output } = lint(root);
99+
assert.equal(code, 1);
100+
assert.match(output, /maturity/u);
101+
});
102+
103+
test('alwaysApply: true fails (on-demand-only contract)', () => {
104+
const root = makeRoot();
105+
writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: x\nalwaysApply: true');
106+
const { code, output } = lint(root);
107+
assert.equal(code, 1);
108+
assert.match(output, /on-demand/u);
109+
});
110+
});

tools/skill-schema.mjs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Single source of truth for the skill contribution schema.
2+
//
3+
// Imported by the lint-skill-entry validator so that the documented schema and
4+
// the enforced schema cannot drift apart. The installer's bundle-directory list
5+
// in tools/install (Bash) mirrors BUNDLE_DIRS; keep the two in sync.
6+
7+
export const REQUIRED_FRONTMATTER = ['name', 'description'];
8+
export const OPTIONAL_FRONTMATTER = ['maturity', 'mandatory', 'scope'];
9+
export const KNOWN_FRONTMATTER = [...REQUIRED_FRONTMATTER, ...OPTIONAL_FRONTMATTER];
10+
11+
export const MATURITY_VALUES = ['experimental', 'stable', 'deprecated'];
12+
13+
// Directories the installer copies alongside skill.md (see tools/install).
14+
export const BUNDLE_DIRS = ['references', 'scripts', 'assets', 'adapters'];
15+
16+
// Directories allowed beside skill.md: the bundle dirs plus the repo-overlay
17+
// dir. Anything else (notably knowledge/) is rejected, because the installer
18+
// does not ship it and the reference would dangle post-install.
19+
export const ALLOWED_SIBLING_DIRS = [...BUNDLE_DIRS, 'repos'];
20+
21+
export const KNOWN_REPOS = ['metamask-extension', 'metamask-mobile', 'core'];
22+
23+
// The description is always loaded into the operator's discovery surface, so it
24+
// is the per-skill always-on cost. The ceiling is the per-operator minimum
25+
// (OpenCode caps description at 1024), so a description that passes here is
26+
// accepted by every target.
27+
export const DESCRIPTION_MAX = 1024;
28+
29+
export const RECOMMENDED_SECTIONS = ['When To Use', 'Workflow'];
30+
31+
// kebab-case, matching the name regex Claude Code and OpenCode require.
32+
export const NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/u;
33+
34+
// The installer prepends this to generated output names; source names must not
35+
// carry it.
36+
export const INSTALLED_PREFIX = 'mms-';

0 commit comments

Comments
 (0)