Repository navigation
chore(build): re-apply build-reorg follow-ups (compat fallback + deploy docs) - #3127
Conversation
Backward-compat hardening: the npm CLI prefers the new dist/ standalone but falls back to the legacy app/ location, so an upgrade over a partially-replaced install (or a package built before the app/->dist/ rename) still boots. Deployed runtimes (VPS app/, Docker /app, Electron resources/app) are unchanged by design.
… error rsync --delete removing chunk files under a live server produced the transient [Shutdown] Cannot find module ./chunks/NNNNN.js. Stop the process first, then start with a clean module graph.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the deployment documentation across several skills to stop the PM2 process before running rsync, preventing transient chunk file errors during deployment. It also introduces a backward-compatibility fallback in bin/cli/commands/serve.mjs to resolve APP_DIR to the legacy app/ directory if dist/server.js is missing, along with corresponding unit tests. Feedback on the changes suggests refactoring a fragile test assertion in tests/unit/cli-serve-port.test.ts that strictly matches the source code formatting of the fallback logic, recommending a more robust and decoupled regular expression assertion instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| assert.match( | ||
| serveSource, | ||
| /existsSync\(join\(ROOT, "dist", "server\.js"\)\)\s*\?\s*join\(ROOT, "dist"\)\s*:\s*join\(ROOT, "app"\)/, | ||
| ); |
There was a problem hiding this comment.
Fragile Test Assertion
Asserting on the exact source code formatting and syntax structure (such as the specific ternary operator layout, quote types, and spacing) via a strict regular expression is highly fragile. Any minor, non-functional change—such as running an auto-formatter (e.g., Prettier), changing single quotes to double quotes, or refactoring the ternary operator into an if/else block—will break this test even though the underlying behavior remains correct.
Recommendation
Make the assertions more robust and less sensitive to exact formatting by decoupling the checks. You can assert that dist/server.js is probed and that app is used as a fallback using separate, more flexible regular expressions.
assert.match(serveSource, /existsSync\s*\(\s*join\s*\(\s*ROOT\s*,\s*['"]dist['"]\s*,\s*['"]server\.js['"]\s*\)\s*\)/);
assert.match(serveSource, /join\s*\(\s*ROOT\s*,\s*['"]app['"]\s*\)/);![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96f3d22d50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| ssh root@69.164.221.35 "pm2 stop omniroute" & ssh root@192.168.0.15 "pm2 stop omniroute" & wait | ||
| rsync -az --delete /home/diegosouzapw/dev/proxys/OmniRoute/dist/ root@69.164.221.35:/usr/lib/node_modules/omniroute/app/ && rsync -az --delete /home/diegosouzapw/dev/proxys/OmniRoute/dist/ root@192.168.0.15:/usr/lib/node_modules/omniroute/app/ |
There was a problem hiding this comment.
In the both-VPS workflow, if either ssh ... pm2 stop fails, this still proceeds to rsync --delete: Bash wait with no job operands returns success after waiting for all current jobs, so a failed stop is masked. In that failure scenario the host whose app is still live can have chunks deleted mid-request, recreating the [Shutdown] Cannot find module ./chunks/... race this step is meant to prevent; capture the PIDs and fail on each wait status, or otherwise chain the rsyncs only after both stops succeed.
Useful? React with 👍 / 👎.
| ssh root@69.164.221.35 "pm2 stop omniroute" | ||
| rsync -az --delete /home/diegosouzapw/dev/proxys/OmniRoute/dist/ root@69.164.221.35:/usr/lib/node_modules/omniroute/app/ |
There was a problem hiding this comment.
For the single-host deploy flow, a transient SSH/PM2 failure on the stop command does not prevent the next line from running when this block is pasted or executed without set -e. That means rsync --delete can still update the bundle while the old process is live, which is exactly the chunk-deletion race described above; use ssh ... "pm2 stop omniroute" && rsync ... or an explicit failure check before syncing. The same pattern appears in the local VPS skill as well.
Useful? React with 👍 / 👎.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 278,288 tokens |
…oy docs) (cheaper-inference#3127) * build(compat): serve CLI falls back dist/ -> app/ for upgrade safety Backward-compat hardening: the npm CLI prefers the new dist/ standalone but falls back to the legacy app/ location, so an upgrade over a partially-replaced install (or a package built before the app/->dist/ rename) still boots. Deployed runtimes (VPS app/, Docker /app, Electron resources/app) are unchanged by design. * docs(deploy): stop pm2 before rsync --delete to avoid transient chunk error rsync --delete removing chunk files under a live server produced the transient [Shutdown] Cannot find module ./chunks/NNNNN.js. Stop the process first, then start with a clean module graph. * test(cli): cover serve APP_DIR dist/->app/ backward-compat fallback
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
…oy docs) (cheaper-inference#3127) * build(compat): serve CLI falls back dist/ -> app/ for upgrade safety Backward-compat hardening: the npm CLI prefers the new dist/ standalone but falls back to the legacy app/ location, so an upgrade over a partially-replaced install (or a package built before the app/->dist/ rename) still boots. Deployed runtimes (VPS app/, Docker /app, Electron resources/app) are unchanged by design. * docs(deploy): stop pm2 before rsync --delete to avoid transient chunk error rsync --delete removing chunk files under a live server produced the transient [Shutdown] Cannot find module ./chunks/NNNNN.js. Stop the process first, then start with a clean module graph. * test(cli): cover serve APP_DIR dist/->app/ backward-compat fallback
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
…oy docs) (cheaper-inference#3127) * build(compat): serve CLI falls back dist/ -> app/ for upgrade safety Backward-compat hardening: the npm CLI prefers the new dist/ standalone but falls back to the legacy app/ location, so an upgrade over a partially-replaced install (or a package built before the app/->dist/ rename) still boots. Deployed runtimes (VPS app/, Docker /app, Electron resources/app) are unchanged by design. * docs(deploy): stop pm2 before rsync --delete to avoid transient chunk error rsync --delete removing chunk files under a live server produced the transient [Shutdown] Cannot find module ./chunks/NNNNN.js. Stop the process first, then start with a clean module graph. * test(cli): cover serve APP_DIR dist/->app/ backward-compat fallback
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
Re-applies two follow-up commits that landed on `refactor/build-output-isolation` after PR #3124 was merged (merge point was `0231fbb33`), so they were orphaned and never reached `release/v3.8.9`.
What's included
Why a separate PR
#3124 was already merged when these were pushed; cherry-picking them onto a fresh branch keeps the release branch correct without touching the closed PR.
🤖 Generated with Claude Code