refactor: simplify Node installation guidance - #60
mldangelo-oai wants to merge 12 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| "Keep an existing writable npm prefix only if its bin is separate from Python's scripts directory. " | ||
| "If its bin is missing from PATH, add it before /usr/bin " | ||
| "and after your Python scripts (for example /opt/venv/bin or /usr/local/bin).", |
| "If an inherited NPM_CONFIG_PREFIX points to an unwritable directory, change it in the Dockerfile " | ||
| "(for example `ENV NPM_CONFIG_PREFIX=/home/app/.npm-global`). Without that override, a non-root user " | ||
| "can run `npm config set prefix ~/.npm-global`. Add the writable bin to PATH in either case.", |
| " RUN dnf --releasever=latest install -y nodejs24 nodejs24-npm " | ||
| "&& /usr/sbin/alternatives --set node /usr/bin/node-24 && dnf clean all", |
| "Node 24 requires repository release 2023.9.20251110 or newer; replace latest with your approved recent " | ||
| "snapshot, or update an older base image before installing.", |
| "If your pinned repository is older than 2023.9.20251110, update the base/repository or install from a " | ||
| "newer approved snapshot: sudo dnf --releasever=latest install -y nodejs24 nodejs24-npm", |
| "If you choose a container image instead, create a new image-based function " | ||
| "and install Node at build time.", |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Serverless guidance still includes misleading runtime installation instructions.
Review effort: Lite
Findings: None
What changed in this PR
Refactors Node.js installation guidance to be shorter and deployment-aware across platforms, containers, serverless environments, CI, and Windows.
Changes:
- Simplifies environment detection and installation instructions.
- Adds Kubernetes, Docker, distribution-specific, serverless, and Windows guidance.
- Expands Docker and platform validation tests.
- Replaces the duplicate release FAQ with the versioning policy link.
| File | Description |
|---|---|
tests/test_instructions.py |
Updated instruction coverage. |
tests/test_environment.py |
Updated environment detection tests. |
tests/smoke/test_installation.py |
Added Windows command smoke tests. |
src/promptfoo/instructions.py |
Generates contextual Node.js guidance. |
src/promptfoo/environment.py |
Simplifies environment detection. |
AGENTS.md |
Consolidates release FAQ guidance. |
.github/workflows/test.yml |
Validates generated Dockerfiles. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What changed
npx.exe.Verification
pytest -q -m 'not smoke': 195 passed, 10 platform skips, 24 deselected on macOS..npmrc, and verifies apt keeps Node 24. New Windows matrix tests execute the printed commands under restricted Windows PowerShell and Git Bash, including a native.exealternative. Neither a local Docker daemon nor native Windows was available.