mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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 <lifecycle>` 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.
This commit is contained in:
Abhay Singh
GitHub
parent
f1c34d336c
commit
82526191a1
2 changed files with 180 additions and 3 deletions
|
|
@ -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 <name>."
|
||||
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:
|
||||
|
|
|
|||
|
|
@ -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] = []
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue