fix: six bugs found by using 0.4.0 on real builds - #21
Merged
Conversation
Each of these was reached by running the tool rather than reading it, and each is a case where a report said something that was not true. audit --browser audited nothing unless it was given a directory. The runner takes the build directory so it can serve the pages over loopback; passing nothing is how a caller says these pages came off a running site. The audit command passed the directory the user typed, which under auto-detection is nothing — so `eaa-kit audit --browser`, the form the README leads with, navigated Chromium to a filesystem path and failed every page. The collection stage now reports the directory it settled on. A baseline stopped suppressing a barrier when anything else on the page changed, and diff reported that same untouched barrier as fixed. The element fingerprint hashed the whole outerHTML, and for the document-level rules the failing element is <html>, whose outerHTML is the entire page. It now covers the opening tag only; axe-core's selector already separates elements that share one. This moves the baseline and JSON report schema versions to 2 and the SARIF fingerprint key to eaaKit/v2, because reading either across the boundary produces exactly the falsehood being fixed. A run where every page failed opened its summary with "No violations" in green. The count was of pages attempted rather than pages audited. A browser run reported criteria as unevaluable and then advised --browser, because the coverage view consulted jsdom's blind-spot table without asking which engine had run. A filter that excluded every page was reported as an empty build directory, with framework advice naming another directory to audit. One unreachable page was counted in the singular and conjugated in the plural. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xCGmHyxdTMCF2GFwr4CAm
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each of these was reached by running 0.4.0 against real builds — a static site, an auto-detected project, a crawled server, both engines — rather than by reading the code. Every one is a case where a report stated something that was not true.
The two that matter most
audit --browseraudited nothing unless it was handed a directory.The browser runner takes the build directory so it can serve the pages over loopback; passing nothing is how a caller says these pages came off a running site and already have somewhere to be fetched from. The audit command passed the directory the user typed — which under auto-detection is nothing, because auto-detection is the thing that works it out. So
eaa-kit audit --browser, the form the README leads with, skipped the loopback server and navigated Chromium to/home/you/site/dist/index.html: a filesystem path, not a URL.Every page came back
Cannot navigate to invalid URL.--browser ./distwas never affected, which is why the suite never saw it: every browser test named a directory. The collection stage now reports the directory it settled on, and bothauditandbaselinepass that along. A real crawl still passes nothing — serving a crawled page back out of a copy on disk would audit the markup with the server that produced it cut out of the picture.A baseline stopped suppressing a barrier when anything else on the page changed, and
diffcalled that same untouched barrier fixed.The identity of a violating element hashed the element's whole outer markup. For the document-level rules —
html-has-lang,document-title, everything that fails against<html>— the element's outer markup is the entire page. Adding one paragraph gave them a new identity, and all three consumers believed it:The page still had no
<title>. Alongside that, the baseline went red on barriers a team had already accepted (5 accepted → 3 accepted and 3 "new" from one added<img>), and SARIF churned its fingerprints so code scanning closed an alert and opened an identical one on every edit.The hash now covers the element's opening tag and not its descendants. Where two elements share an opening tag, axe-core's selector already tells them apart — it qualifies an ambiguous match with
:nth-child(…)— so nothing that could be distinguished before stops being distinguishable, and a barrier that is genuinely fixed still changes identity. Verified both directions: an unrelated edit now yields0 new · 0 fixed · 5 unchanged, while addinglangand a<title>still yields0 new · 2 fixed.The other four
A run where every page failed reported "No violations". The count in that sentence was of pages attempted, not pages audited, so a run that could read none of them opened its summary with a clean result in green — the exact fail-open the rest of the tool is built to refuse. It now says nothing was audited, and where only some pages failed the counts are over the pages actually read.
A browser run reported criteria as unevaluable and then advised
--browser. The coverage view consulted the table of rules jsdom is structurally blind to without asking which engine had run, so a report produced in real Chromium said2.1.1 Keyboard — this engine could not evaluate it (--browser would answer this). It now believes a browser run's own results.A filter that excluded every page was reported as an empty build directory, together with framework advice naming another directory to audit — a fix for a path that was never wrong.
--include/--excludematching nothing now says so and echoes the patterns.One unreachable page was counted in the singular and conjugated in the plural: "1 page could not be reached, and were not audited".
Contract versions
The fingerprint change moves both file contracts, by the project's own rule that a field changing meaning bumps the version:
schemaVersionschemaVersioneaaKit/v1eaaKit/v2Neither is read across the boundary, and that is deliberate rather than incidental: matching 0.4.0 baseline entries against the new rule would suppress nothing while looking as though it had, and diffing a 0.4.0 report against a newer one is precisely the comparison that reports every document-level barrier as both new and fixed. Both refusals name the command that resolves them. Checked-in examples and the docs are regenerated.
Validation
pnpm test— 49 files, 1061 passing, 2 skipped, including the browser suite against real Chromiumpnpm lint,pnpm typecheck,pnpm smoke,pnpm test:packagedall cleantests/audit/fingerprint.test.ts,tests/cli/pages.test.ts, plus cases in the coverage, console-report, CLI-audit, CLI-baseline and Playwright suites--browserregression test was confirmed to fail against the old code before the fix, not just to pass after it🤖 Generated with Claude Code
https://claude.ai/code/session_012xCGmHyxdTMCF2GFwr4CAm
Generated by Claude Code