[lint coverage] ~75 library projects are unlinted by CI + their eslint configs reference uninstalled plugins #419

Closed
opened 2026-07-05 21:33:02 +00:00 by spikerj · 5 comments
Owner

Verified finding (surfaced while getting master to a green working build)

CI's eslint gate covers only a curated 7-project set, leaving the bulk of the workspace's library code with no eslint coverage — and those uncovered libs reference eslint plugins that aren't installed, so they'd fail immediately if added to the lint set.

Verified

  • CI lint scope (main.yml:210pnpm run lint):
    lint = lint:scripts && lint:styles && prettier -c ., where
    lint:scripts = nx run-many -t lint -p spikersoft tools spikersoft-theme spikersoft-environment keycloak-admin marks-site-models spikersoft-models7 projects only. The libraries/features/*, libraries/platform/*, libraries/shared/*, libraries/ui/*, libraries/game/* projects (~75) are not eslint-linted in CI.
  • Running nx run-many -t lint --all fails on ~75 projects with "Definition for rule 'X' was not found"sonarjs/todo-tag, sonarjs/cognitive-complexity, @typescript-eslint/prefer-readonly, @typescript-eslint/no-unused-vars, @typescript-eslint/prefer-for-of, @typescript-eslint/no-empty-function.
  • Root cause: eslint-plugin-sonarjs and typescript-eslint (the flat-config meta) are not declared in package.json (confirmed via git log -S — never were) and are absent from the pnpm store; yet lib configs/sources reference their rules (e.g. the vendored libraries/game/wasm-voxel Minecraft port).
  • The 7 curated projects pass (verified nx lint spikersoft clean), so CI is green — this is latent, not a current CI break.

Why it matters (for a platform aimed at millions of students)

The majority of shared/feature/platform library code gets no static-analysis gate — regressions in those libs (unused vars, complexity, etc.) sail through. And the lib eslint configs are themselves broken (reference uninstalled plugins), so "just add them to the lint set" fails until the deps are reconciled.

Options (your call — may be a deliberate speed tradeoff)

  1. Intentional → close this; the curated set is the accepted gate.
  2. Expand coverage → add eslint-plugin-sonarjs + typescript-eslint to package.json, reconcile the lib flat-configs, and widen lint:scripts (or switch to nx affected -t lint) so libraries are gated. Best done incrementally (it will surface real findings).

Severity: Low–Medium (no current breakage; a coverage gap). Not auto-fixing — expanding lint coverage + adding deps is a decision with real churn.

## Verified finding (surfaced while getting master to a green working build) CI's eslint gate covers only a **curated 7-project set**, leaving the bulk of the workspace's library code with no eslint coverage — and those uncovered libs reference eslint plugins that aren't installed, so they'd fail immediately if added to the lint set. ### Verified - **CI lint scope** (`main.yml:210` → `pnpm run lint`): `lint` = `lint:scripts && lint:styles && prettier -c .`, where `lint:scripts` = `nx run-many -t lint -p spikersoft tools spikersoft-theme spikersoft-environment keycloak-admin marks-site-models spikersoft-models` — **7 projects only**. The `libraries/features/*`, `libraries/platform/*`, `libraries/shared/*`, `libraries/ui/*`, `libraries/game/*` projects (~75) are **not** eslint-linted in CI. - Running `nx run-many -t lint --all` fails on ~75 projects with **"Definition for rule 'X' was not found"** — `sonarjs/todo-tag`, `sonarjs/cognitive-complexity`, `@typescript-eslint/prefer-readonly`, `@typescript-eslint/no-unused-vars`, `@typescript-eslint/prefer-for-of`, `@typescript-eslint/no-empty-function`. - Root cause: **`eslint-plugin-sonarjs` and `typescript-eslint` (the flat-config meta) are not declared in `package.json`** (confirmed via `git log -S` — never were) and are absent from the pnpm store; yet lib configs/sources reference their rules (e.g. the vendored `libraries/game/wasm-voxel` Minecraft port). - The 7 curated projects **pass** (verified `nx lint spikersoft` clean), so **CI is green** — this is latent, not a current CI break. ### Why it matters (for a platform aimed at millions of students) The majority of shared/feature/platform library code gets **no static-analysis gate** — regressions in those libs (unused vars, complexity, etc.) sail through. And the lib eslint configs are themselves broken (reference uninstalled plugins), so "just add them to the lint set" fails until the deps are reconciled. ### Options (your call — may be a deliberate speed tradeoff) 1. **Intentional** → close this; the curated set is the accepted gate. 2. **Expand coverage** → add `eslint-plugin-sonarjs` + `typescript-eslint` to `package.json`, reconcile the lib flat-configs, and widen `lint:scripts` (or switch to `nx affected -t lint`) so libraries are gated. Best done incrementally (it will surface real findings). **Severity:** Low–Medium (no current breakage; a coverage gap). Not auto-fixing — expanding lint coverage + adding deps is a decision with real churn.
Author
Owner

First slice up: angular PR #172 — CI lint coverage goes from the curated 7 projects to 66 (nx run-many -t lint --all with a 9-project exclude list, so new libraries are covered by default and the list can only burn down).

Reality check vs. this ticket's snapshot: after installing the two missing packages (eslint-plugin-sonarjs, typescript-eslint), only 8 of 75 projects have real violations (148 errors total, 78 of them in the vendored wasm-voxel) — the "~75 failing" was almost entirely the unresolvable-rule crash, not real lint debt.

Remaining tail for this ticket:

  1. spikersoft-webnx lint crashes silently with no output (likely OOM); needs its own investigation before it can join the set.
  2. wasm-voxel (78) — vendored code; probably wants a per-project rule override rather than edits.
  3. platform-language-runner (17), feature-games-clue-for-sql (19) — real violations, mechanical but non-trivial.
  4. Five small ones (3–6 errors each) — mostly directive-selector prefix renames = public API changes deserving deliberate PRs: platform-activity-tracking, shared-lesson-panes, feature-child-account-dialog, feature-fundraiser, feature-sponsor.
First slice up: **angular PR #172** — CI lint coverage goes from the curated 7 projects to **66** (`nx run-many -t lint --all` with a 9-project exclude list, so new libraries are covered by default and the list can only burn down). Reality check vs. this ticket's snapshot: after installing the two missing packages (`eslint-plugin-sonarjs`, `typescript-eslint`), only **8 of 75** projects have real violations (148 errors total, 78 of them in the vendored `wasm-voxel`) — the "~75 failing" was almost entirely the unresolvable-rule crash, not real lint debt. **Remaining tail for this ticket**: 1. `spikersoft-web` — `nx lint` **crashes silently with no output** (likely OOM); needs its own investigation before it can join the set. 2. `wasm-voxel` (78) — vendored code; probably wants a per-project rule override rather than edits. 3. `platform-language-runner` (17), `feature-games-clue-for-sql` (19) — real violations, mechanical but non-trivial. 4. Five small ones (3–6 errors each) — mostly `directive-selector` prefix renames = public API changes deserving deliberate PRs: `platform-activity-tracking`, `shared-lesson-panes`, `feature-child-account-dialog`, `feature-fundraiser`, `feature-sponsor`.
Author
Owner

Correction on the spikersoft-web exclude note in PR #172: a completed solo run shows it fails lint normally (exit 1 with failed task, not a silent crash — my earlier check just caught it mid-flight; it is simply a very long lint). Error detail wasn't captured in that run's tail; when someone picks up the tail-item, start with pnpm nx lint spikersoft-web and budget several minutes. The exclude itself stands either way.

Correction on the `spikersoft-web` exclude note in PR #172: a completed solo run shows it **fails lint normally** (exit 1 with failed task, not a silent crash — my earlier check just caught it mid-flight; it is simply a very long lint). Error detail wasn't captured in that run's tail; when someone picks up the tail-item, start with `pnpm nx lint spikersoft-web` and budget several minutes. The exclude itself stands either way.
Author
Owner

Slice 2 up: angular PR #173wasm-voxel is back under lint, coverage 66 → 67.

The real mechanism behind its 78 "errors": nearly all were Definition for rule not found — the vendored port carries inline eslint-disable comments naming sonarjs/* / @typescript-eslint/* rules, and in flat config an unregistered namespace makes every such comment a hard error before any linting happens. Slice 1's package install was necessary but not sufficient; slice 2 adds namespace-only plugin registration in the root config (resolves rule ids, enables nothing).

Also in the PR: a documented vendored-code style override for the port (correctness rules still apply; explicitly marked not-a-template for first-party code), and proper fixes for the 3 real errors, which were in the first-party editor components (inject() migration; keyboard-accessible texture picker).

Remaining excludes (7) are all genuine violations — spikersoft-web (long run), platform-language-runner (17), feature-games-clue-for-sql (19), and the five directive-selector-prefix ones that are public-API renames.

Slice 2 up: **angular PR #173** — `wasm-voxel` is back under lint, coverage **66 → 67**. The real mechanism behind its 78 "errors": nearly all were **`Definition for rule not found`** — the vendored port carries inline `eslint-disable` comments naming `sonarjs/*` / `@typescript-eslint/*` rules, and in flat config an *unregistered namespace* makes every such comment a hard error before any linting happens. Slice 1's package install was necessary but not sufficient; slice 2 adds **namespace-only plugin registration** in the root config (resolves rule ids, enables nothing). Also in the PR: a documented vendored-code style override for the port (correctness rules still apply; explicitly marked not-a-template for first-party code), and proper fixes for the 3 real errors, which were in the *first-party* editor components (`inject()` migration; keyboard-accessible texture picker). Remaining excludes (7) are all genuine violations — `spikersoft-web` (long run), `platform-language-runner` (17), `feature-games-clue-for-sql` (19), and the five directive-selector-prefix ones that are public-API renames.
Author
Owner

spikersoft-web characterization (the last unexamined exclude): it passes lint clean — 0 errors, 37.5s standalone (282% CPU; it's just the biggest single lint target). The earlier standalone exit-1 and the two full-gate silent deaths all happened while several heavy lint workers ran concurrently — i.e. resource contention/OOM on the runner, not lint debt.

Plan: after #173/#174 land (they both edit the exclude line), a final small PR removes spikersoft-web from the excludes — ideally with --parallel=2 or nx's NX_PARALLEL tuned for the CI runner so the gate doesn't OOM with the big target back in the set. That would leave the exclude list at exactly the five directive-selector projects (public-API renames, deliberate PR pending Joey's nod) — from ~75 uncovered projects at ticket filing to 5, all with documented reasons.

`spikersoft-web` characterization (the last unexamined exclude): **it passes lint clean** — 0 errors, 37.5s standalone (282% CPU; it's just the biggest single lint target). The earlier standalone exit-1 and the two full-gate silent deaths all happened while several heavy lint workers ran concurrently — i.e. resource contention/OOM on the runner, not lint debt. Plan: after #173/#174 land (they both edit the exclude line), a final small PR removes `spikersoft-web` from the excludes — ideally with `--parallel=2` or nx's `NX_PARALLEL` tuned for the CI runner so the gate doesn't OOM with the big target back in the set. That would leave the exclude list at exactly the five directive-selector projects (public-API renames, deliberate PR pending Joey's nod) — from ~75 uncovered projects at ticket filing to 5, all with documented reasons.
Author
Owner

Board-sweep verified complete: package.json lint:scripts is now 'nx run-many -t lint --all' (tree-verified) after the burn-down chain PRs #172/#173/#174/#176 (the last re-includes spikersoft-web, titled 'final #419 burn-down'). Closing.

Board-sweep verified complete: package.json lint:scripts is now 'nx run-many -t lint --all' (tree-verified) after the burn-down chain PRs #172/#173/#174/#176 (the last re-includes spikersoft-web, titled 'final #419 burn-down'). Closing.
Sign in to join this conversation.