S270: the documented HTTP port has never matched the shipped one - #102
Merged
Conversation
…tire PHLIX_HTTP_PORT The docs were never right. `git log -L7,7:config/server.php` in phlix-server returns exactly ONE commit — `629348b3` "Step 1.1: Initialize Workerman project structure", which created the file with `'port' => 8096`. The line has never been changed. `git log -S"32400" --all` on phlix-server shows 32400 only ever entering via port-forwarding code and docker port MAPPINGS, never as a listen port. So 32400 in the docs is copied Plex boilerplate, not a former Phlix default — the hypothesis in the step text is confirmed, and no revision of these pages was ever accurate. `PHLIX_HTTP_PORT`: REMOVED FROM THE DOCS, not implemented. `git grep -F PHLIX_HTTP_PORT` over phlix-server's tracked files returns zero hits, and `git log -S"PHLIX_HTTP_PORT" --all` returns zero commits — the name has never existed in any commit on any ref. It is not "unwired", it was never written. Every page that mentioned it now says so in as many words, so an operator who has it in a `.env` finds out that it is doing nothing rather than silently losing the row. The real mechanisms are named instead: `server.port` in `config/server.php`, and `scripts/install.sh --http-port PORT`. NOT a blanket 32400 -> 8096 substitution. Three different ports were conflated: * listen port 8096 config/server.php `server.port` (no env override) * docker HOST port 32400 docker/examples/*/docker-compose.yml `"32400:8096"` * port-forward port 32400 config/port-forward.php / PHLIX_EXTERNAL_PORT 32400 is kept, and now explained, where it means one of the latter two: `install/docker.md` (the shipped compose really does publish on 32400), `reference/cli.md` (`phlix portforward` output prints the port-forward port), `reference/api.md` and `dev/pairing-protocol.md` (hostname candidates are built by `PortForwardService::discoverHostnameCandidates()` from its own port), and `install/kubernetes.md`'s `nodePort`. Also documents a phlix-server defect found while establishing the above, rather than papering over it: `PortForwardService` uses one value as BOTH the external and the internal port of the UPnP/NAT-PMP mapping (`addPortMapping($gateway, $this->port, $localIp, $this->port)`, :111), and that value defaults to 32400 while the server listens on 8096. Left at the shipped defaults the mapping points at a port nothing is bound to, and the hostname candidates advertised to the Hub are unreachable. `hub-admin/network.md`, `hub/remote-access.md` and `reference/env-vars.md` now say to set `PHLIX_EXTERNAL_PORT=8096`. Two adjacent fabrications removed while fixing the port mechanism they claimed to be: `PHLIX_PORT` (`install/docker.md`, no such variable; the compose mapping sets the published port) and `APP_URL` (`install/macos.md`, `install/windows.md`, zero hits in phlix-server, named as the way to change the port). 26 files changed. Remaining `32400` occurrences: 56 lines across 9 files, every one of them either a docker host-port mapping, the port-forwarding port, or new prose explaining the distinction. Gates, each read from its own `$?` on an unpiped command: `npm test` exit 0 (5 files, 85 tests); `npm run docs:build` exit 0, anchor gate 158 page(s), 3454 unique fragment links, 0 dead — byte-identical to the origin/master control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 medium |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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
1 participant
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.
config/server.php:7hardcodes 8096. The docs referenced 32400 in ~30 files, andPHLIX_HTTP_PORT— documented as the way to change it — has zero hits in the server source.The docs were never right
git log -L7,7:config/server.phpreturns exactly one commit: the initial project commit, which created'port' => 8096. The line has never changed. So the hypothesis in the step (that 32400 is copied Plex boilerplate rather than a former Phlix default) is confirmed, not assumed.⚠ It was not a blanket substitution — that was the trap
Three different ports were being conflated:
809632400— the shipped compose really does publish"32400:8096"3240056 lines across 8 files keep 32400, each now explained in place. A find-and-replace would have broken all three.
PHLIX_HTTP_PORT: removed, and the text says whyZero
git grephits and zero commits fromgit log -S ... --all— it has never existed on any ref. All 8 mentions now state plainly that it was never implemented, so an operator who has it set in a.envfinds out rather than continuing to believe it works. Not recommending it be implemented:install.sh --http-portalready covers that case.Routed defect (not fixed here)
PortForwardService.php:111uses one value as both the external and the internal mapping port, defaulting to 32400 while the daemon binds 8096. Automatic port forwarding cannot have worked at defaults, and the hostname candidates advertised to the Hub are unreachable.⚠ Correction to the step brief
The base SHA I gave was stale —
origin/masterisca79f7a, not40fd890; two Windows-doc commits landed after my snapshot. That moves the anchor baseline to 158 pages / 3454 links / 0 dead, not 3474. The −20 is entirely those two commits, confirmed by building both SHAs in clean worktrees.Gates
Clean detached worktree, every exit code from its own
$?:npm test0 (85 tests),docs:build0. Anchors: controlca79f7a158/3454/0 → this branch 158/3454/0.26 files.
🤖 Generated with Claude Code