Skip to content

feat(ci): frontend eslint no-undef gate — catches renamed-function-caller class of bugs (fixes #1342)#1344

Open
Kpa-clawbot wants to merge 2 commits into
masterfrom
fix/issue-1342
Open

feat(ci): frontend eslint no-undef gate — catches renamed-function-caller class of bugs (fixes #1342)#1344
Kpa-clawbot wants to merge 2 commits into
masterfrom
fix/issue-1342

Conversation

@Kpa-clawbot
Copy link
Copy Markdown
Owner

TDD: red commit 03ea965 (canary undef var → CI fails) → green commit b514aeb (canary removed → CI passes). CI URL appears in the Checks tab once GitHub Actions queues this branch.

Fixes #1342

What ships

  • .eslintrc.json at repo root — eslint 8 legacy-config format. no-undef: error, no-unused-vars: warn (with ^_ allowlist).
  • CI step in .github/workflows/deploy.yml (job go-test, after JS unit tests, before proto + Playwright): npm install --no-save eslint@8 && npx eslint public/*.js. --no-save keeps node_modules and package-lock.json out of the tree (already gitignored).
  • One pre-existing fix in public/map.js: typeof esc === 'function'typeof globalThis.esc === 'function'. esc is a local IIFE var in 5 other files, never exported as a true global; the optional lookup was structurally invalid under no-undef. Behavior unchanged.

How this would have caught #1318 / PR #923

PR #923 renamed drawAnimatedLine, updated one caller in public/live.js, missed the other — leaving a reference to the undefined hash var. Playwright didn't hit that path. Reverting #1325 locally (re-introducing the bug) → eslint flags hash as no-undef → red. With the gate in place, #923 never lands.

The "quiet pile of globals" reality

The config declares 257 globals. They were discovered by walking public/*.js for two patterns:

  1. window.X = ... assignments (the explicit exports — 168 of them)
  2. Top-level function/const/let/var declarations in non-IIFE files (the implicit exports — Go-style cross-file linking via shared HTML <script> order)

Plus 9 vendor/runtime names (L, Chart, QRCode, qrcode, module, global, process, require, exports, __filename, __dirname) for dual-runtime files like url-state.js, packet-filter.js, hash-color.js, filter-ux.js that are also require()-d by Node tests.

This is honest documentation of an architectural reality, not a workaround. Future refactor → modules will collapse this list.

Latent bugs discovered

Zero no-undef errors against the current public/*.js tree after globals were enumerated honestly. The would-be-#1318-class bug count today: 0. The gate's job is forward-looking — block the next one.

Out of scope (acknowledged from acceptance criteria)

  • Inline <script> blocks in public/*.html — separate ticket.
  • Per-PR delta-coverage gate — separate ticket.
  • pr-preflight grep for arg-count mismatch — separate ticket.

Preflight

bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master → exit 0, clean.

openclaw-bot added 2 commits May 24, 2026 04:18
Adds the eslint frontend lint gate (.eslintrc.json + CI step in deploy.yml)
PLUS a synthetic canary file (public/_lint_canary.js) that references an
undefined variable. This commit is the RED commit per TDD discipline:

  - CI must FAIL on this commit (canary's undef var trips no-undef:error).
  - Verifies the gate is wired correctly and actually blocks merges.
  - The next commit removes the canary and CI goes GREEN.

Config notes:
  - 257 globals declared in .eslintrc.json — discovered by walking
    public/*.js for window.X = ... assignments AND top-level
    function/const/let/var declarations. Cross-file globals are a
    real (pre-existing) reality of vanilla-JS-no-build CoreScope.
  - Minimal fix in public/map.js: typeof esc -> typeof globalThis.esc
    (esc is a local IIFE var in other files, not a true global).
  - eslint@8 (legacy config). Did NOT migrate to flat-config / eslint@9.

Discovered via local lint run on master snapshot:
  - 0 latent undef-var bugs after globals were declared honestly
    (i.e., would-be #1318 candidates: none currently in tree).
  - 80 unused-var warnings (not blocking; argsIgnorePattern=^_ honored).

Refs #1342
Removes public/_lint_canary.js. With the canary gone, `npx eslint public/*.js`
exits 0 against the current tree (0 errors, 80 unused-var warnings — not blocking).

Mutation check: re-adding the canary re-trips the gate (verified locally).

This is the green commit that completes the red→green TDD cycle for #1342.

Fixes #1342
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