fix(cli): match host service PORT env var name in spawn#3640
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA single environment variable, Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a silent port mismatch bug in the host service spawn flow. The CLI was passing the selected port as Confidence Score: 5/5Safe to merge — single-line targeted fix with no side-effects. The fix is a one-line addition that directly addresses the documented root cause. The host-service env schema (PORT with default(4879)) and the CLI spawn env block are the only two files involved, both have been read and verified. The change is backwards compatible (old HOST_SERVICE_PORT key is preserved), risk of regression is negligible. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/cli/src/lib/host/spawn.ts | Adds PORT env var to the spawned host-service process so it binds on the CLI-selected port instead of always defaulting to 4879. |
Sequence Diagram
sequenceDiagram
participant CLI as CLI (spawn.ts)
participant OS as OS Kernel
participant HS as Host Service (env.ts)
CLI->>OS: findFreePort() → e.g. 51234
CLI->>HS: spawn(hostBin, env={ PORT: "51234", HOST_SERVICE_PORT: "51234" })
HS->>HS: createEnv() reads PORT=51234 ✅
HS->>OS: listen on :51234
CLI->>HS: GET /trpc/health.check on :51234
HS-->>CLI: 200 OK
Note over CLI,HS: Before fix: PORT was unset → host defaulted to 4879,<br/>CLI polled 51234 → timeout after 10 000 ms
Reviews (1): Last reviewed commit: "fix(cli): match host service PORT env va..." | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
packages/cli/src/lib/host/spawn.tspassed the spawned host service port asHOST_SERVICE_PORT, butpackages/host-service/src/env.tsreadsPORT, so the CLI's--portflag (and the random free port chosen byfindFreePort()) was silently ignored. The host always listened on the default4879while the CLI polled a different port, causinghost startto fail withHost service failed to start within 10000ms.PORTalongsideHOST_SERVICE_PORTso the host service picks up the CLI-selected port. Keeping the old name preserves backwards compatibility for any external reader.Linear: https://linear.app/superset-sh/issue/SUPER-448
Test plan
bun run lint:fix && bun run format:checkcleanbun --filter @superset/cli --filter @superset/host-service typecheckpassescd packages/cli && bun run scripts/build-dist.ts --target=linux-x64), extract tarball, runSUPERSET_HOST_BIN=<scratch>/bin/superset-host <scratch>/bin/superset host start— completes health check without--portSummary by cubic
Set
PORTwhen@superset/clispawns the host service so it matches what@superset/host-servicereads, fixing host start failures. The host now binds to the CLI-selected port (--portor a free port) instead of 4879, while keepingHOST_SERVICE_PORTfor compatibility; aligns with Linear SUPER-448.Written for commit c3aad63. Summary will update on new commits.
Summary by CodeRabbit