refactor(brainstorming): remove bundled node_modules#693
Closed
refactor(brainstorming): remove bundled node_modules#693
Conversation
Vendored dependencies bypass npm audit and automated vulnerability scanning. Remove node_modules/, package.json, and package-lock.json, and drop the .gitignore exception that allowed committing them.
Replace express, ws, and chokidar with Node.js built-ins (http, crypto, fs.watch). All behavior preserved: HTTP serving, WebSocket broadcast, file watching, and helper script injection.
RomarQ
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaced the Express/ws/chokidar brainstorming runtime with a minimal custom WebSocket implementation and fs.watch-based file watcher, started persisting events to Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant HTTP as HTTP Server
participant WS as WS Handler
participant Watch as File Watcher
participant FS as File System
Client->>HTTP: GET / (request screen)
HTTP-->>Client: Serve newest screen HTML (with injected helper) or waiting page
Client->>HTTP: HTTP Upgrade (WebSocket handshake)
HTTP->>WS: accept socket, register client
WS-->>Client: send open/ack frames
Client->>WS: send text frame (event JSON)
WS->>FS: append event to `.events` (if choice)
WS->>WS: broadcast(event) to other clients
Watch->>FS: detect screen add/change
Watch->>WS: broadcast(reload)
WS-->>Client: send reload command
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 186-190: The code reads screenFile with fs.readFileSync without
guarding for file churn; wrap the read and subsequent isFullDocument/wrapInFrame
logic in a try/catch so a missing or replaced file doesn't throw and crash the
server—if the read fails, set html to WAITING_PAGE (or fallback behavior) and
log/debug the error; update the block that references screenFile,
isFullDocument, wrapInFrame, and WAITING_PAGE accordingly.
- Around line 150-155: The code currently only persists events when event.choice
exists and logs with ...event after source which allows client data to override
source; update the handling in the frame parse block (the JSON.parse of
frame.data and subsequent console.log / fs.appendFileSync to SCREEN_DIR +
'.events') to normalize choice events by accepting either event.choice or events
where event.type === 'choice' and event.value is set (e.g., treat event.choice =
event.choice || event.value) and persist those, and change the console.log call
so the server-assigned source cannot be overridden (ensure the source:
'user-event' field is applied after spreading event or explicitly set on the
final object).
- Around line 232-255: The fs.watch watcher can emit 'error' events at runtime
and currently has no listener, causing unhandled crashes; after creating the
watcher returned by fs.watch (the variable watcher in this block), add a
watcher.on('error', ...) handler that catches the error, logs/console.errors a
JSON warning including the error message and context (e.g., SCREEN_DIR), sets
watchReady = false (or otherwise disables watching) and optionally calls
broadcast({ type: 'warning', message: ... }) so the rest of the app is informed;
ensure the handler swallows the error to prevent the process from crashing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0503d4a3-509c-408a-b1a5-17af288884c9
⛔ Files ignored due to path filters (298)
skills/brainstorming/scripts/node_modules/.bin/mimeis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/.package-lock.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/accepts/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/accepts/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/accepts/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/accepts/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/accepts/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/anymatch/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/anymatch/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/anymatch/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/anymatch/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/anymatch/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/array-flatten/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/array-flatten/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/array-flatten/array-flatten.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/array-flatten/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/binary-extensions.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/binary-extensions.json.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/licenseis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/binary-extensions/readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/lib/read.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/lib/types/json.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/lib/types/raw.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/lib/types/text.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/lib/types/urlencoded.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/body-parser/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/compile.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/constants.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/expand.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/parse.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/stringify.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/lib/utils.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/braces/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/bytes/History.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/bytes/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/bytes/Readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/bytes/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/bytes/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/actualApply.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/actualApply.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/applyBind.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/applyBind.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/functionApply.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/functionApply.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/functionCall.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/functionCall.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/reflectApply.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/reflectApply.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bind-apply-helpers/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/call-bound/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/lib/constants.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/lib/fsevents-handler.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/lib/nodefs-handler.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/chokidar/types/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-disposition/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-disposition/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-disposition/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-disposition/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-disposition/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-type/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-type/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-type/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-type/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/content-type/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie-signature/History.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie-signature/Readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie-signature/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie-signature/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie/SECURITY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/cookie/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/.coveralls.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/.npmignoreis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/.travis.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/Makefileis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/component.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/karma.conf.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/node.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/src/browser.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/src/debug.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/src/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/src/inspector-log.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/debug/src/node.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/History.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/Readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/lib/browser/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/depd/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/destroy/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/destroy/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/destroy/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/destroy/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/get.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/get.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/set.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/set.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/test/get.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/test/set.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/dunder-proto/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/ee-first/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/ee-first/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/ee-first/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/ee-first/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/encodeurl/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/encodeurl/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/encodeurl/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/encodeurl/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-define-property/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/eval.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/eval.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/range.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/range.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/ref.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/ref.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/syntax.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/syntax.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/type.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/type.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/uri.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-errors/uri.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/RequireObjectCoercible.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/RequireObjectCoercible.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/ToObject.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/ToObject.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/isObject.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/isObject.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/es-object-atoms/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/escape-html/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/escape-html/Readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/escape-html/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/escape-html/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/etag/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/etag/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/etag/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/etag/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/etag/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/History.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/Readme.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/application.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/express.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/middleware/init.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/middleware/query.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/request.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/response.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/router/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/router/layer.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/router/route.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/utils.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/lib/view.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/express/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fill-range/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fill-range/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fill-range/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fill-range/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/SECURITY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/finalhandler/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/forwarded/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/forwarded/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/forwarded/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/forwarded/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/forwarded/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fresh/HISTORY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fresh/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fresh/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fresh/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/fresh/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/.github/SECURITY.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/implementation.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/test/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/function-bind/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-intrinsic/test/GetIntrinsic.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/.nycrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/Object.getPrototypeOf.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/Object.getPrototypeOf.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/Reflect.getPrototypeOf.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/Reflect.getPrototypeOf.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/index.d.tsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/test/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/get-proto/tsconfig.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/glob-parent/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/glob-parent/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/glob-parent/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/glob-parent/index.jsis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/glob-parent/package.jsonis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/.eslintrcis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/.github/FUNDING.ymlis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/CHANGELOG.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/LICENSEis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/README.mdis excluded by!**/node_modules/**skills/brainstorming/scripts/node_modules/gopd/gOPD.d.tsis excluded by!**/node_modules/**
📒 Files selected for processing (2)
.gitignoreskills/brainstorming/scripts/index.js
💤 Files with no reviewable changes (1)
- .gitignore
- Normalize choice events to support both data-choice clicks and the
explicit brainstorm.choice() API which sends {value} not {choice}
- Prevent client data from overriding server-assigned source field
- Guard readFileSync against TOCTOU race when screen files disappear
- Add FSWatcher error listener to prevent unhandled crash on runtime errors
There was a problem hiding this comment.
🧹 Nitpick comments (2)
skills/brainstorming/scripts/index.js (2)
126-133: Optional: Consider adding a buffer size limit for defense-in-depth.The buffer accumulates unbounded data until a complete frame arrives (line 129). A malicious or buggy client sending incomplete frames could cause memory exhaustion. Since the server defaults to
127.0.0.1, the attack surface is minimal, but a cap would harden it for any future exposure.♻️ Suggested guard
function handleWsConnection(socket) { clients.add(socket); let buf = Buffer.alloc(0); + const MAX_BUFFER = 1024 * 1024; // 1MB limit socket.on('data', (chunk) => { buf = Buffer.concat([buf, chunk]); + if (buf.length > MAX_BUFFER) { + socket.destroy(); + return; + } while (true) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 126 - 133, The buffer "buf" used in socket.on('data') while parsing frames via wsParseFrame can grow unbounded; add a defensive maximum size (e.g., MAX_BUF_BYTES) and after each Buffer.concat check if buf.length > MAX_BUF_BYTES, and if so either drop the connection (socket.destroy()/socket.end()) or respond with an error and close; keep references to buf, wsParseFrame and frame.totalLen when implementing this guard so you reject/trim before entering the parse loop to prevent memory exhaustion from malicious/unfinished frames.
140-146: Consider echoing ping payload in pong response.RFC 6455 requires pong frames to echo the ping's payload. Currently the pong is always empty. This works with browser clients (which handle ping/pong internally) but could cause issues with stricter WebSocket clients.
♻️ Suggested fix
if (frame.opcode === 0x09) { // Ping -> Pong - const pong = Buffer.alloc(2); - pong[0] = 0x8a; // fin + pong - pong[1] = 0; - socket.write(pong); + const pongPayload = frame.data; + const pongHeader = Buffer.alloc(2); + pongHeader[0] = 0x8a; // fin + pong + pongHeader[1] = pongPayload.length; + socket.write(Buffer.concat([pongHeader, pongPayload])); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 140 - 146, The pong currently sends an empty payload; change the pong construction to echo the ping's payload: determine the incoming payload length from the received frame (use the existing frame payload property, e.g. frame.payload), allocate the pong Buffer to include header + payload (instead of Buffer.alloc(2) use Buffer.alloc(2 + payloadLength)), keep pong[0] = 0x8a, set the length byte(s) appropriately for payloadLength, copy the incoming payload into pong starting at offset 2, then call socket.write(pong) so the pong echoes the ping payload (use the same frame.opcode handling block where frame.opcode === 0x09 and socket.write is used).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 126-133: The buffer "buf" used in socket.on('data') while parsing
frames via wsParseFrame can grow unbounded; add a defensive maximum size (e.g.,
MAX_BUF_BYTES) and after each Buffer.concat check if buf.length > MAX_BUF_BYTES,
and if so either drop the connection (socket.destroy()/socket.end()) or respond
with an error and close; keep references to buf, wsParseFrame and frame.totalLen
when implementing this guard so you reject/trim before entering the parse loop
to prevent memory exhaustion from malicious/unfinished frames.
- Around line 140-146: The pong currently sends an empty payload; change the
pong construction to echo the ping's payload: determine the incoming payload
length from the received frame (use the existing frame payload property, e.g.
frame.payload), allocate the pong Buffer to include header + payload (instead of
Buffer.alloc(2) use Buffer.alloc(2 + payloadLength)), keep pong[0] = 0x8a, set
the length byte(s) appropriately for payloadLength, copy the incoming payload
into pong starting at offset 2, then call socket.write(pong) so the pong echoes
the ping payload (use the same frame.opcode handling block where frame.opcode
=== 0x09 and socket.write is used).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e3759f7-e603-4ac3-8962-ff6e34fa4922
📒 Files selected for processing (1)
skills/brainstorming/scripts/index.js
There was a problem hiding this comment.
🧹 Nitpick comments (3)
skills/brainstorming/scripts/index.js (3)
230-262: UnusedwatchReadyvariable.The
watchReadyvariable is set totrueon success but never read elsewhere in the code. Consider removing it or using it to conditionally log/expose watch status.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 230 - 262, Remove the unused watchReady variable: it's set to true after creating the fs.watch watcher but never read elsewhere, so delete the watchReady declaration and assignment; keep the watcher creation and error handler (watcher and fs.watch) intact, or alternatively if you want to expose status, replace watchReady with a meaningful status/log call using watcher (e.g., log "file watching enabled") instead of storing an unused boolean.
119-161: Potential unbounded buffer growth on malformed input.If a client sends data that never parses into a complete frame (e.g., malformed length fields),
bufwill grow indefinitely until memory is exhausted. Consider adding a maximum buffer size check:♻️ Suggested improvement
+const MAX_WS_BUFFER = 1024 * 1024; // 1 MB max buffer + function handleWsConnection(socket) { clients.add(socket); let buf = Buffer.alloc(0); socket.on('data', (chunk) => { buf = Buffer.concat([buf, chunk]); + if (buf.length > MAX_WS_BUFFER) { + socket.destroy(); + clients.delete(socket); + return; + } while (true) { const frame = wsParseFrame(buf);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 119 - 161, In handleWsConnection, buf can grow unbounded if wsParseFrame() never returns a complete frame; add a maximum buffer size guard (e.g., MAX_WS_BUF_BYTES) and after concatenating new chunks check buf.length against that limit; if exceeded, log an error (including socket id or remote address), optionally send a close frame and destroy/end the socket, and free/reset buf to prevent memory exhaustion; make changes around the socket.on('data', ...) logic where buf and wsParseFrame(buf) are used so malformed or malicious inputs are dropped promptly.
33-44: Race condition: file may be deleted betweenreaddirSyncandstatSync.If a file is deleted after
readdirSyncbut beforestatSync, the function will throw despite the outer try/catch not covering this case properly (the error occurs insidemapwhich will propagate). Consider handling individual stat failures:♻️ Suggested improvement
try { const files = fs.readdirSync(SCREEN_DIR) .filter(f => f.endsWith('.html')) - .map(f => { + .map(f => { const p = path.join(SCREEN_DIR, f); - return { path: p, mtime: fs.statSync(p).mtime.getTime() }; + try { + return { path: p, mtime: fs.statSync(p).mtime.getTime() }; + } catch { + return null; + } }) + .filter(Boolean) .sort((a, b) => b.mtime - a.mtime); return files.length > 0 ? files[0].path : null; } catch { return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 33 - 44, The current read-sort logic using readdirSync(...).map(...statSync...) can throw if a file is deleted between readdirSync and statSync; update the mapping to handle per-file stat failures by wrapping the fs.statSync(p) call in a try/catch (or use fs.existsSync/fs.promises.stat with Promise.allSettled) so that if stat fails you skip that file instead of allowing the exception to propagate; keep the same output behavior (choose newest .html by mtime or return null if none) and reference SCREEN_DIR, readdirSync, statSync, the .map callback that builds { path, mtime }.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 230-262: Remove the unused watchReady variable: it's set to true
after creating the fs.watch watcher but never read elsewhere, so delete the
watchReady declaration and assignment; keep the watcher creation and error
handler (watcher and fs.watch) intact, or alternatively if you want to expose
status, replace watchReady with a meaningful status/log call using watcher
(e.g., log "file watching enabled") instead of storing an unused boolean.
- Around line 119-161: In handleWsConnection, buf can grow unbounded if
wsParseFrame() never returns a complete frame; add a maximum buffer size guard
(e.g., MAX_WS_BUF_BYTES) and after concatenating new chunks check buf.length
against that limit; if exceeded, log an error (including socket id or remote
address), optionally send a close frame and destroy/end the socket, and
free/reset buf to prevent memory exhaustion; make changes around the
socket.on('data', ...) logic where buf and wsParseFrame(buf) are used so
malformed or malicious inputs are dropped promptly.
- Around line 33-44: The current read-sort logic using
readdirSync(...).map(...statSync...) can throw if a file is deleted between
readdirSync and statSync; update the mapping to handle per-file stat failures by
wrapping the fs.statSync(p) call in a try/catch (or use
fs.existsSync/fs.promises.stat with Promise.allSettled) so that if stat fails
you skip that file instead of allowing the exception to propagate; keep the same
output behavior (choose newest .html by mtime or return null if none) and
reference SCREEN_DIR, readdirSync, statSync, the .map callback that builds {
path, mtime }.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 746e0919-0f51-4b93-b713-b54a833fd372
📒 Files selected for processing (2)
skills/brainstorming/scripts/frame-template.htmlskills/brainstorming/scripts/index.js
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)
skills/brainstorming/scripts/helper.js (1)
64-66:⚠️ Potential issue | 🟡 MinorAvoid innerHTML with unescaped textContent to prevent XSS.
The
labelextracted viatextContentcan contain characters like<or>which, when inserted viainnerHTML, will be interpreted as HTML. While the risk is low in this agent-generated content context, using DOM APIs would be safer.🛡️ Suggested fix using DOM manipulation
} else if (selected.length === 1) { const label = selected[0].querySelector('h3, .content h3, .card-body h3')?.textContent?.trim() || selected[0].dataset.choice; - indicator.innerHTML = '<span class="selected-text">' + label + ' selected</span> — return to terminal to continue'; + indicator.textContent = ''; + const span = document.createElement('span'); + span.className = 'selected-text'; + span.textContent = label + ' selected'; + indicator.appendChild(span); + indicator.appendChild(document.createTextNode(' — return to terminal to continue'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/helper.js` around lines 64 - 66, The code sets indicator.innerHTML using unescaped label (from selected[0].textContent), which risks XSS; change this to DOM-safe manipulation: clear indicator, create a span element with class "selected-text", set span.textContent = label + ' selected', append the span to indicator, then append a text node ' — return to terminal to continue' (or set indicator.appendChild for both). Update the code that references indicator and label (and the selected array handling) to use element creation/textContent instead of innerHTML.
🤖 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 `@skills/brainstorming/scripts/helper.js`:
- Around line 64-66: The code sets indicator.innerHTML using unescaped label
(from selected[0].textContent), which risks XSS; change this to DOM-safe
manipulation: clear indicator, create a span element with class "selected-text",
set span.textContent = label + ' selected', append the span to indicator, then
append a text node ' — return to terminal to continue' (or set
indicator.appendChild for both). Update the code that references indicator and
label (and the selected array handling) to use element creation/textContent
instead of innerHTML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f68e9d68-99bd-47e6-a0c7-3832a90cb840
📒 Files selected for processing (1)
skills/brainstorming/scripts/helper.js
…ugs" This reverts commit cec2f1b.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
skills/brainstorming/scripts/index.js (3)
143-155:⚠️ Potential issue | 🟠 MajorNormalize choice events to handle both click and explicit API calls.
Events sent via
window.brainstorm.choice(value, metadata)have{ type: 'choice', value }but the code only persists events withevent.choice, silently dropping calls from the explicit API. Additionally,...eventspreads aftersource: 'user-event', allowing event data to override the server-assigned source.🐛 Proposed fix
if (frame.opcode === 0x01) { // Text frame try { const event = JSON.parse(frame.data.toString()); - console.log(JSON.stringify({ source: 'user-event', ...event })); - if (event.choice) { + const normalizedEvent = { + ...event, + choice: event.choice ?? event.value + }; + console.log(JSON.stringify({ ...normalizedEvent, source: 'user-event' })); + if (normalizedEvent.choice != null) { const eventsFile = path.join(SCREEN_DIR, '.events'); - fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n'); + fs.appendFileSync(eventsFile, JSON.stringify(normalizedEvent) + '\n'); } } catch { // Ignore malformed JSON }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 143 - 155, The code drops explicit API calls and allows incoming event data to override the source: when handling text frames (check frame.opcode === 0x01) normalize choice events from both forms ({ type: 'choice', value }) and ({ choice }) by treating event.choice || (event.type === 'choice' ? event.value : undefined) as the canonical choice value, and persist when that canonical choice exists to SCREEN_DIR + '/.events' (same fs.appendFileSync call, but write the normalized event). Also prevent incoming data from overriding the server-assigned source by ensuring source: 'user-event' is set after spreading event (or by omitting source from the spread) when building the logged object (the code that currently does console.log(JSON.stringify({ source: 'user-event', ...event })) should place ...event before source or remove any event.source before spreading).
177-182:⚠️ Potential issue | 🟠 MajorGuard the screen read against file churn.
Between checking
screenFileand reading it, the file can be deleted or replaced.fs.readFileSyncwill throw and crash the server instead of falling back cleanly. This was flagged previously and marked as addressed, but the current code does not appear to include the fix.🐛 Proposed fix
if (!screenFile) { html = WAITING_PAGE; } else { - const raw = fs.readFileSync(screenFile, 'utf-8'); - html = isFullDocument(raw) ? raw : wrapInFrame(raw); + try { + const raw = fs.readFileSync(screenFile, 'utf-8'); + html = isFullDocument(raw) ? raw : wrapInFrame(raw); + } catch { + html = WAITING_PAGE; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 177 - 182, The code checks screenFile then calls fs.readFileSync(screenFile, 'utf-8') which can throw if the file is deleted or changed between the check and read; update the block around screenFile to wrap the read in a try/catch (or use fs.existsSync+try) so that any error from fs.readFileSync is caught and handled by falling back to WAITING_PAGE (or other existing fallback), then process the raw content with isFullDocument(raw) ? raw : wrapInFrame(raw); ensure you reference the same symbols (screenFile, fs.readFileSync, isFullDocument, wrapInFrame, WAITING_PAGE) so the fix is applied to the exact code path.
223-247:⚠️ Potential issue | 🟠 MajorAdd error event listener to prevent unhandled FSWatcher crashes.
The
try/catchonly coversfs.watch()construction.FSWatcheremits'error'events for runtime failures (directory deletion, permission issues, EMFILE), and without a listener the process will crash with an unhandled error event. This was flagged previously and marked as addressed, but the current code does not appear to include the fix.🐛 Proposed fix
try { const watcher = fs.watch(SCREEN_DIR, (eventType, filename) => { if (!filename || !filename.endsWith('.html')) return; // Debounce: only act on 'rename' (new file) or 'change' if (eventType === 'rename') { // Check if the file exists (rename fires for both create and delete) const filePath = path.join(SCREEN_DIR, filename); if (fs.existsSync(filePath)) { // Clear events from previous screen const eventsFile = path.join(SCREEN_DIR, '.events'); try { fs.unlinkSync(eventsFile); } catch {} console.log(JSON.stringify({ type: 'screen-added', file: filePath })); broadcast({ type: 'reload' }); } } else if (eventType === 'change') { const filePath = path.join(SCREEN_DIR, filename); console.log(JSON.stringify({ type: 'screen-updated', file: filePath })); broadcast({ type: 'reload' }); } }); + watcher.on('error', (err) => { + console.error(JSON.stringify({ + type: 'warning', + message: 'fs.watch failed, file watching disabled', + error: err.message + })); + }); watchReady = true; } catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 223 - 247, The fs.watch watcher can emit 'error' events that will crash the process if unhandled; attach an error listener to the created watcher (the watcher returned from fs.watch in this block) that catches errors, logs a JSON warning (similar to the existing catch message), sets watchReady = false and closes the watcher if necessary, and optionally broadcast a notification; update the code around fs.watch (the watcher variable and watchReady flag) to add watcher.on('error', ...) handling for runtime FSWatcher failures such as EMFILE or directory deletion.
🧹 Nitpick comments (1)
skills/brainstorming/scripts/index.js (1)
91-117: Consider replacing deprecatedBuffer.slice()withBuffer.subarray().
Buffer.slice()is deprecated in Node.js. While functionally equivalent for this use case,Buffer.subarray()is the recommended alternative.♻️ Proposed fix
if (masked) { if (buffer.length < offset + 4 + payloadLen) return null; - const mask = buffer.slice(offset, offset + 4); + const mask = buffer.subarray(offset, offset + 4); offset += 4; const data = Buffer.alloc(payloadLen); for (let i = 0; i < payloadLen; i++) { data[i] = buffer[offset + i] ^ mask[i % 4]; } return { opcode: buffer[0] & 0x0f, data, totalLen: offset + payloadLen }; } if (buffer.length < offset + payloadLen) return null; - return { opcode: buffer[0] & 0x0f, data: buffer.slice(offset, offset + payloadLen), totalLen: offset + payloadLen }; + return { opcode: buffer[0] & 0x0f, data: buffer.subarray(offset, offset + payloadLen), totalLen: offset + payloadLen };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/scripts/index.js` around lines 91 - 117, The wsParseFrame function uses Buffer.slice (mask = buffer.slice(offset, offset + 4) and returning data: buffer.slice(offset, offset + payloadLen)); replace those Buffer.slice calls with Buffer.subarray to avoid the deprecated API while preserving semantics (use buffer.subarray(start, end) for the mask and for the non-masked data return). Ensure the rest of wsParseFrame logic (masking loop, totalLen, opcode) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 119-128: handleWsConnection accumulates an unbounded Buffer in the
buf variable which can lead to memory exhaustion; add a fixed maximum buffer
size constant and after concatenating chunk check buf.length against that limit
and if exceeded either discard the connection by calling
socket.destroy()/socket.end() with a logged warning or send a protocol-level
error and close; continue to call wsParseFrame(buf) while frames are complete
and only slice processed bytes (as currently done) so partial frames remain, and
document the limit near handleWsConnection and variable buf to make the
safeguard obvious.
---
Duplicate comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 143-155: The code drops explicit API calls and allows incoming
event data to override the source: when handling text frames (check frame.opcode
=== 0x01) normalize choice events from both forms ({ type: 'choice', value })
and ({ choice }) by treating event.choice || (event.type === 'choice' ?
event.value : undefined) as the canonical choice value, and persist when that
canonical choice exists to SCREEN_DIR + '/.events' (same fs.appendFileSync call,
but write the normalized event). Also prevent incoming data from overriding the
server-assigned source by ensuring source: 'user-event' is set after spreading
event (or by omitting source from the spread) when building the logged object
(the code that currently does console.log(JSON.stringify({ source: 'user-event',
...event })) should place ...event before source or remove any event.source
before spreading).
- Around line 177-182: The code checks screenFile then calls
fs.readFileSync(screenFile, 'utf-8') which can throw if the file is deleted or
changed between the check and read; update the block around screenFile to wrap
the read in a try/catch (or use fs.existsSync+try) so that any error from
fs.readFileSync is caught and handled by falling back to WAITING_PAGE (or other
existing fallback), then process the raw content with isFullDocument(raw) ? raw
: wrapInFrame(raw); ensure you reference the same symbols (screenFile,
fs.readFileSync, isFullDocument, wrapInFrame, WAITING_PAGE) so the fix is
applied to the exact code path.
- Around line 223-247: The fs.watch watcher can emit 'error' events that will
crash the process if unhandled; attach an error listener to the created watcher
(the watcher returned from fs.watch in this block) that catches errors, logs a
JSON warning (similar to the existing catch message), sets watchReady = false
and closes the watcher if necessary, and optionally broadcast a notification;
update the code around fs.watch (the watcher variable and watchReady flag) to
add watcher.on('error', ...) handling for runtime FSWatcher failures such as
EMFILE or directory deletion.
---
Nitpick comments:
In `@skills/brainstorming/scripts/index.js`:
- Around line 91-117: The wsParseFrame function uses Buffer.slice (mask =
buffer.slice(offset, offset + 4) and returning data: buffer.slice(offset, offset
+ payloadLen)); replace those Buffer.slice calls with Buffer.subarray to avoid
the deprecated API while preserving semantics (use buffer.subarray(start, end)
for the mask and for the non-masked data return). Ensure the rest of
wsParseFrame logic (masking loop, totalLen, opcode) remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 274ba4ad-3426-44a4-a8a9-6817bfd6ce0e
📒 Files selected for processing (1)
skills/brainstorming/scripts/index.js
Owner
|
Thanks for this. We implemented something similar today |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Closes #688
http,crypto,fs), replacingexpress,ws, andchokidarnode_modules/(4.6MB),package.json, andpackage-lock.json.gitignoreexception that allowed committingnode_modulesMotivation
Bundled
node_modules/bypasses npm audit, Dependabot, and lockfile verification — making injected malicious code persist undetected. A single-route server with file watching doesn't need three npm packages.