From 58f28dc7a6b6ce5bbf0f88524bd78cbe3f3ffa4b Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Tue, 18 Aug 2026 08:50:38 +0530 Subject: [PATCH] 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. --- headroom/cli/install.py | 11 +++- tests/test_cli/test_install_cli.py | 98 ++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/headroom/cli/install.py b/headroom/cli/install.py index 50e897ad4..a81a7d941 100644 --- a/headroom/cli/install.py +++ b/headroom/cli/install.py @@ -505,9 +505,10 @@ def _echo_installed(manifest: DeploymentManifest, *, prefix: str = "Installed pe "--port", "-p", default=8787, + envvar="HEADROOM_PORT", type=click.IntRange(1, 65535), show_default=True, - help="Persistent proxy port.", + help="Persistent proxy port (env: HEADROOM_PORT).", ) @click.option( "--backend", @@ -682,7 +683,13 @@ def install_apply( @main.command("deploy") @click.option("--profile", default="default", show_default=True, help="Deployment profile name.") @click.option( - "--port", "-p", default=8787, type=int, show_default=True, help="Persistent proxy port." + "--port", + "-p", + default=8787, + envvar="HEADROOM_PORT", + type=int, + show_default=True, + help="Persistent proxy port (env: HEADROOM_PORT).", ) @click.option( "--backend", diff --git a/tests/test_cli/test_install_cli.py b/tests/test_cli/test_install_cli.py index 8e82d6ec6..1817312f0 100644 --- a/tests/test_cli/test_install_cli.py +++ b/tests/test_cli/test_install_cli.py @@ -250,6 +250,104 @@ def test_install_apply_forwards_no_http2_to_build_manifest(monkeypatch) -> None: assert captured["no_http2"] is True +def _patch_apply_pipeline(monkeypatch, captured: dict[str, object]): + """Stub out the apply side effects and capture ``build_manifest`` kwargs.""" + + class Manifest: + profile = "default" + preset = "persistent-service" + runtime_kind = "python" + supervisor_kind = "service" + scope = "user" + health_url = "http://127.0.0.1:8787/readyz" + targets = ["claude"] + mutations: list = [] + artifacts: list = [] + + def fake_build_manifest(**kwargs): + captured.update(kwargs) + return Manifest() + + monkeypatch.setattr("headroom.cli.install.build_manifest", fake_build_manifest) + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: None) + monkeypatch.setattr("headroom.cli.install.apply_mutations", lambda deployment: []) + monkeypatch.setattr("headroom.cli.install.install_supervisor", lambda deployment: []) + monkeypatch.setattr("headroom.cli.install.save_manifest", lambda deployment: None) + monkeypatch.setattr("headroom.cli.install.start_supervisor", lambda deployment: None) + monkeypatch.setattr("headroom.cli.install.start_detached_agent", lambda profile: None) + monkeypatch.setattr( + "headroom.cli.install.wait_ready", lambda deployment, timeout_seconds=45: True + ) + + +def test_install_apply_honors_headroom_port_env(monkeypatch) -> None: + """An explicit HEADROOM_PORT must reach build_manifest, like `proxy --port` honors it. + + Regression for #3072 bug 1: `install apply` ignored HEADROOM_PORT and always + configured 8787 because the --port option had no envvar binding. + """ + captured: dict[str, object] = {} + _patch_apply_pipeline(monkeypatch, captured) + monkeypatch.setenv("HEADROOM_PORT", "8788") + + result = CliRunner().invoke(main, ["install", "apply"]) + + assert result.exit_code == 0, result.output + assert captured["port"] == 8788 + + +def test_install_apply_explicit_port_overrides_env(monkeypatch) -> None: + """An explicit --port still wins over HEADROOM_PORT (Click precedence).""" + captured: dict[str, object] = {} + _patch_apply_pipeline(monkeypatch, captured) + monkeypatch.setenv("HEADROOM_PORT", "8788") + + result = CliRunner().invoke(main, ["install", "apply", "--port", "9999"]) + + assert result.exit_code == 0, result.output + assert captured["port"] == 9999 + + +def test_deploy_honors_headroom_port_env(monkeypatch) -> None: + """`headroom deploy` must honor HEADROOM_PORT the same way (#3072 bug 1).""" + captured: dict[str, object] = {} + + plan = SimpleNamespace( + preset="persistent-service", + runtime="python", + reason="test", + supervisor_kind="service", + base_env={}, + ) + manifest = SimpleNamespace( + profile="default", + preset="persistent-service", + runtime_kind="python", + supervisor_kind="service", + scope="user", + port=0, + health_url="http://127.0.0.1:8788/readyz", + targets=["claude"], + ) + + def fake_build(**kwargs): + captured.update(kwargs) + return manifest + + monkeypatch.setattr( + "headroom.cli.install._select_turnkey_plan", lambda prefer_docker=True: plan + ) + monkeypatch.setattr("headroom.cli.install._build_deployment_manifest", fake_build) + monkeypatch.setattr("headroom.cli.install._apply_manifest", lambda m: None) + monkeypatch.setattr("headroom.cli.install._echo_installed", lambda m, prefix="": None) + monkeypatch.setenv("HEADROOM_PORT", "8788") + + result = CliRunner().invoke(main, ["deploy"]) + + assert result.exit_code == 0, result.output + assert captured["port"] == 8788 + + def test_install_apply_help_lists_no_http2() -> None: runner = CliRunner()