Skip to content

Port offset script to support running multiple instances#5295

Draft
Ben-Ho wants to merge 1 commit intomainfrom
set-port-offset
Draft

Port offset script to support running multiple instances#5295
Ben-Ho wants to merge 1 commit intomainfrom
set-port-offset

Conversation

@Ben-Ho
Copy link
Contributor

@Ben-Ho Ben-Ho commented Mar 11, 2026

All Comet DXP applications use the same default ports (API: 4000, Admin auth-proxy: 8000, Site auth-proxy: 3000, etc.). Running comet-demo alongside another project (e.g. comet-starter) is not possible without manually editing environment files.

Adds npm run set-ports -- to shift all development ports by an integer offset and write the result to .env.local. This makes it possible to run two Comet DXP applications simultaneously without port conflicts. Affected env-vars are listed in set-port.js, this list has to be updated if new related env-vars are added.

Also added claude-rule to think about updating set-ports.js if adding a new PORT- or URL-env var in future.

All Comet DXP applications use the same default ports (API: 4000, Admin auth-proxy: 8000, Site auth-proxy: 3000, etc.). Running comet-demo alongside another project (e.g. comet-starter) is not possible without manually editing environment files.

Adds npm run set-ports --  to shift all development ports by an integer offset and write the result to .env.local. This makes it possible to run two Comet DXP applications simultaneously without port conflicts.
Affected env-vars are listed in set-port.js, this list has to be updated if new related env-vars are added.
@Ben-Ho Ben-Ho requested a review from Copilot March 11, 2026 10:54
@Ben-Ho Ben-Ho self-assigned this Mar 11, 2026
@auto-assign auto-assign bot requested a review from johnnyomair March 11, 2026 10:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a root-level set-ports utility to generate port- and URL-overrides for the demo setup so multiple Comet DXP apps can be run side-by-side without manual env editing.

Changes:

  • Introduces set-ports.js to apply an integer offset to selected port env vars and rewrite corresponding URL env vars into demo/.env.local.
  • Adds a pnpm run set-ports script and documents the workflow in README.md.
  • Adds a Claude rule documenting how/when to update set-ports.js when new env vars are introduced.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
set-ports.js New CLI script to generate offset ports + rewritten URL env vars into demo/.env.local.
package.json Adds set-ports script entry.
README.md Documents running the demo with port offsets for multi-instance development.
.claude/rules/env-vars.md Adds guidance to keep set-ports.js in sync with new port/URL env vars.

You can also share your feedback on Copilot code review. Take the survey.

const path = require('node:path');

const ENV_FILE = path.join(__dirname, '.env');
const ENV_LOCAL_FILE = path.join(__dirname, 'demo', '.env.local');
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The script writes the offset ports to demo/.env.local, but docker compose up (used by the demo-docker dev process) reads variable substitutions from the root .env by default. With a non-zero offset this can desync container ports (postgres/imgproxy/jaeger/valkey) from the app’s connection settings and prevent the demo from starting. Consider either generating an env file that docker compose actually consumes (e.g. root .env/.env.local) or adjusting the docker-compose startup command to load demo/.env.local (via --env-file or dotenv -e ... -- docker compose ...).

Suggested change
const ENV_LOCAL_FILE = path.join(__dirname, 'demo', '.env.local');
const ENV_LOCAL_FILE = path.join(__dirname, '.env');

Copilot uses AI. Check for mistakes.
console.error(`Error: ${portVar} in .env is not a valid integer ("${rawEnv[portVar]}")`);
process.exit(1);
}
newPorts[portVar] = base + offset;
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Ports are computed as base + offset without validating the resulting range. Offsets that produce ports <= 0 or > 65535 will generate an unusable .env.local and potentially confusing runtime failures. Consider validating the computed ports and exiting with a clear error when an invalid range is produced.

Suggested change
newPorts[portVar] = base + offset;
const computedPort = base + offset;
if (computedPort <= 0 || computedPort > 65535) {
console.error(
`Error: computed port for ${portVar} (${computedPort}) is outside the valid range 1–65535. ` +
'Check your base port and port offset configuration.',
);
process.exit(1);
}
newPorts[portVar] = computedPort;

Copilot uses AI. Check for mistakes.
Comment on lines +136 to +137
All Comet DXP demo services share the same default ports (API: 4000, Admin: 8001, Site: 3000, etc.).
To run two instances at the same time, shift one app's ports by an integer offset:
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This sentence says “Admin: 8001”, but elsewhere in the README the Demo Admin is documented as available at http://localhost:8000/ (auth-proxy). In .env, AUTHPROXY_PORT is 8000 and ADMIN_PORT is 8001; the user-facing Admin URL is typically 8000. Consider clarifying which port is which (e.g., Admin via auth-proxy 8000 vs internal admin 8001) to avoid confusion.

Copilot uses AI. Check for mistakes.
Comment on lines +139 to +141
pnpm run set-ports -- 100 # shift all ports by +100 (writes demo/.env.local)
pnpm run set-ports -- 0 # reset to default port values

Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The command examples are formatted as an indented code block, while surrounding sections use fenced ```bash blocks. Consider switching these lines to a fenced block for consistency and proper syntax highlighting.

Copilot uses AI. Check for mistakes.
"setup:download-oauth2-proxy": "dotenv -- sh -c 'pnpm exec comet download-oauth2-proxy -v $OAUTH2_PROXY_VERSION'",
"setup:download-mitmproxy": "dotenv -- sh -c 'pnpm exec comet download-mitmproxy -v $MITMPROXY_VERSION'"
"setup:download-mitmproxy": "dotenv -- sh -c 'pnpm exec comet download-mitmproxy -v $MITMPROXY_VERSION'",
"set-ports": "node set-ports.js"
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The PR description mentions “npm run set-ports …”, but this repo is configured to use pnpm (packageManager: pnpm@…) and the docs use pnpm run set-ports. Consider updating the PR description (or any related documentation) so the advertised command matches the actual package manager used in this repo.

Copilot uses AI. Check for mistakes.

The script rewrites `demo/.env.local` with the new port and URL values.
Since `demo/.env.local` is symlinked into `demo/api/`, `demo/admin/`, and `demo/site/`, one command covers the whole demo application.

Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Port offset docs currently suggest you can run two demo stacks side-by-side after running set-ports, but docker-compose.yml still maps Mailpit to fixed host ports 1025/8025. Starting a second instance will fail unless those ports are also made configurable (and included in the offset) or the docs mention how to disable/adjust Mailpit for one instance.

Suggested change
**Mailpit and Docker note:** The `set-ports` script only affects services that read their configuration from `demo/.env.local` (API, Admin, Site, etc.). Mailpit, which is started via Docker, is still mapped to fixed host ports (typically `1025` for SMTP and `8025` for the web UI) in `docker-compose.yml`. When running multiple demo stacks at the same time, you must either adjust the Mailpit port mappings for one stack (e.g. by editing `docker-compose.yml`) or disable Mailpit for that stack to avoid host port conflicts.

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +40
// Port variables to offset
const PORT_VARS = [
'API_PORT',
'ADMIN_PORT',
'AUTHPROXY_PORT',
'SITE_PORT',
'IDP_PORT',
'POSTGRESQL_PORT',
'IMGPROXY_PORT',
'JAEGER_UI_PORT',
'JAEGER_OLTP_PORT',
];

// URL/address variables whose values reference port variables and must be rewritten
const URL_VARS = [
'IMGPROXY_URL',
'AUTHPROXY_URL',
'ADMIN_URL',
'ADMIN_URL_INTERNAL',
'API_URL',
'POST_LOGOUT_REDIRECT_URI',
'IDP_SSO_URL',
'IDP_JWKS_URI',
'IDP_END_SESSION_ENDPOINT',
'SITE_URL',
'OAUTH2_PROXY_OIDC_ISSUER_URL',
'OAUTH2_PROXY_UPSTREAMS',
'OAUTH2_PROXY_REDIRECT_URL',
'OAUTH2_PROXY_HTTP_ADDRESS',
'BREVO_REDIRECT_URL_FOR_IMPORT',
];
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

.claude/rules/env-vars.md asks to “Keep array ordering consistent with .env for readability”, but PORT_VARS/URL_VARS ordering doesn’t currently match the order in .env (e.g. postgres/imgproxy appear earlier in .env but later here). Reordering these arrays to follow .env would make future updates less error-prone.

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +21
const PORT_VARS = [
'API_PORT',
'ADMIN_PORT',
'AUTHPROXY_PORT',
'SITE_PORT',
'IDP_PORT',
'POSTGRESQL_PORT',
'IMGPROXY_PORT',
'JAEGER_UI_PORT',
'JAEGER_OLTP_PORT',
];
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

VALKEY_PORT is used by docker-compose (valkey service binds 127.0.0.1:${VALKEY_PORT}:6379) and by the demo site at runtime, but it isn’t included in PORT_VARS. With a non-zero offset, two demo instances will still conflict on 6379 and the generated .env.local won’t keep valkey aligned with the container port. Add VALKEY_PORT to PORT_VARS (and ensure it’s also reflected wherever docker compose gets its env from).

Copilot uses AI. Check for mistakes.
@nsams
Copy link
Member

nsams commented Mar 11, 2026

AI created a very complicated solution here :)

alternative idea

unfortunately dotenv expand can't do math, but it can do string replaces:

.env:
PORT_OFFSET=0
API_PORT=4{$OFFSET}00

.env.local
PORT_OFFSET=1

-> 4100
  • no script needed
  • needs only a single var in .env.local
  • would limit us to 10 different offsets, but that might be enough (actually no; 41000 is also valid)

@Ben-Ho
Copy link
Contributor Author

Ben-Ho commented Mar 11, 2026

AI created a very complicated solution here :)

alternative idea

unfortunately dotenv expand can't do math, but it can do string replaces:

.env:
PORT_OFFSET=0
API_PORT=4{$OFFSET}00

.env.local
PORT_OFFSET=1

-> 4100
  • no script needed
  • needs only a single var in .env.local
  • would limit us to 10 different offsets, but that might be enough (actually no; 41000 is also valid)

sounds good. 👍
i'm currently preparing another pull-request to also support .env.local for docker-compose.yml using dotenv

@johnnyomair johnnyomair marked this pull request as draft March 12, 2026 16:22
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.

3 participants