From 0ae7b356e03877240b039045e2ca63efda8eef8e Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 20 Aug 2026 15:37:24 -0400 Subject: [PATCH] feat(install): what an installer must decide, in one testable place #439's decision is option A: the desktop product has no self-update. Bundle-aware self-update buys convenience and costs an update channel that must itself be secured, and for a tool that moves real money a user deliberately downloading a signed installer is the better trust posture. That decision has a consequence worth taking seriously: **the installer is the update path**, so "what should happen when this build meets the one already on disk" is a question something has to answer correctly every single time. An Inno Setup script and a `.pkg` postinstall are both places where that answer cannot be tested, so it does not live in either. It lives in `keel/install.py`, which the installer calls and `keel update` reads to explain itself. THE RULE THAT IS NOT OBVIOUS. "Versions differ, so update" is right in one direction only. `keel/data/db.py` migrates with `if current < target` and ships no down-migrations. A database already at schema N, opened by a build that expects N-2, does NOT fail loudly: `migrate` finds nothing to apply and returns, and the old code then runs against tables and columns it was never written against. Silence is the entire hazard. So a downgrade is a confirmation carrying a specific warning -- including the recovery, which is a database backup taken BEFORE the upgrade, never running the older build anyway. An uncomparable pair carries the same warning, because an uncomparable pair might BE a downgrade. Only two outcomes proceed silently: a fresh install, and a genuine upgrade. Same-version and downgrade both stop and ask. That closed statement is itself a test. THE TRAP THIS AVOIDS. The program directory and the deployment directory are different places, and conflating them is how an upgrade destroys an operator's work: the program directory is replaced WHOLESALE, so a config that lived there would not survive one. `default_deployment_dir` delegates to `keel_core.paths.app_data_dir` (#434) rather than restating it -- an installer that proposed a folder the runtime does not discover would produce a deployment that appears EMPTY on first launch: config written, database written, and a dashboard reporting a healthy install with no history. `NEVER_TOUCHED` names every piece of operator state, and is tested against real filenames rather than eyeballed, so a typo in `keel*.db` cannot silently protect nothing. Defaults are per-user on both platforms, because an elevation prompt on a first run is precisely the friction this milestone exists to remove. macOS offers `~/Applications` when `/Applications` is not writable; Windows needs no fallback because its default is already per-user, and inventing one would be a second path for no reason. AND THE MESSAGE THAT REACHES A DESKTOP USER. Every refusal `keel update` produces today is correct and useless to one: they talk about `site-packages` layouts and tell the reader to put `uv` on PATH. A packaged user has no venv and no `uv` and never will. `is_packaged()` now refuses FIRST, naming the actual update path -- download the next signed release, and your config, database and credentials stay exactly as they are. Both freezer markers are checked, because PyInstaller sets `sys.frozen` for every build mode but `sys._MEIPASS` only for `--onefile`, and a false negative here is the exact outcome #439 exists to stop. Version comparison has one home: `plan_install` reads `keel.commands.update.version_key` rather than parsing semver again, pinned by test. A second reader would disagree with the first on exactly the strings nobody tested. 3988 passed, 3 skipped (22 new). ruff clean repo-wide; mypy clean over keel + packages. Refs #439, #438, #18. Co-Authored-By: Claude Opus 5 (1M context) --- keel/commands/update.py | 48 +++++--- keel/install.py | 247 ++++++++++++++++++++++++++++++++++++++++ tests/test_install.py | 228 +++++++++++++++++++++++++++++++++++++ 3 files changed, 506 insertions(+), 17 deletions(-) create mode 100644 keel/install.py create mode 100644 tests/test_install.py diff --git a/keel/commands/update.py b/keel/commands/update.py index 63eb11d..5ab25a5 100644 --- a/keel/commands/update.py +++ b/keel/commands/update.py @@ -70,6 +70,7 @@ import click +from keel.install import is_packaged, packaged_update_refusal from keel.version import BuildInfo, build_info, installed_distributions #: The public, unauthenticated latest-release endpoint. NO auth, NO tokens: this @@ -130,8 +131,10 @@ def parse_release(payload: bytes | str) -> ReleaseInfo: doc = json.loads(payload) except (TypeError, ValueError) as exc: raise UpdateError(f"the releases API returned a non-JSON payload: {exc}") from exc - if not isinstance(doc, dict) or not isinstance(doc.get("tag_name"), str) or not doc.get( - "tag_name" + if ( + not isinstance(doc, dict) + or not isinstance(doc.get("tag_name"), str) + or not doc.get("tag_name") ): raise UpdateError( "unexpected release payload: no tag_name -- the API answered something " @@ -142,8 +145,10 @@ def parse_release(payload: bytes | str) -> ReleaseInfo: raise UpdateError("unexpected release payload: assets is not a list") assets: list[ReleaseAsset] = [] for item in raw_assets: - if not isinstance(item, dict) or not isinstance(item.get("name"), str) or not isinstance( - item.get("browser_download_url"), str + if ( + not isinstance(item, dict) + or not isinstance(item.get("name"), str) + or not isinstance(item.get("browser_download_url"), str) ): raise UpdateError( "unexpected release payload: an asset without a name or a download url" @@ -355,9 +360,7 @@ def _wheel_origin_refusal(package_file: Path) -> str | None: ) -def select_production_wheels( - release: ReleaseInfo, version: str -) -> tuple[ReleaseAsset, ...]: +def select_production_wheels(release: ReleaseInfo, version: str) -> tuple[ReleaseAsset, ...]: """The FOUR production wheel assets for `version`, in `PRODUCTION_WHEEL_PREFIXES` order, matched by exact `--` name -- never `*.whl`, so the fake/robinhood wheels can never ride along. PURE; raises `UpdateError` naming any @@ -424,6 +427,12 @@ def plan_update( except UpdateError as exc: reasons.append(str(exc)) + if is_packaged(): + # First, and on its own terms: every other refusal below talks about `site-packages` + # layouts and tells the reader to put `uv` on PATH. A packaged user has no venv and no + # `uv`, and never will -- so the message that reaches them has to name the real update + # path rather than an impossible one (#439). + reasons.append(packaged_update_refusal()) if info.source != "release": reasons.append( f"this is a [{info.source}] build, not a release install -- the updater " @@ -776,7 +785,10 @@ def say(line: str) -> None: "not poison a later rollback" ) return UpdateResult( - ok=False, steps=tuple(steps), error=str(exc), rolled_back=False, + ok=False, + steps=tuple(steps), + error=str(exc), + rolled_back=False, backups=tuple(backups), ) @@ -818,7 +830,10 @@ def say(line: str) -> None: f"{_MANUAL_RECOVERY}" ) return UpdateResult( - ok=False, steps=tuple(steps), error=error, rolled_back=False, + ok=False, + steps=tuple(steps), + error=error, + rolled_back=False, backups=tuple(backups), ) rolled_back = False @@ -854,7 +869,10 @@ def say(line: str) -> None: f"{_MANUAL_RECOVERY}" ) return UpdateResult( - ok=False, steps=tuple(steps), error=error, rolled_back=rolled_back, + ok=False, + steps=tuple(steps), + error=error, + rolled_back=rolled_back, backups=tuple(backups), ) @@ -923,8 +941,7 @@ def _relaunch() -> NoReturn: f"start the console by hand: `{new_keel} tui`" ) from exc raise UpdateError( - f"relaunch failed: execv returned -- start the console by hand: " - f"`{new_keel} tui`" + f"relaunch failed: execv returned -- start the console by hand: `{new_keel} tui`" ) return _relaunch @@ -978,17 +995,14 @@ def render_plan_lines(plan: UpdatePlan) -> list[str]: The exact lines the CLI prints and the console's update view renders -- ONE renderer, so the two front-ends cannot drift. PURE.""" lines = [ - f"current: {plan.current_version} latest: {plan.latest_version} " - f"(tag {plan.latest_tag})" + f"current: {plan.current_version} latest: {plan.latest_version} (tag {plan.latest_tag})" ] if not plan.offered: if plan.refusal_reasons: lines.append("update NOT offered:") lines.extend(f" - {reason}" for reason in plan.refusal_reasons) else: - lines.append( - f"already at the latest release ({plan.latest_version}) -- nothing to do." - ) + lines.append(f"already at the latest release ({plan.latest_version}) -- nothing to do.") return lines lines.append(f"update available: {plan.current_version} -> {plan.latest_version}") lines.append(f"the plan (launch folder {plan.launch_dir}):") diff --git a/keel/install.py b/keel/install.py new file mode 100644 index 0000000..9fd8071 --- /dev/null +++ b/keel/install.py @@ -0,0 +1,247 @@ +"""What an installer must decide, in one place both the installer and the app can read (#438/#439). + +The desktop product has no self-update. That is D6's decision and it is deliberate: bundle-aware +self-update buys convenience and costs an update channel that must itself be secured, and for a +tool that moves real money a user deliberately downloading a signed installer is the better trust +posture. So **the installer is the update path**, and "what should happen when this build meets +the one already on disk" is a question the installer has to answer correctly every time. + +It lives here rather than inside an Inno Setup script or a `.pkg` postinstall because it is real +logic with a real failure mode, and neither of those is a place where logic can be tested. The +installer calls it; `keel update` reads the same module to explain itself on a packaged install. + +**The rule that is not obvious.** "Versions differ, so update" is right in one direction only. +`keel/data/db.py` migrates with `if current < target` and ships **no down-migrations**. A database +already at schema N, opened by a build that expects N-2, does not fail loudly: `migrate` finds +nothing to apply and returns, and the old code then runs against tables and columns it was never +written against. So a downgrade is a confirmation with a specific warning, never a silent update. + +**And the rule that is absolute.** An installer replaces the PROGRAM. It never touches the +DEPLOYMENT -- `config.yaml`, `keel*.db`, `.env`, `logs/`. An operator's allowlist, caps and +trading mode are hand-edited and irreplaceable, and a database is the only record of what the +engine has done. `keel_core.paths` already separates the two (#434); this module keeps them +separate at the point where a wizard would be most tempted to conflate them. +""" + +from __future__ import annotations + +import os +import sys +from dataclasses import dataclass +from enum import Enum +from pathlib import Path + +#: Where a release is downloaded from, named in every refusal that tells a desktop user to update +#: by downloading rather than by running a command. +RELEASES_URL = "https://github.com/CodeGateSoftware/keel/releases/latest" + + +def is_packaged() -> bool: + """True when running inside a frozen bundle rather than from a venv. + + Both markers are checked because PyInstaller sets `sys.frozen` for every build mode but only + sets `sys._MEIPASS` for `--onefile`, and other freezers set one or the other. A false positive + here costs a refusal an operator can work around; a false negative sends a desktop user to + install `uv`, which is the outcome #439 exists to stop. + """ + return bool(getattr(sys, "frozen", False)) or hasattr(sys, "_MEIPASS") + + +# -- where things go --------------------------------------------------------------------------- + + +def default_program_dir(platform: str | None = None, *, home: Path | None = None) -> Path: + """Where the installer proposes to put the BINARY. + + Per-user on both platforms, deliberately: a machine-wide install needs elevation, and an + elevation prompt on a first run is exactly the friction this milestone exists to remove. It + also means an uninstall cannot need admin either. + + `platform`/`home` are parameters rather than reads of `sys.platform` so both answers are + testable from one machine -- the same reason `keel_core.paths`' tests can check the Windows + branch on a Mac. + """ + platform = sys.platform if platform is None else platform + home = Path.home() if home is None else home + if platform == "darwin": + return Path("/Applications/keel.app") + if platform == "win32": + base = os.environ.get("LOCALAPPDATA") + root = Path(base) if base else home / "AppData" / "Local" + return root / "Programs" / "keel" + return home / ".local" / "share" / "keel-app" + + +def fallback_program_dir(platform: str | None = None, *, home: Path | None = None) -> Path | None: + """The alternative to offer when the default is not writable, or `None` where there isn't one. + + macOS only: `/Applications` needs admin on a managed machine, and `~/Applications` is the + documented per-user equivalent that does not. Windows' default is already per-user.""" + platform = sys.platform if platform is None else platform + home = Path.home() if home is None else home + if platform == "darwin": + return home / "Applications" / "keel.app" + return None + + +def default_deployment_dir(platform: str | None = None) -> Path: + """Where the installer proposes to put CONFIG, DATABASE, `.env` and LOGS. + + A separate question from `default_program_dir`, and conflating them is the trap this module + exists to avoid: the program directory is replaced wholesale on every update, so anything of + the operator's that lived there would be destroyed by an upgrade. + + Delegates to `keel_core.paths.app_data_dir` rather than restating it, so the installer's + default and the runtime's discovery cannot disagree -- an installer that proposed a folder + the app then did not look in would produce a deployment that appears empty on first launch. + """ + from keel_core import paths + + if platform is None or platform == sys.platform: + return paths.app_data_dir() + # Asked about a platform that is not this one: mirror `app_data_dir`'s branches. Only useful + # for building an installer for the other OS, and for testing both from one machine. + home = Path.home() + if platform == "darwin": + return home / "Library" / "Application Support" / "keel" + if platform == "win32": + base = os.environ.get("LOCALAPPDATA") or os.environ.get("APPDATA") + return Path(base) / "keel" if base else home / "AppData" / "Local" / "keel" + base = os.environ.get("XDG_DATA_HOME") + return Path(base) / "keel" if base else home / ".local" / "share" / "keel" + + +# -- what to do about what is already there ---------------------------------------------------- + + +class InstallDecision(str, Enum): + """What the installer is about to do. Every member except `FRESH` and `UPGRADE` needs the + user to agree first.""" + + FRESH = "fresh" + UPGRADE = "upgrade" + REINSTALL = "reinstall" + DOWNGRADE = "downgrade" + UNCOMPARABLE = "uncomparable" + + +@dataclass(frozen=True) +class InstallPlan: + decision: InstallDecision + installed_version: str | None + incoming_version: str + #: Whether the installer must stop and ask before proceeding. + needs_confirmation: bool + #: One sentence naming what is about to happen, for the confirmation dialog. + summary: str + #: The specific hazard, where there is one. Shown in addition to `summary`, never instead. + warning: str | None = None + + @property + def may_proceed_silently(self) -> bool: + return not self.needs_confirmation + + +#: Said in full wherever a downgrade is confirmed, because the failure it describes is silent. +DOWNGRADE_WARNING = ( + "This is OLDER than the version already installed. keel's database migrations are " + "forward-only -- there are no down-migrations -- so a database already at a newer schema " + "will not be converted back. The older build will simply find nothing to migrate and then " + "run against tables it was never written against. If you need to go back, restore a database " + "backup taken before the upgrade rather than running the older build against the newer " + "database." +) + + +def plan_install(incoming_version: str, installed_version: str | None) -> InstallPlan: + """Decide what installing `incoming_version` over `installed_version` should do. + + `installed_version` is `None` when the target directory holds no keel. It should be read from + the on-disk artifact's METADATA and never by executing the installed binary: running an old + build to decide whether to replace it is fragile, and it is the exact case `keel versions` + exists to catch -- a partially-upgraded tree reports the new number from `--version` while + running old libraries. + """ + if installed_version is None: + return InstallPlan( + decision=InstallDecision.FRESH, + installed_version=None, + incoming_version=incoming_version, + needs_confirmation=False, + summary=f"Install keel {incoming_version}.", + ) + + # Lazy: `keel.commands.update` imports THIS module for its packaged refusal, so a + # module-level import here would close the cycle. `version_key` is the ONE semver reader -- + # a second one in this module would be a second place for "is this newer" to be wrong. + from keel.commands.update import version_key + + incoming_key = version_key(incoming_version) + installed_key = version_key(installed_version) + if incoming_key is None or installed_key is None: + return InstallPlan( + decision=InstallDecision.UNCOMPARABLE, + installed_version=installed_version, + incoming_version=incoming_version, + needs_confirmation=True, + summary=( + f"keel {installed_version} is already installed here and cannot be compared with " + f"{incoming_version}." + ), + warning=( + "One of these versions is not semver, so keel cannot tell which is newer. " + "Proceeding replaces the installed program with this one. " + DOWNGRADE_WARNING + ), + ) + + if incoming_key > installed_key: + return InstallPlan( + decision=InstallDecision.UPGRADE, + installed_version=installed_version, + incoming_version=incoming_version, + needs_confirmation=False, + summary=f"Update keel {installed_version} to {incoming_version}.", + ) + + if incoming_key == installed_key: + return InstallPlan( + decision=InstallDecision.REINSTALL, + installed_version=installed_version, + incoming_version=incoming_version, + needs_confirmation=True, + summary=f"keel {installed_version} is already installed here.", + warning=( + "Reinstalling the same version replaces the program files. Your config, database " + "and credentials are not touched." + ), + ) + + return InstallPlan( + decision=InstallDecision.DOWNGRADE, + installed_version=installed_version, + incoming_version=incoming_version, + needs_confirmation=True, + summary=f"Replace keel {installed_version} with the OLDER {incoming_version}.", + warning=DOWNGRADE_WARNING, + ) + + +#: Everything an installer must leave exactly as it found it. Not a suggestion: an operator's +#: allowlist, caps and trading mode are hand-edited and irreplaceable, and a database is the only +#: record of what the engine has done. `keel.commands.setup.create_config` already refuses to +#: overwrite a config and offers no `force` for a caller to pass; an installer must match it. +NEVER_TOUCHED: tuple[str, ...] = ("config*.yaml", "keel*.db", ".env", "logs/") + + +def packaged_update_refusal() -> str: + """Why `keel update` cannot run here, phrased for someone who has never opened a terminal. + + The refusals `keel update` already produces are all correct and all useless to a desktop user: + they talk about `site-packages` layouts and tell the reader to put `uv` on PATH. A packaged + user has no venv and no `uv`, and never will -- so the honest message names the actual update + path, which is downloading the next signed installer (#439's option A).""" + return ( + "this is a packaged install, which updates by downloading the next release rather than " + f"from the command line. Get it from {RELEASES_URL} and run it -- it will keep your " + "config, database and credentials exactly as they are." + ) diff --git a/tests/test_install.py b/tests/test_install.py new file mode 100644 index 0000000..fead45d --- /dev/null +++ b/tests/test_install.py @@ -0,0 +1,228 @@ +"""What an installer decides, and the one direction where "versions differ, so update" is wrong. + +The desktop product has no self-update (#439's option A), so the installer IS the update path. +That makes "what happens when this build meets the one already on disk" a question something has +to answer correctly every time -- and an Inno Setup script or a `.pkg` postinstall is not a place +where an answer can be tested. It is answered here instead. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +from keel.install import ( + DOWNGRADE_WARNING, + NEVER_TOUCHED, + RELEASES_URL, + InstallDecision, + default_deployment_dir, + default_program_dir, + fallback_program_dir, + is_packaged, + packaged_update_refusal, + plan_install, +) + +# -- the decision -------------------------------------------------------------------------- + + +def test_nothing_installed_is_a_fresh_install_and_asks_nothing() -> None: + plan = plan_install("0.11.0", None) + assert plan.decision is InstallDecision.FRESH + assert plan.may_proceed_silently + + +def test_a_newer_build_updates_without_asking() -> None: + plan = plan_install("0.11.0", "0.10.0") + assert plan.decision is InstallDecision.UPGRADE + assert plan.may_proceed_silently + assert "0.10.0" in plan.summary and "0.11.0" in plan.summary + + +def test_the_same_version_asks_before_overwriting() -> None: + """Reinstalling the same build is almost always a repair, so offer it -- but never silently.""" + plan = plan_install("0.10.0", "0.10.0") + assert plan.decision is InstallDecision.REINSTALL + assert plan.needs_confirmation + assert plan.warning is not None + + +def test_an_older_build_asks_AND_warns_that_migrations_do_not_reverse() -> None: + """The case that makes "versions differ, so update" wrong in one direction. + + `keel/data/db.py` migrates with `if current < target` and ships no down-migrations. A + database already at schema N, opened by a build expecting N-2, does NOT fail loudly: + `migrate` finds nothing to apply and returns, and the old code then runs against tables it + was never written against. Silence is the whole hazard, so the warning has to be loud.""" + plan = plan_install("0.9.0", "0.10.0") + assert plan.decision is InstallDecision.DOWNGRADE + assert plan.needs_confirmation + assert plan.warning == DOWNGRADE_WARNING + assert "forward-only" in plan.warning + assert "OLDER" in plan.summary + + +def test_the_downgrade_warning_names_the_recovery_and_not_just_the_risk() -> None: + """A warning that says only "this is dangerous" leaves the reader with no move. The recovery + is a database backup taken BEFORE the upgrade -- not running the old build anyway.""" + assert "backup" in DOWNGRADE_WARNING + assert "before the upgrade" in DOWNGRADE_WARNING + + +@pytest.mark.parametrize( + ("incoming", "installed"), + [("nightly", "0.10.0"), ("0.10.0", "nightly"), ("", "0.10.0"), ("0.10.0", "")], +) +def test_a_version_that_cannot_be_compared_asks_rather_than_guessing( + incoming: str, installed: str +) -> None: + """ "Cannot tell which is newer" must never resolve to "probably fine". It carries the + downgrade warning too, because an uncomparable pair might BE a downgrade.""" + plan = plan_install(incoming, installed) + assert plan.decision is InstallDecision.UNCOMPARABLE + assert plan.needs_confirmation + assert plan.warning is not None + assert "forward-only" in plan.warning + + +def test_only_a_fresh_install_or_a_genuine_upgrade_proceeds_silently() -> None: + """The closed statement of the rule: every other outcome stops and asks.""" + silent = { + plan_install(a, b).decision + for a, b in [("0.11.0", None), ("0.11.0", "0.10.0")] + if plan_install(a, b).may_proceed_silently + } + assert silent == {InstallDecision.FRESH, InstallDecision.UPGRADE} + for incoming, installed in [("0.10.0", "0.10.0"), ("0.9.0", "0.10.0"), ("x", "0.10.0")]: + assert not plan_install(incoming, installed).may_proceed_silently + + +def test_version_comparison_has_exactly_one_home() -> None: + """`plan_install` reads `keel.commands.update.version_key` rather than parsing semver again. + A second reader would be a second place for "is this newer" to disagree -- and the two would + disagree on exactly the strings nobody tested.""" + import inspect as inspect_mod + + from keel import install + + assert "version_key" in inspect_mod.getsource(install.plan_install) + + +# -- where things go ----------------------------------------------------------------------- + + +def test_the_program_and_the_deployment_are_different_directories() -> None: + """The trap this module exists to avoid. The program directory is replaced WHOLESALE on every + update, so anything of the operator's that lived there would be destroyed by an upgrade.""" + for platform in ("darwin", "win32"): + assert default_program_dir(platform) != default_deployment_dir(platform) + + +@pytest.mark.parametrize( + ("platform", "expected"), + [("darwin", "/Applications/keel.app"), ("win32", "Programs")], +) +def test_the_proposed_program_directory_is_per_user_and_os_appropriate( + platform: str, expected: str +) -> None: + """Per-user on both, deliberately: a machine-wide install needs elevation, and an elevation + prompt on a first run is the friction this milestone exists to remove.""" + assert expected in str(default_program_dir(platform, home=Path("/home/tester"))) + + +def test_macos_offers_a_per_user_fallback_and_windows_needs_none() -> None: + """`/Applications` needs admin on a managed machine; `~/Applications` is the documented + per-user equivalent. Windows' default is already per-user, so there is nothing to fall back + to -- and inventing one would be a second path for no reason.""" + mac = fallback_program_dir("darwin", home=Path("/home/tester")) + assert mac is not None and "Applications" in str(mac) + assert fallback_program_dir("win32", home=Path("/home/tester")) is None + + +def test_the_proposed_deployment_directory_is_the_one_the_app_will_look_in() -> None: + """An installer that proposed a folder the runtime does not discover would produce a + deployment that appears EMPTY on first launch -- config written, database written, and a + dashboard reporting a healthy install with no history.""" + from keel_core import paths + + assert default_deployment_dir() == paths.app_data_dir() + assert default_deployment_dir(sys.platform) == paths.app_data_dir() + + +def test_the_untouchable_list_covers_every_piece_of_operator_state() -> None: + """Config, databases, credentials and logs. An operator's allowlist, caps and trading mode + are hand-edited and irreplaceable; a database is the only record of what the engine did.""" + assert "config*.yaml" in NEVER_TOUCHED + assert "keel*.db" in NEVER_TOUCHED + assert ".env" in NEVER_TOUCHED + + +def test_the_untouchable_patterns_match_a_real_deployments_files(tmp_path: Path) -> None: + """The patterns are matched against real filenames, not eyeballed: `keel*.db` must actually + catch `keel-live.db`, and a typo in one of them would silently protect nothing.""" + for name in ("config.yaml", "config.live-sandbox.yaml", "keel.db", "keel-live.db", ".env"): + (tmp_path / name).touch() + matched = { + path.name for pattern in NEVER_TOUCHED for path in tmp_path.glob(pattern.rstrip("/")) + } + assert matched == { + "config.yaml", + "config.live-sandbox.yaml", + "keel.db", + "keel-live.db", + ".env", + } + + +# -- what a packaged install says about updating ------------------------------------------- + + +def test_a_venv_install_is_not_packaged() -> None: + """The test suite runs from a venv, so this also proves the detector is not simply True.""" + assert is_packaged() is False + + +def test_both_freezer_markers_are_detected(monkeypatch: pytest.MonkeyPatch) -> None: + """PyInstaller sets `sys.frozen` for every build mode but `sys._MEIPASS` only for + `--onefile`, and other freezers set one or the other. A false negative is the outcome #439 + exists to stop: a desktop user told to install `uv`.""" + monkeypatch.setattr(sys, "frozen", True, raising=False) + assert is_packaged() is True + monkeypatch.delattr(sys, "frozen", raising=False) + monkeypatch.setattr(sys, "_MEIPASS", "/tmp/whatever", raising=False) + assert is_packaged() is True + + +def test_the_packaged_refusal_names_the_download_and_not_a_command() -> None: + """Every other refusal `keel update` produces is correct and useless to a desktop user: they + talk about `site-packages` layouts and tell the reader to put `uv` on PATH.""" + message = packaged_update_refusal() + assert RELEASES_URL in message + assert "uv" not in message.split() + assert "site-packages" not in message + assert "config" in message and "database" in message + + +def test_keel_update_refuses_a_packaged_install_and_says_why( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Wired into the plan and driven through it, not merely available -- a refusal nobody calls + is documentation. Built against an OTHERWISE-VALID deployment layout, so the packaged + refusal is the thing being observed rather than one of the four the layout would produce + anyway.""" + import keel.commands.update as update_mod + from tests.commands.test_update import _plan + + plan = _plan(tmp_path) + assert plan.offered, "the fixture must be updatable, or this test proves nothing" + + monkeypatch.setattr(update_mod, "is_packaged", lambda: True) + packaged = _plan(tmp_path) + assert not packaged.offered + assert any(RELEASES_URL in reason for reason in packaged.refusal_reasons) + # And it is the FIRST thing said: every other refusal talks about venv layouts and `uv`, + # which is exactly the advice a desktop user cannot act on. + assert RELEASES_URL in packaged.refusal_reasons[0]