chore: add bug-focused ESLint baseline#634
Conversation
📝 WalkthroughWalkthroughThis PR establishes ESLint linting infrastructure by introducing a flat ESLint configuration scoped to TypeScript and Solid/TSX files, adding lint scripts and dependencies to ChangesLinting Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~18 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Astro-Han
added
tech-debt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates ESLint into the project by adding the necessary dependencies and a new configuration file, eslint.config.mjs, which defines rules for TypeScript and Solid.js. It also introduces lint and lint:ci scripts to the root package.json. Feedback from the reviewer correctly identified that the defineConfig helper from eslint/config is invalid for ESLint 9 and recommended using tseslint.config for better type support. Additionally, it was suggested to simplify the linting script glob to eslint . to leverage the configuration's native file discovery and ensure the configuration file itself is linted.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
package.json (1)
14-14: ⚡ Quick winConsider using
eslint .to rely on config file patterns.The lint script explicitly passes file globs to ESLint, duplicating the patterns defined in
eslint.config.mjsfilesarrays. This creates two sources of truth that can drift out of sync.Since the ESLint config already defines which files to lint via the
filesproperty in each rule block, you can simplify to:- "lint": "eslint \"packages/{app,ui,desktop-electron}/src/**/*.{ts,tsx}\"", + "lint": "eslint .",This makes the config file the single source of truth for which files to lint. If you prefer explicit CLI globs for visibility or other reasons, consider adding a comment explaining the rationale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 14, The "lint" npm script currently hardcodes CLI globs ("lint" script) that duplicate patterns already defined in eslint.config.mjs; update the package.json "lint" script to use a generic eslint invocation (e.g., eslint .) so the eslint.config.mjs files arrays remain the single source of truth, or if you must keep explicit globs, add a comment in package.json explaining why the CLI globs are intentionally duplicated; ensure you reference and update the "lint" script entry so it no longer conflicts with eslint.config.mjs file patterns..github/workflows/ci.yml (1)
132-167: ⚡ Quick winConsider adding Turbo cache for the lint job.
The
typecheckjob includes Turbo caching (lines 119-125), but thelintjob only has Bun install cache. Since ESLint withprojectServiceenabled can be resource-intensive, adding Turbo cache would improve CI performance and developer experience:📦 Add Turbo cache for lint job
- uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # actions/cache@v5 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: | bun-${{ runner.os }}- + - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # actions/cache@v5 + with: + path: .turbo/cache + key: turbo-${{ runner.os }}-lint-${{ hashFiles('turbo.json', '**/package.json', 'bun.lock') }}-${{ github.sha }} + restore-keys: | + turbo-${{ runner.os }}-lint-${{ hashFiles('turbo.json', '**/package.json', 'bun.lock') }}- + turbo-${{ runner.os }}-lint- + - run: bun install --frozen-lockfileThis matches the pattern used by other CI jobs and would speed up repeat lint runs. Since this is a baseline PR, you may choose to defer this optimization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 132 - 167, The lint job currently only caches Bun install artifacts but should also use the Turbo cache like the typecheck job; update the lint job (job name "lint", steps around "actions/cache", "bun install --frozen-lockfile", and the "lint" step that runs "bun run lint:ci") to add Turbo cache restore/save steps (same placement as typecheck) so repeated lint runs reuse the .turbo cache and speed up ESLint with projectService enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 132-167: The lint job currently only caches Bun install artifacts
but should also use the Turbo cache like the typecheck job; update the lint job
(job name "lint", steps around "actions/cache", "bun install --frozen-lockfile",
and the "lint" step that runs "bun run lint:ci") to add Turbo cache restore/save
steps (same placement as typecheck) so repeated lint runs reuse the .turbo cache
and speed up ESLint with projectService enabled.
In `@package.json`:
- Line 14: The "lint" npm script currently hardcodes CLI globs ("lint" script)
that duplicate patterns already defined in eslint.config.mjs; update the
package.json "lint" script to use a generic eslint invocation (e.g., eslint .)
so the eslint.config.mjs files arrays remain the single source of truth, or if
you must keep explicit globs, add a comment in package.json explaining why the
CLI globs are intentionally duplicated; ensure you reference and update the
"lint" script entry so it no longer conflicts with eslint.config.mjs file
patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c594a7bf-1da0-4362-9729-244b24c72f23
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.coderabbit.yaml.github/workflows/ci.ymleslint.config.mjspackage.json
Astro-Han
force-pushed
the
codex/eslint-bug-baseline
branch
from
d87be10 to
8689ae4
Compare
|
CodeRabbit nit follow-up: kept the explicit |
Astro-Han
force-pushed
the
codex/eslint-bug-baseline
branch
from
8689ae4 to
65b360e
Compare
github-actions
Bot
added
the
harness
1 participant
Summary
Adds a low-noise ESLint baseline for PawWork-owned product TypeScript/Solid code:
eslint.config.mjsscoped topackages/app/src,packages/ui/src, andpackages/desktop-electron/src.bun run lint/bun run lint:ciscripts.lintCI job that is not included in the blockingci / checkaggregate.Why
CodeRabbit recently warned that ESLint was skipped because the repo had no ESLint configuration. The goal here is not to satisfy CodeRabbit specifically, and not to introduce formatting or style enforcement.
This PR establishes a first bug-focused lint baseline that can be run locally and in CI. The initial rule set intentionally stays narrow because broader Promise and
@eslint/jsrecommended rules currently surface many existing historical findings. Those should be handled in follow-up slices rather than bundled into the first baseline.Related Issue
No dedicated issue. This follows the maintainer discussion from PR #630 CodeRabbit review tooling feedback.
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
srcfiles and does not includepackages/opencode.check.needsaggregate.Risk Notes
typescript-eslint, andeslint-plugin-solidas dev dependencies only.continue-on-error: truefor the first PR, so it should not block merge while we observe noise.@typescript-eslintrules, so this PR does not rely on it as the source of truth.bun.lockalso refreshed workspace package metadata while adding the lint dependencies; no source files were changed for that metadata drift.packages/opencode/src/provider/models-snapshot.jswas dirty in the handoff worktree before this work and is intentionally not included in this PR.How To Verify
Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit