Skip to content

chore(build): re-apply build-reorg follow-ups (compat fallback + deploy docs) #3127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 3 commits into from
Jun 3, 2026
Merged

chore(build): re-apply build-reorg follow-ups (compat fallback + deploy docs) #3127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ce2e18
build(compat): serve CLI falls back dist/ -> app/ for upgrade safety
diegosouzapw Jun 3, 2026
757d88e
docs(deploy): stop pm2 before rsync --delete to avoid transient chunk…
diegosouzapw Jun 3, 2026
96f3d22
test(cli): cover serve APP_DIR dist/->app/ backward-compat fallback
diegosouzapw Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .agents/skills/deploy-vps-akamai-cc/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,20 @@ OMNIROUTE_VERSION=$(node -p "require('/home/diegosouzapw/dev/proxys/OmniRoute/pa
ssh root@69.164.221.35 "cp -a /usr/lib/node_modules/omniroute/app /usr/lib/node_modules/omniroute/app.bak-${OMNIROUTE_VERSION}"
```

### 3. Rsync `dist/` → remote `app/`
### 3. Stop the process, then rsync `dist/` → remote `app/`

The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`:
The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`. **Stop the running process FIRST** so `rsync --delete` never removes chunk files out from under the live server — that race produces the transient `[Shutdown] Cannot find module ./chunks/NNNNN.js` seen mid-deploy:

// turbo-all

```bash
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/
Comment on lines +61 to 62

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Prevent rsync after a failed stop

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 👍 / 👎.

```

### 4. Restart + health check
### 4. Start + health check

`pm2 restart` on a stopped app starts it again with a clean module graph:

```bash
ssh root@69.164.221.35 "pm2 restart omniroute --update-env && pm2 save"
Expand Down
9 changes: 6 additions & 3 deletions .agents/skills/deploy-vps-both-cc/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,20 @@ ssh root@192.168.0.15 "cp -a /usr/lib/node_modules/omniroute/app /usr/lib/node_m
wait
```

### 3. Rsync `dist/` → remote `app/` on both VPSs
### 3. Stop the processes, then rsync `dist/` → remote `app/` on both VPSs

The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`:
The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`. **Stop both running processes FIRST** so `rsync --delete` never removes chunk files out from under a live server — that race produces the transient `[Shutdown] Cannot find module ./chunks/NNNNN.js` seen mid-deploy:

// turbo-all

```bash
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/
Comment on lines +64 to 65

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate both rsyncs on successful PM2 stops

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 👍 / 👎.

```

### 4. Restart + health check on both VPSs
### 4. Start + health check on both VPSs

`pm2 restart` on a stopped app starts it again with a clean module graph:

```bash
ssh root@69.164.221.35 "pm2 restart omniroute --update-env && pm2 save"
Expand Down
9 changes: 6 additions & 3 deletions .agents/skills/deploy-vps-local-cc/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,20 @@ OMNIROUTE_VERSION=$(node -p "require('/home/diegosouzapw/dev/proxys/OmniRoute/pa
ssh root@192.168.0.15 "cp -a /usr/lib/node_modules/omniroute/app /usr/lib/node_modules/omniroute/app.bak-${OMNIROUTE_VERSION}"
```

### 3. Rsync `dist/` → remote `app/`
### 3. Stop the process, then rsync `dist/` → remote `app/`

The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`:
The VPS image directory is still named `app/`; we ship the **contents** of our local `dist/`. **Stop the running process FIRST** so `rsync --delete` never removes chunk files out from under the live server — that race produces the transient `[Shutdown] Cannot find module ./chunks/NNNNN.js` seen mid-deploy:

// turbo-all

```bash
ssh root@192.168.0.15 "pm2 stop omniroute"
rsync -az --delete /home/diegosouzapw/dev/proxys/OmniRoute/dist/ root@192.168.0.15:/usr/lib/node_modules/omniroute/app/
```

### 4. Restart + health check
### 4. Start + health check

`pm2 restart` on a stopped app starts it again with a clean module graph:

```bash
ssh root@192.168.0.15 "pm2 restart omniroute --update-env && pm2 save"
Expand Down
6 changes: 5 additions & 1 deletion bin/cli/commands/serve.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import { isTermux } from "../../../scripts/build/postinstallSupport.mjs";

const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = join(__dirname, "..", "..", "..");
const APP_DIR = join(ROOT, "dist");
// The standalone bundle ships in `dist/` (since the build-output-isolation
// refactor). Fall back to the legacy `app/` location so an upgrade over a
// partially-replaced install — or a package built before the rename — still
// boots. Backward-compatible by design: every deployed runtime keeps its path.
const APP_DIR = existsSync(join(ROOT, "dist", "server.js")) ? join(ROOT, "dist") : join(ROOT, "app");

function parsePort(value, fallback) {
const parsed = parseInt(String(value), 10);
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/cli-serve-port.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,35 @@ test("serve command: --port option has no Commander default", async () => {
// Ensure the option does NOT have a third argument (Commander default)
assert.match(serveSource, /\.option\("--port <port>",\s*t\("serve\.port"\)\)/);
});

/**
* Replicate the APP_DIR resolution from bin/cli/commands/serve.mjs to verify the
* backward-compatibility fallback introduced by the build-output-isolation refactor:
* the standalone bundle now ships in `dist/`, but an upgrade over a partially-replaced
* install — or a package built before the rename — must still boot from legacy `app/`.
*/
function resolveAppDir(root: string, distServerExists: boolean): string {
return distServerExists ? `${root}/dist` : `${root}/app`;
}

test("serve app dir: resolves to dist/ when dist/server.js exists (current layout)", () => {
assert.equal(resolveAppDir("/opt/omniroute", true), "/opt/omniroute/dist");
});

test("serve app dir: falls back to legacy app/ when dist/server.js is absent (upgrade safety)", () => {
assert.equal(resolveAppDir("/opt/omniroute", false), "/opt/omniroute/app");
});

test("serve command: APP_DIR keeps the dist/ -> app/ backward-compat fallback", async () => {
const fs = await import("node:fs");
const path = await import("node:path");
const serveSource = fs.readFileSync(
path.resolve(import.meta.dirname, "../../bin/cli/commands/serve.mjs"),
"utf-8",
);
// Must probe dist/server.js and fall back to app/ — never hard-code dist/ only.
assert.match(
serveSource,
/existsSync\(join\(ROOT, "dist", "server\.js"\)\)\s*\?\s*join\(ROOT, "dist"\)\s*:\s*join\(ROOT, "app"\)/,
);
Comment on lines +84 to +87

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

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

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*\)/);

});