ReviewOS

stacks/stx

stx a11y reports a clean pass for any file path or glob — the argument shape its sibling commands take

#1918
Closed glennmichael123 opened this 14 days ago · 1 comment
14 days ago

stx a11y takes a directory, stx typecheck and stx codemod take a glob, and only one of them tells you when it matched nothing. Give a11y the argument shape its siblings document and it prints ✓ No accessibility issues found! and exits 0 — on a file that has three SERIOUS findings.

Measured on 0.2.175, against one real file (resources/emails/subscription-confirmation.stx, a layout table in an email template):

argumentresultexit
stx a11y resourcesTotal: 3 issues in 1 files1
stx a11y resources/emailsTotal: 3 issues in 1 files1
stx a11y resources/emails/subscription-confirmation.stx✓ No accessibility issues found!0
stx a11y 'resources/**/*.stx'✓ No accessibility issues found!0
stx a11y 'zzz/**/*.stx'✓ No accessibility issues found!0

The third row is the whole bug in one line: the exact path of a file with three findings, reported clean.

Compare the sibling commands on the same tree:

$ stx typecheck resources
  No .stx files matched resources

$ stx typecheck 'resources/**/*.stx'
  Checked 47 script block(s) and 275 template expression(s) in 37 file(s) — 109 error(s)

typecheck takes the glob and, given a directory, says it matched nothing. a11y takes the directory and, given a glob, says everything is fine. Both halves are inverted, and the inversion is silent in the direction that costs you.

Why this is worse than a wrong finding

A false positive gets triaged the first time someone reads it. This gets triaged never. The natural way to add the checker to CI is to copy the line that already works:

- run: stx typecheck 'resources/**/*.stx'
- run: stx codemod 'resources/**/*.stx'
- run: stx a11y 'resources/**/*.stx'      # green forever

That third line passes on every commit, for the life of the project, no matter what the markup does. Nothing distinguishes it from a project that has no accessibility problems.

It cost me directly: I have been quoting "a11y: 3" for a week, re-ran it after an upgrade with a glob instead of a directory, got 0, and wrote "down from 3" into a commit message before checking. The count had not moved. The argument had.

Cause

The path looks like it is joined into a scan pattern rather than tested. A file argument becomes <file>/**/*.stx, which matches nothing; zzz/**/*.stx becomes zzz/**/*.stx/**/*.stx, which also matches nothing; both then take the empty-set branch, which prints the success banner. A directory that does not exist throws ENOENT instead, so the failure mode is not even consistent with itself.

Suggested fix

Two independent changes, and the second matters more:

  1. Accept a directory, a file, or a glob in all three commands — whichever the user passes. typecheck and codemod already resolve globs; sharing that resolver is the smallest change.
  2. An empty match set is not a pass. typecheck's No .stx files matched <arg> is the right behaviour and should be what a11y does too — ideally non-zero exit, since a CI step that scanned nothing has not verified anything. The success banner should require having actually read at least one file.

Point 2 alone would have surfaced this the first time anyone pointed the command at a glob.

Environment

  • stx 0.2.175 (built from source at 6e29f859a6)
  • bun 1.3.14, macOS 15
  • app: a 38-view Stacks app, run as the e2e surface for stx
14 days ago

Fixed in c5965f1, both points — and point 2 is the one I treated as the real fix, as you argued.

Your table, re-run:

argumentbeforeafter
stx a11y resources4 issues, exit 14 issues, exit 1
stx a11y resources/emails4 issues, exit 14 issues, exit 1
stx a11y resources/emails/subscription-confirmation.stx✓ clean, exit 04 issues, exit 1
stx a11y 'resources/**/*.stx'✓ clean, exit 04 issues, exit 1
stx a11y 'zzz/**/*.stx'✓ clean, exit 0No .stx files matched, exit 1

Point 1 — one resolver now backs both a11y and typecheck, taking a directory, a file, or a glob. stx typecheck resources works now too. A named file is accepted whether or not it ends in .stx: naming one is an explicit request, and silently dropping it is the failure being removed.

Point 2 — results only ever contained files with violations, so an empty map meant both "everything is clean" and "nothing was scanned", and the banner was printed for both. scanA11yTargets now reports what it read, separately from what it found. Empty read set → No .stx files matched <arg> → exit 1. And the success banner states the count (✓ No accessibility issues found in 37 file(s)!), so a real pass no longer looks like a scan that never happened.

You were right that point 2 generalises. It also caught something while I was writing the tests: Bun.Glob is asymmetric — a relative pattern whose base directory is missing yields nothing, but an absolute one throws ENOENT. So stx a11y '/abs/nope/**/*.stx' would have crashed while stx a11y 'nope/**/*.stx' succeeded. Both now mean "no matches", which makes the failure mode at least consistent with itself — the same complaint you made about ENOENT-vs-success-banner.

One deliberate omission. I did not make typecheck exit non-zero on an empty match set. It already prints No .stx files matched, so it's not silent, and flipping its exit code would break anyone running stx typecheck in a repo that legitimately has no .stx files. Say the word if you want it aligned anyway — the argument for it is the same one you made.

Also: #1916 says a11y "exits 0 regardless". It doesn't — it exits 1 when issues are found, as your table shows. Worth knowing if you're relying on it being advisory.

Verification

11 tests. Sabotage-checked by restoring the path.join scan — 6 of 11 fail, including all three argument shapes and both empty-set cases.

Full suite 11857 pass. The 4 failures on that run (craft binary resolution ×2, X-STX-Runtime ×2) reproduce on a clean checkout of HEAD and are unrelated.

Sign in to comment on this issue.