From 879f5750de49ee16b07e5dac2ee30bcbd5780b45 Mon Sep 17 00:00:00 2001 From: t Date: Thu, 29 Oct 2026 01:34:55 +0000 Subject: [PATCH] chore: add pre-commit and pre-push hooks that lint changed files - husky installs the hooks from the npm prepare script - pre-commit: lint-staged runs scripts/lint-files.mjs on staged files, then npm run validate - pre-push: npm run check (everything CI runs) - hooks:check (run by pretest) fails when core.hooksPath or the hook files are not as installed - CI installs with npm ci and runs npm run lint as the backstop for skipped hooks --- .github/workflows/ci.yml | 11 +++- .husky/pre-commit | 4 ++ .husky/pre-push | 2 + AGENTS.md | 15 ++++- package-lock.json | 139 +++++++++++++++++++++++++++++++++++++-- package.json | 15 ++++- scripts/check-hooks.mjs | 36 ++++++++++ scripts/lint-files.mjs | 89 +++++++++++++++++++++++++ tests/lint.test.mjs | 48 ++++++++++++++ 9 files changed, 348 insertions(+), 11 deletions(-) create mode 100644 .husky/pre-commit create mode 100644 .husky/pre-push create mode 100644 scripts/check-hooks.mjs create mode 100644 scripts/lint-files.mjs create mode 100644 tests/lint.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d4e66f..a760f81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,16 @@ jobs: with: node-version: 22 - # The skill validator and the plugin version check need no dependencies. + # HUSKY=0: CI does not need the git hooks. They run the same checks as the steps below, so a + # contributor who skips them (--no-verify, no npm install) is caught here. + - name: Install dependencies + run: npm ci --no-audit --no-fund + env: + HUSKY: "0" + + - name: Lint files + run: npm run lint + - name: Validate skills run: npm run validate diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100644 index 0000000..58ec0ef --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1,4 @@ +# Runs on every commit. Lints the staged files, then the package-wide rules. +# Skip once with `git commit --no-verify`; CI runs the same checks, so the skip only defers them. +npx --no-install lint-staged +npm run --silent validate diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100644 index 0000000..f3f9371 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1,2 @@ +# Runs before every push: everything CI runs. +npm run --silent check diff --git a/AGENTS.md b/AGENTS.md index af62b92..5ed9103 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,12 +55,25 @@ Boilerplate every skill shares lives once in the protocol. Do not copy it back i ## Checks ```bash +npm run lint # syntax of .mjs, .sh, .json, .yml and skill front matter (scripts/lint-files.mjs) npm run validate -npm test +npm test # starts with hooks:check npm run sync:check npm run check-plugin-version +npm run check # lint, validate, check-plugin-version and test: everything CI runs ``` +## Git hooks + +`npm install` runs the `prepare` script, which installs [husky](https://typicode.github.io/husky/) and points `core.hooksPath` at `.husky/_`. The hooks live in `.husky/`. + +- **`pre-commit`**: `lint-staged` runs `scripts/lint-files.mjs` on the staged files, then `npm run validate` checks the package-wide rules (this includes the generated-files check, `sync:check`). +- **`pre-push`**: `npm run check`. + +CI runs the same checks, so `git commit --no-verify` only defers a failure. A hook does not run when `core.hooksPath` points elsewhere, when `npm install` was never run, or when `HUSKY=0` is set. `npm test` starts with `npm run hooks:check`, which fails and says how to fix it; `npm run prepare` reinstalls. + +Add a check in `scripts/lint-files.mjs` (per file) or `scripts/validate-skills.mjs` (package-wide), with a test. The hooks and CI both call those scripts, so there is nowhere else to wire it in. + ## Releasing Add a changeset (`npm run changeset`) for user-visible changes. diff --git a/package-lock.json b/package-lock.json index 27404c0..539f0f6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,21 @@ { "name": "github-agent-workflow", - "version": "0.1.0", + "version": "1.9.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "github-agent-workflow", - "version": "0.1.0", + "version": "1.9.0", "license": "MIT", + "dependencies": { + "mattpocock-skills": "1.3.0" + }, "devDependencies": { - "@changesets/cli": "2.31.0" + "@changesets/cli": "2.31.0", + "husky": "9.1.7", + "lint-staged": "17.6.0", + "yaml": "2.9.1" } }, "node_modules/@babel/runtime": { @@ -299,6 +305,13 @@ "fs-extra": "^8.1.0" } }, + "node_modules/@manypkg/find-root/node_modules/@types/node": { + "version": "12.20.55", + "resolved": "https://registry.npmjs.org/@types/node/-/node-12.20.55.tgz", + "integrity": "sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==", + "dev": true, + "license": "MIT" + }, "node_modules/@manypkg/find-root/node_modules/fs-extra": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-8.1.0.tgz", @@ -390,11 +403,16 @@ } }, "node_modules/@types/node": { - "version": "12.20.55", - "resolved": "https://registry.npmjs.org/@types/node/-/node-12.20.55.tgz", - "integrity": "sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==", + "version": "26.6.6", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.6.6.tgz", + "integrity": "sha512-MzQj+8Kx3XI40OmUAE6Qoh58ZMNstNNMwn2nfd3qWsfe7nDNSqJNzes4njhz6AHwrSHOffSLwXU6yb474Q4aog==", "dev": true, - "license": "MIT" + "license": "MIT", + "optional": true, + "peer": true, + "dependencies": { + "undici-types": "~8.9.0" + } }, "node_modules/ansi-colors": { "version": "4.1.3", @@ -659,6 +677,22 @@ "human-id": "dist/cli.js" } }, + "node_modules/husky": { + "version": "9.1.7", + "resolved": "https://registry.npmjs.org/husky/-/husky-9.1.7.tgz", + "integrity": "sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA==", + "dev": true, + "license": "MIT", + "bin": { + "husky": "bin.js" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/typicode" + } + }, "node_modules/iconv-lite": { "version": "0.7.3", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.3.tgz", @@ -782,6 +816,43 @@ "graceful-fs": "^4.1.6" } }, + "node_modules/lint-staged": { + "version": "17.6.0", + "resolved": "https://registry.npmjs.org/lint-staged/-/lint-staged-17.6.0.tgz", + "integrity": "sha512-nOhBfYkZEwkFtNzD5pK9s9LkcWoxJ+whLzEnhqi9nl7zTB5n7tg8+9iYgMGx2Wm3VlLu8cbNj0RACzY+nWi5iQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "picomatch": "^4.0.7", + "string-argv": "^0.3.2", + "tinyexec": "^1.3.1" + }, + "bin": { + "lint-staged": "bin/lint-staged.js" + }, + "engines": { + "node": ">=22.22.1" + }, + "funding": { + "url": "https://opencollective.com/lint-staged" + }, + "optionalDependencies": { + "yaml": "^2.9.1" + } + }, + "node_modules/lint-staged/node_modules/picomatch": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.7.tgz", + "integrity": "sha512-qcJu88Q2IWqJsDD529JKMdwGm/dvInW4HvQnRwiH9JtihJvzGOscDtHE3x1pBKeUOTysQ8kVmLnJ2kJu7yhcGA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/jonschlinkert" + } + }, "node_modules/locate-path": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-5.0.0.tgz", @@ -802,6 +873,15 @@ "dev": true, "license": "MIT" }, + "node_modules/mattpocock-skills": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/mattpocock-skills/-/mattpocock-skills-1.3.0.tgz", + "integrity": "sha512-dlzg5e8vdBCjrpybZAiqBfH+Utwlgy2y0HuMnBbbiPe43o25l4EB1OUkeYFYQTZTlpkOrVxbKQAmMQRGhTHXQA==", + "license": "MIT", + "bin": { + "my-skills": "cli/index.js" + } + }, "node_modules/merge2": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/merge2/-/merge2-1.4.1.tgz", @@ -1198,6 +1278,16 @@ "dev": true, "license": "BSD-3-Clause" }, + "node_modules/string-argv": { + "version": "0.3.2", + "resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.2.tgz", + "integrity": "sha512-aqD2Q0144Z+/RqG52NeHEkZauTAUWJO8c6yTftGJKO3Tja5tUgIfmIl6kExvhtxSDP7fXB6DvzkfMpCd/F3G+Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.6.19" + } + }, "node_modules/strip-ansi": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz", @@ -1234,6 +1324,16 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/tinyexec": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/tinyexec/-/tinyexec-1.3.1.tgz", + "integrity": "sha512-GCvB3aoys96IuDFBMcTB46JOR6mdMtAToqwiW8JlWhsoh1mhHi/xn9ss/Dg7N555GiJyEt2qzoG/NHCwM6h1EA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + } + }, "node_modules/to-regex-range": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", @@ -1247,6 +1347,15 @@ "node": ">=8.0" } }, + "node_modules/undici-types": { + "version": "8.9.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.9.0.tgz", + "integrity": "sha512-KTDyRTYX8sWmKXAikPHHSyc63CRPETMctyjKFupcC6OBLXT3xsN0e9aF7m+mIXutFWpUXuedtowG7iLOzp0kQg==", + "dev": true, + "license": "MIT", + "optional": true, + "peer": true + }, "node_modules/universalify": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", @@ -1272,6 +1381,22 @@ "engines": { "node": ">= 8" } + }, + "node_modules/yaml": { + "version": "2.9.1", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.1.tgz", + "integrity": "sha512-3NxN8+78OdzbT7C/WjGsyfPAtJaN3FNDsWxv7Y7mcDsT/oOmgW8BpyQQFFBnvZE3j9Y2Sdz1ULFLezL7Eb2yFw==", + "dev": true, + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } } } } diff --git a/package.json b/package.json index 785c96d..eab3234 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,13 @@ "license": "MIT", "scripts": { "validate": "node scripts/validate-skills.mjs", - "test": "bash tests/lib.test.sh && node --test tests/helpers.test.mjs", + "lint": "node scripts/lint-files.mjs --all", + "pretest": "node scripts/check-hooks.mjs", + "test": "bash tests/lib.test.sh && node --test tests/helpers.test.mjs tests/lint.test.mjs", "check-plugin-version": "node scripts/sync-plugin-version.mjs --check", + "check": "npm run lint && npm run validate && npm run check-plugin-version && npm test", + "hooks:check": "node scripts/check-hooks.mjs", + "prepare": "husky || true", "link": "bash scripts/link-skills.sh", "changeset": "changeset", "version": "changeset version && node scripts/sync-plugin-version.mjs", @@ -18,6 +23,12 @@ "mattpocock-skills": "1.3.0" }, "devDependencies": { - "@changesets/cli": "2.31.0" + "@changesets/cli": "2.31.0", + "husky": "9.1.7", + "lint-staged": "17.6.0", + "yaml": "2.9.1" + }, + "lint-staged": { + "*": "node scripts/lint-files.mjs" } } diff --git a/scripts/check-hooks.mjs b/scripts/check-hooks.mjs new file mode 100644 index 0000000..b1f3901 --- /dev/null +++ b/scripts/check-hooks.mjs @@ -0,0 +1,36 @@ +#!/usr/bin/env node +// Fails when this checkout's git hooks are not the ones the package installs, so a hook that is +// silently not running shows up as an error instead of a hunch. `npm test` runs it first. +// +// node scripts/check-hooks.mjs +// +// A hook never runs when `core.hooksPath` points somewhere else, when `npm install` was skipped +// (`prepare` is what installs husky), or when HUSKY=0. The fix is `npm install` (or `npm run prepare`). +// The check is skipped where hooks do not apply: CI, no .git directory, or HUSKY=0. + +import { existsSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), ".."); +const HOOKS = ["pre-commit", "pre-push"]; + +if (process.env.CI || process.env.HUSKY === "0" || !existsSync(join(root, ".git"))) { + console.log("hooks: skipped (CI, HUSKY=0 or no .git)"); + process.exit(0); +} + +const problems = []; +const hooksPath = spawnSync("git", ["config", "--get", "core.hooksPath"], { cwd: root, encoding: "utf8" }).stdout.trim(); +if (hooksPath !== ".husky/_") problems.push(`core.hooksPath is ${hooksPath ? `"${hooksPath}"` : "not set"}, expected ".husky/_"`); +for (const h of HOOKS) { + if (!existsSync(join(root, ".husky", h))) problems.push(`.husky/${h} is missing`); + else if (!existsSync(join(root, ".husky", "_", h))) problems.push(`.husky/_/${h} is not installed`); +} + +if (problems.length) { + console.error(`hooks: not installed\n - ${problems.join("\n - ")}\nRun \`npm run prepare\` (or \`npm install\`) in the package root.`); + process.exit(1); +} +console.log("hooks: installed"); diff --git a/scripts/lint-files.mjs b/scripts/lint-files.mjs new file mode 100644 index 0000000..3881215 --- /dev/null +++ b/scripts/lint-files.mjs @@ -0,0 +1,89 @@ +#!/usr/bin/env node +// Lints files without any configuration. The pre-commit hook passes it the staged files (through +// lint-staged); CI runs it on every tracked file, so a skipped hook is caught there. +// +// node scripts/lint-files.mjs FILE... lint these files +// node scripts/lint-files.mjs --all lint every file git tracks +// +// What it checks, by file type: +// *.mjs, *.js the syntax parses (node --check) +// *.sh the syntax parses (bash -n); shellcheck at error severity when installed +// *.json valid JSON +// *.yml, *.yaml valid YAML +// *.md front matter, when present, is valid YAML (an unquoted ": " in a skill's +// description makes harnesses skip the skill) +// any text file no unresolved merge conflict markers +// +// Package-wide rules (names, links, generated files) stay in `npm run validate`. Exit code 1 on any error. + +import { existsSync, readFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { extname } from "node:path"; +import { parse, parseAllDocuments } from "yaml"; + +const args = process.argv.slice(2); +const git = (a) => spawnSync("git", a, { encoding: "utf8" }); + +let files; +if (args.includes("--all")) { + const r = git(["ls-files", "-z"]); + if (r.status !== 0) { console.error("lint: git ls-files failed"); process.exit(2); } + files = r.stdout.split("\0").filter(Boolean); +} else { + files = args.filter((a) => !a.startsWith("-")); +} + +const hasShellcheck = spawnSync("shellcheck", ["--version"]).status === 0; +const errors = []; +const err = (file, msg) => errors.push(`${file}: ${msg}`); +const run = (cmd, a) => spawnSync(cmd, a, { encoding: "utf8" }); +const firstLine = (s) => (s || "").trim().split("\n").filter(Boolean).slice(0, 3).join(" | "); + +export function lintYaml(text, file) { + for (const d of parseAllDocuments(text)) { + for (const e of d.errors) err(file, `invalid YAML: ${e.message.split("\n")[0]}`); + } +} + +function frontMatter(text) { + const m = text.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/); + return m ? m[1] : null; +} + +for (const file of files) { + if (!existsSync(file)) continue; // deleted in the commit + const ext = extname(file); + let text; + try { text = readFileSync(file, "utf8"); } catch { continue; } + if (text.includes("\0")) continue; // binary + + if (/^(<{7}|>{7})( |$)/m.test(text)) err(file, "unresolved merge conflict marker"); + + if (ext === ".mjs" || ext === ".js") { + const r = run("node", ["--check", file]); + if (r.status !== 0) err(file, `syntax error: ${firstLine(r.stderr)}`); + } else if (ext === ".sh") { + const r = run("bash", ["-n", file]); + if (r.status !== 0) err(file, `syntax error: ${firstLine(r.stderr)}`); + else if (hasShellcheck) { + const s = run("shellcheck", ["--severity=error", "--format=gcc", file]); + if (s.status !== 0) err(file, `shellcheck: ${firstLine(s.stdout)}`); + } + } else if (ext === ".json") { + try { JSON.parse(text); } catch (e) { err(file, `invalid JSON: ${e.message}`); } + } else if (ext === ".yml" || ext === ".yaml") { + lintYaml(text, file); + } else if (ext === ".md") { + const fm = frontMatter(text); + if (fm !== null) { + try { parse(fm, { strict: true }); } catch (e) { err(file, `invalid front matter YAML: ${e.message.split("\n")[0]}`); } + } + } +} + +if (errors.length) { + console.error(errors.join("\n")); + console.error(`\n${errors.length} lint error(s).`); + process.exit(1); +} +console.log(`lint: ${files.length} file(s) OK`); diff --git a/tests/lint.test.mjs b/tests/lint.test.mjs new file mode 100644 index 0000000..96805e1 --- /dev/null +++ b/tests/lint.test.mjs @@ -0,0 +1,48 @@ +// Tests for scripts/lint-files.mjs. Run with: node --test tests/ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { spawnSync } from 'node:child_process'; + +const script = join(dirname(fileURLToPath(import.meta.url)), '..', 'scripts', 'lint-files.mjs'); +const dir = mkdtempSync(join(tmpdir(), 'lint-')); + +function lint(name, content) { + const file = join(dir, name); + writeFileSync(file, content); + const r = spawnSync('node', [script, file], { encoding: 'utf8' }); + return { ok: r.status === 0, out: r.stdout + r.stderr }; +} + +const cases = [ + ['valid json', 'a.json', '{"a": 1}\n', true], + ['invalid json', 'b.json', '{bad', false], + ['valid yaml', 'a.yml', 'a:\n - b\n', true], + ['invalid yaml', 'b.yml', 'a: [b\n', false], + ['valid node syntax', 'a.mjs', 'export const a = 1;\n', true], + ['invalid node syntax', 'b.mjs', 'export const = ;\n', false], + ['valid shell syntax', 'a.sh', '#!/usr/bin/env bash\necho hi\n', true], + ['invalid shell syntax', 'b.sh', 'if then\n', false], + ['markdown without front matter', 'a.md', '# Title\n\nSome: text\n', true], + ['valid front matter', 'b.md', '---\nname: x\ndescription: "Note: quoted"\n---\nBody\n', true], + ['unquoted ": " in front matter', 'c.md', '---\nname: x\ndescription: Note: not quoted\n---\nBody\n', false], + ['merge conflict marker', 'd.md', 'a\n<<<<<<< HEAD\nb\n', false], + ['unknown file types are ignored', 'e.txt', 'anything goes\n', true], +]; + +for (const [name, file, content, ok] of cases) { + test(name, () => { + const r = lint(file, content); + assert.equal(r.ok, ok, r.out); + }); +} + +test('a path that no longer exists (a deleted file) is skipped', () => { + const r = spawnSync('node', [script, join(dir, 'gone.json')], { encoding: 'utf8' }); + assert.equal(r.status, 0); +}); + +test.after(() => rmSync(dir, { recursive: true, force: true }));