From 6fb5f3bc3dfa60e56744f85cf049524d43104a31 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 7 Jul 2026 12:37:23 -0400 Subject: [PATCH] fix(install): persist --no-http2 override through install apply (#1676) ## Description `headroom install apply` regenerates the deployment manifest on every run, and that regeneration silently drops any manually-added `--no-http2` override. The HTTP/2 workaround itself is already real and already supported by `headroom proxy`, but persistent installs had no first-class way to keep it. This PR adds `--no-http2` to `install apply`, threads it into `build_manifest()`, and persists the flag in `manifest.proxy_args` so it survives reapply. Closes #1615 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added `--no-http2` to `headroom install apply`, and forwarded the flag into `build_manifest()`. - Extended `headroom/install/planner.py` so `build_manifest(..., no_http2=True)` persists `--no-http2` into `manifest.proxy_args`. - Added planner-level regression coverage for both the override path and the default-preservation path. - Added CLI-level regression coverage that proves `install apply --no-http2` forwards correctly and that the help surface advertises the flag. - `CHANGELOG.md` intentionally not touched: repo policy generates changelog entries from conventional commits rather than manual PR edits. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_install/test_planner.py` and `uv run pytest tests/test_cli/test_install_cli.py`) - [x] Linting passes (`uv run ruff check .` and `uv run ruff format . --check`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text > rtk uv run pytest tests/test_install/test_planner.py -k no_http2 -q collected 7 items / 5 deselected / 2 selected tests\test_install\test_planner.py .. [100%] 2 passed, 5 deselected in 0.18s > rtk uv run pytest tests/test_cli/test_install_cli.py -k no_http2 -q collected 19 items / 17 deselected / 2 selected tests\test_cli\test_install_cli.py .. [100%] 2 passed, 17 deselected in 0.23s > rtk uv run pytest tests/test_install/test_runtime.py -q collected 19 items tests\test_install\test_runtime.py ..........F........ [100%] FAILED tests/test_install/test_runtime.py::test_runtime_start_lock_blocks_another_process 1 failed, 18 passed in 0.44s (Confirmed pre-existing on unmodified origin/main via `git stash` in this worktree, identical failure with none of this PR's changes applied. Environment-specific lock-file flakiness in this sandbox, unrelated to install-manifest persistence; runtime.py was not touched by this change.) > rtk uv run ruff check headroom/cli/install.py headroom/install/planner.py tests/test_install/test_planner.py tests/test_cli/test_install_cli.py All checks passed! > rtk uv run ruff format --check headroom/cli/install.py headroom/install/planner.py tests/test_install/test_planner.py tests/test_cli/test_install_cli.py 4 files already formatted ``` ## Real Behavior Proof - Environment: local source checkout with `uv` dev environment, using the existing install CLI and manifest builder, in worktree `D:\Repos\headroom-pr-1615-persist-install-http2-override`. - Exact command / steps: ran `headroom install apply --help` through `CliRunner`, ran a direct `build_manifest(..., no_http2=True)` proof, and ran the focused planner, CLI, runtime, and lint checks. - Observed result: on `origin/main`, `install apply --help` lacked `--no-http2` and `build_manifest(..., no_http2=True)` raised `TypeError: build_manifest() got an unexpected keyword argument 'no_http2'`; on this branch, `install apply --help` lists `--no-http2`, `build_manifest(..., no_http2=True)` returns a manifest whose `proxy_args` contains exactly one `--no-http2` entry (`['--host', '127.0.0.1', '--port', '8787', '--mode', 'token', '--backend', 'anthropic', '--telemetry', '--no-http2']`), persistent installs now preserve the existing HTTP/2 disable flag across `install apply` regeneration, and runtime behavior still comes entirely from replaying manifest `proxy_args` (`runtime.py` was not modified). - Not tested: a full persistent-service supervisor round-trip or full CI suite locally. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable (not applicable, changelog entries are generated from conventional commits per repo policy) ## Additional Notes This stays scoped to the install-manifest persistence seam only; it does not revisit HTTP/2 default policy, retry behavior, or proxy transport construction. Attribution: the implementation shape follows the persistence pattern already established by #1365, and the remaining install-layer gap was confirmed by `sarkarsital1959` in the 2026-07-01 comment on #1615. --- headroom/cli/install.py | 7 +++++ headroom/install/planner.py | 3 ++ tests/test_cli/test_install_cli.py | 47 ++++++++++++++++++++++++++++++ tests/test_install/test_planner.py | 44 ++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+) diff --git a/headroom/cli/install.py b/headroom/cli/install.py index 40da76f52..66221fd36 100644 --- a/headroom/cli/install.py +++ b/headroom/cli/install.py @@ -202,6 +202,11 @@ def _reject_task_lifecycle(manifest: DeploymentManifest, action: str) -> None: show_default=True, help="Docker image to use when runtime=docker or preset=persistent-docker.", ) +@click.option( + "--no-http2", + is_flag=True, + help="Disable HTTP/2 in the persistent runtime (enabled by default).", +) def install_apply( preset: str, runtime: str, @@ -218,6 +223,7 @@ def install_apply( telemetry: bool, no_telemetry: bool, image: str, + no_http2: bool, ) -> None: """Install a persistent Headroom deployment.""" @@ -245,6 +251,7 @@ def install_apply( memory_enabled=memory, telemetry_enabled=telemetry and not no_telemetry, image=image, + no_http2=no_http2, ) try: diff --git a/headroom/install/planner.py b/headroom/install/planner.py index 60899e548..d2f6e9cce 100644 --- a/headroom/install/planner.py +++ b/headroom/install/planner.py @@ -119,6 +119,7 @@ def build_manifest( memory_enabled: bool, telemetry_enabled: bool, image: str, + no_http2: bool = False, ) -> DeploymentManifest: """Create a normalized deployment manifest.""" @@ -166,6 +167,8 @@ def build_manifest( proxy_args.extend(["--anyllm-provider", anyllm_provider]) if region: proxy_args.extend(["--region", region]) + if no_http2: + proxy_args.append("--no-http2") container_name = f"headroom-{normalized_profile}" return DeploymentManifest( diff --git a/tests/test_cli/test_install_cli.py b/tests/test_cli/test_install_cli.py index 07ffe3e0f..2ba8b5577 100644 --- a/tests/test_cli/test_install_cli.py +++ b/tests/test_cli/test_install_cli.py @@ -52,6 +52,53 @@ def test_install_apply_starts_service_supervisor(monkeypatch) -> None: assert calls == ["save", "start_service"] +def test_install_apply_forwards_no_http2_to_build_manifest(monkeypatch) -> None: + runner = CliRunner() + captured: dict[str, object] = {} + + 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 = [] + artifacts = [] + + manifest = Manifest() + + 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 + ) + + result = runner.invoke(main, ["install", "apply", "--no-http2"]) + + assert result.exit_code == 0, result.output + assert captured["no_http2"] is True + + +def test_install_apply_help_lists_no_http2() -> None: + runner = CliRunner() + + result = runner.invoke(main, ["install", "apply", "--help"]) + + assert result.exit_code == 0, result.output + assert "--no-http2" in result.output + + def test_install_status_includes_backend_from_health_probe(monkeypatch) -> None: runner = CliRunner() diff --git a/tests/test_install/test_planner.py b/tests/test_install/test_planner.py index 2832bb08c..1eac07d5e 100644 --- a/tests/test_install/test_planner.py +++ b/tests/test_install/test_planner.py @@ -102,3 +102,47 @@ def test_resolve_targets_manual_dedupes_and_filters_invalid() -> None: ) assert targets == [ToolTarget.CLAUDE.value, ToolTarget.COPILOT.value] + + +def test_build_manifest_omits_no_http2_by_default() -> None: + manifest = build_manifest( + profile="default", + preset=InstallPreset.PERSISTENT_SERVICE.value, + runtime_kind="python", + scope="user", + provider_mode="manual", + targets=["claude"], + port=8787, + backend="anthropic", + anyllm_provider=None, + region=None, + proxy_mode="token", + memory_enabled=False, + telemetry_enabled=True, + image="ghcr.io/chopratejas/headroom:latest", + ) + + assert "--no-http2" not in manifest.proxy_args + + +def test_build_manifest_persists_no_http2_override() -> None: + manifest = build_manifest( + profile="default", + preset=InstallPreset.PERSISTENT_SERVICE.value, + runtime_kind="python", + scope="user", + provider_mode="manual", + targets=["claude"], + port=8787, + backend="anthropic", + anyllm_provider=None, + region=None, + proxy_mode="token", + memory_enabled=False, + telemetry_enabled=True, + image="ghcr.io/chopratejas/headroom:latest", + no_http2=True, + ) + + assert manifest.proxy_args.count("--no-http2") == 1 + assert "HEADROOM_HTTP2" not in manifest.base_env