Skip to content

fix(cli): check only JS and TS files, gate on nothing checked, and summarize long runs - #180

Merged
dgesteves merged 2 commits into
mainfrom
fix/os-cli-summary-scope-and-summary
Oct 16, 2026
Merged

dgesteves merged 2 commits into
mainfrom
fix/os-cli-summary-scope-and-summary

Conversation

@dgesteves

@dgesteves dgesteves commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Roadmap item: os-cli-summary (OS2-36 + OS2-48, wave 2).

What was wrong

Files that are not JS or TS were parsed as TSX. check and check_ui read every file they were given as TSX. In a git copy of examples/shadcn-demo, check $(git ls-files) on main reported README.md 241 errors, app/globals.css 209, package.json 26, tsconfig.json 14 and every docs/*.md 48 to 86, all syntax, and exited 1. Globs did the same: check "app/**/*.css" gave 209 errors, 0 warnings in 1 file, and check "**/*" gave 827 errors, 3 warnings in 11 of 13 files checked. check_ui on the demo with { path: "app/globals.css" } answered ok: false with the same 209 syntax errors. With { code: "<template><button>x</button></template>", filename: "x.vue" } it answered ok: false with a prefer-design-system-component error, because it checked the Vue template as TSX.

A JavaScript app had nothing checked, and the gate passed. Folders expand to .tsx and .jsx files. In an app with components/ui/button.jsx and its pages in .js files (as Next.js's JavaScript template writes app/page.js), main gave:

$ onsystem check .
Nothing to check: 1 design-system file skipped. Pass --include-design-system to check it.   (exit 0)
$ onsystem check . --require-design-system --format github
                                                                                             (exit 0, no output)
$ onsystem check app
check: no files match app                                                                    (exit 2)
$ onsystem check app/page.js
2 errors, 1 warning in 1 file                                                                (exit 1)

A long run had no summary. check . in Dub's apps/web (corpus checkout, Dub 70c96f9) prints 14,228 lines on main, one per finding, ending with 11962 errors, 245 warnings in 1009 of 1477 files checked.

What changed

  • check checks only files that end in .tsx, .jsx, .ts, .js, .mts, .cts, .mjs or .cjs (any case), whether named outright or matched by a glob. The summary line counts the rest: (10 files that are not JS or TS skipped). In the demo copy, check "**/*" now gives 7 errors, 3 warnings in 1 of 3 files checked (5 design-system files skipped; 10 files that are not JS or TS skipped), and check "app/**/*.css" gives Nothing to check: 1 file that is not JS or TS skipped. (exit 0). If every file named outright is skipped, it prints Nothing to check: ... and exits 0. Before, a list with no match exited 2 with "no files match".
  • check_ui returns an error result for a path or filename that is not JS or TS: check_ui checks .tsx, .jsx, .ts and .js files; app/globals.css is not one, so it was not checked. The path input's description names the extensions. A path outside the root still gets the "outside the project root" error first. Both use one SCRIPT_FILE from src/util/paths.ts.
  • --require-design-system exits 2 when the folders and globs it is given leave no file to check: a JavaScript app whose pages are .js files, a design-system package whose files are all its own components (check --require-design-system: nothing was checked (1 design-system file skipped). Pass --include-design-system to check it.), or a glob that matches only styles. If only files named outright were passed, it does not fail: check README.md app/globals.css (a pull request that changes only styles and docs) has no UI code to check, and a lint-staged run on a commit that only touches components/ui should still pass.
  • When folders are given, check finds the .js files in them that contain JSX. It leaves out what git ignores, what exclude covers (tests by default), the design system's own files, anything a glob already names, *.min.js, files over 512 KB and files it cannot read. It names the files it found and the command that checks them. When that is all there is, the notice takes the place of the --include-design-system hint, which would have checked the button and passed with the pages still unchecked:
$ onsystem check .
Nothing to check: 1 design-system file skipped. 2 .js files have JSX (app/layout.js and app/page.js), but folders are checked for .tsx and .jsx files, so they were not checked. Name them to check them: `check . "**/*.js"`.   (exit 0)
$ onsystem check . --require-design-system
check --require-design-system: nothing was checked (1 design-system file skipped). 2 .js files have JSX (app/layout.js and app/page.js), ... Name them to check them: `check . "**/*.js"`.   (exit 2, one line on stderr)
$ onsystem check app
check: nothing to check in app. 2 .js files have JSX (app/layout.js and app/page.js), ... Name them to check them: `check app "app/**/*.js"`.   (exit 2)

The gate failure is one line because the GitHub Action puts only the first stderr line in its error annotation. Run locally with action/index.mjs on that app, the annotation is now ::error title=onsystem::onsystem check exited with 2: check --require-design-system: nothing was checked (1 design-system file skipped). 2 .js files have JSX ..., and paths: . **/*.js checks the pages (2 errors, 1 warning). Next to files that were checked, the notice goes to stdout in pretty output, as a ::warning with --format github, and to stderr with JSON or SARIF. It works per project at a monorepo root.

  • In pretty output, a run with 50 findings or more ends with the count per rule and the ten most common Tailwind palette classes, each with the token to use. Pairs are (nearest token) when the rule says "Nearest token" rather than "Matches token", and no close token when the theme has none. JSON, SARIF and GitHub output do not change. On Dub's apps/web the end of the output is now:
By rule: no-hardcoded-color 11,155, prefer-design-system-component 1,000, icon-button-accessible-name 52.
Tailwind palette classes: 9,200 in 943 files. The most common, with the token to use:
  1,023× border-neutral-200 → border-border-subtle
    902× text-neutral-500 → text-content-subtle
    736× text-neutral-900 → text-content-emphasis
    515× bg-neutral-200 → bg-bg-emphasis
    510× text-neutral-700 → text-content-default
    439× text-neutral-600 → text-content-default (nearest token)
    423× border-neutral-300 → border-border-default
    381× bg-neutral-50 → bg-bg-muted
    379× bg-neutral-100 → bg-bg-subtle
    326× text-neutral-800 → text-content-emphasis (nearest token)

11962 errors, 245 warnings in 1009 of 1477 files checked (7 image or email files not checked)

The ten pairs cover 5,634 of the 9,200 palette classes (61%). The output grows from 14,228 to 14,241 lines. Diffing the two outputs shows only the 13-line block above.

  • docs/ci.md covers the skipped files, the .js notice, the wider --require-design-system gate and the summary. A test checks the summary example in the docs against real output. action.yml and docs/required-check.md say that paths takes globs without quotes (paths: . **/*.js for a JavaScript app) and that require-design-system also fails when paths leaves nothing to check.
  • Changeset: minor, since --require-design-system, which the Action turns on by default, can now fail a job that passed before.

The cost of the .js scan

Every check <folder> reads the .js files under the folder. A file is parsed only if it has </ or /> and an element where only JSX can start one (after (, =>, =, return and the like, with a closing tag or a /> after it). Over the .tsx, .jsx and .js files of the corpus checkouts, 12,771 have an element by the TypeScript parse, and that check passes all 12,771. Timings, three runs each unless noted:

check . over main first commit of this PR now
400 library .js files copied from node_modules into public/js (96 MB; 135 have </, 48 are over 512 KB) 0.50 to 0.60 s 4.18 to 4.37 s 0.98 to 1.03 s
3,000 synthetic .js files cut from those (71 MB), two runs 0.54 to 0.63 s 5.22 to 5.55 s 1.25 to 1.29 s
Dub apps/web, two runs 4.14 to 4.43 s 4.45 to 4.46 s

Of the 352 library files under 512 KB, 122 have </ or />, and 20 of those are still parsed (mostly React's own builds, whose comments and strings hold JSX examples). None is reported as having JSX.

Choices that differ from the briefs

  • .js files are named, not checked. The finding's first option was to add .js to folder globs and check only the files that contain JSX. I chose its fallback (a notice, plus the gate) for three reasons. Checking them would give teams with a baseline new findings without a choice. The hook (plugins/onsystem/hooks/check-ui.mjs, which OS2-38 owns this wave) and the ESLint config (src/eslint/index.ts, outside this bundle) would still skip .js, so the surfaces would disagree. And corpus/snapshot.json may change only once per wave, in X2-37.
  • No folding of findings in pretty output. The prototype folded palette warnings into the by-class block to cut Dub's output to about 3,100 lines. That hides file and line locations in the format both people and agents read. Instead the summary sits at the end, so it is the last screen of a long run.
  • No token coverage line. Counting color classes that already use a token needs a count from the lint engine (src/lint/rules, outside this bundle). init (OS2-38's file this wave) is not changed either.
  • (nearest token) still comes from the message text. The summary looks for . Nearest token in the rule's message. A flag on the diagnostic would add a field to the JSON output and check_ui's results, and change src/lint/rules/hardcoded.ts, which other items edit this wave. test/check-summary.test.ts fails if the wording changes.

How it was tested

New tests:

  • test/check-scope.test.ts (12 tests): named files that are not JS or TS (pretty, JSON, the gate with named files only, uppercase .SCSS); globs that match them (app/**/*.css, alone and with the gate, **/*, a file both named and globbed counted once); .ts, .js, .mjs and .cjs named files still checked; the JS app (the notice in place of the hint, the command it gives, exclude, tests and files without JSX left out, the JSON, GitHub and pretty channels); the gate in the JS app (one line) and in a design-system package; a folder of .js pages (check app, exit 2 with the notice, and a folder with no page still "no files match"); JSX after a comment, an arrow, a generic call in props and in a fragment found, and HTML in strings, a named regex group, *.min.js and a file over 512 KB not counted; an unreadable .js file left out (skipped on Windows and as root); a pnpm monorepo root (including check apps/web/app and check "**/*"); and check_ui on app/globals.css, x.vue, x.mtsx, x.cjsx, a .mts filename, a .tsx path and a .js filename.
  • test/check-summary.test.ts (3 tests): the summary lines and where they go, the top-10 cut and ordering (including no close token), its absence from short runs and from JSON, SARIF and GitHub output, and the docs/ci.md example.

On main, 11 of the 12 scope tests fail; on the first commit of this PR, 8 do. The one that passes on main guards behaviour that must not change: named .ts, .js, .mjs and .cjs files are still checked.

Gate, all passing: pnpm install --frozen-lockfile --prefer-offline, pnpm format:check, pnpm lint, pnpm typecheck, pnpm test (74 files passed, 1 skipped; 969 tests passed, 3 skipped), pnpm build (no change to schema.json), pnpm smoke (passed).

Corpus: pnpm corpus on this branch prints Counts match corpus/snapshot.json., and the blind sample fp-sample-20261016 still has 80 of its 80 labels. No count changes.

Follow-ups

  • Hook regex and ESLint files for .js files with JSX: plugins/onsystem/hooks/check-ui.mjs (OS2-38 this wave) and src/eslint/index.ts. They should move together with folder globs if .js pages are to be checked rather than named.
  • Token coverage line in check, inspect and init (OS2-48, items 3 and 4), which needs the engine to count color classes that use tokens.
  • docs/tools.md (the check_ui row) could mention the JS/TS-only error.
  • src/detect.ts keeps its own script-file pattern (it also leaves out .d.ts); feat: read installed design-system packages that publish their sources #176 changes that file now.

…mmarize long runs

check parsed every file named outright as TSX, so the CSS, Markdown and
JSON that `check $(git ls-files)` passes gave hundreds of syntax errors
(209 for the demo's app/globals.css), and check_ui did the same for a
path or filename such as globals.css or x.vue. check now skips such a
file and counts it in the summary line; check_ui answers with an error.

A JavaScript app whose pages are .js files had nothing checked by
`check .`, which passed with --require-design-system. The gate now
fails when the folders and globs given leave no file to check, and
check names the .js files with JSX that folders left out, with the
command that checks them.

A run with 50 findings or more ends, in the default output, with the
count per rule and the ten Tailwind palette classes found most, each
with the token to use.
… .js pages when nothing was checked

- A glob such as `app/**/*.css` or `**/*` no longer parses styles, docs and
  JSON as TSX: they are counted with the named ones as "files that are not JS
  or TS skipped".
- When the only code left is `.js` pages with JSX, the "Nothing to check" line
  and the `--require-design-system` failure name them and the command that
  checks them, in place of the --include-design-system hint. The failure is
  one line, which is what the GitHub Action's annotation shows.
- A folder of `.js` pages (`check app`) names them instead of "no files match",
  and still exits 2.
- The JSX scan skips files it cannot read, `*.min.js` and files over 512 KB,
  and parses only files with an element where JSX can start one.
- One SCRIPT_FILE in src/util/paths.ts for check and check_ui, without the
  made-up `.mtsx` and `.cjsx` extensions.
- The changeset is a minor; action.yml and docs/required-check.md say how
  `paths` takes `. **/*.js` and when require-design-system fails.
@dgesteves
dgesteves merged commit 8052fc5 into main Oct 16, 2026
12 checks passed
@dgesteves
dgesteves deleted the fix/os-cli-summary-scope-and-summary branch October 16, 2026 07:57
@dgesteves-release dgesteves-release Bot mentioned this pull request Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant