Skip to content

Fix port auto-allocation on host conflict#820

Merged
malberts merged 2 commits into
masterfrom
fix-port-allocation
May 13, 2026
Merged

Fix port auto-allocation on host conflict#820
malberts merged 2 commits into
masterfrom
fix-port-allocation

Conversation

@alistair3149
Copy link
Copy Markdown
Member

@alistair3149 alistair3149 commented May 12, 2026

Follows-up to #811

The promise of make dev "just works" across worktrees was not being kept:
since .env.dist ships MW_SERVER_PORT=8484, every fresh clone hit the
"explicit request" branch in set-port.sh on first run and skipped the
host-port-free check, so a busy 8484 (or any second worktree) failed at
docker compose up with a cryptic bind error.

Commits

1. Fix port auto-allocation on host conflict

Three fixes, scoped to the same code path:

  • Makefile ensure-port: skip allocation when our compose stack is
    already running, so an idempotent make dev re-run does not silently
    reallocate and force-recreate the live mediawiki container. When the
    stack is down and no port= override is given, pass empty string to
    set-port.sh so the existing allocate() logic (reuse if free, else
    range-scan) actually runs.
  • set-port.sh explicit-request branch: validate that the requested
    port is free and exit with a clear error if not, rather than writing
    it back and letting docker bind fail later with an opaque message.
  • set-port.sh allocate(): when the existing .env value is held
    AND outside the configured range, treat it as a deliberate user
    override and error out instead of silently swapping it for an
    in-range port.

2. Add Docker/tests/ with smoke test rename and set-port regression suite

Establishes Docker/tests/ as the home for shell-script tests at the
Docker layer, keeping the top-level tests/ subtree PHP-only.

  • Docker/test.shDocker/tests/smoke.sh. Renamed (was an ambiguous
    "test.sh") and now resolves .env relative to the script so it can
    be invoked from anywhere.
  • Docker/tests/test-set-port.sh locks in the coverage matrix as an
    executable regression suite. Uses python3 to hold contested ports
    (backlog 128 + accept-and-close loop), allocates into a high MW range
    (38484-38499) so a busy 8484-8499 on the host does not perturb
    assertions.
  • make test-scripts runs the regression suite. Kept out of
    make test / make ci so it does not become a CI gate on the first
    iteration. Promote to CI after a few sprints once stability is
    confirmed.

Heads-up: PR #818 references Docker/test.sh in its planned
smoke-test target — that path becomes Docker/tests/smoke.sh after
this PR lands. (Will leave a note on #818.)

Coverage matrix

Scenario Before After
Fresh clone, 8484 free works works
Fresh clone, 8484 held by other fails at docker bind auto-picks next free in range
Second worktree, baked .env=8484, A holds it fails at docker bind auto-picks next free in range
Re-run while stack up works works (gate skips allocator, no recreate)
port=N, N free works works
port=N, N held by other fails at docker bind clear error, exits 1
Manual .env=9999, 9999 free works works (preserved)
Manual .env=9999, 9999 held fails at docker bind clear error, exits 1

Test plan

  • All 11 checks in make test-scripts pass.
  • Coverage matrix above verified against a sandboxed .env with python TCP listeners holding the contested port for each row.

alistair3149 and others added 2 commits May 12, 2026 19:37
The promise of `make dev` "just works" across worktrees was not being kept:
since `.env.dist` ships `MW_SERVER_PORT=8484`, every fresh clone hit the
"explicit request" branch in `set-port.sh` on first run and skipped the
host-port-free check, so a busy 8484 (or any second worktree) failed at
`docker compose up` with a cryptic bind error.

Three fixes, scoped to the same code path:

* `Makefile` `ensure-port`: skip allocation when our compose stack is
  already running, so an idempotent `make dev` re-run does not silently
  reallocate and force-recreate the live mediawiki container. When the
  stack is down and no `port=` override is given, pass empty string to
  `set-port.sh` so the existing `allocate()` logic (reuse if free, else
  range-scan) actually runs.

* `set-port.sh` explicit-request branch: validate that the requested
  port is free and exit with a clear error if not, rather than writing
  it back and letting docker bind fail later with an opaque message.

* `set-port.sh` `allocate()`: when the existing `.env` value is held
  AND outside the configured range, treat it as a deliberate user
  override and error out instead of silently swapping it for an
  in-range port.

Follows-up to #811

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Establishes Docker/tests/ as the home for shell-script tests at the
Docker layer, keeping the top-level tests/ subtree PHP-only.

* Docker/test.sh -> Docker/tests/smoke.sh. Renamed to a descriptive
  name (was ambiguous "test.sh"), and now resolves Docker/.env relative
  to its own script directory so it can be invoked from anywhere.
* Docker/tests/test-set-port.sh added. Locks in the coverage matrix
  from the previous commit as an executable regression suite, catching
  the Morne fresh-clone bug and the three new behaviours of the fix:
  idempotent re-run, explicit-port-validation, and out-of-range
  override protection. Uses python3 to hold contested ports
  (backlog 128 + accept-and-close loop to avoid kernel SYN drops once
  the accept queue fills). Allocates into a high MW range
  (38484-38499) so a busy 8484-8499 on the host does not perturb
  assertions.
* New `make test-scripts` target runs the regression suite. Kept out
  of `make test` / `make ci` so it does not become a CI gate on the
  first iteration. Promote to CI after a few sprints once stability
  is confirmed.

Note: PR #818's draft `smoke-test` target needs to point at
`Docker/tests/smoke.sh` after this lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alistair3149 alistair3149 requested a review from malberts May 13, 2026 00:00
@malberts malberts merged commit 17c304b into master May 13, 2026
6 checks passed
@malberts malberts deleted the fix-port-allocation branch May 13, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants