Skip to content

fix: repair pnpm run lint and run it in CI (not gating yet) - #194

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

JohnMcLear merged 2 commits into
mainfrom
ci/repair-lint

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Why

pnpm run lint was broken in this repo (and in ~82 of the 85 ether/* plugin
repos). The devDependency 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 via eslint-config-etherpad — throws
Cannot read properties of undefined (reading 'Intrinsic') while loading, which
takes the whole ESLint config down before a single file is checked.

Nothing noticed because lint was never run in CI.

What changed

  • typescript pinned to ~6.0.3 (must satisfy >=4.8.4 <6.1.0).
  • eslint-config-etherpad bumped to ^5.0.0. 5.x declares that TypeScript
    range as a peer dependency, so a future TypeScript major now fails loudly at
    install time instead of silently breaking lint.
  • pnpm-lock.yaml regenerated. The diff is large because TypeScript 7 ships
    ~20 per-platform native binaries that TypeScript 6 does not.
  • New .github/workflows/lint.yml (reusable workflow running pnpm run lint),
    added to test-and-release.yml as a lint job. It is deliberately NOT in the
    release job's needs: list
    — see below.

Lint result

With lint working again this repo reports 105 problems (35 errors, 70
warnings)
. The errors are almost entirely mocha/no-synchronous-tests in the
backend specs, plus a handful of quotes; clearing them means rewriting tests,
not a mechanical fix, and these rules have never actually run here.

So this PR makes lint run and report without letting it block releases:
lint.yml is added as a job, but lint is not added to the release job's
needs:. The findings can be worked through in a follow-up, and lint added to
needs: at that point.

No source or test files are changed by this PR.

Follows ether/ep_cursortrace#117, ether/ep_clear_formatting#96 and
ether/ep_git_commit_saved_revision#107.

🤖 Generated with Claude Code

https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw

`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 so lint runs in CI. It is deliberately NOT
added to the `release` job's `needs:` list: with lint working again the repo
reports 105 findings (35 errors, 70 warnings), mostly
`mocha/no-synchronous-tests` in the backend specs, which needs real changes to
the test suite rather than a mechanical fix. Lint therefore reports but does
not block until that is cleaned up separately.

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 add non-gating CI lint

🐞 Bug fix ✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Restores ESLint compatibility by pinning TypeScript 6 and upgrading Etherpad’s shared
 configuration.
• Adds reusable lint CI reporting without making releases depend on current lint failures.
• Regenerates the lockfile for the compatible TypeScript and typescript-eslint dependency graph.
Diagram

graph TD
  A["Git push"] --> B["CI orchestrator"] --> C["Lint workflow"] --> D["Pinned toolchain"] --> E["ESLint report"]
  B --> F["Test workflows"] --> G["Release workflow"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Baseline existing lint findings
  • ➕ Could block newly introduced violations immediately
  • ➕ Would protect releases without first resolving all legacy findings
  • ➖ Requires custom comparison or baseline maintenance
  • ➖ Can become stale and obscure the actual lint status
2. Fix all lint findings now
  • ➕ Would allow lint to gate releases immediately
  • ➕ Avoids a temporary period with failing CI lint
  • ➖ Substantially expands scope into backend test rewrites
  • ➖ Combines toolchain repair with higher-risk behavioral changes

Recommendation: Keep the PR’s staged approach: restore a peer-constrained lint toolchain and expose findings without blocking releases. Resolve the legacy findings separately, then add lint to the release dependencies; a baseline is worth considering only if immediate regression prevention is required.

Files changed (4) +574 / -742

Bug fix (1) +2 / -2
package.jsonPin a compatible TypeScript and ESLint configuration +2/-2

Pin a compatible TypeScript and ESLint configuration

• Pins TypeScript to the 6.0 patch line and upgrades eslint-config-etherpad to 5.x. The updated shared configuration declares a TypeScript peer range that prevents incompatible future majors from installing silently.

package.json

Other (3) +572 / -740
lint.ymlAdd reusable ESLint workflow +35/-0

Add reusable ESLint workflow

• Introduces a reusable GitHub Actions workflow that provisions Node.js and pnpm, restores the pnpm cache, installs dependencies without scripts, and runs the repository lint command.

.github/workflows/lint.yml

test-and-release.ymlRun lint alongside tests without gating releases +3/-0

Run lint alongside tests without gating releases

• Calls the new reusable lint workflow from the primary CI workflow. The release job continues to depend only on backend and frontend tests, allowing publication despite known lint failures.

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

pnpm-lock.yamlRegenerate the lint dependency graph +534/-740

Regenerate the lint dependency graph

• Locks TypeScript 6.0.3, eslint-config-etherpad 5.0.2, and the updated typescript-eslint stack. Removes TypeScript 7’s platform-specific native packages and refreshes affected transitive 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 (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Every push workflow reports failure 🐞 Bug ≡ Correctness
Description
The Run ESLint step executes the existing eslint . script without continue-on-error, so any
lint error fails both the reusable lint job and its parent workflow. With the repository's current
lint errors, this happens on every push and marks the entire Node.js Package workflow failed even
though the release job does not depend on lint.
Code

.github/workflows/lint.yml[35]

+        run: pnpm run lint
Evidence
The reusable workflow installs dependencies and runs the lint script as an ordinary failing step,
while package.json maps that script to eslint .. The active Etherpad preset covers synchronous
test bodies present in the repository, and the parent workflow runs on every push; only the release
job is insulated because its needs list contains backend and frontend but not lint.

.github/workflows/lint.yml[32-35]
package.json[37-40]
.eslintrc.cjs[4-9]
static/tests/backend/specs/default_type.js[12-23]
.github/workflows/test-and-release.yml[1-25]

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 lint workflow runs a command that is currently known to return a nonzero status, causing every parent workflow run to be marked failed rather than merely reporting the violations.
## Fix Focus Areas
- .github/workflows/lint.yml[34-35]
## Recommended Fix
Add `continue-on-error: true` to the `Run ESLint` step while the existing lint violations are being remediated. This preserves the ESLint output in the job log without failing the reusable job; remove the setting when lint is ready to gate 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 .github/workflows/lint.yml
Add a job-level `permissions: contents: read` to the lint workflow. `eslint .`
executes the repo's ESLint config and every installed plugin, so it should not
inherit the caller workflow's contents:write / id-token:write token.

Raise engines.node from >=18.0.0 to >=22.0.0. eslint-config-etherpad@5 pulls
eslint-visitor-keys@5.0.1, which excludes Node 18, so the old floor was a false
claim; >=22.0.0 is the documented ether plugin floor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
@JohnMcLear
JohnMcLear merged commit 005b2f1 into main Sep 21, 2026
3 of 4 checks passed
@JohnMcLear
JohnMcLear deleted the ci/repair-lint branch September 21, 2026 08:14
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