Add ESLint baseline: no-floating-promises enforced, recommended rules as warnings#201
Conversation
…es as warnings
Installs ESLint 9 + typescript-eslint. Uses a flat config (eslint.config.mjs)
scoped to src/**/*.{ts,tsx} and scripts/**/*.mjs with build artifacts ignored.
no-floating-promises is ERROR (type-aware); 18 violations fixed across 7 files
using void + comment or .catch() per the PR #195 error-visibility conventions.
All other recommended rules are WARN so they inform without blocking CI.
Adds "lint" npm script and a CI gate immediately after verify:mcp-resources-drift.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request integrates ESLint and TypeScript-ESLint into the project, adding a linting script and a configuration file that enforces type-aware rules, specifically targeting un-awaited promises. Consequently, numerous floating promises across the codebase have been prefixed with void to satisfy the new rules. Feedback on these changes highlights a critical issue in eslint.config.mjs where the extends property is incorrectly used inside a configuration object, which is unsupported in ESLint 9 flat config and will cause a runtime error. Additionally, for the promise chain in ProjectSidebar.tsx, it is recommended to append a .catch() block to properly handle potential rejections rather than solely silencing the linter with void.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Apply to all TS/TSX source and mjs scripts — no type-aware rules here | ||
| { | ||
| files: ['src/**/*.{ts,tsx}', 'scripts/**/*.mjs'], | ||
| extends: [ | ||
| ...tseslint.configs.recommended, | ||
| ], | ||
| rules: { |
There was a problem hiding this comment.
In ESLint 9 flat config, the extends property is not supported inside a configuration object. Using it will cause ESLint to throw a TypeError and fail to run.
To fix this, you should spread tseslint.configs.recommended directly into the tseslint.config() call, and then define your custom rules in a separate configuration object.
...tseslint.configs.recommended,
// Apply to all TS/TSX source and mjs scripts — no type-aware rules here
{
files: ['src/**/*.{ts,tsx}', 'scripts/**/*.mjs'],
rules: {There was a problem hiding this comment.
False positive: extends is not supported in raw flat-config objects, but this file wraps everything in tseslint.config(), whose documented signature feature is exactly per-object extends (https://typescript-eslint.io/packages/typescript-eslint#config). Verified empirically: npx eslint . runs on this branch — 0 errors, 54 warnings, no TypeError.
|
|
||
| useEffect(() => { | ||
| pear.auth.status().then(setAuth) | ||
| void pear.auth.status().then(setAuth) |
There was a problem hiding this comment.
Using void on a promise chain without a .catch() block silences the linter but leaves potential promise rejections unhandled. If pear.auth.status() fails, it will result in an unhandled promise rejection at runtime.
Since the comment indicates this is a best-effort probe where failures can be safely ignored, explicitly catch the error to prevent unhandled rejections.
| void pear.auth.status().then(setAuth) | |
| void pear.auth.status().then(setAuth).catch(() => {}) |
There was a problem hiding this comment.
Applied in c9d9cea — explicit .catch(() => {}) on the best-effort probe.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bffaaf5ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }).catch((err) => { | ||
| console.error('[main] app initialization failed:', err) |
There was a problem hiding this comment.
Don't swallow startup initialization failures
If any startup step inside the app.whenReady().then(...) callback throws before the window is created or IPC is registered, this new catch only logs the error and lets the Electron process keep running in a partially initialized/no-window state. Previously the unhandled rejection would fail fast under the Node/Electron default; please rethrow/quit after logging, or handle recovery explicitly, so startup failures don't leave a hung app.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed — fixed in c9d9cea: the init catch now logs, sets process.exitCode = 1, and calls app.quit() so a partially-initialized app fails fast instead of lingering windowless (preserving the pre-lint exit-non-zero behavior).
- ProjectSidebar auth.status() best-effort probe now catches rejections explicitly instead of leaving an unhandled rejection path. - app initialization catch now sets exitCode and quits instead of leaving a partially-initialized, windowless app running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
What this does
Adds ESLint 9 flat config (
eslint.config.mjs) withtypescript-eslint. This is the first linter in the repo — designed as a low-noise, genuinely useful baseline that passes in CI without becoming a style crusade.Config philosophy
src/**/*.{ts,tsx}andscripts/**/*.mjs; ignoresout/,build/,dist/,node_modules/@typescript-eslint/no-floating-promises→ ERROR (type-aware): un-awaited promises are a real bug category. 18 violations fixed (see below).prefer-const,naming-convention,consistent-type-importsetc. are all off or warn-only. No Prettier in this PR..test.ts,__tests__/**,.dom.test.ts) are excluded from type-aware analysis — they're excluded from all tsconfigs soprojectServicecan't resolve them.Violation counts
Warning breakdown by rule:
@typescript-eslint/no-unused-vars: 35 warnings (underscore-prefixed params, dead exports)prefer-const: 4 warnings@typescript-eslint/prefer-as-const: 1 warningno-console(unused disable directives): 4 warnings54 warnings is intentionally non-zero — they're real signals for future cleanup, just not blocking.
no-floating-promises fixes (18 violations across 7 files)
All fixes follow the convention from PR #195 (error-visibility): either
void exprwith a justifying comment, or a proper.catch()handler.src/main/index.tsvoid shell.openExternal(url)— OS-level fire-and-forgetsrc/main/index.tsvoid mainWindow.loadURL/File()— load errors surface in devtoolssrc/main/index.ts.catch((err) => console.error('[main] app initialization failed:', err))src/renderer/src/App.tsxvoid load()inside useEffectsrc/renderer/src/components/diff/DiffViewer.tsxvoid Promise.all(...).then(...)inside useEffect with ignore-flag patternsrc/renderer/src/components/sidebar/ProjectSidebar.tsxvoid pear.auth.status().then(setAuth)src/renderer/src/components/terminal/TerminalPane.tsxvoid pear.broker.releaseAgent(...)inside onClicksrc/renderer/src/hooks/use-broker-events.tsvoid pear.broker.releaseAgent(...)inside menu event handlersrc/renderer/src/stores/git-store.tsvoid get().fetchStatus/Summary/Diff/ProjectStatus(...)— zustand store polling, results update state directlyCI change
One line added to
.github/workflows/ci.ymlimmediately afterverify:mcp-resources-drift:New dependencies (devDependencies only)
eslint@^9.39.4typescript-eslint@^8.61.0(meta-package: plugin + parser)Follow-ups (not in this PR)
no-floating-promisescatch-allvoidusages to proper error handlers where warranted@typescript-eslint/no-unused-varsas ERROR once the warnings are cleaned uppackaged-mcp-smokejob if desired