From c2c3cdb1ed0dcf69659d45d4248d0be5800a488e Mon Sep 17 00:00:00 2001 From: Daniel Maksymilian Syrnicki Date: Sun, 28 Jun 2026 13:03:33 +0200 Subject: [PATCH] fix(cli): fire on_install hooks on named CLI installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `furtka app install ` copied app folders (install_plan) then ran a bare reconcile, which fires on_start but never on_install — so a CLI install of a consumer brought its provider up without ever provisioning it (no account, empty MQTT_* in the consumer .env). Route named installs through install_runner.run_install (writing the plan file first), the same docker phase the API dispatches, so providers come up before consumers and on_install hooks run. Path installs keep the copy+reconcile dev/test path. Co-Authored-By: Claude Opus 4.8 (1M context) --- furtka/cli.py | 71 ++++++++++++++++++++++++++++++++--------------- tests/test_cli.py | 14 ++++++++-- 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/furtka/cli.py b/furtka/cli.py index 87091f4..71337aa 100644 --- a/furtka/cli.py +++ b/furtka/cli.py @@ -69,37 +69,62 @@ def _cmd_app_list(args: argparse.Namespace) -> int: def _cmd_app_install(args: argparse.Namespace) -> int: # If the user passed a path (or a path-ish thing), bypass dep resolution — # local paths are dev/test workflows where the caller knows what they want. - # Catalog/bundled name installs go through plan_install() so transitive - # `requires` are pulled in. + # These just copy + reconcile; no `requires`/hook handling. src_path = Path(args.source) is_path = src_path.is_dir() or "/" in args.source or args.source.startswith(".") - try: - if is_path: + if is_path: + try: src = installer.resolve_source(args.source) target = installer.install_from(src) - print(f"installed {target.name} to {target}") + except installer.InstallError as e: + print(f"error: {e}", file=sys.stderr) + return 2 + print(f"installed {target.name} to {target}") + actions = reconciler.reconcile(apps_dir()) + for a in actions: + print(f" {a.describe()}") + return 1 if reconciler.has_errors(actions) else 0 + + # Catalog/bundled name install: resolve transitive `requires`, copy every + # app folder (the synchronous phase the API runs inline), then drive the + # exact same docker phase the API dispatches via systemd-run — so providers + # come up before consumers and `on_install` hooks fire to provision them. + # Going through run_install (not a bare reconcile) is what makes a CLI + # install of a consumer actually provision against its provider. + from furtka import install_runner + + try: + plan = deps.plan_install(args.source) + except deps.DependencyError as e: + print(f"error: {e}", file=sys.stderr) + return 2 + try: + if plan.to_install: + installer.install_plan(plan) + to_install = list(plan.to_install) else: - try: - plan = deps.plan_install(args.source) - except deps.DependencyError as e: - print(f"error: {e}", file=sys.stderr) - return 2 - if not plan.to_install: - # Target is already installed — re-run as a single-app install - # to refresh files (matches reinstall semantics). - target_path = installer.install_from(installer.resolve_source(args.source)) - print(f"reinstalled {target_path.name} to {target_path}") - else: - targets = installer.install_plan(plan) - for t in targets: - print(f"installed {t.name} to {t}") + # Target already installed — reinstall to refresh files, then still + # run the docker phase so hooks re-fire (matches API reinstall). + installer.install_from(installer.resolve_source(args.source)) + to_install = [args.source] except installer.InstallError as e: print(f"error: {e}", file=sys.stderr) return 2 - actions = reconciler.reconcile(apps_dir()) - for a in actions: - print(f" {a.describe()}") - return 1 if reconciler.has_errors(actions) else 0 + + # Stage the plan file run_install consumes (it removes it after reading). + install_runner.plan_path().parent.mkdir(parents=True, exist_ok=True) + install_runner.plan_path().write_text( + json.dumps({"target": args.source, "to_install": to_install}) + ) + try: + install_runner.run_install(args.source) + except Exception as e: + # run_install already wrote state="error"; surface it to the caller. + print(f"error: {e}", file=sys.stderr) + return 1 + for name in to_install: + print(f"installed {name}") + return 0 def _cmd_app_install_bg(args: argparse.Namespace) -> int: diff --git a/tests/test_cli.py b/tests/test_cli.py index bcf1ff8..7275e9b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -158,7 +158,7 @@ def test_app_install_uses_plan_for_named_install(tmp_path, monkeypatch, capsys): _write_manifest(bundled, "mosquitto") _write_manifest(bundled, "zigbee2mqtt", requires=[{"app": "mosquitto"}]) - from furtka import installer, reconciler + from furtka import install_runner, installer # Stub install_from so we don't actually copy files / mess with placeholders. install_calls: list[str] = [] @@ -168,12 +168,22 @@ def test_app_install_uses_plan_for_named_install(tmp_path, monkeypatch, capsys): return tmp_path / src.name monkeypatch.setattr(installer, "install_from", fake_install_from) - monkeypatch.setattr(reconciler, "reconcile", lambda *a, **k: []) + monkeypatch.setattr(install_runner, "_INSTALL_PLAN", tmp_path / "install-plan.json") + # Named installs drive the docker phase through run_install (so on_install + # hooks fire) — stub it and assert it's invoked with the chosen target. + run_calls: list[str] = [] + monkeypatch.setattr(install_runner, "run_install", lambda name: run_calls.append(name)) rc = main(["app", "install", "zigbee2mqtt"]) assert rc == 0 # Provider installed before consumer. assert install_calls == ["mosquitto", "zigbee2mqtt"] + # The docker phase (which fires on_install hooks) ran for the target. + assert run_calls == ["zigbee2mqtt"] + # And the plan file was staged for run_install to consume. + plan = json.loads(install_runner.plan_path().read_text()) + assert plan["target"] == "zigbee2mqtt" + assert plan["to_install"] == ["mosquitto", "zigbee2mqtt"] def test_app_install_named_with_cycle_exits_2(tmp_path, monkeypatch, capsys):