mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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.
This commit is contained in:
parent
3f14eac060
commit
6fb5f3bc3d
4 changed files with 101 additions and 0 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue