chore: enforce no-floating-promises in activate/#251
Conversation
First slice of a ratchet that re-enables @typescript-eslint/no-floating-promises directory by directory. The rule and type-aware linting are scoped to activate/** via a files-block in eslint.config.mjs, so pnpm lint (eslint --max-warnings=0) stays green; later PRs widen the scope. registerCommands.ts had 7 un-awaited postMessageToWebview calls: the 5 in synchronous command handlers are marked void (intentional fire-and-forget); the 2 in async handlers are awaited — one sits inside a try/catch, so awaiting routes a rejected post into the existing error handling.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCommand handlers in the activate module are updated to explicitly handle promise returns from webview message posting: most use ChangesPromise handling updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/activate/registerCommands.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/eslint.config.mjsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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.
Pull request overview
This PR re-enables @typescript-eslint/no-floating-promises in a scoped, incremental way by enforcing it only under src/activate/**, and fixes the newly-detected floating promises in activate/registerCommands.ts without broadly changing async behavior elsewhere in the repo.
Changes:
- Scoped
@typescript-eslint/no-floating-promisestoactivate/**/*.tswith type-aware parsing enabled for that scope. - Annotated intentional fire-and-forget webview messages with
voidin sync handlers. - Awaited two webview messages in existing async handlers to satisfy the rule.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/eslint.config.mjs | Adds a flat-config files override to enforce no-floating-promises only for activate/** using type-aware parser options. |
| src/activate/registerCommands.ts | Resolves floating promises by marking intentional fire-and-forget calls as void and awaiting calls in async handlers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
First slice of a ratchet that re-enables
@typescript-eslint/no-floating-promisesdirectory by directory.The repo's lint is zero-tolerance (
eslint --max-warnings=0+ theonly-warnplugin), so the rule can't be enabled globally without first clearing all ~194 floating promises acrosssrc/. Instead, this enables the rule scoped toactivate/**via afiles-block ineslint.config.mjs(with type-aware parsing for that scope).pnpm lintstays green; subsequent PRs widen the scope until it's global.Fixes
activate/registerCommands.tshad 7 un-awaitedpostMessageToWebview(...)calls (fire-and-forget UI messages):void— in synchronous command handlers (settingsButtonClicked,historyButtonClicked,marketplaceButtonClicked,acceptInput); these are intentional fire-and-forget and the handler is notasync.async(focusInput,toggleAutoApprove).focusInput's call sits inside atry/catch, so awaiting routes a rejected post into the existing error handler.No sync→async handler conversions; behavior is unchanged.
Test plan
eslint activate/clean.eslint . --ext=ts --max-warnings=0(the repo lint command) passes.tsc --noEmitclean.activate/test suite passes (12 tests).Summary by CodeRabbit
Bug Fixes
Chores