From 82526191a103a8d0e079d170e47631b3c2bcb0d9 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Thu, 13 Aug 2026 22:21:34 +0530 Subject: [PATCH] fix(cli/install): resolve the deployment profile instead of dead-ending on default (#2832) ## Description `headroom init` installs its persistent deployment under a non-`default` profile name (`init-user` for a global-scope install), but every `headroom install ` subcommand hardcodes `--profile default`. The docs show those commands without `--profile`, so on a machine set up by `headroom init` every documented lifecycle command fails while the real deployment is running fine: ```console $ headroom install status Error: No deployment profile named 'default' is installed. $ headroom install status --profile init-user Status: running Healthy: yes ``` The error named neither the installed profile nor the `--profile` flag, so there was nothing to lead the user to `init-user`, which exists only as an internal constant. When the requested profile is not installed, `_require_manifest` now resolves the real target instead of dead-ending on a name the user never chose: 1. an explicit `HEADROOM_DEPLOYMENT_PROFILE` (which the runtime already exports) wins; 2. otherwise, when `--profile` was left at its `default` default and exactly one deployment is installed, that one is used; 3. when it still cannot decide, the error lists the installed profiles and points at `--profile`. This changes only the not-found path. An installed `default` still loads exactly as before, and an explicit typo'd `--profile` still fails, now with a helpful list. Fixes #2811 ## 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 - `headroom/cli/install.py` (`_require_manifest`): on a manifest miss, resolve via `HEADROOM_DEPLOYMENT_PROFILE`, then a single installed deployment when the request is the bare `default`, and otherwise raise an error that lists installed profiles and points at `--profile`. Imported `list_manifests` (already present in `headroom.install.state`) for the enumeration. - `tests/test_cli/test_install_cli.py`: added `test_require_manifest_resolves_single_profile_when_default_missing`, `test_require_manifest_honors_env_profile`, and `test_require_manifest_lists_installed_profiles_when_ambiguous`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text # Before/after on the exact reported scenario (one installed profile "init-user"): # ORIGINAL: _require_manifest("default") -> raises "No deployment profile named 'default' is installed." # FIXED: _require_manifest("default") -> resolves to "init-user" # Pass-after, install suites: tests/test_cli/test_install_cli.py 33 passed tests/test_install/ 174 passed, 1 skipped, 1 pre-existing failure # the 1 failure is tests/test_install/test_native_installers.py:: # test_powershell_native_installer_supports_persistent_docker_lifecycle, which # runs scripts/install.ps1 and fails identically on clean main with these changes # stashed (an environment-specific PowerShell exit, unrelated to this diff). # uvx ruff@0.15.17 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.17 and mypy 1.20.2 via uvx. - Exact command / steps: read `detect`/`_require_manifest` and the lifecycle command options (`--profile default` at install.py:720/748/763/775/789/818/830) to confirm the mismatch with `init.py`'s `_GLOBAL_PROFILE = "init-user"`, then demonstrated before/after by monkeypatching `load_manifest`/`list_manifests`: on the original code `_require_manifest("default")` raises "No deployment profile named 'default' is installed."; with the fix it returns the single installed manifest (`init-user`). Fail-before via `git stash push headroom/cli/install.py` and a direct call; pass-after with `git stash pop` and the install suites (33 passed in the CLI file, 174 passed in test_install with one pre-existing environment failure). - Observed result: a bare lifecycle command on an init'd machine now targets the running deployment instead of failing, matching the `--profile init-user` command the issue reporter confirmed works. An explicit `HEADROOM_DEPLOYMENT_PROFILE` selects the target, and an ambiguous multi-profile machine gets an error naming the installed profiles and the `--profile` flag. - Not tested: a full end-to-end `headroom init` then `headroom install status` on a fresh host (that flow spawns a real deployment and supervisor). The resolution logic is a pure function verified directly, and the manifest loading it calls is existing, tested code. ## 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 - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [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 (a CI guard enforces this) ## Additional Notes The resolution deliberately only triggers on the not-found path and only auto-selects when a single deployment is installed or an explicit env profile names one, so it never silently picks the wrong deployment on a multi-profile host. The docs that show the bare commands (`docs/content/docs/persistent-installs.mdx`, `wiki/persistent-installs.md`, `wiki/cli.md`) become correct again without needing a `--profile` on every line. --- headroom/cli/install.py | 70 +++++++++++++++++- tests/test_cli/test_install_cli.py | 113 +++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 3 deletions(-) diff --git a/headroom/cli/install.py b/headroom/cli/install.py index 417f73e37..2d9402054 100644 --- a/headroom/cli/install.py +++ b/headroom/cli/install.py @@ -11,6 +11,7 @@ from copy import deepcopy from dataclasses import dataclass import click +from click.core import ParameterSource from headroom._subprocess import run from headroom.install.health import probe_json, probe_ready @@ -36,6 +37,7 @@ from headroom.install.runtime import ( from headroom.install.state import ( ManifestError, delete_manifest, + list_manifests, load_manifest, save_manifest, ) @@ -65,14 +67,76 @@ def install() -> None: """Install and manage persistent Headroom deployments.""" +def _profile_selection_was_explicit() -> bool: + """True when the current command received an explicit ``--profile``. + + An explicit selection must be honored verbatim or rejected, never redirected + to ``HEADROOM_DEPLOYMENT_PROFILE`` or a lone installed deployment: silently + operating ``stop``/``restart``/``remove`` on a different profile than the one + the user typed is dangerous. Only a defaulted (omitted) ``--profile`` is + eligible for the recovery fallback. Outside a Click command context (direct + calls / unit tests) there is no explicit selection to protect. + """ + ctx = click.get_current_context(silent=True) + if ctx is None: + return False + return bool(ctx.get_parameter_source("profile") == ParameterSource.COMMANDLINE) + + +def _missing_profile_error( + name: str, + installed: list[DeploymentManifest], + *, + source: str | None = None, +) -> click.ClickException: + if installed: + names = ", ".join(sorted(m.profile for m in installed)) + hint = f" Installed: {names}. Select one with --profile ." + else: + hint = " No deployments are installed; run `headroom init` or `headroom install apply`." + origin = f" (from {source})" if source else "" + return click.ClickException(f"No deployment profile named '{name}'{origin} is installed.{hint}") + + def _require_manifest(profile: str) -> DeploymentManifest: try: manifest = load_manifest(profile) except ManifestError as e: raise click.ClickException(str(e)) from None - if manifest is None: - raise click.ClickException(f"No deployment profile named '{profile}' is installed.") - return manifest + if manifest is not None: + return manifest + + # The requested profile isn't installed. `headroom init` installs under a + # non-"default" profile name (e.g. init-user), while every lifecycle command + # defaults --profile to "default" -- so on an init'd machine the documented + # bare commands (`headroom install status`, etc.) would all dead-end (#2811). + installed = list_manifests() + + # An EXPLICIT --profile is honored or rejected verbatim, never redirected: a + # typo must not silently act on the env/lone profile (#2832 review). + if _profile_selection_was_explicit(): + raise _missing_profile_error(profile, installed) + + # --profile was defaulted. A non-empty HEADROOM_DEPLOYMENT_PROFILE (which the + # runtime exports) is itself an explicit selection: honor it when installed, + # otherwise fail naming it. It must never fall through to the lone-manifest + # fallback and silently operate on a different deployment (#2832 review). + env_profile = os.environ.get("HEADROOM_DEPLOYMENT_PROFILE", "").strip() + if env_profile: + if env_profile != profile: + try: + resolved = load_manifest(env_profile) + except ManifestError: + resolved = None + if resolved is not None: + return resolved + raise _missing_profile_error(env_profile, installed, source="HEADROOM_DEPLOYMENT_PROFILE") + + # Neither CLI nor environment named a profile. A single installed deployment + # is unambiguous, so use it; otherwise report what is available. + if len(installed) == 1: + return installed[0] + raise _missing_profile_error(profile, installed) def _start_deployment(manifest: DeploymentManifest, *, assume_start_lock: bool = False) -> None: diff --git a/tests/test_cli/test_install_cli.py b/tests/test_cli/test_install_cli.py index 691f12143..cf84e063b 100644 --- a/tests/test_cli/test_install_cli.py +++ b/tests/test_cli/test_install_cli.py @@ -1,11 +1,124 @@ from __future__ import annotations +from types import SimpleNamespace + import click +import pytest from click.testing import CliRunner +from headroom.cli import install as inst from headroom.cli.main import main +def test_require_manifest_resolves_single_profile_when_default_missing(monkeypatch): + """On an init'd machine (one profile, e.g. init-user), a bare lifecycle + command whose --profile defaults to 'default' resolves to the single + installed deployment instead of dead-ending (#2811).""" + only = SimpleNamespace(profile="init-user") + monkeypatch.delenv("HEADROOM_DEPLOYMENT_PROFILE", raising=False) + monkeypatch.setattr(inst, "load_manifest", lambda profile: None) + monkeypatch.setattr(inst, "list_manifests", lambda: [only]) + + assert inst._require_manifest("default") is only + + +def test_require_manifest_honors_env_profile(monkeypatch): + """An explicit HEADROOM_DEPLOYMENT_PROFILE (exported by the runtime) selects + the target even when the requested profile is not installed.""" + target = SimpleNamespace(profile="init-user") + monkeypatch.setenv("HEADROOM_DEPLOYMENT_PROFILE", "init-user") + monkeypatch.setattr( + inst, "load_manifest", lambda profile: target if profile == "init-user" else None + ) + monkeypatch.setattr(inst, "list_manifests", lambda: [target]) + + assert inst._require_manifest("default") is target + + +def test_require_manifest_lists_installed_profiles_when_ambiguous(monkeypatch): + """With several installed profiles and no signal, the error names them and + points at --profile instead of dead-ending on 'default'.""" + monkeypatch.delenv("HEADROOM_DEPLOYMENT_PROFILE", raising=False) + monkeypatch.setattr(inst, "load_manifest", lambda profile: None) + monkeypatch.setattr( + inst, + "list_manifests", + lambda: [SimpleNamespace(profile="init-user"), SimpleNamespace(profile="ci")], + ) + + with pytest.raises(click.ClickException) as exc: + inst._require_manifest("default") + msg = str(exc.value) + assert "ci" in msg and "init-user" in msg and "--profile" in msg + + +def _status_manifest(profile: str) -> SimpleNamespace: + return SimpleNamespace( + profile=profile, + preset="persistent-task", + runtime_kind="python", + supervisor_kind="none", + scope="user", + port=8787, + health_url="http://127.0.0.1:8787/readyz", + backend="anthropic", + ) + + +def test_install_status_explicit_missing_profile_is_not_redirected_to_env(monkeypatch): + """An explicit --profile must be honored or rejected verbatim, never + redirected to HEADROOM_DEPLOYMENT_PROFILE or a lone installed deployment: a + typo must fail even when the env profile exists (#2832 review). Only a + CliRunner invocation exercises the default-vs-explicit distinction.""" + init_user = _status_manifest("init-user") + monkeypatch.setenv("HEADROOM_DEPLOYMENT_PROFILE", "init-user") + monkeypatch.setattr(inst, "load_manifest", lambda p: init_user if p == "init-user" else None) + monkeypatch.setattr(inst, "list_manifests", lambda: [init_user]) + + res = CliRunner().invoke(main, ["install", "status", "--profile", "typo"]) + + assert res.exit_code != 0 + assert "typo" in res.output + # The error names the installed profile, but the command never operated on it. + assert "Preset:" not in res.output + assert "Status:" not in res.output + + +def test_install_status_stale_env_profile_is_not_redirected_to_lone_manifest(monkeypatch): + """A non-empty HEADROOM_DEPLOYMENT_PROFILE is an explicit selection: if it + names a missing/stale profile the command must fail naming that profile, never + silently redirect to a different lone installed deployment (#2832 review).""" + init_user = _status_manifest("init-user") + monkeypatch.setenv("HEADROOM_DEPLOYMENT_PROFILE", "missing") + monkeypatch.setattr(inst, "load_manifest", lambda p: init_user if p == "init-user" else None) + monkeypatch.setattr(inst, "list_manifests", lambda: [init_user]) + + res = CliRunner().invoke(main, ["install", "status"]) + + assert res.exit_code != 0 + assert "missing" in res.output + # Never operated on the lone init-user deployment. + assert "Preset:" not in res.output + assert "Status:" not in res.output + + +def test_install_status_omitted_profile_resolves_env_deployment(monkeypatch): + """With --profile omitted (Click default), HEADROOM_DEPLOYMENT_PROFILE selects + the target so the documented bare command works on an init'd machine.""" + init_user = _status_manifest("init-user") + monkeypatch.setenv("HEADROOM_DEPLOYMENT_PROFILE", "init-user") + monkeypatch.setattr(inst, "load_manifest", lambda p: init_user if p == "init-user" else None) + monkeypatch.setattr(inst, "list_manifests", lambda: [init_user]) + monkeypatch.setattr(inst, "probe_json", lambda url: None) + monkeypatch.setattr(inst, "runtime_status", lambda m: "running") + monkeypatch.setattr(inst, "probe_ready", lambda url: True) + + res = CliRunner().invoke(main, ["install", "status"]) + + assert res.exit_code == 0, res.output + assert "Profile: init-user" in res.output + + def test_install_apply_starts_service_supervisor(monkeypatch) -> None: runner = CliRunner() calls: list[str] = []