fix(cli): print usable startup URLs for wildcard and IPv6 hosts - #709
Conversation
cevheri
left a comment
There was a problem hiding this comment.
CI is red on the required Unit & Integration Tests job, and the three failures are all in the new launcher startup URL block.
Root cause: those cases spawn bin/studio.js with the runner's ambient node. That job has no actions/setup-node, the ubuntu-24.04 image ships Node 22.23.2, and the launcher correctly refuses anything below its Node 24 floor. So it exits 1 immediately, which is why each case dies in about 40ms, well before any download. Your Node 26 is why the same block is green locally.
Please drop that describe block. #670 asked for a pure case table and your 12 startupUrl cases are exactly that. I checked the consequences: bin/lib/launcher-utils.mjs stays at 100% lines without the subprocess block, and bin/studio.js never enters the lcov because it is spawned rather than imported, so the coverage gate is unaffected. The real --host 0.0.0.0 output you pasted in the PR body already covers the issue's evidence request.
Everything else is right: the helper rules match the spec, DEFAULT_PORT is shared with the help text, the Starting LibreDB Studio <version> prefix is byte-identical, and src/lib/startup-banner.ts is untouched. Fix the tests and this is ready to merge.
The missing setup-node in that job is our gap, not yours. I am tracking it separately.
|
Addressed in 39c9965: removed the launcher startup URL subprocess block and retained all 12 pure startupUrl cases. The launcher helper suite passes 99 tests / 178 assertions; focused launcher-utils.mjs line coverage remains 100%. Production code is unchanged. Updated the PR description to distinguish the final test scope and current validation from prior results. Full CI is running on the new head; I will check its result. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Final validation for 39c9965: the previously failing Unit & Integration Tests job now passes, including merged core/component coverage and the 100% line-coverage gate. Lint/typecheck/build, browser E2E and subpath E2E, PostgreSQL functional smoke, both Node engine smoke jobs, and channel/security checks also passed. Re-review is requested; no further source changes after this validated head. |
|
That is exactly the change I asked for: 39c9965 removes the subprocess block and nothing else, and the PR is now 3 files, +40/-2. All 22 checks are green, including the unit job that was red. I probed the case table rather than trusting the count. Four mutations of I also appreciated the revised description separating prior-revision results from this head. That distinction is what makes a validation section worth reading. The three cases you removed were good tests aimed at the wrong runner. The gap is ours: that job never pins Node. Once it does I will bring them back from your commits and credit them there. Merging shortly. Thanks for the quick turnaround. |
Description
The launcher prints unusable startup links for wildcard bind addresses:
0.0.0.0is shown directly and::produceshttp://:::3000. Format the displayed URL with a purestartupUrlhelper while preserving the server bind settings and version-parsing log prefix.Closes #670.
Changes
Actual launcher output previously verified with
--host 0.0.0.0and an isolated cached stub payload:Validation
AI-assisted implementation and validation. The manual launcher fixture used a cached stub server, not a real Next.js server or downloaded release. No dependencies added; server bind settings and the startup version prefix are preserved.