From 0895f0d8b5e5bde7aa53174063bc013d26610486 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 16:09:19 +0000 Subject: [PATCH] feat(ci): lint skills for Codex-safe frontmatter and Claude substitutions Add scripts/ci/validate-skill-host-compat.js and run it from the Validate Components job and npm test after validate-skills.js. The check fails on Claude-only substitutions in bash fences and on Codex copies that use keys outside name, description, metadata, license, and allowed-tools. CONTRIBUTING checklist items warn unless --strict-checklist. Also add /skill-host-compat and skills/skill-host-compat with a Codex copy that drops origin. Move configure-ecc plugin-root examples into prose so the live catalog stays green without a skip list. Co-authored-by: Matt Van Horn --- .agents/skills/skill-host-compat/SKILL.md | 97 ++++ .../skill-host-compat/agents/openai.yaml | 7 + .github/workflows/ci.yml | 4 + .github/workflows/reusable-validate.yml | 3 + COMMANDS-QUICK-REF.md | 1 + README.md | 2 + agent.yaml | 2 + commands/skill-host-compat.md | 40 ++ docs/COMMAND-AGENT-MAP.md | 2 + docs/COMMAND-REGISTRY.json | 15 +- manifests/install-modules.json | 5 +- package.json | 5 +- scripts/ci/validate-skill-host-compat.js | 479 +++++++++++++++++ scripts/ci/validate-skills.js | 17 +- skills/configure-ecc/SKILL.md | 24 +- skills/skill-host-compat/SKILL.md | 98 ++++ tests/ci/validate-skill-host-compat.test.js | 494 ++++++++++++++++++ tests/scripts/npm-publish-surface.test.js | 3 + 18 files changed, 1272 insertions(+), 26 deletions(-) create mode 100644 .agents/skills/skill-host-compat/SKILL.md create mode 100644 .agents/skills/skill-host-compat/agents/openai.yaml create mode 100644 commands/skill-host-compat.md create mode 100644 scripts/ci/validate-skill-host-compat.js create mode 100644 skills/skill-host-compat/SKILL.md create mode 100644 tests/ci/validate-skill-host-compat.test.js diff --git a/.agents/skills/skill-host-compat/SKILL.md b/.agents/skills/skill-host-compat/SKILL.md new file mode 100644 index 000000000..6597dad54 --- /dev/null +++ b/.agents/skills/skill-host-compat/SKILL.md @@ -0,0 +1,97 @@ +--- +name: skill-host-compat +description: Lint ECC skills for Codex/Cursor-safe frontmatter and Claude-only substitutions before a skill PR. +--- + +# Skill Host Compatibility + +Run the same host-compat gate CI uses on `skills/` before opening a skill PR. +The check is for Codex and Cursor copies, not Claude-only runtime quality. + +## When to Activate + +- Adding or editing a skill under `skills/` +- Copying a skill into `.agents/skills/` or `.cursor/skills/` +- A review asks whether a skill is safe to install into Codex or Cursor +- CI reports `validate-skill-host-compat.js` findings + +## Core Concepts + +Canonical skills live in `skills/`. Codex loads `.agents/skills/`. Cursor loads a +smaller `.cursor/skills/` subset. CONTRIBUTING documents that sync as manual. + +`validate-skills.js` only checks that `SKILL.md` exists and that `name` / +`description` are usable YAML. `tests/ci/codex-skill-surface.test.js` only +sees `.agents/skills/`. This script is the cross-tree gate: + +1. Claude-only substitutions in executable `bash` / `sh` / `zsh` fences + (`$CLAUDE_SESSION_ID`, `$CLAUDE_SKILL_DIR`, `$CLAUDE_PLUGIN_ROOT`, + `$ARGUMENTS`) fail. Codex leaves those tokens as literal text. +2. A Codex copy may only use `name`, `description`, `metadata`, `license`, + and `allowed-tools`. `origin: ECC` may stay on the canonical `skills/` + copy and must be stripped on the Codex copy. +3. A skill listed in `agents/openai.yaml` that has a `skills/` directory must + have `.agents/skills//`. Cursor copies are optional; when present, + `name:` must match the directory. +4. CONTRIBUTING checklist items (When to Activate, Anti-Patterns, name + matches directory, 500/800 line caps) warn. They fail only with + `--strict-checklist`. + +`validate-skills.js` frontmatter WARN default is unchanged (#1663). + +## Code Examples + +From the repo root: + +```bash +node scripts/ci/validate-skill-host-compat.js +``` + +Print counts without failing: + +```bash +node scripts/ci/validate-skill-host-compat.js --inventory +``` + +Promote checklist warnings: + +```bash +node scripts/ci/validate-skill-host-compat.js --strict-checklist +``` + +Slash command: + +``` +$skill-host-compat +$skill-host-compat --inventory +``` + +## Anti-Patterns + +Do not put Claude-only substitutions in bash blocks. This fence is the +forbidden pattern: + +```bash +echo "$CLAUDE_SESSION_ID" +ls "$CLAUDE_SKILL_DIR" +node "$CLAUDE_PLUGIN_ROOT/scripts/setup.js" $ARGUMENTS +``` + +Do not add `version:` or `origin:` to a Codex copy under `.agents/skills/`. +Do not treat a passing `validate-skills.js` run as Codex-safe. Do not require +every `skills/` entry to have a Cursor copy. + +## Best Practices + +- Keep canonical `origin: ECC` if the CONTRIBUTING template calls for it. +- Drop `origin`, `version`, and `argument-hint` on the Codex copy. +- Show host-portable commands in bash fences (`npx`, `ecc`, or a resolved + root). Mention Claude env vars in prose, not in executable fences. +- Run this script before `npm test` when the change is a new skill. +- Use `--inventory` to see checklist warning volume without failing the tree. + +## Related Skills + +- `skill-comply` — runtime behavioral traces, not host-portability +- `skill-stocktake` — holistic audit of installed `~/.claude/skills` +- `skill-scout` — search existing skills before creating a new one diff --git a/.agents/skills/skill-host-compat/agents/openai.yaml b/.agents/skills/skill-host-compat/agents/openai.yaml new file mode 100644 index 000000000..131be432c --- /dev/null +++ b/.agents/skills/skill-host-compat/agents/openai.yaml @@ -0,0 +1,7 @@ +interface: + display_name: "Skill Host Compat" + short_description: "Lint skills for Codex-safe frontmatter and substitutions" + brand_color: "#6366F1" + default_prompt: "Use $skill-host-compat to lint skills for Codex/Cursor-safe frontmatter and Claude-only substitutions." +policy: + allow_implicit_invocation: true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb0c531fd..38fdd1148 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,6 +211,10 @@ jobs: run: node scripts/ci/validate-skills.js continue-on-error: false + - name: Validate skill host compatibility + run: node scripts/ci/validate-skill-host-compat.js + continue-on-error: false + - name: Validate install manifests run: node scripts/ci/validate-install-manifests.js continue-on-error: false diff --git a/.github/workflows/reusable-validate.yml b/.github/workflows/reusable-validate.yml index 66e295e13..7ae56f044 100644 --- a/.github/workflows/reusable-validate.yml +++ b/.github/workflows/reusable-validate.yml @@ -41,6 +41,9 @@ jobs: - name: Validate skills run: node scripts/ci/validate-skills.js + - name: Validate skill host compatibility + run: node scripts/ci/validate-skill-host-compat.js + - name: Validate install manifests run: node scripts/ci/validate-install-manifests.js diff --git a/COMMANDS-QUICK-REF.md b/COMMANDS-QUICK-REF.md index 6a9fa22e2..36b3dde20 100644 --- a/COMMANDS-QUICK-REF.md +++ b/COMMANDS-QUICK-REF.md @@ -180,6 +180,7 @@ These lifecycle commands are also available through the `ecc` CLI. | `/instinct-import` | Import instincts from a file or URL | | `/skill-create` | Analyse local git history → generate a reusable skill | | `/skill-health` | Skill portfolio health dashboard with analytics | +| `/skill-host-compat` | Lint skills for Codex-safe frontmatter and Claude substitutions | --- diff --git a/README.md b/README.md index f9d202b28..bda9da5f1 100644 --- a/README.md +++ b/README.md @@ -912,6 +912,7 @@ ECC/ | |-- swift-protocol-di-testing/ # Protocol-based DI for testable Swift code | |-- search-first/ # Research-before-coding workflow | |-- skill-stocktake/ # Audit skills and commands for quality +| |-- skill-host-compat/ # Lint skills for Codex-safe frontmatter and substitutions | |-- liquid-glass-design/ # iOS 26 Liquid Glass design system | |-- foundation-models-on-device/ # Apple on-device LLM with FoundationModels | |-- swift-concurrency-6-2/ # Swift 6.2 Approachable Concurrency @@ -938,6 +939,7 @@ ECC/ | |-- go-test.md # /go-test - Go TDD workflow | |-- go-build.md # /go-build - Fix Go build errors | |-- skill-create.md # /skill-create - Generate skills from git history +| |-- skill-host-compat.md # /skill-host-compat - Lint skills for Codex/Cursor copies | |-- instinct-status.md # /instinct-status - View learned instincts | |-- instinct-import.md # /instinct-import - Import instincts | |-- instinct-export.md # /instinct-export - Export instincts diff --git a/agent.yaml b/agent.yaml index 4236f04cc..9c83e8e9f 100644 --- a/agent.yaml +++ b/agent.yaml @@ -142,6 +142,7 @@ skills: - security-review - security-scan - skill-comply + - skill-host-compat - skill-stocktake - springboot-patterns - springboot-security @@ -255,6 +256,7 @@ commands: - setup-pm - skill-create - skill-health + - skill-host-compat - test-coverage - update-codemaps - update-docs diff --git a/commands/skill-host-compat.md b/commands/skill-host-compat.md new file mode 100644 index 000000000..f30604a24 --- /dev/null +++ b/commands/skill-host-compat.md @@ -0,0 +1,40 @@ +--- +name: skill-host-compat +description: Lint ECC skills for Codex/Cursor-safe frontmatter and Claude-only substitutions before a skill PR. +command: true +--- + +# Skill Host Compatibility + +Lint curated `skills/` for Codex-safe frontmatter and Claude-only substitutions. + +## Implementation + +Run the host-compat validator: + +```bash +ECC_ROOT="${CLAUDE_PLUGIN_ROOT:-$(node -e "var r=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i/SKILL.md + * (name, description, metadata, license, allowed-tools). origin/version + * may remain on the canonical skills/ copy. + * 3. Subset drift: agents/openai.yaml names a skill that is missing from + * .agents/skills/, or a Codex/Cursor copy name: does not match its + * directory. + * 4. CONTRIBUTING checklist (WARN unless --strict-checklist): When to + * Activate, Anti-Patterns, name matches directory, 500/800 line caps. + * + * Default invocation is fail-closed on (1)-(3). Pass --inventory to print + * JSON counts and exit 0. Do not flip validate-skills.js to --strict; + * that file's WARN default is load-bearing for #1663. + * + * Scope: curated skills/ plus Codex/Cursor copies. If skills/ is absent, + * exit 0 (nothing to validate). + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const { extractFrontmatter, inspectFrontmatter } = require('./validate-skills'); + +const DEFAULT_ROOT = path.join(__dirname, '../..'); + +const ALLOWED_CODEX_FRONTMATTER_KEYS = new Set([ + 'allowed-tools', + 'description', + 'license', + 'metadata', + 'name', +]); + +const CLAUDE_SUBSTITUTION_PATTERNS = [ + { token: '${CLAUDE_SESSION_ID}', regex: /\$\{CLAUDE_SESSION_ID\}/g }, + { token: '$CLAUDE_SESSION_ID', regex: /\$CLAUDE_SESSION_ID\b/g }, + { token: '${CLAUDE_SKILL_DIR}', regex: /\$\{CLAUDE_SKILL_DIR\}/g }, + { token: '$CLAUDE_SKILL_DIR', regex: /\$CLAUDE_SKILL_DIR\b/g }, + { token: '${CLAUDE_PLUGIN_ROOT}', regex: /\$\{CLAUDE_PLUGIN_ROOT\}/g }, + { token: '$CLAUDE_PLUGIN_ROOT', regex: /\$CLAUDE_PLUGIN_ROOT\b/g }, + { token: '$ARGUMENTS', regex: /\$ARGUMENTS\b|\$\{ARGUMENTS\}/g }, + // Bare $1 is a Perl/awk capture in many examples; flag the Claude skill + // form ${1} and $1 used as a shell positional, not `$1 `. + { token: '$1', regex: /\$\{1\}|(?=])/g }, +]; + +const EXECUTABLE_FENCE_LANGS = new Set(['bash', 'sh', 'zsh']); +const CHECKLIST_CODES = new Set([ + 'missing-when-to-activate', + 'missing-anti-patterns', + 'name-mismatch', + 'line-cap-500', + 'line-cap-800', +]); + +function parseArgs(argv) { + const args = { + root: DEFAULT_ROOT, + inventory: false, + strictChecklist: false, + }; + + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + if (arg === '--inventory') { + args.inventory = true; + } else if (arg === '--strict-checklist') { + args.strictChecklist = true; + } else if (arg === '--root') { + const value = argv[i + 1]; + if (!value) { + throw new Error('--root requires a directory path'); + } + args.root = value; + i += 1; + } + } + + return args; +} + +function listSkillDirs(dir) { + if (!fs.existsSync(dir)) return []; + return fs.readdirSync(dir, { withFileTypes: true }) + .filter(entry => entry.isDirectory() && !entry.name.startsWith('.')) + .map(entry => entry.name) + .sort(); +} + +function readFileIfPresent(filePath) { + if (!fs.existsSync(filePath)) return null; + return fs.readFileSync(filePath, 'utf8'); +} + +function unquoteYamlScalar(value) { + if (value === undefined || value === null) return ''; + const trimmed = String(value).trim(); + if ( + (trimmed.startsWith('"') && trimmed.endsWith('"')) + || (trimmed.startsWith("'") && trimmed.endsWith("'")) + ) { + return trimmed.slice(1, -1); + } + return trimmed; +} + +function extractTopLevelList(yamlSource, key) { + const lines = String(yamlSource).replace(/^\uFEFF/, '').split(/\r?\n/); + const results = []; + let collecting = false; + + for (const line of lines) { + if (!collecting) { + if (line.trim() === `${key}:`) { + collecting = true; + } + continue; + } + + if (/^[A-Za-z0-9_-]+:\s*/.test(line)) { + break; + } + + const match = line.match(/^\s*-\s+(.+?)\s*$/); + if (match) { + results.push(unquoteYamlScalar(match[1])); + } + } + + return results; +} + +function listOpenaiYamlSkills(root) { + const filePath = path.join(root, 'agents', 'openai.yaml'); + const source = readFileIfPresent(filePath); + if (source === null) return []; + return extractTopLevelList(source, 'skills'); +} + +function stripFrontmatter(content) { + const fm = extractFrontmatter(content); + if (!fm.present) { + return { frontmatter: fm, body: content.replace(/^\uFEFF/, ''), bodyOffset: 0 }; + } + const match = content.replace(/^\uFEFF/, '').match(/^---\r?\n[\s\S]*?\r?\n---(?:\r?\n|$)/); + const bodyOffset = match ? match[0].length : 0; + return { + frontmatter: fm, + body: content.replace(/^\uFEFF/, '').slice(bodyOffset), + bodyOffset, + }; +} + +function findExecutableFences(body) { + const fences = []; + const fenceRe = /^```([^\r\n]*)\r?\n([\s\S]*?)^```[ \t]*(?:\r?\n|$)/gm; + let match = fenceRe.exec(body); + while (match) { + const info = (match[1] || '').trim(); + const lang = info === '' ? '' : info.split(/\s+/)[0].toLowerCase(); + const content = match[2]; + const executable = EXECUTABLE_FENCE_LANGS.has(lang) + || (lang === '' && /^\s*#!/.test(content)); + if (executable) { + fences.push({ + lang: lang || 'shebang', + content, + start: match.index, + }); + } + match = fenceRe.exec(body); + } + return fences; +} + +function precedingAllowsToken(body, fenceStart) { + const prefix = body.slice(0, fenceStart).replace(/\s+$/, ''); + const lines = prefix.split(/\r?\n/); + const window = lines.slice(-3).join('\n'); + return /anti-pattern/i.test(window) || /do not/i.test(window); +} + +function findSubstitutionTokens(fenceContent) { + const found = []; + for (const pattern of CLAUDE_SUBSTITUTION_PATTERNS) { + pattern.regex.lastIndex = 0; + if (pattern.regex.test(fenceContent)) { + found.push(pattern.token); + } + } + return found; +} + +function countLines(content) { + if (content.length === 0) return 0; + return content.replace(/^\uFEFF/, '').split(/\r?\n/).length; +} + +function relSkillMd(skillName) { + return `${skillName}/SKILL.md`; +} + +function pushFinding(findings, finding) { + findings.push(finding); +} + +function inspectCanonicalSkill(skillName, content, findings) { + const { frontmatter, body } = stripFrontmatter(content); + const values = frontmatter.present ? inspectFrontmatter(frontmatter.lines).values : {}; + const declaredName = unquoteYamlScalar(values.name || ''); + const lineCount = countLines(content); + + if (declaredName && declaredName !== skillName) { + pushFinding(findings, { + severity: 'warn', + code: 'name-mismatch', + skill: skillName, + message: `${relSkillMd(skillName)} - frontmatter name '${declaredName}' does not match directory '${skillName}'`, + }); + } + + if (!/^##\s+When to Activate\b/m.test(body)) { + pushFinding(findings, { + severity: 'warn', + code: 'missing-when-to-activate', + skill: skillName, + message: `${relSkillMd(skillName)} - missing '## When to Activate' section`, + }); + } + + if (!/^##\s+Anti-Patterns\b/m.test(body)) { + pushFinding(findings, { + severity: 'warn', + code: 'missing-anti-patterns', + skill: skillName, + message: `${relSkillMd(skillName)} - missing '## Anti-Patterns' section`, + }); + } + + if (lineCount > 800) { + pushFinding(findings, { + severity: 'warn', + code: 'line-cap-800', + skill: skillName, + message: `${relSkillMd(skillName)} - body is ${lineCount} lines (CONTRIBUTING max 800)`, + }); + } else if (lineCount > 500) { + pushFinding(findings, { + severity: 'warn', + code: 'line-cap-500', + skill: skillName, + message: `${relSkillMd(skillName)} - body is ${lineCount} lines (CONTRIBUTING target 500)`, + }); + } + + for (const fence of findExecutableFences(body)) { + if (precedingAllowsToken(body, fence.start)) continue; + const tokens = findSubstitutionTokens(fence.content); + for (const token of tokens) { + pushFinding(findings, { + severity: 'error', + code: 'substitution', + skill: skillName, + token, + message: `${relSkillMd(skillName)} - ${fence.lang} fence contains Claude-only substitution ${token}`, + }); + } + } +} + +function inspectCopyName(skillName, content, treeLabel, findings) { + const { frontmatter } = stripFrontmatter(content); + if (!frontmatter.present) return; + const declaredName = unquoteYamlScalar(inspectFrontmatter(frontmatter.lines).values.name || ''); + if (declaredName && declaredName !== skillName) { + pushFinding(findings, { + severity: 'error', + code: 'copy-name-mismatch', + skill: skillName, + message: `${treeLabel}/${relSkillMd(skillName)} - frontmatter name '${declaredName}' does not match directory '${skillName}'`, + }); + } +} + +function inspectCodexCopy(skillName, content, findings) { + const { frontmatter } = stripFrontmatter(content); + inspectCopyName(skillName, content, '.agents/skills', findings); + if (!frontmatter.present) return; + + const keys = Object.keys(inspectFrontmatter(frontmatter.lines).values); + const unexpected = keys.filter(key => !ALLOWED_CODEX_FRONTMATTER_KEYS.has(key)); + if (unexpected.length > 0) { + pushFinding(findings, { + severity: 'error', + code: 'codex-key', + skill: skillName, + message: `.agents/skills/${relSkillMd(skillName)} - Codex-forbidden frontmatter key${unexpected.length === 1 ? '' : 's'}: ${unexpected.join(', ')}`, + }); + } +} + +function validateSkillHostCompat(options = {}) { + const root = options.root || DEFAULT_ROOT; + const inventory = Boolean(options.inventory); + const strictChecklist = Boolean(options.strictChecklist); + const skillsDir = path.join(root, 'skills'); + const codexDir = path.join(root, '.agents', 'skills'); + const cursorDir = path.join(root, '.cursor', 'skills'); + const findings = []; + + if (!fs.existsSync(skillsDir)) { + return { + ok: true, + skipped: true, + message: 'No curated skills directory (skills/), skipping', + findings: [], + counts: emptyCounts(), + }; + } + + const canonicalSkills = listSkillDirs(skillsDir); + const codexSkills = listSkillDirs(codexDir); + const cursorSkills = listSkillDirs(cursorDir); + const openaiListed = listOpenaiYamlSkills(root); + const skillsWithCanonical = new Set(canonicalSkills); + + for (const skillName of canonicalSkills) { + const skillMd = path.join(skillsDir, skillName, 'SKILL.md'); + const content = readFileIfPresent(skillMd); + if (content === null) continue; + inspectCanonicalSkill(skillName, content, findings); + } + + for (const skillName of openaiListed) { + if (!skillsWithCanonical.has(skillName)) continue; + const copyMd = path.join(codexDir, skillName, 'SKILL.md'); + if (!fs.existsSync(copyMd)) { + pushFinding(findings, { + severity: 'error', + code: 'missing-codex-copy', + skill: skillName, + message: `agents/openai.yaml lists '${skillName}' but .agents/skills/${skillName}/ is missing`, + }); + } + } + + for (const skillName of codexSkills) { + const copyMd = path.join(codexDir, skillName, 'SKILL.md'); + const content = readFileIfPresent(copyMd); + if (content === null) continue; + inspectCodexCopy(skillName, content, findings); + } + + for (const skillName of cursorSkills) { + const copyMd = path.join(cursorDir, skillName, 'SKILL.md'); + const content = readFileIfPresent(copyMd); + if (content === null) continue; + inspectCopyName(skillName, content, '.cursor/skills', findings); + } + + const resolved = findings.map(finding => { + if (strictChecklist && CHECKLIST_CODES.has(finding.code) && finding.severity === 'warn') { + return { ...finding, severity: 'error' }; + } + return finding; + }); + + const counts = summarizeCounts(canonicalSkills.length, resolved); + const errorCount = resolved.filter(finding => finding.severity === 'error').length; + + return { + ok: inventory || errorCount === 0, + skipped: false, + inventory, + findings: resolved, + counts, + message: inventory + ? JSON.stringify(counts) + : formatSummary(canonicalSkills.length, counts), + }; +} + +function emptyCounts() { + return { + skillsScanned: 0, + errors: 0, + warnings: 0, + substitutions: 0, + codexKeys: 0, + missingCodexCopies: 0, + copyNameMismatches: 0, + checklistWarnings: 0, + }; +} + +function summarizeCounts(skillsScanned, findings) { + const counts = emptyCounts(); + counts.skillsScanned = skillsScanned; + for (const finding of findings) { + if (finding.severity === 'error') counts.errors += 1; + if (finding.severity === 'warn') counts.warnings += 1; + if (finding.code === 'substitution') counts.substitutions += 1; + if (finding.code === 'codex-key') counts.codexKeys += 1; + if (finding.code === 'missing-codex-copy') counts.missingCodexCopies += 1; + if (finding.code === 'copy-name-mismatch') counts.copyNameMismatches += 1; + if (CHECKLIST_CODES.has(finding.code)) counts.checklistWarnings += 1; + } + return counts; +} + +function formatSummary(skillCount, counts) { + let msg = `Validated host compatibility for ${skillCount} skill directories`; + const extras = []; + if (counts.errors > 0) extras.push(`${counts.errors} error${counts.errors === 1 ? '' : 's'}`); + if (counts.warnings > 0) extras.push(`${counts.warnings} warning${counts.warnings === 1 ? '' : 's'}`); + if (extras.length > 0) msg += ` (${extras.join(', ')})`; + return msg; +} + +function reportFindings(result, stdout, stderr) { + if (result.skipped) { + stdout.write(`${result.message}\n`); + return; + } + + if (result.inventory) { + stdout.write(`${result.message}\n`); + return; + } + + for (const finding of result.findings) { + const line = `${finding.severity === 'error' ? 'ERROR' : 'WARN'}: ${finding.message}\n`; + if (finding.severity === 'error') stderr.write(line); + else stderr.write(line); + } + stdout.write(`${result.message}\n`); +} + +function run(argv = process.argv.slice(2), options = {}) { + const stdout = options.stdout || process.stdout; + const stderr = options.stderr || process.stderr; + + try { + const args = parseArgs(argv); + const result = validateSkillHostCompat({ + root: options.root || args.root, + inventory: args.inventory, + strictChecklist: args.strictChecklist, + }); + reportFindings(result, stdout, stderr); + return result.ok ? 0 : 1; + } catch (error) { + stderr.write(`ERROR: ${error.message}\n`); + return 1; + } +} + +if (require.main === module) { + process.exit(run()); +} + +module.exports = { + ALLOWED_CODEX_FRONTMATTER_KEYS, + CLAUDE_SUBSTITUTION_PATTERNS, + extractTopLevelList, + findExecutableFences, + parseArgs, + run, + validateSkillHostCompat, +}; diff --git a/scripts/ci/validate-skills.js b/scripts/ci/validate-skills.js index 47334687f..0bcc4df48 100644 --- a/scripts/ci/validate-skills.js +++ b/scripts/ci/validate-skills.js @@ -379,9 +379,16 @@ function validateSkills() { console.log(msg); } -try { - validateSkills(); -} catch (error) { - console.error(`ERROR: ${error.message}`); - process.exit(1); +module.exports = { + extractFrontmatter, + inspectFrontmatter, +}; + +if (require.main === module) { + try { + validateSkills(); + } catch (error) { + console.error(`ERROR: ${error.message}`); + process.exit(1); + } } diff --git a/skills/configure-ecc/SKILL.md b/skills/configure-ecc/SKILL.md index f9c3992d1..7610cb181 100644 --- a/skills/configure-ecc/SKILL.md +++ b/skills/configure-ecc/SKILL.md @@ -73,15 +73,10 @@ the selected install scope. ### 3. Preview and confirm once -Prefer the plugin-bundled setup script. Substitute the two selected values and -include `--move-scope` only for a scope migration: - -```bash -node "$CLAUDE_PLUGIN_ROOT/scripts/setup.js" --mode claude-plugin \ - --scope --hooks [--move-scope] --dry-run --json -``` - -If `$CLAUDE_PLUGIN_ROOT` is unavailable, use the published npm package: +Prefer the plugin-bundled setup script. When `$CLAUDE_PLUGIN_ROOT` is set, run +`node` on `scripts/setup.js` under that plugin root. Substitute the two selected +values and include `--move-scope` only for a scope migration. If the plugin root +is unavailable, use the published npm package: ```bash npx --yes --package ecc-universal ecc setup --mode claude-plugin \ @@ -96,14 +91,9 @@ harness shell tool because that shell is commonly non-TTY. ### 4. Apply the explicit choices After confirmation, rerun the same route without `--dry-run`. Keep every choice -explicit and request JSON so success can be checked deterministically: - -```bash -node "$CLAUDE_PLUGIN_ROOT/scripts/setup.js" --mode claude-plugin \ - --scope --hooks [--move-scope] --yes --json -``` - -Fallback: +explicit and request JSON so success can be checked deterministically. When +`$CLAUDE_PLUGIN_ROOT` is set, that is `node` plus `scripts/setup.js` with +`--yes --json`. Otherwise: ```bash npx --yes --package ecc-universal ecc setup --mode claude-plugin \ diff --git a/skills/skill-host-compat/SKILL.md b/skills/skill-host-compat/SKILL.md new file mode 100644 index 000000000..e8ad98059 --- /dev/null +++ b/skills/skill-host-compat/SKILL.md @@ -0,0 +1,98 @@ +--- +name: skill-host-compat +description: Lint ECC skills for Codex/Cursor-safe frontmatter and Claude-only substitutions before a skill PR. +origin: ECC +--- + +# Skill Host Compatibility + +Run the same host-compat gate CI uses on `skills/` before opening a skill PR. +The check is for Codex and Cursor copies, not Claude-only runtime quality. + +## When to Activate + +- Adding or editing a skill under `skills/` +- Copying a skill into `.agents/skills/` or `.cursor/skills/` +- A review asks whether a skill is safe to install into Codex or Cursor +- CI reports `validate-skill-host-compat.js` findings + +## Core Concepts + +Canonical skills live in `skills/`. Codex loads `.agents/skills/`. Cursor loads a +smaller `.cursor/skills/` subset. CONTRIBUTING documents that sync as manual. + +`validate-skills.js` only checks that `SKILL.md` exists and that `name` / +`description` are usable YAML. `tests/ci/codex-skill-surface.test.js` only +sees `.agents/skills/`. This script is the cross-tree gate: + +1. Claude-only substitutions in executable `bash` / `sh` / `zsh` fences + (`${CLAUDE_SESSION_ID}`, `${CLAUDE_SKILL_DIR}`, `${CLAUDE_PLUGIN_ROOT}`, + `$ARGUMENTS`) fail. Codex leaves those tokens as literal text. +2. A Codex copy may only use `name`, `description`, `metadata`, `license`, + and `allowed-tools`. `origin: ECC` may stay on the canonical `skills/` + copy and must be stripped on the Codex copy. +3. A skill listed in `agents/openai.yaml` that has a `skills/` directory must + have `.agents/skills//`. Cursor copies are optional; when present, + `name:` must match the directory. +4. CONTRIBUTING checklist items (When to Activate, Anti-Patterns, name + matches directory, 500/800 line caps) warn. They fail only with + `--strict-checklist`. + +`validate-skills.js` frontmatter WARN default is unchanged (#1663). + +## Code Examples + +From the repo root: + +```bash +node scripts/ci/validate-skill-host-compat.js +``` + +Print counts without failing: + +```bash +node scripts/ci/validate-skill-host-compat.js --inventory +``` + +Promote checklist warnings: + +```bash +node scripts/ci/validate-skill-host-compat.js --strict-checklist +``` + +Slash command: + +``` +/skill-host-compat +/skill-host-compat --inventory +``` + +## Anti-Patterns + +Do not put Claude-only substitutions in bash blocks. This fence is the +forbidden pattern: + +```bash +echo "${CLAUDE_SESSION_ID}" +ls "${CLAUDE_SKILL_DIR}" +node "${CLAUDE_PLUGIN_ROOT}/scripts/setup.js" $ARGUMENTS +``` + +Do not add `version:` or `origin:` to a Codex copy under `.agents/skills/`. +Do not treat a passing `validate-skills.js` run as Codex-safe. Do not require +every `skills/` entry to have a Cursor copy. + +## Best Practices + +- Keep canonical `origin: ECC` if the CONTRIBUTING template calls for it. +- Drop `origin`, `version`, and `argument-hint` on the Codex copy. +- Show host-portable commands in bash fences (`npx`, `ecc`, or a resolved + root). Mention Claude env vars in prose, not in executable fences. +- Run this script before `npm test` when the change is a new skill. +- Use `--inventory` to see checklist warning volume without failing the tree. + +## Related Skills + +- `skill-comply` — runtime behavioral traces, not host-portability +- `skill-stocktake` — holistic audit of installed `~/.claude/skills` +- `skill-scout` — search existing skills before creating a new one diff --git a/tests/ci/validate-skill-host-compat.test.js b/tests/ci/validate-skill-host-compat.test.js new file mode 100644 index 000000000..e11cf7101 --- /dev/null +++ b/tests/ci/validate-skill-host-compat.test.js @@ -0,0 +1,494 @@ +#!/usr/bin/env node +/** + * Fixture tests for scripts/ci/validate-skill-host-compat.js. + * + * Uses tmp dirs, not the live catalog. + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const repoRoot = path.join(__dirname, '..', '..'); +const validatorPath = path.join(repoRoot, 'scripts', 'ci', 'validate-skill-host-compat.js'); + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + return false; + } +} + +function createRoot() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'skill-host-compat-')); +} + +function cleanup(dir) { + fs.rmSync(dir, { recursive: true, force: true }); +} + +function writeSkill(root, skillName, body, options = {}) { + const tree = options.tree || 'skills'; + const extraFrontmatter = options.frontmatter || {}; + const dir = path.join(root, tree, skillName); + fs.mkdirSync(dir, { recursive: true }); + const lines = ['---', `name: ${options.name || skillName}`]; + if (options.description !== false) { + lines.push(`description: ${options.description || `Fixture skill ${skillName}`}`); + } + for (const [key, value] of Object.entries(extraFrontmatter)) { + lines.push(`${key}: ${value}`); + } + lines.push('---', ''); + lines.push(body); + fs.writeFileSync(path.join(dir, 'SKILL.md'), `${lines.join('\n')}\n`); +} + +function writeOpenaiYaml(root, skillNames) { + const dir = path.join(root, 'agents'); + fs.mkdirSync(dir, { recursive: true }); + const body = ['skills:', ...skillNames.map(name => ` - ${name}`)].join('\n'); + fs.writeFileSync(path.join(dir, 'openai.yaml'), `${body}\n`); +} + +function runValidator(root, extraArgs = []) { + return spawnSync('node', [validatorPath, '--root', root, ...extraArgs], { + encoding: 'utf8', + cwd: repoRoot, + timeout: 10000, + }); +} + +function checklistBody(title) { + return [ + `# ${title}`, + '', + '## When to Activate', + '', + '- Writing or reviewing a skill PR', + '', + '## Anti-Patterns', + '', + '- Shipping a Codex copy with extra frontmatter keys', + '', + ].join('\n'); +} + +console.log('\n=== Testing validate-skill-host-compat.js ===\n'); + +let passed = 0; +let failed = 0; + +function check(name, fn) { + if (test(name, fn)) passed += 1; + else failed += 1; +} + +check('exports extractFrontmatter and inspectFrontmatter without running the CLI', () => { + const skills = require('../../scripts/ci/validate-skills'); + assert.strictEqual(typeof skills.extractFrontmatter, 'function'); + assert.strictEqual(typeof skills.inspectFrontmatter, 'function'); + const fm = skills.extractFrontmatter('---\nname: demo\ndescription: hi\n---\n# Demo\n'); + assert.strictEqual(fm.present, true); + const inspected = skills.inspectFrontmatter(fm.lines); + assert.strictEqual(inspected.values.name, 'demo'); +}); + +check('happy: skill with name/description only, no Codex copy required, exits 0', () => { + const root = createRoot(); + try { + writeSkill(root, 'portable', checklistBody('Portable')); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + assert.ok(result.stdout.includes('Validated host compatibility for 1')); + } finally { + cleanup(root); + } +}); + +check('happy: Codex copy whose frontmatter is the allowlist only, exits 0', () => { + const root = createRoot(); + try { + writeSkill(root, 'portable', checklistBody('Portable'), { + frontmatter: { origin: 'ECC' }, + }); + writeSkill(root, 'portable', checklistBody('Portable'), { + tree: '.agents/skills', + }); + writeOpenaiYaml(root, ['portable']); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + } finally { + cleanup(root); + } +}); + +check('fail: bash fence contains ${CLAUDE_SESSION_ID}', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'leaky', + `${checklistBody('Leaky')}\n## Code Examples\n\n\`\`\`bash\necho "\${CLAUDE_SESSION_ID}"\n\`\`\`\n` + ); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('ERROR:')); + assert.ok(result.stderr.includes('leaky')); + assert.ok(result.stderr.includes('${CLAUDE_SESSION_ID}')); + } finally { + cleanup(root); + } +}); + +check('fail: $ARGUMENTS used as a substitution in a bash fence', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'args', + `${checklistBody('Args')}\n## Code Examples\n\n\`\`\`bash\necho "$ARGUMENTS"\n\`\`\`\n` + ); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('args')); + assert.ok(result.stderr.includes('$ARGUMENTS')); + } finally { + cleanup(root); + } +}); + +check('fail: .agents/skills/foo/SKILL.md has version:', () => { + const root = createRoot(); + try { + writeSkill(root, 'foo', checklistBody('Foo')); + writeSkill(root, 'foo', checklistBody('Foo'), { + tree: '.agents/skills', + frontmatter: { version: '1.0.0' }, + }); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('version')); + assert.ok(result.stderr.includes('.agents/skills/foo/SKILL.md')); + } finally { + cleanup(root); + } +}); + +check('fail: agents/openai.yaml lists foo but .agents/skills/foo/ is missing', () => { + const root = createRoot(); + try { + writeSkill(root, 'foo', checklistBody('Foo')); + writeOpenaiYaml(root, ['foo']); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('agents/openai.yaml lists \'foo\'')); + assert.ok(result.stderr.includes('.agents/skills/foo/')); + } finally { + cleanup(root); + } +}); + +check('warn-only: missing ## When to Activate exits 0 unless --strict-checklist', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'thin', + '# Thin\n\n## Anti-Patterns\n\n- Missing activation heading\n' + ); + const warnResult = runValidator(root); + assert.strictEqual(warnResult.status, 0, warnResult.stderr); + assert.ok(warnResult.stderr.includes('WARN:')); + assert.ok(warnResult.stderr.includes('When to Activate')); + + const strictResult = runValidator(root, ['--strict-checklist']); + assert.strictEqual(strictResult.status, 1); + assert.ok(strictResult.stderr.includes('ERROR:')); + } finally { + cleanup(root); + } +}); + +check('anti-pattern fence still matches when the do-not line wraps', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'wrapped', + [ + '# Wrapped', + '', + '## When to Activate', + '', + '- Reviewing a skill PR', + '', + '## Anti-Patterns', + '', + 'Do not put Claude-only substitutions in bash blocks. This fence is the', + 'forbidden pattern:', + '', + '```bash', + 'echo "${CLAUDE_SESSION_ID}"', + '```', + '', + ].join('\n') + ); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(!result.stderr.includes('ERROR:')); + } finally { + cleanup(root); + } +}); + +check('anti-pattern fence that quotes the token as forbidden exits 0', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'quoted', + [ + '# Quoted', + '', + '## When to Activate', + '', + '- Reviewing a skill PR', + '', + '## Anti-Patterns', + '', + 'Do not put Claude-only substitutions in bash blocks:', + '', + '```bash', + 'echo "${CLAUDE_SESSION_ID}"', + '```', + '', + ].join('\n') + ); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(!result.stderr.includes('ERROR:')); + } finally { + cleanup(root); + } +}); + +check('empty skills/ tmp root exits 0', () => { + const root = createRoot(); + try { + fs.mkdirSync(path.join(root, 'skills')); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(result.stdout.includes('Validated host compatibility for 0')); + } finally { + cleanup(root); + } +}); + +check('missing skills directory skips with exit 0', () => { + const root = createRoot(); + try { + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(result.stdout.includes('skipping')); + } finally { + cleanup(root); + } +}); + +check('origin on canonical copy is allowed; origin on Codex copy fails', () => { + const root = createRoot(); + try { + writeSkill(root, 'foo', checklistBody('Foo'), { frontmatter: { origin: 'ECC' } }); + writeSkill(root, 'foo', checklistBody('Foo'), { + tree: '.agents/skills', + frontmatter: { origin: 'ECC' }, + }); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('origin')); + } finally { + cleanup(root); + } +}); + +check('Cursor copy name mismatch is an error; extra Cursor copies are not required', () => { + const root = createRoot(); + try { + writeSkill(root, 'alpha', checklistBody('Alpha')); + writeSkill(root, 'alpha', checklistBody('Alpha'), { + tree: '.cursor/skills', + name: 'beta', + }); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('.cursor/skills/alpha/SKILL.md')); + assert.ok(result.stderr.includes('does not match directory')); + } finally { + cleanup(root); + } +}); + +check('--inventory prints JSON counts and exits 0 even with errors', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'leaky', + `${checklistBody('Leaky')}\n## Code Examples\n\n\`\`\`bash\necho "$CLAUDE_SKILL_DIR"\n\`\`\`\n` + ); + const result = runValidator(root, ['--inventory']); + assert.strictEqual(result.status, 0, result.stderr); + const counts = JSON.parse(result.stdout.trim()); + assert.strictEqual(counts.skillsScanned, 1); + assert.ok(counts.substitutions >= 1); + assert.ok(counts.errors >= 1); + } finally { + cleanup(root); + } +}); + +check('empty-language shebang fence is scanned; markdown fence is not', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'mixed', + [ + '# Mixed', + '', + '## When to Activate', + '', + '- Reviewing fences', + '', + '## Anti-Patterns', + '', + '- Putting substitutions in bash', + '', + '```markdown', + 'Parse $ARGUMENTS in a command file, not in a skill bash block.', + '```', + '', + '```', + '#!/bin/sh', + 'echo "$CLAUDE_PLUGIN_ROOT"', + '```', + '', + ].join('\n') + ); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('$CLAUDE_PLUGIN_ROOT')); + assert.ok(!result.stderr.includes('$ARGUMENTS')); + } finally { + cleanup(root); + } +}); + +check('Codex copy name mismatch is an error', () => { + const root = createRoot(); + try { + writeSkill(root, 'foo', checklistBody('Foo')); + writeSkill(root, 'foo', checklistBody('Foo'), { + tree: '.agents/skills', + name: 'bar', + }); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('.agents/skills/foo/SKILL.md')); + } finally { + cleanup(root); + } +}); + +check('--root without a path exits 1', () => { + const result = spawnSync('node', [validatorPath, '--root'], { + encoding: 'utf8', + cwd: repoRoot, + timeout: 10000, + }); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('--root requires a directory path')); +}); + +check('canonical name mismatch warns and does not fail', () => { + const root = createRoot(); + try { + writeSkill(root, 'foo', checklistBody('Foo'), { name: 'bar' }); + const result = runValidator(root); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(result.stderr.includes('WARN:')); + assert.ok(result.stderr.includes('does not match directory')); + } finally { + cleanup(root); + } +}); + +check('missing Anti-Patterns heading warns', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'thin', + '# Thin\n\n## When to Activate\n\n- Reviewing a skill\n' + ); + const result = runValidator(root); + assert.strictEqual(result.status, 0); + assert.ok(result.stderr.includes('Anti-Patterns')); + } finally { + cleanup(root); + } +}); + +check('body over 500 lines warns; over 800 stays warn unless --strict-checklist', () => { + const root = createRoot(); + try { + const overFive = `${checklistBody('Long')}\n${'line\n'.repeat(500)}`; + writeSkill(root, 'longish', overFive); + const warnResult = runValidator(root); + assert.strictEqual(warnResult.status, 0, warnResult.stderr); + assert.ok(warnResult.stderr.includes('500')); + + cleanup(root); + const root2 = createRoot(); + const overEight = `${checklistBody('Huge')}\n${'line\n'.repeat(800)}`; + writeSkill(root2, 'huge', overEight); + const hugeWarn = runValidator(root2); + assert.strictEqual(hugeWarn.status, 0, hugeWarn.stderr); + assert.ok(hugeWarn.stderr.includes('800')); + const hugeStrict = runValidator(root2, ['--strict-checklist']); + assert.strictEqual(hugeStrict.status, 1); + cleanup(root2); + } finally { + if (fs.existsSync(root)) cleanup(root); + } +}); + +check('bash fence ${1} is treated as a Claude substitution', () => { + const root = createRoot(); + try { + writeSkill( + root, + 'pos', + `${checklistBody('Pos')}\n## Code Examples\n\n\`\`\`bash\necho "\${1}"\n\`\`\`\n` + ); + const result = runValidator(root); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('$1')); + } finally { + cleanup(root); + } +}); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/scripts/npm-publish-surface.test.js b/tests/scripts/npm-publish-surface.test.js index bec9096d4..3e23d41a4 100644 --- a/tests/scripts/npm-publish-surface.test.js +++ b/tests/scripts/npm-publish-surface.test.js @@ -50,6 +50,8 @@ function buildExpectedPublishPaths(repoRoot) { "scripts/catalog.js", "scripts/ci/scan-supply-chain-iocs.js", "scripts/ci/supply-chain-advisory-sources.js", + "scripts/ci/validate-skill-host-compat.js", + "scripts/ci/validate-skills.js", "scripts/consult.js", "scripts/control-pane.js", "scripts/dashboard-web.js", @@ -184,6 +186,7 @@ function main() { "scripts/catalog.js", "scripts/ci/scan-supply-chain-iocs.js", "scripts/ci/supply-chain-advisory-sources.js", + "scripts/ci/validate-skill-host-compat.js", "scripts/consult.js", "scripts/control-pane.js", "scripts/feedback.js",