Skip to content

docs: fix outdated Node 24 warnings in TROUBLESHOOTING.md - #1343

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.6.7from
oyi77:fix/troubleshooting-node24-refs
Apr 16, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.6.7from
oyi77:fix/troubleshooting-node24-refs

Conversation

@oyi77

@oyi77 oyi77 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Address Gemini code review feedback from PR #1340 by removing outdated Node.js 24 warnings and improving documentation consistency.

Changes

Documentation Fixes

  • ✅ Line 18 (Quick Fixes table): Changed "You may be on Node.js 24+" to "Check Node.js version"
  • ✅ Line 32 (Node.js Compatibility): Removed "Node.js 24+: better-sqlite3 is not supported" warning
  • ✅ Line 52 (Supported versions): Updated from "Node.js 24+ is not supported" to "Node.js 24.x LTS (Krypton) is fully supported"
  • ✅ Updated fix instructions to recommend Node 24 instead of Node 22

Code Improvements

  • ✅ Dockerfile: Pinned base images to node:24.14.1-trixie-slim for build reproducibility
  • ✅ nodeRuntimeSupport.ts: Clarified SUPPORTED_NODE_DISPLAY to show 24.0.0+ minimum version

Context

This PR addresses all 4 review comments from Gemini on PR #1340:

  1. ✅ Pin Dockerfile builder stage to specific version
  2. ✅ Pin Dockerfile runner stage to specific version
  3. ✅ Fix outdated Node 24 warnings in TROUBLESHOOTING.md (lines 18, 32, 52)
  4. ✅ Standardize version display string to show minimum patch version

Testing

Documentation changes only — no functional changes to code.

Related

Address Gemini code review feedback on PR diegosouzapw#1340:

- Remove outdated 'Node.js 24+ not supported' warnings from lines 18, 32, 52
- Update Quick Fixes table to remove Node 24 reference
- Update Node.js Compatibility section to reflect Node 24 support
- Update recommended version to Node 24 in fix instructions
- Pin Dockerfile base images to 24.14.1 for reproducibility
- Clarify SUPPORTED_NODE_DISPLAY to show '24.0.0+' minimum

All documentation now consistent with Node 24 LTS support.
@oyi77
oyi77 requested a review from diegosouzapw as a code owner April 16, 2026 17:38

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the project to officially support and recommend Node.js 24.x. Key changes include pinning the Docker base image to Node 24.14.1, updating the troubleshooting documentation to reflect Node 24 compatibility, and refining runtime support utility strings. The review feedback suggests minor documentation improvements to ensure consistency regarding Node 24's status and to clarify that the recommended version verification applies specifically to the 24.x LTS line.

Comment thread docs/TROUBLESHOOTING.md Outdated

1. **Node.js 24+**: `better-sqlite3` is not supported here and startup can fail hard.
2. **Older Node 20/22 patch levels**: the runtime may start, but it falls below the patched security floor OmniRoute now requires.
**Cause:** You are running a Node.js version outside OmniRoute's approved secure runtime floor. The most common case is running an older Node 20/22 patch level that falls below the patched security floor OmniRoute requires.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The description of the "most common case" for Node.js version issues should be updated to include Node 24, as it is now a supported and recommended version. This ensures the documentation remains consistent with the changes made elsewhere in this PR.

Suggested change
**Cause:** You are running a Node.js version outside OmniRoute's approved secure runtime floor. The most common case is running an older Node 20/22 patch level that falls below the patched security floor OmniRoute requires.
**Cause:** You are running a Node.js version outside OmniRoute's approved secure runtime floor. The most common case is running an older Node 20, 22, or 24 patch level that falls below the patched security floor OmniRoute requires.

Comment thread docs/TROUBLESHOOTING.md Outdated
nvm use 24
```
2. Verify your version: `node --version` should show `v22.22.2` or newer on the 22.x LTS line
2. Verify your version: `node --version` should show `v24.0.0` or newer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The verification step should specify that the version should be on the 24.x line. Without this clarification, "v24.0.0 or newer" could be interpreted as including Node 25+, which is explicitly excluded from the supported range (<25) and the engines field in package.json.

Suggested change
2. Verify your version: `node --version` should show `v24.0.0` or newer
2. Verify your version: node --version should show v24.0.0 or newer on the 24.x LTS line

- Clarify that Node 24 patch levels can also fall below security floor
- Specify verification should be on 24.x LTS line to avoid confusion with Node 25+
@oyi77

oyi77 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Gemini Review Addressed ✅

Applied both suggestions from the code review:

  1. Line 30: Updated to include Node 24 in the "most common case" description:

    • Now: "older Node 20, 22, or 24 patch level"
    • Was: "older Node 20/22 patch level"
  2. Line 45: Clarified version verification to specify 24.x LTS line:

    • Now: "should show v24.0.0 or newer on the 24.x LTS line"
    • Was: "should show v24.0.0 or newer"

This prevents confusion with Node 25+ which is explicitly excluded from the supported range.

Commit: 9a8b7c8

@diegosouzapw
diegosouzapw merged commit 5cbc08a into diegosouzapw:release/v3.6.7 Apr 16, 2026
2 checks passed
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @oyi77 for this great contribution! 🎉 This PR has been evaluated and successfully integrated into the release/v3.6.7 branch and will be part of the final production release. We appreciate your effort!

Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants