[brandonmarshal]

Refactor Pull Request

This repository enforces changelog, release, and label automation for all PRs and issues.
See the organisation-wide Automation Governance & Release Strategy for required rules.

Stack

• Position: PR 1 of 6
• Issue/Epic: LS-4750 under epic LS-4744
• Followed by: refactor: ls-wp-release - Fix helper script side effects, SVN layout and packaging docs #2 (helper scripts and packaging docs, based on this branch)
• Review scope: validators, hooks, tests and docs for the sub-skills/ layout

Linked issues

Relates to LS-4750 (Linear). This is batch 1 of several branches for that issue, so it does not close it. Parent epic: LS-4744 (coordination only, never a closing target).

There is no GitHub issue for this work, so there is nothing to auto-close.

Summary

The repository's own validators and hooks looked for a skills/ folder that does not exist (the bundled children live in sub-skills/), reported success when nothing was checked, and npm test was a no-op. This PR makes the existing checks look at the real layout and fail when they should, and adds focused tests for them. No release behaviour, routing or publishing capability changes.

Structure validator (scripts/validate-skill-structure.js)

• Required paths and directories point at sub-skills/ instead of skills/.
• The release workflow heading check accepts both ## Release Workflow and ### Release Workflow.
• Every sub-skills/*/SKILL.md must have a frontmatter name that matches its directory name.
• release-approval-workflow gains frontmatter and multi-package-release uses name: instead of skill_name:, so both pass the new check.

JSON and link validators

• scripts/validate-json.js: a missing required .json file now fails (it previously exited 0). A run that validated nothing says so instead of reporting "valid".
• scripts/validate-links.js: stays network-free and now reports external links and anchors as "not checked" instead of counting them as valid.

package.json and Lefthook

• Lint globs point at sub-skills/**.
• Removed the non-existent yamllint npm dependency (it is a Python tool), declared the semver dependency, and fixed the repository URL.
• lint:yaml fails with a clear message when yamllint is missing and no longer swallows real YAML errors. Zero YAML files is reported as "nothing was linted".
• Hooks fail when a required tool is missing instead of silently passing. The YAML glob is fixed and pre-push runs npm test and npm run lint.
• The commit-msg conventional-commit rule is intentionally unchanged.

Tests (new, Node built-in runner only)

• tests/validate-json.test.cjs, validate-links.test.cjs, validate-skill-structure.test.cjs, lint-yaml.test.cjs: fixture tests that copy each validator into a temp project, run it from an unrelated working directory with an isolated PATH, and assert nothing is written.
• npm test now runs node --test tests/*.test.cjs. No Jest, no new dependencies, no coverage tooling (Jest work is on hold pending a testing-tool decision).

Docs

• README.md, docs/DEVELOPMENT.md, tests/README.md: skills/ paths corrected to sub-skills/, the npm install side effect (prepare runs lefthook install) documented, and tests/README.md states what npm test covers versus the downstream-project reference material.

Scope and exclusions

• Not in this PR: helper scripts, WordPress.org sync skills, the broken documentation links in SKILL.md and agents/, the changelog handoff, TypeScript conversion and the lightspeed-release-ops plugin skills. These are planned as later branches of the same issue.
• validate-links.js still exits 1 on 50 broken links in files outside this PR. Placeholder links in release-notes templates are also counted as real links by that validator.
• Review size: 14 files, 605 insertions, 87 deletions (about 440 lines are new test files). This is within the file budget and above the preferred 400 lines. No files were excluded from the count.
• Screenshots/video: not applicable, there is no visible UI change.

Safety Nets

• Existing tests covering behaviour: none. npm test was an echo placeholder.
• New/refined tests added: 4 fixture test files, 31 tests (see above).
• Static analysis/lint rules touched: lint globs, lint:yaml, and the Lefthook markdown and YAML hooks.

Approach

• Structural changes (APIs, patterns): no CLI or output contract changes beyond the intended failures and "not checked" wording. Validators keep their current interfaces.
• Dead code removed? Yes: the unpublished yamllint npm dependency and the no-op npm test.

Metrics / Benchmarks (if applicable)

• Before: npm test ran 0 tests. Structure validator exit 1. JSON validator exit 0 on a missing file. Link validator 53 broken links.
• After: npm test runs 31 tests, all pass. Structure validator exit 0. JSON validator exit 1 on a missing file. Link validator 50 broken links (remaining ones are outside this PR).

Verification

• Unit tests pass locally (node --test tests/*.test.cjs: 31 pass, 0 fail)
• Key flows smoke-tested: node scripts/validate-skill-structure.js exits 0, node scripts/validate-json.js exits 0 on the default files and 1 for a missing file, node scripts/validate-links.js runs network-free (258 links, 169 external and 17 anchors reported as not checked)
• No user-visible diffs observed: not true for contributors. Hooks and lint:yaml now fail when yamllint or markdownlint is not installed.
• lint:yaml against a real yamllint install was not re-run for this PR (covered with a mock in tests)

Risk & Rollback

• Risk level: Medium for contributor tooling, Low for skill content.
• Known risk: npm run lint already failed on main against existing markdown violations. Pointing the globs at the real sub-skills/ folder means it now covers those files too, so once Lefthook is installed the pre-push lint step fails until that markdown debt is cleaned up or the rules relaxed. No file edited here gained a violation.
• Rollback plan: revert the commit. No data, release state or published artefact is touched.

Changelog

This repository has no CHANGELOG.md, and none of the meta:needs-changelog / meta:no-changelog labels exist here, so no changelog file entry is added. Notes for release automation if one is added later:

Added

• Fixture tests for the JSON, link and structure validators and for lint:yaml, run by npm test. (Relates to LS-4750)

Changed

• Validators, lint globs and docs now use sub-skills/ instead of skills/. (Relates to LS-4750)
• Link validation reports external links and anchors as not checked. (Relates to LS-4750)

Fixed

• The JSON validator no longer passes when a required file is missing.
• lint:yaml and the Lefthook hooks no longer pass when a required tool is missing.

Removed

• The non-existent yamllint npm dependency.

---

Checklist (Global DoD / PR)

• All AC met and demonstrated
• Tests added/updated (unit/E2E as appropriate)
• Accessibility checklist completed (where relevant):
  • Semantic HTML and heading order verified
  • Keyboard navigation and visible focus states verified
  • ARIA used only where needed
  • Contrast and non-colour cues reviewed (WCAG 2.2 AA)
• Docs/readme/changelog updated (if user-facing)
• Security checklist completed (where relevant):
  • Untrusted input validated and sanitised
  • Output escaped for its rendering context
  • Privileged actions enforce nonce and capability checks
  • No secrets/sensitive data introduced; OWASP risks reviewed
• Code/design reviews approved
• CI green; linked issues closed; release notes prepared (if shipping)

---

[linear-code]

LS-4750

LS-4768

[brandonmarshal]

Changes needed — static review only

Reviewed all 14 changed files at head da3b3efe38536dc4dcb4d9e221d5efe36a46b186 against base aa59481b1a80598a65dba1738e1c4603dad49e87.

• P1: The YAML hook’s Bash-only redirection under sh blocks valid YAML commits when the shell is dash.
• P2: All four new suites require node:test.after, introduced in Node18.8.0, while declared support starts at 18.0.0.

The layout repairs, missing-JSON failure, network-free link implementation and 31 fixture definitions are statically supported. Tests exercise the npm YAML command, not the actual hook; only three validator suites assert unchanged fixture contents.

No commands were executed. Reported passing results remain author evidence. No subsequent stack layer was reviewed.

[brandonmarshal]

Review fixes pushed (e15b719)

• P1, YAML hook: &> replaced with >/dev/null 2>&1 in .lefthook.yml. Four new tests run the real hook command with a mock yamllint (installed, missing, failing, no bash-only redirection) and fail against the old hook.
• P2, Node 18: after() replaced with process.on('exit') in the four suites. The Node range is unchanged.

Checked in a throwaway copy: node --test tests/*.test.cjs gives 35 pass, 0 fail (31 before, plus 4 hook tests). Both review threads are replied to and resolved.

🤖 Generated with Claude Code