Uh oh!
There was an error while loading. Please reload this page.
Format all files properly - #71
Merged
Merged
Conversation
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (78)
✨ 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 |
heavyrubberslave
commented
Mar 7, 2026
MemberAuthor
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/controller/patchDeviceController.ts (1)
32-32:⚠️ Potential issue | 🔴 CriticalFix incorrect response method chaining in error handler.
Chaining
.send()and.sendStatus()is incorrect—both methods send the response, so the second call will fail with "Cannot set headers after they are sent to the client."🐛 Proposed fix
- res.send((err as Error).message).sendStatus(500);+ res.status(500).send((err as Error).message);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controller/patchDeviceController.ts` at line 32, The error handler in patchDeviceController.ts currently chains res.send(...).sendStatus(500), which sends the response twice and breaks header handling; change it to set the status first using res.status(500) and then send the error message (or send a JSON error) in a single response call instead of chaining send and sendStatus on res.send(...).sendStatus(500).
🧹 Nitpick comments (3)
src/util/devNullStream.ts (1)
2-2: Remove unused EventEmitter import and events property.The
eventsproperty on line 7 is instantiated but never used. The class correctly usesthis.emit('idle')on line 27, which leverages the inheritedemitmethod fromWritable(which extendsEventEmitter). The separateeventsinstance is redundant and creates unnecessary memory overhead.♻️ Proposed fix to remove unused code
-import { Writable } from 'stream';-import EventEmitter from 'events';+import { Writable } from 'stream'; export default class DevNullStream extends Writable { private readonly timeoutMs: number; private timer?: NodeJS.Timeout; - private readonly events = new EventEmitter();Also applies to: 7-7
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/devNullStream.ts` at line 2, Remove the redundant EventEmitter import and the unused events property in the DevNullStream class: delete the "import EventEmitter" line and remove the class property named "events" (the instantiated EventEmitter) and any related references, leaving the existing this.emit('idle') calls intact since Writable already provides emit; ensure no other code relies on the removed events instance (check for "events" property usages) and run tests.src/controller/automation/deleteScriptController.ts (1)
20-21: Usingtext/plaincontent-type would prevent reflected input from being interpreted as HTML.While Express defaults
res.send(string)toContent-Type: text/html, theisValidAutomationScriptFileName()whitelist regex only allows[a-z\d._-]+\.js, which excludes all HTML special characters. Reflection is not an XSS risk here. Settingtext/plainis still good practice for defense-in-depth clarity, but not critical given the validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controller/automation/deleteScriptController.ts` around lines 20 - 21, Update the error response to explicitly use a text/plain content type for the invalid filename path: when the filename fails isValidAutomationScriptFileName(...) in deleteScriptController (the branch that currently calls res.status(400).send(...)), set the response content type to text/plain (for example via res.type('text/plain') or res.set('Content-Type','text/plain')) before sending the message so reflected input is returned as plain text.src/device/protocol/buttplugIo/buttplugIoDevice.ts (1)
86-91: Consider removing the extra blank lines.The blank lines between each property in this small object literal seem excessive and reduce readability. If this wasn't intentional formatting from a linter rule, consider condensing:
♻️ Suggested format
return await this.buttplugClientDevice.scalar({ - 'ActuatorType': command, - 'Scalar': value, - 'Index': index });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/device/protocol/buttplugIo/buttplugIoDevice.ts` around lines 86 - 91, The object literal containing 'ActuatorType', 'Scalar', and 'Index' in buttplugIoDevice.ts has unnecessary blank lines; remove the empty lines so the properties are placed consecutively (e.g., 'ActuatorType': command, 'Scalar': value, 'Index': index) to improve readability and match surrounding formatting for the code that constructs the command payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/controller/patchDeviceController.ts`:
- Line 32: The error handler in patchDeviceController.ts currently chains
res.send(...).sendStatus(500), which sends the response twice and breaks header
handling; change it to set the status first using res.status(500) and then send
the error message (or send a JSON error) in a single response call instead of
chaining send and sendStatus on res.send(...).sendStatus(500).
---
Nitpick comments:
In `@src/controller/automation/deleteScriptController.ts`:
- Around line 20-21: Update the error response to explicitly use a text/plain
content type for the invalid filename path: when the filename fails
isValidAutomationScriptFileName(...) in deleteScriptController (the branch that
currently calls res.status(400).send(...)), set the response content type to
text/plain (for example via res.type('text/plain') or
res.set('Content-Type','text/plain')) before sending the message so reflected
input is returned as plain text.
In `@src/device/protocol/buttplugIo/buttplugIoDevice.ts`:
- Around line 86-91: The object literal containing 'ActuatorType', 'Scalar', and
'Index' in buttplugIoDevice.ts has unnecessary blank lines; remove the empty
lines so the properties are placed consecutively (e.g., 'ActuatorType': command,
'Scalar': value, 'Index': index) to improve readability and match surrounding
formatting for the code that constructs the command payload.
In `@src/util/devNullStream.ts`:
- Line 2: Remove the redundant EventEmitter import and the unused events
property in the DevNullStream class: delete the "import EventEmitter" line and
remove the class property named "events" (the instantiated EventEmitter) and any
related references, leaving the existing this.emit('idle') calls intact since
Writable already provides emit; ensure no other code relies on the removed
events instance (check for "events" property usages) and run tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efddc9af-cb48-41d7-b16e-fbae8fcc1aa0
📒 Files selected for processing (77)
src/automation/automationEventType.tssrc/automation/scriptRuntime.tssrc/controller/automation/createScriptController.tssrc/controller/automation/deleteScriptController.tssrc/controller/automation/getLogController.tssrc/controller/automation/getScriptController.tssrc/controller/automation/getScriptsController.tssrc/controller/automation/runScriptController.tssrc/controller/automation/statusScriptController.tssrc/controller/automation/stopScriptController.tssrc/controller/getDeviceController.tssrc/controller/getDevicesController.tssrc/controller/healthController.tssrc/controller/patchDeviceController.tssrc/controller/settings/getSettingsController.tssrc/controller/settings/putSettingsController.tssrc/controller/versionController.tssrc/device/attribute/boolDeviceAttribute.tssrc/device/attribute/floatDeviceAttribute.tssrc/device/attribute/intDeviceAttribute.tssrc/device/attribute/intRangeDeviceAttribute.tssrc/device/attribute/numberDeviceAttribute.tssrc/device/attribute/strDeviceAttribute.tssrc/device/deviceManager.tssrc/device/deviceManagerEvent.tssrc/device/deviceNameGenerator.tssrc/device/deviceState.tssrc/device/genericDeviceUpdater.tssrc/device/protocol/buttplugIo/buttplugIoDevice.tssrc/device/protocol/buttplugIo/buttplugIoDeviceFactory.tssrc/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProviderFactory.tssrc/device/protocol/deviceProtocol.tssrc/device/protocol/virtual/audio/piperVirtualDeviceLogic.tssrc/device/protocol/zc95/Zc95SerialReader.tssrc/device/protocol/zc95/zc95Device.tssrc/device/protocol/zc95/zc95MessageFactory.tssrc/device/provider/deviceProviderEvent.tssrc/device/provider/deviceProviderFactory.tssrc/device/provider/deviceProviderLoader.tssrc/device/transport/serialPortObserver.tssrc/device/updater/abstractDeviceUpdater.tssrc/device/updater/bufferedDeviceUpdater.tssrc/device/updater/deviceUpdaterInterface.tssrc/device/webSocketEvent.tssrc/entity/automationScript.tssrc/entity/deviceList.tssrc/entity/list.tssrc/index.tssrc/repository/automationScriptRepository.tssrc/repository/automationScriptRepositoryInterface.tssrc/repository/connectedDeviceRepository.tssrc/repository/deviceRepositoryInterface.tssrc/schemaValidation/JsonSchemaValidator.tssrc/schemaValidation/JsonSchemaValidatorFactory.tssrc/serialization/classToPlainSerializer.tssrc/serialization/discriminator/genericDeviceAttributeDiscriminator.tssrc/serialization/discriminator/objectDiscriminator.tssrc/serviceMap.tssrc/serviceProvider/automationServiceProvider.tssrc/serviceProvider/controllerServiceProvider.tssrc/serviceProvider/repositoryServiceProvider.tssrc/serviceProvider/schemaValidationServiceProvider.tssrc/serviceProvider/serializationServiceProvider.tssrc/serviceProvider/serverServiceProvider.tssrc/serviceProvider/settingsServiceProvider.tssrc/serviceProvider/socketServiceProvider.tssrc/settings/deviceSource.tssrc/settings/knownDevice.tssrc/settings/settings.tssrc/settings/settingsEventType.tssrc/settings/settingsManager.tssrc/socket/deviceUpdateHandler.tssrc/socket/types.tssrc/util/devNullStream.tssrc/util/numbers.tssrc/util/process.tstsconfig.json
💤 Files with no reviewable changes (3)
- tsconfig.json
- src/util/numbers.ts
- src/device/protocol/zc95/zc95MessageFactory.ts
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Style
Chores