[INFRA] feat: respect pre-set port env vars when initializing worktree ports#6558
Open
ntilwalli wants to merge 2 commits into
Open
[INFRA] feat: respect pre-set port env vars when initializing worktree ports#6558ntilwalli wants to merge 2 commits into
ntilwalli wants to merge 2 commits into
Conversation
…e ports
init_worktree_ports() unconditionally overwrote NGINX_PORT (and every sibling
port) with BASE + PORT_OFFSET, so callers couldn't pin a single port via env
without also disabling the worktree offset for the rest. Switch each assignment
to the ${VAR:-default} idiom so pre-set values win, fix the docker-compose
default for OPIK_REVERSE_PROXY_URL to follow NGINX_PORT, and document the knob
in the docker-compose README.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
init_worktree_ports()inscripts/worktree-utils.shunconditionally overwritesNGINX_PORT(and every sibling port) withBASE_* + PORT_OFFSET, so callers can't pin a single port via env (e.g. when5173is already taken on the host by another local app that can't move). The compose file already plumbsNGINX_PORTthrough to the frontend container's port mapping, healthcheck, and nginx config — only the unconditional overwrite blocks it from working.This PR switches each port assignment in
init_worktree_portsto the${VAR:-default}idiom so pre-set env vars win while preserving the worktree-offset behavior for everything else. It also:OPIK_REVERSE_PROXY_URL's default indocker-compose.yamlto follow${NGINX_PORT:-5173}instead of hardcoding5173, so the python-backend can still reach the frontend on a non-default port.deployment/docker-compose/README.mddocumentingNGINX_PORT(and pointing atOPIK_PORT_OFFSETfor the all-ports-shifted case).Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
scripts/worktree-utils.shanddeployment/docker-compose/docker-compose.yaml, README addition indeployment/docker-compose/README.md, commit message, and this PR description.NGINX_PORT=5293 ./opik.shagainst this branch — confirmed boot banner showshttp://localhost:5293, frontend container binds0.0.0.0:5293->5293/tcp, UI returns 200,/api/is-alive/verand/api/v1/private/projectsreturn 200 through the reverse proxy, andpython-backend's renderedOPIK_REVERSE_PROXY_URLresolves tohttp://frontend:5293/api.Testing
bash -n scripts/worktree-utils.sh— syntax check.NGINX_PORT=5293 ./opik.sh: confirmeddocker psshows0.0.0.0:5293->5293/tcponfrontend-1, the boot banner printshttp://localhost:5293, the UI loads,/api/is-alive/verreturns 200 through the proxy, andpython-backend'sOPIK_REVERSE_PROXY_URLenv var resolves tohttp://frontend:5293/api(verified viadocker inspect).docker compose --profile opik config | grep OPIK_REVERSE_PROXY_URL— confirmed compose substitution renders correctly with bothNGINX_PORTset and unset../opik.shwith no env overrides) is unchanged —${VAR:-…}falls through to the existingBASE_* + PORT_OFFSETexpression, andOPIK_REVERSE_PROXY_URLresolves tohttp://frontend:5173/apias before.Documentation
Added a "Changing the Frontend Port" subsection to
deployment/docker-compose/README.mdcoveringNGINX_PORTand pointing atOPIK_PORT_OFFSETas the all-ports alternative.