Fixes#890
Conversation
WalkthroughNode.js version bumped to v24.4.1 across env and Docker. docker-compose adds LOCAL_MODULES and NODE_OPTIONS flags and propagates NODE_OPTIONS. Server code shifts key handlers to async I/O, adjusts sandbox static serving with latest-version resolution and index fallback. Minor SCSS gradient tweak. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server
participant FS as FileSystem
rect rgb(240,248,255)
note over C,S: GET /channels/:id (broadcast page)
C->>S: GET /channels/:id
S->>FS: readFile(channelHtmlPath)
alt file found
FS-->>S: HTML content
S-->>C: 200 HTML
else not found
S-->>C: 404
end
end
%% Sandbox static serving with latest-version fallback
rect rgb(245,255,245)
note over C,S: GET /sandbox/* (async, latest-version resolution)
C->>S: GET /sandbox/{path}
S->>FS: readdir(sandboxRoot) to find latest vN
S->>FS: readFile(resolvedPath)
alt file found
FS-->>S: Content
S-->>C: 200 (content-type per file)
else not found
note right of S: onError fallback
S->>FS: readFile(latestVersion/index.html)
FS-->>S: HTML content
S-->>C: 200 text/html
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
✅ Deploy Preview for livecodes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Size Change: -215 B (-0.02%) Total Size: 955 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.nvmrc(1 hunks)Dockerfile(2 hunks)docker-compose.yml(3 hunks)server/src/broadcast/index.ts(1 hunks)server/src/sandbox.ts(1 hunks)server/src/utils.ts(2 hunks)src/livecodes/styles/inc-menu.scss(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
server/src/broadcast/index.ts (1)
server/src/app.ts (1)
app(13-13)
server/src/sandbox.ts (1)
server/src/utils.ts (1)
dirname(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: tests (18.x, 2)
- GitHub Check: tests (18.x, 4)
- GitHub Check: tests (18.x, 1)
- GitHub Check: tests (18.x, 5)
- GitHub Check: tests (18.x, 3)
- GitHub Check: build
- GitHub Check: build (18.x)
- GitHub Check: Cloudflare Pages
| ARG NODE_OPTIONS | ||
|
|
There was a problem hiding this comment.
NODE_OPTIONS build arg is never applied
Declaring ARG NODE_OPTIONS alone doesn’t expose it to the build environment, so npm ci/npm run build still run with the default heap limit. If the goal was to avoid Node OOMs during the build, this change has no effect. Please export it (or drop the arg). Example fix:
ARG NODE_OPTIONS
+ENV NODE_OPTIONS=${NODE_OPTIONS}🤖 Prompt for AI Agents
In Dockerfile around lines 24-25, ARG NODE_OPTIONS is declared but never applied
to the image environment so build steps still use default Node heap; change to
export the value by adding ENV NODE_OPTIONS=${NODE_OPTIONS} (or set a default
ARG and then ENV) so subsequent RUN npm ci / npm run build inherit the intended
NODE_OPTIONS, or remove the ARG if not needed.
| const version = | ||
| dirs | ||
| .filter((v) => v.startsWith('v')) | ||
| .map((v) => Number(v.slice(1))) | ||
| .filter((v) => !Number.isNaN(v)) | ||
| .sort((a, b) => b - a) | ||
| .map((v) => 'v' + v) | ||
| .pop() || ''; | ||
| const sandboxVersionDir = path.resolve(sandboxDir, version); |
There was a problem hiding this comment.
Latest sandbox version detection is inverted
After sorting versions in descending order, calling .pop() returns the smallest entry, so the sandbox now serves the oldest bundle whenever multiple v* folders exist. Use .shift() (or grab index 0) to keep the newest directory.
- const version =
- dirs
- .filter((v) => v.startsWith('v'))
- .map((v) => Number(v.slice(1)))
- .filter((v) => !Number.isNaN(v))
- .sort((a, b) => b - a)
- .map((v) => 'v' + v)
- .pop() || '';
+ const version =
+ dirs
+ .filter((v) => v.startsWith('v'))
+ .map((v) => Number(v.slice(1)))
+ .filter((v) => !Number.isNaN(v))
+ .sort((a, b) => b - a)
+ .map((v) => 'v' + v)[0] || '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const version = | |
| dirs | |
| .filter((v) => v.startsWith('v')) | |
| .map((v) => Number(v.slice(1))) | |
| .filter((v) => !Number.isNaN(v)) | |
| .sort((a, b) => b - a) | |
| .map((v) => 'v' + v) | |
| .pop() || ''; | |
| const sandboxVersionDir = path.resolve(sandboxDir, version); | |
| const version = | |
| dirs | |
| .filter((v) => v.startsWith('v')) | |
| .map((v) => Number(v.slice(1))) | |
| .filter((v) => !Number.isNaN(v)) | |
| .sort((a, b) => b - a) | |
| .map((v) => 'v' + v)[0] || ''; | |
| const sandboxVersionDir = path.resolve(sandboxDir, version); |
🤖 Prompt for AI Agents
In server/src/sandbox.ts around lines 16 to 24, the code sorts version numbers
in descending order but then calls .pop(), which returns the smallest (oldest)
version; change that to .shift() (or use [0]) so the newest version is selected,
and keep the existing fallback to '' for the sandboxVersionDir resolution.



Summary by CodeRabbit
Bug Fixes
Style
Chores