refactor(validate-skills): extract lint rules into a shared lib

[nucliweb]

What

Extracts the skill validation rules out of the validate-skills.js CLI into a shared scripts/lib/skill-lint.js: a pure lintSkillContent() (no I/O) plus a thin lintSkill() file wrapper. validate-skills.js becomes a thin CLI over the lib.

Why

The rules were only reachable by spawning the CLI, so they could not be unit-tested directly. Extracting a pure function makes them importable and testable, which unblocks a follow-up test battery (linter self-tests + per-skill checks). This PR is the small, behavior-preserving first half of that split, kept separate to avoid a large refactor of a hot file landing together with new test code.

No behavior change

node scripts/validate-skills.js produces byte-identical output and the same exit codes as before, verified against all 24 skills. One intentional string update: the exemption-guard error now points contributors to scripts/lib/skill-lint.js (where the SECTION_EXEMPT_SKILLS allowlist now lives); that path only fires on an exemption-bypass attempt, so no real skill output changes.

Follow-up

PR-B will add the node:test battery, linter self-tests, and length metrics on top of this lib (supersedes the now-closed #324).

[nucliweb]

Hi @addyosmani @federicobartoli, could you take a look when you have a moment?

[addyosmani]

I like where this is headed. A pure lintSkillContent() with no I/O is the right shape and makes the rules testable on their own. My one hesitation is timing: this touches the same file as a couple of other in-flight script PRs (the naming/description enforcement, the draft test battery), so I want to sequence it rather than rebase three ways. Can you hold while I clear those, or rebase once they land?

[nucliweb]

Thanks, and that sequencing instinct is exactly why I split this out, so let me give you the current state, because I think the collision you are guarding against is already gone:

• The draft test battery (test: add skill validation battery with node:test #324) is closed. I closed it in favor of splitting it into this extraction plus a follow-up battery, so it is no longer in flight.
• The naming/description enforcement (feat(scripts): enforce naming + description-trigger rules in skill validator #323) already merged, and this PR sits on top of it (its base already has the kebab-case and trigger rules). So there is no three-way rebase with it; it is the base.
• Right now refactor(validate-skills): extract lint rules into a shared lib #379 is the only open PR touching scripts/validate-skills.js, and in fact the only open PR touching anything under scripts/.

So this is already the sequenced, on-top-of-merged-work small piece, not one of three racing edits. PR-B (the node:test battery + linter self-tests + metrics) will only come after this lands, so it stays one at a time.

Happy to hold if you would still rather clear the decks first, but there is nothing left for it to collide with that I can see. Shout if I am missing an in-flight PR you are tracking.

[federicobartoli]

Thanks for splitting this out, @nucliweb !! I took a deeper pass and I agree with the overall direction. The extraction is clean, the I/O boundary is much better, and the current CLI behavior is preserved.
There is one small API detail I think would be worth tightening in this PR: the policy collections (REQUIRED_SECTIONS, SECTION_EXEMPT_SKILLS, and the reference patterns) are exported by reference. A test or future consumer could accidentally mutate them and change subsequent lint results for the whole process. Keeping the canonical policy state private and exporting only the linting functions, or defensive snapshots, would preserve the intended purity and avoid order-dependent tests.
While exercising the new test seam, I also found a few useful cases for PR-B:

• The current frontmatter helper implements a YAML-like subset: it can accept malformed YAML or non-string descriptions, reject valid folded/commented YAML, and miss the real length of multiline descriptions.
• Section checks currently use substring matching, so headings inside fenced examples or ### headings can satisfy the rule. There is also an existing contract question to resolve: the docs describe these sections as recommended and allow equivalent headings, while the validator treats the exact headings as required.
• The cross-reference patterns miss some reference styles already used in the corpus and can inspect references inside fenced examples.
• A negative phrase such as “Do not use when…” currently satisfies the positive trigger check.
  These look like existing validator limitations rather than regressions introduced by this extraction, so I’m not suggesting we expand PR-A to solve them. I would just like to make sure they are captured in PR-B - or in a linked follow-up -before the new tests lock the current heuristics in as the contract.
  With the exported-policy state encapsulated, this looks good to me.

[nucliweb]

Thanks for the deep pass. Encapsulated the exported policy state in 495b5a8: the lib now exports only the functions (parseFrontmatter, extractSkillReferences, lintSkillContent, lintSkill), while REQUIRED_SECTIONS, SECTION_EXEMPT_SKILLS, SKILL_REF_PATTERNS, and the regexes stay private, so no shared state can be mutated by a test or future consumer. Validator output stays byte-identical.

On the heuristic limitations (YAML-subset frontmatter, substring section matching plus the recommended-vs-required contract question, missed or over-broad cross-ref patterns, and "Do not use when..." satisfying the positive trigger): agreed these are pre-existing validator behavior, not regressions from the extraction, and agreed they should be settled before PR-B's tests lock the current heuristics in as the contract. I'll capture them in a linked follow-up issue so PR-B references it and we decide bug-vs-intended per case first. Will link it here.

[nucliweb]

Captured as #387.