fix: never kill the process holding an explicitly requested port#64
Merged
Conversation
`serve --port N` used to run `lsof -ti tcp:N`, SIGTERM every PID it found, then SIGKILL the survivors. Requesting a busy port silently destroyed whatever owned it — including unrelated processes that happened to hold it. Startup now fails on an occupied explicit port and prints the alternative command instead. Default (non-explicit) startup keeps walking to the next free port as before. Drops `ensurePortFree`, `findPidOnPort`, and the `node:child_process` import. Also drops the `probeLoopbackPort` preflight from the first draft of this fix: its guard block duplicated the existing `catch` in `listenOnAvailablePort` and changed no observable behavior, while opening a probe-close-to-bind window that did not exist before. `listenOnce` was already the authoritative, race-safe bind check. README documents the PowerShell equivalents for forcing a port and for inspecting what holds one.
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.
Closes #63
Summary
serve --port Nno longer runslsof -ti tcp:Nand SIGTERM/SIGKILLs the PIDs it finds. An occupied explicit port now fails startup and prints the alternative command. RemovesensurePortFree,findPidOnPort, and thenode:child_processimport.probeLoopbackPortpreflight that the first draft of this fix introduced. Its guard block duplicated the existingcatchinlistenOnAvailablePort(samelastErrorassignment, same throw guard, sameonRetrycall) and produced identical observable behavior for every input, while adding a probe-close-to-bind TOCTOU window that did not exist before.listenOncewas already the authoritative, race-safe bind check. Netsrc/commands/serve.js: -50 lines.$env:PORT) and for inspecting what holds one (Get-NetTCPConnection), plusnpx --yes @ipv9/tokentracker-cliinvocations that work in PowerShell.Behavior change to be aware of
This is intentional but it is a breaking change for any supervisor that restarts over a still-live instance on a pinned port. Previously the new process killed the old one and took the port; now it exits. A
launchd/ systemd / Task Scheduler unit pinning an explicit port must fully stop the old process before starting the new one.Test plan
node --test test/serve-port-hint.test.js— 5/5 passEADDRINUSEand that the pre-existing listener is stilllisteningafterward (the actual regression guard)npm run docs:openwiki:check— 0 findingsuser01@10.100.94.171, Node 20.20.2) — pending releasePre-existing CI failure — not from this PR
npm run ci:localis RED on this branch, with 4 failures intest/rollout-parser.test.js:The identical 4 failures reproduce on
mainat4a9aea8. Failing-test names were diffed between this branch andmainand are byte-identical, so this PR introduces zero new failures.mainis red on its own and needs a separate fix.Release state
Not released. This touches
src/, so per CLAUDE.md it is release-bound and needs the 4-way version lockstep (package.json, bothproject.ymlMARKETING_VERSIONentries,TokenTrackerWin.csproj) plus a DMG workflow run. No version bump is in this PR — awaiting the release decision.