feat(cli): shift off a busy port + openable banner URL on jss start — closes #557 - #574
melvincarvalho merged 2 commits into
Conversation
Sorry, something went wrong.
…557) Port-back from jspod's lib/start.js (proven downstream). 1. Busy port → shift, not crash. `jss start` on an in-use port died with a raw EADDRINUSE — a first-run papercut when a stale instance is still running. It now probes upward Vite-style (next free port, up to 10 tries) and binds there. The shift goes to stderr so it surfaces even under --quiet (a port the operator didn't ask for is operationally significant), and runs BEFORE the IdP issuer / baseUrl are derived so they reflect the port actually bound. If the whole window is taken, it exits with a clear message instead of EADDRINUSE. 2. Openable banner URL. The banner rewrote only 0.0.0.0 → localhost; formatUrl now also maps :: / * → localhost and brackets IPv6 literals, so the printed URL is always clickable. findFreePort + formatUrl live in src/utils/port.js (not inline in the executable bin/jss.js) so they're unit-testable without running the CLI. Tests: - test/port.test.js (7): formatUrl wildcard/IPv6/protocol cases; findFreePort returns a free port, shifts off a busy one, and returns null when the window is full. - test/port-shift-cli.test.js (1): spawns `jss start` on an occupied port and asserts it logs the shift notice AND serves on the shifted port — the real end-to-end behaviour. No behaviour change when the requested port is free. Full suite 979/979. Closes #557.
There was a problem hiding this comment.
Pull request overview
This PR improves the jss start CLI experience by (1) shifting to the next available port when the requested port is already in use (instead of crashing with EADDRINUSE), and (2) ensuring the startup URL is “openable” (wildcard hosts → localhost, IPv6 literals bracketed). The change is factored into src/utils/port.js and covered by both unit tests and an end-to-end CLI spawn test.
Changes:
- Add findFreePort() to probe upward for a bindable port (up to 10 tries) and wire it into bin/jss.js with a stderr shift notice.
- Add formatUrl() to print a clickable URL for wildcard hosts and IPv6 literals, and use it for the derived base URL/issuer.
- Add unit tests for the utilities plus an integration test that spawns jss start on an occupied port and verifies the shift + serving behavior.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils/port.js | Introduces findFreePort and formatUrl helpers used by the CLI. |
| bin/jss.js | Uses the new helpers to shift off busy ports and print an openable base URL. |
| test/port.test.js | Unit tests for formatUrl and findFreePort. |
| test/port-shift-cli.test.js | End-to-end CLI test validating port shift notice + serving on the shifted port. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| export async function findFreePort(startPort, host, maxTries = 10) { | ||
| for (let p = startPort; p < startPort + maxTries; p++) { | ||
| const free = await new Promise((resolve) => { | ||
| const srv = createServer(); | ||
| srv.once('error', () => resolve(false)); | ||
| srv.once('listening', () => srv.close(() => resolve(true))); | ||
| srv.listen(p, host); | ||
| }); | ||
| if (free) return p; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Fixed in 84b87e1 — and a good catch (the ported jspod version swallowed all errors). The probe now resolves false only on EADDRINUSE; every other bind error rejects and propagates to the start action's existing catch (bin/jss.js:338 → Error: ${err.message}), so an EACCES on a privileged port or EADDRNOTAVAIL on a bad --host surfaces the real cause instead of 'no free port found'. Test added: findFreePort against a non-local host (192.0.2.1 → EADDRNOTAVAIL) rejects rather than returning null.
Sorry, something went wrong.
…rows other errors Copilot: the probe treated ANY listen error as 'port busy', so EACCES (privileged port) or EADDRNOTAVAIL (invalid bind host) would probe the whole window and report a misleading 'no free port found' instead of the real cause. Now only EADDRINUSE resolves false (try next port); every other error rejects, propagating to the CLI's catch (bin/jss.js line 338) which surfaces the actual message. An improvement over jspod's original, which swallowed all errors. New test: findFreePort against a non-local host (192.0.2.1 → EADDRNOTAVAIL) rejects rather than returning null. 8/8 in file; full suite green.
Bundles three fixes merged since 0.0.208: - fix(nostr): NIP-98 payload hash verifies the raw request bytes instead of a re-serialization, so binary/whitespace-sensitive bodies validate (#573, closes #565) - feat(idp): served did:nostr documents use the CID v1.0 @context so Multikey / publicKeyMultibase are actually defined (#569) - feat(cli): `jss start` shifts off a busy port (Vite-style) instead of crashing on EADDRINUSE, and prints an openable banner URL (#574, closes #557)
Closes #557. Port-back from jspod's lib/start.js, proven downstream.
1. Busy port → shift, not crash
jss start on an in-use port died with a raw EADDRINUSE — a first-run papercut when a stale instance is still running. It now probes upward Vite-style (next free port, up to 10 tries) and binds there.
The issue noted explicit --port could shift-with-notice or fail-hard; this mirrors jspod (shift, loudly).
2. Openable banner URL
The banner only rewrote 0.0.0.0 → localhost. formatUrl now also maps :: / * → localhost and brackets IPv6 literals, so the printed URL is always clickable.
Structure
findFreePort + formatUrl live in src/utils/port.js, not inline in the executable bin/jss.js, so they're unit-testable without running the CLI (importing bin/jss.js would execute program.parse()).
Tests
No behaviour change when the requested port is free. Full suite: 979/979.
Interaction with jspod
jspod already finds a free port and passes --port <free>, so under jspod jss's probe just confirms it — one redundant check, harmless. This fixes the gap for direct jss start users and other embedders, who never had the wrapper's protection.