#!/usr/bin/env node 'use strict'; /** * lint-allowed-tools-parity.cjs — catch `allowed-tools` frontmatter that * declares `Bash` but omits `Grep` (#4394, follow-up to #3085). * * ## Why * * `gen-plugin-skills.cjs --check` already guarantees `skills/*` /SKILL.md` * matches byte-for-byte what `commands/msd/*.md` generates, so the two trees * cannot silently diverge FROM EACH OTHER. Nothing guarded the shape #3085 * actually found: a command shipping `Bash` without `Grep` purely by * omission, identical in both trees and therefore invisible to a parity * check that only compares them to each other. * * That drift ran long enough for 29 of 71 skills to lack a tool most of * their siblings already declared, and it surfaced through a manual audit * rather than any gate. A command that can shell out but cannot Grep does * not fail loudly — it quietly reaches for `Bash` + `grep` instead, which is * slower, less structured, and (per `lint-portable-grep.cjs`) a portability * hazard of its own on hosts without GNU grep. * * ## The rule * * A command whose `allowed-tools` includes `Bash` must also include `Grep`, * unless it is on the exemption list below. * * Detection only. This lint never edits a command's `allowed-tools`. * * ## Why an exemption list rather than a heuristic * * The alternative — inferring from a command's body whether it "really" * needs Grep — would make the rule's verdict depend on prose that changes * constantly, and produce a lint whose failures nobody can predict. A short * literal list keeps every exemption a reviewable one-line diff, and the * staleness check below stops it becoming a dumping ground: an entry that no * longer needs to be there fails just as loudly as a missing tool. */ const fs = require('fs'); const path = require('path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); const ROOT = path.resolve(__dirname, '..'); const DEFAULT_ROOT = 'commands/msd'; /** * Commands allowed to declare `Bash` without `Grep`. * * Keyed by command stem (the filename without `.md`), with the reason inline * so changing the set is a one-line, reviewable diff. * * #4394 named eight candidates from the #3085 review: the six `msd-ns-*` * namespace dispatchers, `msd-help`, and `msd-surface`. Seven of those turn * out not to need an entry at all — they do not declare `Bash` in the first * place, so the rule never reaches them: * * ns-context/ns-ideate/ns-manage/ns-project/ns-review/ns-workflow Read + Skill * help Read * * Listing them anyway would be seven pre-forgiven commands: the day one of * them gained `Bash`, the omission it was granted an exemption for would * pass silently. The staleness check in `scan()` below is what surfaced * this, and it is why the list stays minimal — an exemption is a real * suppression, not documentation. */ const EXEMPT = new Map([ // Toggles which skills are surfaced. Mutates the install tree through the // installer's own seams rather than by searching project files, so `Bash` // here is not standing in for a search it cannot perform. ['surface', 'mutates the install surface via installer seams, not by searching the project'], ]); /** * Parse an `allowed-tools` frontmatter value. * * Handles both shapes the corpus uses: a YAML block sequence * * allowed-tools: * - Read * - Bash * * and an inline scalar or flow sequence (`allowed-tools: Read, Bash` / * `allowed-tools: [Read, Bash]`). Returns `null` when the key is absent, * which is NOT a violation — a command with no `allowed-tools` at all * declares no `Bash` either, so this rule has nothing to say about it. * * Deliberately not a YAML parser: the frontmatter here is a fixed, shallow * shape, and pulling in a parser to read one list would be a dependency * bought for a single key. * * @param {string} text full file contents * @returns {string[] | null} declared tool names, or null when the key is absent */ function parseAllowedTools(text) { const lines = String(text).split(/\r?\n/); const keyIndex = lines.findIndex((l) => /^allowed-tools:/.test(l)); if (keyIndex === -1) return null; const inline = lines[keyIndex].replace(/^allowed-tools:/, '').trim(); if (inline) { return inline .replace(/^\[/, '') .replace(/\]$/, '') .split(',') .map((s) => s.trim().replace(/^['"]|['"]$/g, '')) .filter(Boolean); } const tools = []; for (let i = keyIndex + 1; i < lines.length; i += 1) { const line = lines[i]; const item = line.match(/^\s*-\s+(.+?)\s*$/); if (item) { tools.push(item[1].replace(/^['"]|['"]$/g, '')); continue; } // The first line that is neither a list item nor blank ends the block — // the next frontmatter key, or the closing `---`. if (line.trim() === '') continue; break; } return tools; } /** * List command stems under `dir`, sorted. * * @param {string} dir absolute path to the commands directory * @returns {{ stem: string, file: string }[]} */ function listCommands(dir) { let entries; try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return []; } return entries .filter((e) => e.isFile() && e.name.endsWith('.md')) .map((e) => ({ stem: e.name.replace(/\.md$/, ''), file: path.join(dir, e.name) })) .sort((a, b) => a.stem.localeCompare(b.stem)); } /** * Evaluate the corpus. * * Reports two independent failure classes, because an exemption list that * can only ever grow rots into a list of things nobody re-examined: * * - `violations`: declares Bash, omits Grep, not exempt. * - `staleExemptions`: on the list but no longer needs to be — either the * command is gone, or it no longer declares Bash without Grep. Removing * the entry is then a one-line diff, and the next omission in that * command is caught rather than silently pre-forgiven. * * @param {string} [root] repo-relative commands directory * @returns {{ scanned: number, violations: {stem: string, tools: string[]}[], staleExemptions: {stem: string, reason: string}[] }} */ function scan(root = DEFAULT_ROOT) { const abs = path.isAbsolute(root) ? root : path.join(ROOT, root); const commands = listCommands(abs); const violations = []; const seen = new Set(); for (const { stem, file } of commands) { const tools = parseAllowedTools(fs.readFileSync(file, 'utf8')); if (!tools) continue; if (!tools.includes('Bash') || tools.includes('Grep')) continue; seen.add(stem); if (EXEMPT.has(stem)) continue; violations.push({ stem, tools }); } const present = new Set(commands.map((c) => c.stem)); const staleExemptions = []; for (const [stem, reason] of EXEMPT) { if (!present.has(stem)) { staleExemptions.push({ stem, reason: `no such command (${reason})` }); } else if (!seen.has(stem)) { staleExemptions.push({ stem, reason: `no longer declares Bash without Grep (${reason})` }); } } return { scanned: commands.length, violations, staleExemptions }; } function main() { const root = process.env.MSD_LINT_ALLOWED_TOOLS_ROOT || DEFAULT_ROOT; const { scanned, violations, staleExemptions } = scan(root); if (violations.length > 0 || staleExemptions.length > 0) { const parts = []; if (violations.length > 0) { parts.push( 'lint-allowed-tools-parity: these commands declare `Bash` but not `Grep` (#4394).\n' + 'A command that can shell out but cannot Grep reaches for `Bash` + `grep` instead —\n' + 'slower, unstructured, and a portability hazard on hosts without GNU grep. Add `Grep`\n' + 'to the frontmatter, or add the command to EXEMPT in this script with a reason:\n' + violations.map((v) => ` ${root}/${v.stem}.md [${v.tools.join(', ')}]`).join('\n'), ); } if (staleExemptions.length > 0) { parts.push( 'lint-allowed-tools-parity: these EXEMPT entries are stale — delete them so the next\n' + 'omission in those commands is caught rather than silently pre-forgiven:\n' + staleExemptions.map((s) => ` ${s.stem}: ${s.reason}`).join('\n'), ); } throw new ExitError(1, parts.join('\n\n')); } console.log( `ok lint-allowed-tools-parity: ${scanned} command(s) checked, ${EXEMPT.size} exemption(s) all still needed`, ); } module.exports = { parseAllowedTools, scan, EXEMPT, DEFAULT_ROOT }; if (require.main === module) runMain(main);