Skip to content

fix: repair pnpm run lint and gate it in CI - #119

Merged
JohnMcLear merged 3 commits into
mainfrom
ci/repair-lint
Sep 21, 2026
Merged

JohnMcLear merged 3 commits into
mainfrom
ci/repair-lint

Conversation

@JohnMcLear

@JohnMcLear JohnMcLear commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

The bug

pnpm run lint fails on the default branch of this repo (and of ~80 other ether/ep_* plugins):

$ pnpm run lint
Oops! Something went wrong! :(
ESLint: 8.57.1
TypeError: Failed to load plugin '@typescript-eslint' declared in
'.eslintrc.cjs » eslint-config-etherpad/plugin#overrides[0] » ./node.js » ./index.js#overrides[0]':
Cannot read properties of undefined (reading 'Intrinsic')
    at .../ts-api-utils@1.4.3_typescript@7.0.2/node_modules/ts-api-utils/lib/index.cjs:779:57

Root cause

Dependabot bumped typescript to ^7.0.2. TypeScript 7 is the native (Go) port and no longer exposes the legacy compiler API surface, so ts.TypeFlags is undefined. ts-api-utils evaluates this at module load:

var IntrinsicTypeFlags = ts.TypeFlags.Intrinsic ?? ...

…which throws, taking @typescript-eslint — and therefore all of eslint-config-etherpad — down with it. ESLint exits without linting a single file.

No released @typescript-eslint supports TypeScript 7. 8.70.0 and every 8.70.1-alpha.* declare typescript: ">=4.8.4 <6.1.0", so this cannot be fixed by upgrading the lint toolchain today; typescript has to be held inside the supported range.

The fix

  • typescript → ~6.0.3, the newest line @typescript-eslint supports and an exact match for its <6.1.0 ceiling.
  • eslint-config-etherpad → ^5.0.0. 5.x declares that same typescript range itself, so a future TypeScript major fails resolution loudly instead of breaking lint silently. The .eslintrc.cjs entry points are unchanged, so this repo's existing config keeps working.

The lockfile diff is large because TypeScript 7 ships ~20 per-platform native binary packages (@typescript/typescript-linux-x64, -darwin-arm64, …) that TypeScript 6 doesn't have.

Why it rotted: lint ran in no CI job

That's the real defect — the breakage sat on the default branch unnoticed. This PR adds a reusable .github/workflows/lint.yml and wires it into test-and-release.yml:

jobs:
  lint:
    uses: ./.github/workflows/lint.yml
    secrets: inherit
  backend: ...
  frontend: ...
  release:
    needs:
      - lint
      - backend
      - frontend

Because automerge.yml only auto-merges Dependabot PRs when the Node.js Package run succeeds, a future dependency bump that breaks lint now turns that run red and is held back instead of landing silently. No Dependabot ignore rule is added — CI is the gate.

Lint findings

One error, fixed in a separate commit so the dependency/CI commit stays reviewable on its own:

File Rule Fix
index.js max-len (103 cols) hoisted the themes.js path into a const src so the template literal fits in 100 cols. The emitted HTML is byte-for-byte identical.

One camelcase warning (theme_default) remains; it's a persisted settings key, so renaming it is not a mechanical change. Warnings don't fail the build.

Verification

$ pnpm install && pnpm run lint
> ep_themes@11.0.34 lint
> eslint .
✖ 1 problem (0 errors, 1 warning)

$ echo $?
0

This repo has no test script; the reusable backend/frontend workflows cover the rest in CI.

Review follow-up

A third commit (fix: least-privilege lint job and an honest engines.node) addresses two findings from the automated review:

  • permissions: contents: read on the lint job. It declared no permissions, so it inherited contents: write / id-token: write from test-and-release.yml while running eslint . — which executes .eslintrc.cjs and every installed ESLint plugin. This is a property of the shared plugin lint workflow and should be propagated to the other ether/ep_* repos that already took it.
  • engines.node → >=22.0.0. eslint-config-etherpad@5 pulls in eslint-visitor-keys@5.0.1 (^20.19.0 || ^22.13.0 || >=24), so advertising >=18.0.0 would break an engine-strict dev install. >=22.0.0 is the current ether plugin floor.

Two further findings were declined, with reasons, in the review thread: the "no regression test" one (the new lint job is the regression test) and the "fork PRs skip lint" one (a property of the whole plugin CI template — backend and frontend already miss fork PRs identically — so it belongs in a template-wide change).

🤖 Generated with Claude Code

https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw

JohnMcLear and others added 2 commits September 20, 2026 18:56
`typescript: ^7.0.2` resolved to TypeScript 7, the native port, which no
longer exposes the legacy compiler API. `ts-api-utils` (pulled in by
`@typescript-eslint` through `eslint-config-etherpad`) throws
`Cannot read properties of undefined (reading 'Intrinsic')` on load, taking
the whole ESLint config down before any file is checked.

Pin `typescript` to `~6.0.3` and bump `eslint-config-etherpad` to `^5.0.0`,
which declares `typescript: ">=4.8.4 <6.1.0"` so a future TypeScript major
fails loudly at install time instead of silently breaking lint.

Add a reusable `lint.yml` workflow and wire it into `test-and-release.yml` so
lint actually runs in CI and gates the release job — the reason this rotted
unnoticed is that it never ran.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
With lint restored, ESLint reports findings that were latent while it
could not run at all. These are mechanical and behaviour-preserving:
partly `eslint --fix`, partly by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Restore ESLint compatibility and gate releases on lint

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Restores ESLint by aligning TypeScript and Etherpad lint configuration compatibility.
• Adds reusable lint CI and blocks releases when lint fails.
• Resolves the latent max-length violation without changing rendered HTML.
Diagram

graph TD
  A["Push or dispatch"] --> B["Test and release"] --> C["Lint workflow"] --> D["Pinned toolchain"] --> E["ESLint"] --> G{"Release gate"}
  B --> F["Backend and frontend"] --> G
Loading
High-Level Assessment

The chosen approach is appropriate: pinning TypeScript within the supported peer range repairs lint immediately, while eslint-config-etherpad 5 enforces compatibility during future installs. Upgrading to TypeScript 7 support is not currently possible, and adding a CI gate is preferable to suppressing dependency updates because it detects any future lint regression.

Files changed (5) +306 / -475

Bug fix (1) +2 / -1
index.jsResolve timeslider script max-length violation +2/-1

Resolve timeslider script max-length violation

• Hoists the timeslider theme script path into a constant so the generated template satisfies the line-length rule. The emitted HTML remains unchanged.

index.js

Other (4) +304 / -474
lint.ymlAdd reusable ESLint workflow +35/-0

Add reusable ESLint workflow

• Introduces a callable GitHub Actions workflow that configures Node.js and pnpm, restores the dependency cache, installs without lifecycle scripts, and runs the lint script.

.github/workflows/lint.yml

test-and-release.ymlRequire lint before release +4/-0

Require lint before release

• Invokes the reusable lint workflow alongside backend and frontend tests. Adds lint to the release job dependencies so publishing cannot proceed after a lint failure.

.github/workflows/test-and-release.yml

package.jsonAlign TypeScript with the supported lint toolchain +2/-2

Align TypeScript with the supported lint toolchain

• Upgrades eslint-config-etherpad to 5.x and constrains TypeScript to the compatible 6.0 patch line, restoring ESLint startup and enforcing the configuration package's peer range.

package.json

pnpm-lock.yamlRegenerate the compatible lint dependency graph +263/-472

Regenerate the compatible lint dependency graph

• Locks TypeScript 6.0.3, eslint-config-etherpad 5.0.2, and the corresponding TypeScript ESLint 8.70 toolchain. Removes TypeScript 7 native platform packages and refreshes transitive lint dependencies.

pnpm-lock.yaml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Strict installs reject declared Node ✓ Resolved 🐞 Bug ☼ Reliability
Description
eslint-config-etherpad@5.0.2 pulls in eslint-visitor-keys@5.0.1, whose engine range excludes
Node 18 and Node 20 below 20.19 even though package.json declares every Node version from 18
onward supported. A contributor using an engine-strict installer on those declared runtimes will be
blocked during dependency installation, while the new lint job only exercises Node 25 and cannot
catch the mismatch.
Code

package.json[30]

+    "eslint-config-etherpad": "^5.0.0",
Evidence
The package declares Node >=18.0.0, but the newly resolved lint tree contains
eslint-visitor-keys@5.0.1 with engines ^20.19.0 || ^22.13.0 || >=24. The lint workflow's fixed
Node 25 environment means CI does not validate the lower versions still advertised by the package.

package.json[15-17]
package.json[28-31]
pnpm-lock.yaml[581-583]
.github/workflows/lint.yml[13-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The upgraded lint dependency tree requires newer Node releases than the package's declared `>=18.0.0` engine range. Engine-strict installations can therefore reject versions that the package claims to support.
## Fix Focus Areas
- package.json[15-17]
- package.json[28-31]
- pnpm-lock.yaml[581-583]
- .github/workflows/lint.yml[13-16]
## Recommended Fix
Either select a lint configuration and dependency tree that supports the full declared Node range, or update the documented engine constraints to the strictest resolved tooling requirement and exercise the minimum supported version in CI.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread package.json
Two points from the automated review on this PR:

* The reusable lint job declared no `permissions`, so it inherited
  `contents: write` and `id-token: write` from test-and-release.yml while
  running `eslint .` — which executes `.eslintrc.cjs` and every installed
  ESLint plugin. It only needs to read the checkout, so it now pins
  `permissions: contents: read`. The release job keeps its own elevated
  block.

* `eslint-config-etherpad@5` pulls in `eslint-visitor-keys@5.0.1`, whose
  engines are `^20.19.0 || ^22.13.0 || >=24`, while this package still
  advertised `node: >=18.0.0`. Under an engine-strict installer a
  contributor on an advertised Node 18 could no longer install the dev
  dependencies. `engines.node` is raised to `>=22.0.0`, the current ether
  plugin floor, so the manifest states something true again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
@JohnMcLear

Copy link
Copy Markdown
Member Author

Thanks — went through the review. Two of the findings were real and are fixed in fix: least-privilege lint job and an honest engines.node; two are declined, with reasons.

Fixed — "lint code can modify the repository" (High). Correct. The reusable lint job declared no permissions, so it inherited contents: write and id-token: write from test-and-release.yml while running eslint ., which executes .eslintrc.cjs and every installed ESLint plugin. It now pins:

  lint:
    name: ESLint
    runs-on: ubuntu-latest
    permissions:
      contents: read

The release job keeps its own elevated block. Note this applies to the shared plugin lint workflow, not just this repo, so it needs propagating to the other ether/ep_* repos that already took it.

Fixed — "strict installs reject declared node". Also correct, and caused by this PR: eslint-config-etherpad@5 pulls in eslint-visitor-keys@5.0.1, whose engines are ^20.19.0 || ^22.13.0 || >=24, while the manifest still advertised >=18.0.0. engines.node is now >=22.0.0, the current ether plugin floor. (That range still nominally admits 22.0–22.12, which eslint-visitor-keys excludes; tightening further would mean departing from the project-wide floor for a devDependency, so it's left at the policy value.)

Declined — "the fix adds no regression test". The regression test is the new lint job: before this PR pnpm run lint exited non-zero without linting a file, and the job would have been red. A separate unit test that shells out to ESLint to assert exit 0 would only restate what the CI job already asserts, and would itself break on the next toolchain bump for reasons unrelated to this repo.

Declined here — "fork pull requests skip lint". True, but it is a property of the whole ether plugin CI template rather than of this change: test-and-release.yml triggers on push/workflow_dispatch only, so the existing backend and frontend jobs already miss fork PRs in exactly the same way. Adding a pull_request trigger changes run semantics for all three jobs and interacts with automerge.yml, so it belongs in a template-wide change across the ~85 plugin repos, not in this one-repo lint repair.

🤖 Generated with Claude Code

https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw

@JohnMcLear
JohnMcLear merged commit f3a6d70 into main Sep 21, 2026
4 checks passed
@JohnMcLear
JohnMcLear deleted the ci/repair-lint branch September 21, 2026 08:16
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