[ChristianMurphy]

Replaces validate-output.sh with a validator that reads the page as the tree a browser builds and queries it with CSS selectors. It keeps the arguments, the exit codes, and the pass: and FAIL: lines. Closes #21.

The skill ships the validator as one built file, scripts/validate-output.js, with its packages inlined, so there's no npm install step and nothing to download.

What changes

• src/validate-output.ts: the source. It parses with parse5 and queries with css-select. Each check is a named function that returns its result. Text checks use string parsing and Intl.Segmenter, not regular expressions.
• skills/explain-diff-html/scripts/: what the skill ships.
  • validate-output.js is the bundle, built by npm run build. It runs on Node 22.18 or later.
  • THIRD-PARTY-LICENSES.txt carries the license of each inlined package. The build writes it too.
  • package.json holds only { "type": "module" }. Without it, Node takes the module type from the user's project, and a CommonJS project can't load the bundle.
• vite.config.ts: Vite+ config. vp pack builds the bundle, and a small inline plugin writes the license notices. vp check runs oxfmt, oxlint with warnings denied, and type checks. vp test runs Vitest.
• tests/validate-output/: a page in pass/ must pass every check, and a page in fail/<check id>/ must fail that check and no other. To add a case, add a page. The command-line tests run the bundle and check the pass:, FAIL:, and usage: lines, not only the exit code. One test copies the skill into a CommonJS project and runs it there.
• .github/workflows/checks.yml: check, then rebuild and fail when scripts/ differs from what's committed. Run the tests on Node 22.18.0 and 24. Lint the workflows with actionlint and audit them with zizmor. The samples workflow doesn't install anything.
• Docs: the skill file, references/validation.md, README, and CONTRIBUTING name the single node command and the 22.18 floor. SKILL.md checks the Node version up front. Step 7 explains a usage: line, and treats output with no pass:, FAIL:, or usage: line as a setup failure.

Working on the validator needs a Node version Vite+ supports, so the root engines field says ^22.18.0 || ^24.11.0 || >=26.0.0. The shipped bundle doesn't need Vite+, so the skill's own floor stays at 22.18.

Behavior

On every sample, the output matches the shell script byte for byte, including the exit codes and the advisory list.

These cases change on purpose. The shell script got each of them wrong, and each one has a test page:

Page	Shell script	New script
<p class="filename wide">b.py:9</p>, unlinked	pass	fail
An unlinked reference, plus a spare a.srcref link elsewhere	pass	fail
A Markdown line link with ?notplain=1	pass	fail
A Markdown link written README.md#L3?plain=1	pass	fail
<p data-bare="outside-repo" class="filename">, with the attribute before the class	fail	pass
A <script> inside the quiz that holds "the first option"	fail	pass
<p class="filename" data-bare="outside-repo"><code>main.py:8</code></p>	fail	pass
A Markdown heading anchor such as README.md#license	fail	pass

Review turned up a couple more, also with test pages:

• Only an a.srcref counts as a link. A <span class="srcref"> around a reference fails.
• A Markdown link with a column anchor such as #L3C5-L4C9 and no ?plain=1 fails. A heading anchor such as #l1cache still passes.

The unlinked-reference failure lists each reference after #18's line, so the agent can see what to fix.

Dependencies

The bundle inlines these packages, and their licenses ship in scripts/THIRD-PARTY-LICENSES.txt:

Package	Version	License
parse5	8.0.1	MIT
parse5-htmlparser2-tree-adapter	8.0.1	MIT
css-select	7.0.0	BSD-2-Clause
domutils	4.0.2	BSD-2-Clause
domhandler	6.0.1	BSD-2-Clause
css-what	8.0.0	BSD-2-Clause
domelementtype	3.0.0	BSD-2-Clause
entities	8.1.0	BSD-2-Clause
nth-check	3.0.1	BSD-2-Clause
dom-serializer	3.1.1	MIT
boolbase	2.0.0	ISC

The root package.json holds the development tools, which never reach the installed skill:

Package	Version	License
vite-plus	1.1.0	MIT
@types/node	22.20.5	MIT

vite-plus 1.1.0 brings oxlint 1.87.0, oxfmt 0.72.0, and Vitest 5.0.3. It's pinned at 1.1.0 rather than 1.0.0 because 1.0.0 carries the advisory GHSA-r825-w8mm-5pxw in its built-in launch-editor copy. npm audit is clean.

How I checked it

• npm run check passes.
• npm test passes all 57 tests on Node 22.18.0 and on Node 24.20.0.
• sh .github/scripts/validate-samples.sh passes every sample on both versions.
• actionlint and zizmor pass on .github/workflows/.
• Rebuilding gives byte-identical files, and the CI freshness step fails when the source changes and the bundle doesn't.
• For each branch the reviews listed, I removed the branch and a test failed. The exception is template in the quiz's skipped tags. The tree adapter puts template content under a fragment node, which the text walk already skips, so that entry can't change the result. Its page still pins the behavior.
• Without scripts/package.json, the CommonJS test fails with "Cannot use import statement outside a module".
• The bundle runs on Node 20.20.2. On Node 18.20.8 it exits 1 without printing a pass: or FAIL: line.
• Not run by me: a fresh npx skills add install. In the first round, @malav2110 confirmed it copies the files in scripts/, including the lockfile that was there at the time.

Follow-ups

• CI doesn't check the Mermaid diagrams in finished pages. A follow-up could run pinned maid on each pre.mermaid block.

[malav2110]

Thanks so much for this, @ChristianMurphy ! Going with Node and adding a test page for each case covers everything I asked about in #21.

It's a big change, so it'll take me some time to review properly. I'll get back to you once I've been through it.

[malav2110]

Hey @ChristianMurphy, #18 is merged now (squashed as 16ed6df), so this is ready for the rebase you mentioned.

One behavior from that round needs to come across too. #18 now requires the extension to start with a letter, so <code>127.0.0.1:8000</code> isn't counted as a reference. isReferenceText takes 1 as the extension, so it counts it and fails the page. Could you add that, with a pass page for it?

[malav2110]

Hey @ChristianMurphy, are you still up for the rebase now that #18 is in? If it's easier, I can resolve the conflicts on your branch. Which would you prefer?

[malav2110]

Thanks for the rebase and the letter rule, @ChristianMurphy! I'll go through the full review next and get back to you.

[malav2110]

Thanks, @ChristianMurphy. I re-ran everything under How I checked it and got the same results, and the output matches the shell script on all nine samples. A fresh npx skills add from this branch ships the lockfile, and the validator runs from the installed copy.

I've left some comments inline. The main one is the open question from #21 about bundling the validator, since the answer changes a few of the others. Two more are blocking: where the install puts node_modules, and what step 7 tells the agent after a usage error. The rest are questions and small suggestions.

[ChristianMurphy]

Thanks taking a look, @malav2110. I pushed 8b176d6, which answers every comment below.

The main change follows the bundling question. The skill now ships one built file, scripts/validate-output.js. The file carries its packages inline, and scripts/THIRD-PARTY-LICENSES.txt carries their licenses. An installed copy does not run an install, reach the registry, or create a node_modules folder. That retires the .gitignore comment, the stale-install question, and the README registry note. The Node floor is now 22.18.

[malav2110]

Thanks, @ChristianMurphy. The bundle addresses every comment from the last round, and a clean build reproduces the committed bundle and notices byte for byte.

One blocking issue is left, inline: the bundle fails to load in a project whose package.json says "type": "commonjs". The rest are non-blocking. Could you also update the PR description? It still describes the first-use npm ci, scripts/package.json, and the Node 24.12 floor.

[ChristianMurphy]

Thanks, @malav2110. 8644489 fixes the CommonJS load and picks up the rest of the suggestions. I also rewrote the PR description, so it now covers the bundle, the scripts/package.json, and the 22.18 floor.

[malav2110]

Thanks, @ChristianMurphy. The CommonJS fix works, and everything from the last round checks out. A clean build reproduces the committed bundle, and all 57 tests and nine samples pass. I've left a few non-blocking comments, mostly on SKILL.md text that still describes the old validator, plus an optional test for the advisory list. Once those are addressed, this is good to merge from my side.

[malav2110]

suggestion (non-blocking): Lines 110-111 still say a run with no Mermaid diagram "needs no Mermaid, and therefore no Node". That sentence was true when this table row said Node was only for pages with a Mermaid diagram. Now step 7 runs the validator on every page, and the check at line 52 asks for Node 22.18 on every run. Something like:

A run whose change warrants none of those four shapes needs no Mermaid. The
hand-built families cover it. Step 7 still needs Node to run the validator.

[malav2110]

suggestion (non-blocking): Lines 590-591 of SKILL.md still describe the shell script. They say it reads the quiz "from its tag to </main>", so anything after the quiz is checked as quiz text. This function reads only main #quiz, so a section after the quiz now gets no quiz check. A page with "The first option is wrong." after the quiz fails on main and passes here. The advice to keep the quiz last still holds; only the reason changed:

Keep the quiz as the last section in `<main>`, where the template puts it, and
add no section after it. The validation script reads only the element whose `id`
is `quiz`, so text placed after it gets none of the quiz checks.

validation.md already says this, so only SKILL.md needs it.

[malav2110]

suggestion (non-blocking): The advisory list itself has no test. The test at this line only matches the advisory: header. No pass page has an ordinal in its visible quiz text, so a change that always printed none would keep the suite green. A pass page whose quiz says "Which is the second step in the flow? Pick one." prints the second step in the flow? Pick one under the header:

test("the advisory lists each ordinal with the rest of its sentence", () => {
  const { stdout } = runScript(join(passFolder, "advisory-ordinal.html"), sha);
  expect(stdout).toMatch(/^the second step in the flow\? Pick one$/m);
});