ci: add lint-skill-entry structural validator for skill contributions

.github/scripts/lint-skill-entry.mjs

@@ -0,0 +1,152 @@
+#!/usr/bin/env node
+//
+// Structural validator for skill contributions.
+//
+// Reuses the installer's own parser (bin/metamask-skills.mjs collectSkills /
+// parseFrontmatter) so that it validates exactly what ships, rather than a
+// parallel model. Errors block; warnings advise. Exits non-zero on any error.
+//
+// Run against the repo:        node .github/scripts/lint-skill-entry.mjs
+// Run against another tree:     SKILLS_LINT_ROOT=/path node .github/scripts/lint-skill-entry.mjs
+
+import { readFileSync, readdirSync } from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+import { collectSkills, parseFrontmatter } from '../../bin/metamask-skills.mjs';
+import {
+  ALLOWED_SIBLING_DIRS,
+  DESCRIPTION_MAX,
+  INSTALLED_PREFIX,
+  KNOWN_FRONTMATTER,
+  KNOWN_REPOS,
+  MATURITY_VALUES,
+  NAME_PATTERN,
+  RECOMMENDED_SECTIONS,
+} from '../../tools/skill-schema.mjs';
+
+const ROOT = process.env.SKILLS_LINT_ROOT
+  ? path.resolve(process.env.SKILLS_LINT_ROOT)
+  : path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
+
+const allowedSiblings = new Set(ALLOWED_SIBLING_DIRS);
+const TRUTHY = new Set(['1', 'true', 'yes', 'on']);
+
+export function lintSkill(skill) {
+  const errors = [];
+  const warnings = [];
+  const dirName = skill.id.slice(skill.domain.length + 1);
+
+  let raw;
+  try {
+    raw = parseFrontmatter(readFileSync(path.join(skill.path, 'skill.md'), 'utf8'));
+  } catch (error) {
+    return { errors: [`could not read skill.md: ${error.message}`], warnings };
+  }
+
+  if (!raw.name) {
+    errors.push('missing required `name` in frontmatter');
+  } else {
+    if (raw.name !== dirName) {
+      errors.push(`\`name\` "${raw.name}" must match the directory "${dirName}"`);
+    }
+    if (!NAME_PATTERN.test(raw.name)) {
+      errors.push(`\`name\` "${raw.name}" must be kebab-case`);
+    }
+    if (raw.name.startsWith(INSTALLED_PREFIX)) {
+      errors.push(`source \`name\` must not carry the \`${INSTALLED_PREFIX}\` prefix; the installer adds it`);
+    }
+  }
+
+  if (!raw.description) {
+    errors.push('missing required `description` in frontmatter');
+  } else if (raw.description.length > DESCRIPTION_MAX) {
+    errors.push(`\`description\` is ${raw.description.length} chars, over the ${DESCRIPTION_MAX}-char operator ceiling`);
+  }
+
+  if (raw.maturity && !MATURITY_VALUES.includes(raw.maturity)) {
+    errors.push(`\`maturity\` "${raw.maturity}" must be one of: ${MATURITY_VALUES.join(', ')}`);
+  }
+
+  // On-demand-only contract: a source skill must not force persistent loading.
+  if (raw.alwaysApply !== undefined && TRUTHY.has(String(raw.alwaysApply).toLowerCase())) {
+    errors.push('skills are on-demand only; remove `alwaysApply: true` (always-on guidance belongs in AGENTS.md)');
+  }
+
+  // Sibling directories: bundle dirs and repos/ only. knowledge/ is rejected.
+  let entries = [];
+  try {
+    entries = readdirSync(skill.path, { withFileTypes: true });
+  } catch {
+    // skill dir vanished mid-run; nothing to check
+  }
+  for (const entry of entries) {
+    if (entry.isDirectory() && !allowedSiblings.has(entry.name)) {
+      errors.push(`unexpected directory "${entry.name}/" beside skill.md (allowed: ${[...allowedSiblings].join(', ')}); domain knowledge belongs in references/`);
+    }
+  }
+
+  for (const repo of skill.repos) {
+    if (!KNOWN_REPOS.includes(repo)) {
+      warnings.push(`repos/${repo}.md targets an unknown consumer (known: ${KNOWN_REPOS.join(', ')})`);
+    }
+  }
+
+  for (const key of Object.keys(raw)) {
+    if (!KNOWN_FRONTMATTER.includes(key) && key !== 'alwaysApply') {
+      warnings.push(`unknown frontmatter key "${key}"; operators silently ignore unrecognised keys (typo?)`);
+    }
+  }
+
+  for (const section of RECOMMENDED_SECTIONS) {
+    if (!new RegExp(`^#{1,4}\\s+${section}\\b`, 'imu').test(skill.body)) {
+      warnings.push(`missing recommended section "## ${section}"`);
+    }
+  }
+
+  return { errors, warnings };
+}
+
+// Restrict to skills touched by the given file paths (the CI gate passes the
+// PR's changed files, so pre-existing drift in untouched skills never blocks an
+// unrelated change). With no paths, every skill is linted (a full audit).
+function skillsForPaths(skills, paths) {
+  const resolved = paths.map((file) => path.resolve(ROOT, file));
+  return skills.filter((skill) =>
+    resolved.some((file) => file === skill.path || file.startsWith(`${skill.path}${path.sep}`)),
+  );
+}
+
+function main() {
+  const paths = process.argv.slice(2).filter((arg) => !arg.startsWith('-'));
+  const all = collectSkills([ROOT]);
+  const skills = paths.length > 0 ? skillsForPaths(all, paths) : all;
+  let errorCount = 0;
+  let warningCount = 0;
+
+  for (const skill of skills) {
+    const { errors, warnings } = lintSkill(skill);
+    if (errors.length > 0 || warnings.length > 0) {
+      console.log(`\n${skill.id}`);
+      for (const message of errors) {
+        console.log(`  error:   ${message}`);
+      }
+      for (const message of warnings) {
+        console.log(`  warning: ${message}`);
+      }
+    }
+    errorCount += errors.length;
+    warningCount += warnings.length;
+  }
+
+  console.log(`\n${skills.length} skill(s) checked, ${errorCount} error(s), ${warningCount} warning(s).`);
+  // Set exitCode rather than process.exit() so buffered stdout flushes when it
+  // is a pipe (e.g. under CI or execFileSync), instead of being truncated.
+  process.exitCode = errorCount > 0 ? 1 : 0;
+}
+
+const invokedDirectly =
+  process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url);
+if (invokedDirectly) {
+  main();
+}

.github/workflows/lint-skill-entry.yml

@@ -0,0 +1,33 @@
+name: Lint skill entries
+
+on:
+  pull_request:
+    paths:
+      - 'domains/**'
+      - 'tools/**'
+      - '.github/scripts/lint-skill-entry.mjs'
+      - '.github/workflows/lint-skill-entry.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  lint-skills:
+    name: Lint skill entries
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 24.x
+
+      - name: Get changed skill files
+        id: changed
+        uses: tj-actions/changed-files@v45
+        with:
+          files: domains/**
+
+      - name: Lint changed skills
+        if: steps.changed.outputs.any_changed == 'true'
+        run: node .github/scripts/lint-skill-entry.mjs ${{ steps.changed.outputs.all_changed_files }}

package.json

@@ -21,7 +21,8 @@
     "test": "node --test test/*.test.mjs",
     "pack:dry-run": "yarn pack --dry-run",
     "lint": "yarn lint:changelog",
-    "lint:changelog": "auto-changelog validate --formatter oxfmt"
+    "lint:changelog": "auto-changelog validate --formatter oxfmt",
+    "lint:skills": "node .github/scripts/lint-skill-entry.mjs"
   },
   "publishConfig": {
     "access": "public",

test/lint-skill-entry.test.mjs

@@ -0,0 +1,110 @@
+import assert from 'node:assert/strict';
+import { spawnSync } from 'node:child_process';
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { afterEach, describe, test } from 'node:test';
+
+const LINTER = path.resolve(
+  path.dirname(fileURLToPath(import.meta.url)),
+  '..',
+  '.github',
+  'scripts',
+  'lint-skill-entry.mjs',
+);
+
+const roots = [];
+
+function makeRoot() {
+  const root = mkdtempSync(path.join(os.tmpdir(), 'skill-lint-'));
+  roots.push(root);
+  return root;
+}
+
+afterEach(() => {
+  while (roots.length > 0) {
+    rmSync(roots.pop(), { recursive: true, force: true });
+  }
+});
+
+function writeSkill(root, domain, name, frontmatter, body) {
+  const dir = path.join(root, 'domains', domain, 'skills', name);
+  mkdirSync(dir, { recursive: true });
+  const defaultBody = '## When To Use\n\n- always\n\n## Workflow\n\n1. do the thing\n';
+  writeFileSync(path.join(dir, 'skill.md'), `---\n${frontmatter}\n---\n\n${body ?? defaultBody}`);
+  return dir;
+}
+
+function lint(root) {
+  const result = spawnSync(process.execPath, [LINTER], {
+    env: { ...process.env, SKILLS_LINT_ROOT: root },
+    encoding: 'utf8',
+  });
+  return { code: result.status, output: `${result.stdout}${result.stderr}` };
+}
+
+describe('lint-skill-entry', () => {
+  test('a well-formed skill passes', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: Write unit tests');
+    assert.equal(lint(root).code, 0);
+  });
+
+  test('missing name fails', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', 'description: x');
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /missing required `name`/u);
+  });
+
+  test('name not matching the directory fails', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', 'name: wrong-name\ndescription: x');
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /must match the directory/u);
+  });
+
+  test('a knowledge/ sibling directory fails (the conversion guarantee)', () => {
+    const root = makeRoot();
+    const dir = writeSkill(root, 'perps', 'fix-bug', 'name: fix-bug\ndescription: x');
+    mkdirSync(path.join(dir, 'knowledge'));
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /knowledge/u);
+  });
+
+  test('an mms- prefix in the source name fails', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'mms-unit', 'name: mms-unit\ndescription: x');
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /prefix/u);
+  });
+
+  test('a description over the operator ceiling fails', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', `name: unit-testing\ndescription: ${'x'.repeat(1100)}`);
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /operator ceiling/u);
+  });
+
+  test('an invalid maturity value fails', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: x\nmaturity: beta');
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /maturity/u);
+  });
+
+  test('alwaysApply: true fails (on-demand-only contract)', () => {
+    const root = makeRoot();
+    writeSkill(root, 'testing', 'unit-testing', 'name: unit-testing\ndescription: x\nalwaysApply: true');
+    const { code, output } = lint(root);
+    assert.equal(code, 1);
+    assert.match(output, /on-demand/u);
+  });
+});

tools/skill-schema.mjs

@@ -0,0 +1,36 @@
+// Single source of truth for the skill contribution schema.
+//
+// Imported by the lint-skill-entry validator so that the documented schema and
+// the enforced schema cannot drift apart. The installer's bundle-directory list
+// in tools/install (Bash) mirrors BUNDLE_DIRS; keep the two in sync.
+
+export const REQUIRED_FRONTMATTER = ['name', 'description'];
+export const OPTIONAL_FRONTMATTER = ['maturity', 'mandatory', 'scope'];
+export const KNOWN_FRONTMATTER = [...REQUIRED_FRONTMATTER, ...OPTIONAL_FRONTMATTER];
+
+export const MATURITY_VALUES = ['experimental', 'stable', 'deprecated'];
+
+// Directories the installer copies alongside skill.md (see tools/install).
+export const BUNDLE_DIRS = ['references', 'scripts', 'assets', 'adapters'];
+
+// Directories allowed beside skill.md: the bundle dirs plus the repo-overlay
+// dir. Anything else (notably knowledge/) is rejected, because the installer
+// does not ship it and the reference would dangle post-install.
+export const ALLOWED_SIBLING_DIRS = [...BUNDLE_DIRS, 'repos'];
+
+export const KNOWN_REPOS = ['metamask-extension', 'metamask-mobile', 'core'];
+
+// The description is always loaded into the operator's discovery surface, so it
+// is the per-skill always-on cost. The ceiling is the per-operator minimum
+// (OpenCode caps description at 1024), so a description that passes here is
+// accepted by every target.
+export const DESCRIPTION_MAX = 1024;
+
+export const RECOMMENDED_SECTIONS = ['When To Use', 'Workflow'];
+
+// kebab-case, matching the name regex Claude Code and OpenCode require.
+export const NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/u;
+
+// The installer prepends this to generated output names; source names must not
+// carry it.
+export const INSTALLED_PREFIX = 'mms-';