Skip to content

Commit 58f28dc

Browse files
fix(install): honor HEADROOM_PORT in install apply and deploy (#3085)
## Description `headroom install apply --preset persistent-service` and `headroom deploy` ignored an explicit `HEADROOM_PORT` and always configured port 8787, even though `headroom proxy --port` honors `HEADROOM_PORT`. Anyone running a second instance, or avoiding a port conflict, got a silently wrong configuration, and the failure is especially confusing because the override *appears* supported on the direct proxy path. Root cause: the `--port` options on the `install apply` and `deploy` commands were declared with a hardcoded `default=8787` and **no** `envvar` binding: ```python @click.option("--port", "-p", default=8787, type=int, show_default=True, help="Persistent proxy port.") ``` The proxy command's `--port` already carries `envvar="HEADROOM_PORT"`, so the two paths disagreed. `build_manifest` / `_build_deployment_manifest` already thread the `port` argument all the way through to the generated `HEADROOM_PORT` base-env and the health URL, so the value was simply never resolved from the environment at the CLI boundary. ## Fix Bind both `--port` options to `envvar="HEADROOM_PORT"`, matching the proxy command. Click resolves the value from the environment when `--port` is not passed, and an explicit `--port` still wins over the env var (standard Click precedence: explicit CLI argument over `envvar` over `default`). ## Scope This addresses **bug 1** of #3072. Bug 2 (`install status` reporting `Status: stopped` alongside `Healthy: yes`, disagreeing with `doctor`) is an unrelated status-reporting concern that the reporter offered a live repro for; it is left for a separate follow-up rather than bundled here. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/cli/install.py`: add `envvar="HEADROOM_PORT"` to the `--port` option on both `install apply` and `deploy` (and note the env var in each help string), matching `headroom proxy --port`. - `tests/test_cli/test_install_cli.py`: added `test_install_apply_honors_headroom_port_env`, `test_install_apply_explicit_port_overrides_env`, and `test_deploy_honors_headroom_port_env`, capturing the `port` that reaches the manifest builder. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy`) - [x] New tests added ### Test Output ```text tests/test_cli/test_install_cli.py 40 passed # uvx ruff@0.15.22 check -> All checks passed! # uvx mypy@1.20.2 headroom/cli/install.py -> Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.15.22 and mypy 1.20.2 via uvx. - Exact command / steps: reverted the source fix and ran the two new env-var tests to capture the bug (`python -m pytest tests/test_cli/test_install_cli.py::test_install_apply_honors_headroom_port_env tests/test_cli/test_install_cli.py::test_deploy_honors_headroom_port_env` -> both failed with `assert 8787 == 8788`, proving `HEADROOM_PORT=8788` was dropped); restored the fix; re-ran the full file (`python -m pytest tests/test_cli/test_install_cli.py` -> 40 passed); then `uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx mypy@1.20.2 headroom/cli/install.py`. - Observed result: with the fix, `HEADROOM_PORT=8788 headroom install apply` (and `deploy`) resolves `port=8788` into `build_manifest`, so the generated service config and `HEADROOM_PORT` base-env use 8788; passing `--port 9999` alongside the env var still yields 9999. - Not tested: an end-to-end persistent-service install on a machine with a running supervisor (the CLI-to-manifest port resolution is verified through the manifest builder, which already owns the downstream wiring covered by the existing planner tests). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This is a CLI option-binding fix on the install/deploy commands, not a rollout-channel-gated runtime feature. - Minimum rollout channel: N/A (no rollout-managed behavior). - Stable/default behavior changed: only when `HEADROOM_PORT` is set in the environment. Previously it was ignored (config wired to 8787); now the install/deploy path honors it, matching `headroom proxy`. With no `HEADROOM_PORT` set and no `--port`, the default is still 8787, so existing installs are unaffected. - Kill switch / disable path: unset `HEADROOM_PORT` (or pass `--port 8787`) to keep the prior port. - Unsafe override required: no. - Qualification impact: `install apply` / `deploy` now provision the proxy on the operator's requested port instead of always 8787, so a second instance or a port-conflict workaround configures correctly. - Rollback path: revert this PR; the `--port` options return to ignoring `HEADROOM_PORT`. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md`: it is generated by release-please from my Conventional Commit PR title ## Additional Notes Reported by @vsg-prog (split out of #3040 into #3072). The `--port` option already carried the correct `type`/range validation and threaded through the manifest builder; the only gap was the missing `envvar` binding at the CLI boundary.
1 parent 3ed8f76 commit 58f28dc

2 files changed

Lines changed: 107 additions & 2 deletions

File tree

headroom/cli/install.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,10 @@ def _echo_installed(manifest: DeploymentManifest, *, prefix: str = "Installed pe
505505
"--port",
506506
"-p",
507507
default=8787,
508+
envvar="HEADROOM_PORT",
508509
type=click.IntRange(1, 65535),
509510
show_default=True,
510-
help="Persistent proxy port.",
511+
help="Persistent proxy port (env: HEADROOM_PORT).",
511512
)
512513
@click.option(
513514
"--backend",
@@ -682,7 +683,13 @@ def install_apply(
682683
@main.command("deploy")
683684
@click.option("--profile", default="default", show_default=True, help="Deployment profile name.")
684685
@click.option(
685-
"--port", "-p", default=8787, type=int, show_default=True, help="Persistent proxy port."
686+
"--port",
687+
"-p",
688+
default=8787,
689+
envvar="HEADROOM_PORT",
690+
type=int,
691+
show_default=True,
692+
help="Persistent proxy port (env: HEADROOM_PORT).",
686693
)
687694
@click.option(
688695
"--backend",

tests/test_cli/test_install_cli.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,104 @@ def fake_build_manifest(**kwargs):
250250
assert captured["no_http2"] is True
251251

252252

253+
def _patch_apply_pipeline(monkeypatch, captured: dict[str, object]):
254+
"""Stub out the apply side effects and capture ``build_manifest`` kwargs."""
255+
256+
class Manifest:
257+
profile = "default"
258+
preset = "persistent-service"
259+
runtime_kind = "python"
260+
supervisor_kind = "service"
261+
scope = "user"
262+
health_url = "http://127.0.0.1:8787/readyz"
263+
targets = ["claude"]
264+
mutations: list = []
265+
artifacts: list = []
266+
267+
def fake_build_manifest(**kwargs):
268+
captured.update(kwargs)
269+
return Manifest()
270+
271+
monkeypatch.setattr("headroom.cli.install.build_manifest", fake_build_manifest)
272+
monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: None)
273+
monkeypatch.setattr("headroom.cli.install.apply_mutations", lambda deployment: [])
274+
monkeypatch.setattr("headroom.cli.install.install_supervisor", lambda deployment: [])
275+
monkeypatch.setattr("headroom.cli.install.save_manifest", lambda deployment: None)
276+
monkeypatch.setattr("headroom.cli.install.start_supervisor", lambda deployment: None)
277+
monkeypatch.setattr("headroom.cli.install.start_detached_agent", lambda profile: None)
278+
monkeypatch.setattr(
279+
"headroom.cli.install.wait_ready", lambda deployment, timeout_seconds=45: True
280+
)
281+
282+
283+
def test_install_apply_honors_headroom_port_env(monkeypatch) -> None:
284+
"""An explicit HEADROOM_PORT must reach build_manifest, like `proxy --port` honors it.
285+
286+
Regression for #3072 bug 1: `install apply` ignored HEADROOM_PORT and always
287+
configured 8787 because the --port option had no envvar binding.
288+
"""
289+
captured: dict[str, object] = {}
290+
_patch_apply_pipeline(monkeypatch, captured)
291+
monkeypatch.setenv("HEADROOM_PORT", "8788")
292+
293+
result = CliRunner().invoke(main, ["install", "apply"])
294+
295+
assert result.exit_code == 0, result.output
296+
assert captured["port"] == 8788
297+
298+
299+
def test_install_apply_explicit_port_overrides_env(monkeypatch) -> None:
300+
"""An explicit --port still wins over HEADROOM_PORT (Click precedence)."""
301+
captured: dict[str, object] = {}
302+
_patch_apply_pipeline(monkeypatch, captured)
303+
monkeypatch.setenv("HEADROOM_PORT", "8788")
304+
305+
result = CliRunner().invoke(main, ["install", "apply", "--port", "9999"])
306+
307+
assert result.exit_code == 0, result.output
308+
assert captured["port"] == 9999
309+
310+
311+
def test_deploy_honors_headroom_port_env(monkeypatch) -> None:
312+
"""`headroom deploy` must honor HEADROOM_PORT the same way (#3072 bug 1)."""
313+
captured: dict[str, object] = {}
314+
315+
plan = SimpleNamespace(
316+
preset="persistent-service",
317+
runtime="python",
318+
reason="test",
319+
supervisor_kind="service",
320+
base_env={},
321+
)
322+
manifest = SimpleNamespace(
323+
profile="default",
324+
preset="persistent-service",
325+
runtime_kind="python",
326+
supervisor_kind="service",
327+
scope="user",
328+
port=0,
329+
health_url="http://127.0.0.1:8788/readyz",
330+
targets=["claude"],
331+
)
332+
333+
def fake_build(**kwargs):
334+
captured.update(kwargs)
335+
return manifest
336+
337+
monkeypatch.setattr(
338+
"headroom.cli.install._select_turnkey_plan", lambda prefer_docker=True: plan
339+
)
340+
monkeypatch.setattr("headroom.cli.install._build_deployment_manifest", fake_build)
341+
monkeypatch.setattr("headroom.cli.install._apply_manifest", lambda m: None)
342+
monkeypatch.setattr("headroom.cli.install._echo_installed", lambda m, prefix="": None)
343+
monkeypatch.setenv("HEADROOM_PORT", "8788")
344+
345+
result = CliRunner().invoke(main, ["deploy"])
346+
347+
assert result.exit_code == 0, result.output
348+
assert captured["port"] == 8788
349+
350+
253351
def test_install_apply_help_lists_no_http2() -> None:
254352
runner = CliRunner()
255353

0 commit comments

Comments
 (0)