site serve: refuse to start on a port something else holds #253

Merged

vnprc merged 2 commits from agent/site-serve-held-port into master

2026-09-29 02:51:33 +01:00

allod-agent commented

2026-09-29 02:35:22 +01:00

Member

allod site serve now refuses to start a preview on a port that something else already holds. Before this it printed the other program's address and reported success.

What changes for a running machine: a start on a held port exits 1, names the port, and starts nothing.
What stays identical: a site whose port is free is started with the same command as before. A site with no port is refused as before. --stop, --vm and view are untouched.
What proves it: run for real on a dev VM with another program holding the port, and a test that fails when the check is removed.
If it is wrong: revert this PR. It keeps no state.

Closes allod/tools#252

What changed

cmd/allod/site_serve.go: before starting a unit that is not running, serve connects to 127.0.0.1:<port> once. An answer is port <n> is already in use: <unit> was not started, exit 1, and no systemd-run. The check comes after the checks of the checkout path and the flake's preview app. The connection attempt is one function, shared with the wait that follows a start.
docs/allod-site-preview.md and serve --help: the contract no longer says a held port is reported "as serving", and the exit codes name a port in use.
cmd/allod/site_serve_test.go: one new test. Four existing tests that start a unit now get their listener when the stubbed systemd-run is called, where they used to open it beforehand, since a listener that is already there is now a port in use. Their assertions are unchanged.

This replaces pull request allod/tools#251, closed unmerged. Picking a port for a site that has none was dropped; the reasoning is in the closing comment on allod/tools#249.

Risk

R2. One added refusal ahead of a start, in a command that starts a systemd unit.

Worst credible failure: a port that is free is refused. That would need something answering on the port, which is what the check is for. Recovery is a revert.
Left open, as the issue says: another program can take the port in the seconds between the check and the app's bind. serve then prints the address with exit 0 while the unit's journal shows the app failing.
Left open, found in review: serve counts a unit as running when systemctl is-active exits 3, which covers a unit that has failed and not yet been cleared away. In that moment the check is skipped, and if another program holds the port, its address is printed. This was so before the change. Units are started with --collect, so the moment is brief. Closing it means changing what "running" means on every path, which is outside this issue.

Validation

Run at 93450a6, the head of this branch, on a dev VM:

go test ./..., go test -tags site ./..., go test -tags secret ./..., go test -tags site,secret ./..., each with -count=1: all seven packages ok in each of the four runs.
go vet ./... and gofmt -l .: no output.
allod check: All 11 steps passed (checks: 9 built).
With the check removed, the new test fails on both of its cases: systemd-run ran 1 times, want 0.

Run at 5966627, which differs from the head by one deleted comment line in a test file, against a throwaway site whose preview app is live-server:

$ allod site serve example/fixed          # port held by socat
allod: port 18649 is already in use: allod-preview-example-fixed was not started
[exit 1]
$ allod site serve --port 18649 example/site
allod: port 18649 is already in use: allod-preview-example-site was not started
[exit 1]

No unit existed after either. With the port free:

$ allod site serve example/fixed
http://127.0.0.1:18649
[exit 0]

The page answered 200 and the listener was the unit's live-server. A second serve printed the same address with one unit running. --stop exited 0. A site with no port was refused with today's message. Afterwards no allod-preview-* unit, no socat and no live-server was left.

Not run: nothing from a second machine. view is not changed by this pull request.

Review

One reviewer from another model family (gpt-5.6-terra) read the diff cold at 5966627.

Declined: the exit-3 case above, recorded under Risk.
Acted on: one comment in the test stub that only described a field was deleted.
No other finding.

`allod site serve` now refuses to start a preview on a port that something else already holds. Before this it printed the other program's address and reported success. - **What changes for a running machine:** a start on a held port exits 1, names the port, and starts nothing. - **What stays identical:** a site whose port is free is started with the same command as before. A site with no port is refused as before. `--stop`, `--vm` and `view` are untouched. - **What proves it:** run for real on a dev VM with another program holding the port, and a test that fails when the check is removed. - **If it is wrong:** revert this PR. It keeps no state. Closes allod/tools#252 ## What changed - `cmd/allod/site_serve.go`: before starting a unit that is not running, `serve` connects to `127.0.0.1:<port>` once. An answer is `port <n> is already in use: <unit> was not started`, exit 1, and no `systemd-run`. The check comes after the checks of the checkout path and the flake's `preview` app. The connection attempt is one function, shared with the wait that follows a start. - `docs/allod-site-preview.md` and `serve --help`: the contract no longer says a held port is reported "as serving", and the exit codes name a port in use. - `cmd/allod/site_serve_test.go`: one new test. Four existing tests that start a unit now get their listener when the stubbed `systemd-run` is called, where they used to open it beforehand, since a listener that is already there is now a port in use. Their assertions are unchanged. This replaces pull request allod/tools#251, closed unmerged. Picking a port for a site that has none was dropped; the reasoning is in the closing comment on allod/tools#249. ## Risk R2. One added refusal ahead of a start, in a command that starts a systemd unit. - **Worst credible failure:** a port that is free is refused. That would need something answering on the port, which is what the check is for. Recovery is a revert. - **Left open, as the issue says:** another program can take the port in the seconds between the check and the app's bind. `serve` then prints the address with exit 0 while the unit's journal shows the app failing. - **Left open, found in review:** `serve` counts a unit as running when `systemctl is-active` exits 3, which covers a unit that has failed and not yet been cleared away. In that moment the check is skipped, and if another program holds the port, its address is printed. This was so before the change. Units are started with `--collect`, so the moment is brief. Closing it means changing what "running" means on every path, which is outside this issue. ## Validation Run at `93450a6`, the head of this branch, on a dev VM: - `go test ./...`, `go test -tags site ./...`, `go test -tags secret ./...`, `go test -tags site,secret ./...`, each with `-count=1`: all seven packages `ok` in each of the four runs. - `go vet ./...` and `gofmt -l .`: no output. - `allod check`: `All 11 steps passed (checks: 9 built).` - With the check removed, the new test fails on both of its cases: `systemd-run ran 1 times, want 0`. Run at `5966627`, which differs from the head by one deleted comment line in a test file, against a throwaway site whose preview app is `live-server`: ``` $ allod site serve example/fixed # port held by socat allod: port 18649 is already in use: allod-preview-example-fixed was not started [exit 1] $ allod site serve --port 18649 example/site allod: port 18649 is already in use: allod-preview-example-site was not started [exit 1] ``` No unit existed after either. With the port free: ``` $ allod site serve example/fixed http://127.0.0.1:18649 [exit 0] ``` The page answered 200 and the listener was the unit's `live-server`. A second `serve` printed the same address with one unit running. `--stop` exited 0. A site with no port was refused with today's message. Afterwards no `allod-preview-*` unit, no `socat` and no `live-server` was left. Not run: nothing from a second machine. `view` is not changed by this pull request. ## Review One reviewer from another model family (`gpt-5.6-terra`) read the diff cold at `5966627`. - Declined: the exit-3 case above, recorded under Risk. - Acted on: one comment in the test stub that only described a field was deleted. - No other finding.

allod-agent added 2 commits

2026-09-29 02:35:22 +01:00

Refuse to start a preview on a port something else holds 5966627b38

serve took a connection to the port as a running preview while the unit
was up, and the unit is up while nix evaluates, so a port held by another
program was printed as the site's address with exit 0. Before starting a
unit, serve now connects to the port once and refuses if anything answers.

The tests that start a unit get their listener from the start, since one
that is already there is now a port in use.

Refs allod/tools#252

Drop a comment that only described a test field 93450a6e4c