fix(selfhost): derive local port URLs from env (MUL-2506)#2939
Conversation
|
@YOMXXX is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
Bohan-J
left a comment
There was a problem hiding this comment.
Request changes — .env.example defaults break the derivation, and check.sh doesn't export PORT
Thanks for the rework — direction is right and the abstraction is clean. But two blockers need fixing before merge, plus a couple of nits.
Blockers
1. .env.example non-empty defaults make the compose fallback inert.
docker-compose.selfhost.yml now does the right thing in isolation:
FRONTEND_ORIGIN: ${FRONTEND_ORIGIN:-http://localhost:${FRONTEND_PORT:-3000}}
GOOGLE_REDIRECT_URI: ${GOOGLE_REDIRECT_URI:-http://localhost:${FRONTEND_PORT:-3000}/auth/callback}
MULTICA_APP_URL: ${MULTICA_APP_URL:-http://localhost:${FRONTEND_PORT:-3000}}…but .env.example already ships these as hardcoded localhost:3000 values:
.env.example:36MULTICA_APP_URL=http://localhost:3000.env.example:99GOOGLE_REDIRECT_URI=http://localhost:3000/auth/callback.env.example:178FRONTEND_ORIGIN=http://localhost:3000
After the standard cp .env.example .env, the ${VAR:-...} fallback never fires, so a user who only sets FRONTEND_PORT=3100 ends up with backend env still pointing at localhost:3000. Reproduces with:
FRONTEND_PORT=3100 BACKEND_PORT=9100 docker compose \
--env-file .env.example -f docker-compose.selfhost.yml config
Published ports become 3100 / 9100, but FRONTEND_ORIGIN, GOOGLE_REDIRECT_URI, MULTICA_APP_URL stay at localhost:3000. That's CORS / WebSocket origin, Google OAuth redirect, and the CLI/app URL all pointing at the wrong port — i.e. the exact failure mode #2876 was opened to fix.
Fix should make port-derived URLs follow FRONTEND_PORT. Either approach works:
- Drop those three values from
.env.example(or comment them out) and let the compose / scripts derivation be the single source of truth, or - Have
setup_server/install.*rewrite the derived URLs in the generated.envwheneverFRONTEND_PORTdiffers from the default.
Whichever you pick, please add a self-host smoke step that runs the docker compose config command above and asserts the URLs reflect FRONTEND_PORT.
2. scripts/check.sh derives PORT but never exports it.
PORT="${BACKEND_PORT:-${API_PORT:-${SERVER_PORT:-${PORT:-8080}}}}"This assigns a shell variable; the subshell (cd server && go run ./cmd/server) won't inherit it unless PORT was already exported in the calling environment. If a user sets only BACKEND_PORT=9100 in .env, check.sh polls localhost:9100/health but the backend boots on 8080. scripts/dev.sh:43 already exports — please make check.sh match.
Nits (non-blocking)
apps/web/config/runtime-urls.test.tscovers each alias vs.PORT, but not the relative precedence between aliases (e.g.BACKEND_PORTwinning overAPI_PORTandSERVER_PORTwhen all three are set) and not whitespace-only fallback (" "should fall through). Two extra cases will lock the contract in.- The semantic change where
NEXT_PUBLIC_API_URLnow also feeds the Next SSR proxy isn't called out anywhere. Worth a line in the PR description or an inline comment in.env.exampleso a future archaeologist doesn't have to git-blame to figure out why SSR started honoring it. - Follow-up only: consider adding a
BACKEND_PORT=9100smoke job to the installer CI matrix so the alias chain is regression-protected.install.ps1in particular has no automated coverage yet.
Re-request review once the two blockers are addressed — happy to take another pass quickly.
2fbf5c9 to
ed59f36
Compare
ed59f36 to
80112c1
Compare
|
Addressed the requested changes in
Verification:
Re-requesting review now. |
Bohan-J
left a comment
There was a problem hiding this comment.
Re-review @ 80112c1 — Request changes
Thanks for tackling the two blockers from the last round. Compose-side .env.example defaults are now commented out and scripts/check.sh exports PORT — both look good. However, the new revision still doesn't deliver the product goal ("change one port var, everything follows") on the local scripts path.
Must-fix: scripts/dev.sh / scripts/check.sh don't derive FRONTEND_ORIGIN, MULTICA_APP_URL, GOOGLE_REDIRECT_URI
.env.example now comments those three URLs out and notes that local scripts derive them from FRONTEND_PORT. But:
scripts/dev.sh:43-44only derives/exportsPORT.scripts/check.sh:24-29only derives/exportsPORT,FRONTEND_PORT,PLAYWRIGHT_BASE_URL.
Result: if a user sets FRONTEND_PORT=3100, the frontend starts on 3100 but the backend gets no FRONTEND_ORIGIN, so it falls back to the default origin list in server/cmd/server/router.go / server/internal/realtime/hub.go (3000 / 5173 / 5174) and CORS / WebSocket origin checks still match the old port.
Please derive and export the frontend-port-dependent URLs in both dev.sh and check.sh (or extract a shared helper), at minimum:
FRONTEND_ORIGIN="${FRONTEND_ORIGIN:-http://localhost:${FRONTEND_PORT}}"
MULTICA_APP_URL="${MULTICA_APP_URL:-${FRONTEND_ORIGIN}}"
GOOGLE_REDIRECT_URI="${GOOGLE_REDIRECT_URI:-${FRONTEND_ORIGIN}/auth/callback}"
export FRONTEND_ORIGIN MULTICA_APP_URL GOOGLE_REDIRECT_URIAlso: backend-port-dependent URLs in .env.example
.env.example:35 still has MULTICA_SERVER_URL=ws://localhost:8080/ws and .env.example:133 has LOCAL_UPLOAD_BASE_URL=http://localhost:8080 as hardcoded values. With set -a, changing BACKEND_PORT leaves these pointing at 8080 for local daemon / upload. Same treatment as the frontend URLs: either comment them out and derive in the scripts, or rewrite at script time.
A small scripts/selfhost-config.test.sh-style smoke test that asserts the derived *_ORIGIN / *_URL values after sourcing dev.sh / check.sh would lock this down.
Nit (non-blocking, please split)
packages/views/issues/components/issue-detail.tsx has an aria-hidden change unrelated to selfhost port config. The change itself is fine, but please pull it into a separate PR so this one stays scoped to the port-derivation work.
Verification I ran on 80112c19
bash scripts/selfhost-config.test.sh✅bash -n scripts/install.sh scripts/dev.sh scripts/check.sh scripts/selfhost-config.test.sh✅git diff --check origin/main...HEAD✅- Manual compose matrix:
BACKEND_PORT > API_PORT > SERVER_PORT > PORTprecedence holds ✅ - CI all green ✅
Compose path is right. Once dev.sh / check.sh derive the full URL set (and ideally .env.example stops hardcoding the backend-port URLs), happy to re-review.
|
Addressed the remaining review blockers in
Verification:
|
What does this PR do?
Fixes self-host port configuration drift by deriving local frontend/backend URLs from configured environment ports instead of leaving several
localhost:3000/localhost:8080defaults hardcoded.The project already has canonical
PORTandFRONTEND_PORTknobs. This PR keeps those working and also accepts the explicit backend aliases requested in #2876:BACKEND_PORT,API_PORT, andSERVER_PORT. The alias precedence isBACKEND_PORT→API_PORT→SERVER_PORT→PORT→8080.Related Issue
Closes #2876
Type of Change
Changes Made
apps/web/config/runtime-urls.tssonext.config.tsderives the backend proxy URL fromREMOTE_API_URL,NEXT_PUBLIC_API_URL, backend port aliases, or the historical8080fallback.apps/web/config/runtime-urls.test.tsto cover explicit API URL precedence,PORT, backend aliases, and the fallback.docker-compose.selfhost.ymlso backend host port mapping honorsBACKEND_PORT/API_PORT/SERVER_PORT/PORT, and default frontend-derived URLs honorFRONTEND_PORT.scripts/install.shandscripts/install.ps1so installer health checks and final printed URLs use configured ports.Makefile,scripts/dev.sh, andscripts/check.shso local workflows accept the same backend port aliases..env.example.How to Test
FRONTEND_PORT=3100 BACKEND_PORT=9100and rundocker compose -f docker-compose.selfhost.yml config; backend should publish9100, frontend should publish3100, and default frontend URLs should use3100.API_PORT=9200orSERVER_PORT=9300and run the same compose config command; backend should publish that port.Verification
pnpm --filter @multica/web exec vitest run config/runtime-urls.test.tspnpm --filter @multica/web typecheckpnpm --filter @multica/web testpnpm --filter @multica/web build(exited 0; Next logged a GitHub API 403 while prerendering the download page)pnpm --filter @multica/web lint(exited 0 with one pre-existing warning inapps/web/app/(auth)/login/page.tsx)for f in scripts/install.sh scripts/dev.sh scripts/check.sh; do bash -n "$f"; donemake -pn BACKEND_PORT=9100 | rg '^PORT :='BACKEND_PORT=9100 FRONTEND_PORT=3100 docker compose -f docker-compose.selfhost.yml config | rg 'FRONTEND_ORIGIN|GOOGLE_REDIRECT_URI|MULTICA_APP_URL|published: "(9100|3100)"'API_PORT=9200 docker compose -f docker-compose.selfhost.yml config | rg 'published: "9200"'SERVER_PORT=9300 docker compose -f docker-compose.selfhost.yml config | rg 'published: "9300"'git diff --checkNote: I could not execute
scripts/install.ps1locally becausepwshis not installed in this environment; the same helper logic is mirrored from the verified shell installer.Checklist
apps/web/features/landing/i18n/) and relevant docs (apps/docs/content/docs/)apps/docs/content/docs/developers/conventions.zh.mdx(terminology, mixed-rule fortask/issue/skill)AI Disclosure
AI tool used: Codex
Prompt / approach: Investigated #2876 from the self-host flow outward: existing env examples, Makefile defaults, compose mappings, Next proxy rewrites, and install scripts. Used TDD for the Next runtime URL resolver, then verified compose interpolation and local web checks before opening the PR.
Screenshots (optional)
N/A - self-host configuration and installer behavior only.